Message ID | 1347224784-19472-6-git-send-email-rth@twiddle.net |
---|---|
State | New |
Headers | show |
On 09/09/2012 11:04 PM, Richard Henderson wrote: > The real gdb protocol doesn't split out pc or cc as real registers. > Those are pseudos that are extracted as needed from the PSW. Don't > modify env->cc_op during read -- that way lies heisenbugs. > > Fill in the XXX for the fp registers. > > Remove duplicated defines in cpu.h. > > Signed-off-by: Richard Henderson <rth@twiddle.net> > --- > gdbstub.c | 78 +++++++++++++++++++++++++++++++++--------------------- > target-s390x/cpu.h | 73 -------------------------------------------------- > 2 files changed, 48 insertions(+), 103 deletions(-) > > diff --git a/gdbstub.c b/gdbstub.c > index 5d37dd9..6aed0b4 100644 > --- a/gdbstub.c > +++ b/gdbstub.c > @@ -1499,27 +1499,35 @@ static int cpu_gdb_write_register(CPUAlphaState *env, uint8_t *mem_buf, int n) > } > #elif defined (TARGET_S390X) > > -#define NUM_CORE_REGS S390_NUM_TOTAL_REGS > +#define NUM_CORE_REGS S390_NUM_REGS > > static int cpu_gdb_read_register(CPUS390XState *env, uint8_t *mem_buf, int n) > { > + uint64_t val; > + int cc_op; > + > switch (n) { > - case S390_PSWM_REGNUM: GET_REGL(env->psw.mask); break; > - case S390_PSWA_REGNUM: GET_REGL(env->psw.addr); break; > - case S390_R0_REGNUM ... S390_R15_REGNUM: > - GET_REGL(env->regs[n-S390_R0_REGNUM]); break; > - case S390_A0_REGNUM ... S390_A15_REGNUM: > - GET_REG32(env->aregs[n-S390_A0_REGNUM]); break; > - case S390_FPC_REGNUM: GET_REG32(env->fpc); break; > - case S390_F0_REGNUM ... S390_F15_REGNUM: > - /* XXX */ > - break; > - case S390_PC_REGNUM: GET_REGL(env->psw.addr); break; > - case S390_CC_REGNUM: > - env->cc_op = calc_cc(env, env->cc_op, env->cc_src, env->cc_dst, > - env->cc_vr); > - GET_REG32(env->cc_op); > - break; > + case S390_PSWM_REGNUM: > + val = env->psw.mask & ~(3ull << 13); > + cc_op = calc_cc(env, env->cc_op, env->cc_src, env->cc_dst, env->cc_vr); > + val |= cc_op << 13; > + GET_REGL(val); > + break; > + case S390_PSWA_REGNUM: > + GET_REGL(env->psw.addr); > + break; > + case S390_R0_REGNUM ... S390_R15_REGNUM: > + GET_REGL(env->regs[n-S390_R0_REGNUM]); > + break; > + case S390_A0_REGNUM ... S390_A15_REGNUM: > + GET_REG32(env->aregs[n-S390_A0_REGNUM]); > + break; > + case S390_FPC_REGNUM: > + GET_REG32(env->fpc); > + break; > + case S390_F0_REGNUM ... S390_F15_REGNUM: > + GET_REG64(env->fregs[n-S390_F0_REGNUM].ll); > + break; > } > > return 0; > @@ -1534,20 +1542,30 @@ static int cpu_gdb_write_register(CPUS390XState *env, uint8_t *mem_buf, int n) > tmp32 = ldl_p(mem_buf); > > switch (n) { > - case S390_PSWM_REGNUM: env->psw.mask = tmpl; break; > - case S390_PSWA_REGNUM: env->psw.addr = tmpl; break; > - case S390_R0_REGNUM ... S390_R15_REGNUM: > - env->regs[n-S390_R0_REGNUM] = tmpl; break; > - case S390_A0_REGNUM ... S390_A15_REGNUM: > - env->aregs[n-S390_A0_REGNUM] = tmp32; r=4; break; > - case S390_FPC_REGNUM: env->fpc = tmp32; r=4; break; > - case S390_F0_REGNUM ... S390_F15_REGNUM: > - /* XXX */ > - break; > - case S390_PC_REGNUM: env->psw.addr = tmpl; break; > - case S390_CC_REGNUM: env->cc_op = tmp32; r=4; break; > + case S390_PSWM_REGNUM: > + env->psw.mask = tmpl; > + env->cc_op = (tmpl >> 13) & 3; Are you sure this is correct? I thought gdbstub would just ignore the cc bits. Alex > + break; > + case S390_PSWA_REGNUM: > + env->psw.addr = tmpl; > + break; > + case S390_R0_REGNUM ... S390_R15_REGNUM: > + env->regs[n-S390_R0_REGNUM] = tmpl; > + break; > + case S390_A0_REGNUM ... S390_A15_REGNUM: > + env->aregs[n-S390_A0_REGNUM] = tmp32; > + r = 4; > + break; > + case S390_FPC_REGNUM: > + env->fpc = tmp32; > + r = 4; > + break; > + case S390_F0_REGNUM ... S390_F15_REGNUM: > + env->fregs[n-S390_F0_REGNUM].ll = tmpl; > + break; > + default: > + return 0; > } > - > return r; > } > #elif defined (TARGET_LM32) > diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h > index ed81af3..471fb91 100644 > --- a/target-s390x/cpu.h > +++ b/target-s390x/cpu.h > @@ -430,79 +430,6 @@ static inline void cpu_set_tls(CPUS390XState *env, target_ulong newtls) > /* Total. */ > #define S390_NUM_REGS 51 > > -/* Pseudo registers -- PC and condition code. */ > -#define S390_PC_REGNUM S390_NUM_REGS > -#define S390_CC_REGNUM (S390_NUM_REGS+1) > -#define S390_NUM_PSEUDO_REGS 2 > -#define S390_NUM_TOTAL_REGS (S390_NUM_REGS+2) > - > - > - > -/* Program Status Word. */ > -#define S390_PSWM_REGNUM 0 > -#define S390_PSWA_REGNUM 1 > -/* General Purpose Registers. */ > -#define S390_R0_REGNUM 2 > -#define S390_R1_REGNUM 3 > -#define S390_R2_REGNUM 4 > -#define S390_R3_REGNUM 5 > -#define S390_R4_REGNUM 6 > -#define S390_R5_REGNUM 7 > -#define S390_R6_REGNUM 8 > -#define S390_R7_REGNUM 9 > -#define S390_R8_REGNUM 10 > -#define S390_R9_REGNUM 11 > -#define S390_R10_REGNUM 12 > -#define S390_R11_REGNUM 13 > -#define S390_R12_REGNUM 14 > -#define S390_R13_REGNUM 15 > -#define S390_R14_REGNUM 16 > -#define S390_R15_REGNUM 17 > -/* Access Registers. */ > -#define S390_A0_REGNUM 18 > -#define S390_A1_REGNUM 19 > -#define S390_A2_REGNUM 20 > -#define S390_A3_REGNUM 21 > -#define S390_A4_REGNUM 22 > -#define S390_A5_REGNUM 23 > -#define S390_A6_REGNUM 24 > -#define S390_A7_REGNUM 25 > -#define S390_A8_REGNUM 26 > -#define S390_A9_REGNUM 27 > -#define S390_A10_REGNUM 28 > -#define S390_A11_REGNUM 29 > -#define S390_A12_REGNUM 30 > -#define S390_A13_REGNUM 31 > -#define S390_A14_REGNUM 32 > -#define S390_A15_REGNUM 33 > -/* Floating Point Control Word. */ > -#define S390_FPC_REGNUM 34 > -/* Floating Point Registers. */ > -#define S390_F0_REGNUM 35 > -#define S390_F1_REGNUM 36 > -#define S390_F2_REGNUM 37 > -#define S390_F3_REGNUM 38 > -#define S390_F4_REGNUM 39 > -#define S390_F5_REGNUM 40 > -#define S390_F6_REGNUM 41 > -#define S390_F7_REGNUM 42 > -#define S390_F8_REGNUM 43 > -#define S390_F9_REGNUM 44 > -#define S390_F10_REGNUM 45 > -#define S390_F11_REGNUM 46 > -#define S390_F12_REGNUM 47 > -#define S390_F13_REGNUM 48 > -#define S390_F14_REGNUM 49 > -#define S390_F15_REGNUM 50 > -/* Total. */ > -#define S390_NUM_REGS 51 > - > -/* Pseudo registers -- PC and condition code. */ > -#define S390_PC_REGNUM S390_NUM_REGS > -#define S390_CC_REGNUM (S390_NUM_REGS+1) > -#define S390_NUM_PSEUDO_REGS 2 > -#define S390_NUM_TOTAL_REGS (S390_NUM_REGS+2) > - Hrm. Mind to quickly explain why it's ok to remove these? Where to they come from now? Or were they defined twice in cpu.h? Alex > /* CC optimization */ > > enum cc_op {
On 09/12/2012 06:25 AM, Alexander Graf wrote: >> + case S390_PSWM_REGNUM: >> + env->psw.mask = tmpl; >> + env->cc_op = (tmpl >> 13) & 3; > > Are you sure this is correct? I thought gdbstub would just ignore the cc bits. Well... no it won't ignore the cc bits. But it would appear that I've got them at the wrong location. From gdb/s390-tdep.c: if (regnum == tdep->cc_regnum) { enum register_status status; status = regcache_raw_read_unsigned (regcache, S390_PSWM_REGNUM, &val); if (status == REG_VALID) { if (register_size (gdbarch, S390_PSWA_REGNUM) == 4) val = (val >> 12) & 3; else val = (val >> 44) & 3; store_unsigned_integer (buf, regsize, byte_order, val); } return status; } r~
On 09/12/2012 05:11 PM, Richard Henderson wrote: > On 09/12/2012 06:25 AM, Alexander Graf wrote: >>> + case S390_PSWM_REGNUM: >>> + env->psw.mask = tmpl; >>> + env->cc_op = (tmpl >> 13) & 3; >> Are you sure this is correct? I thought gdbstub would just ignore the cc bits. > Well... no it won't ignore the cc bits. So the CC pseudo-register is never written to? > But it would appear that I've got > them at the wrong location. From gdb/s390-tdep.c: > > if (regnum == tdep->cc_regnum) > { > enum register_status status; > > status = regcache_raw_read_unsigned (regcache, S390_PSWM_REGNUM, &val); > if (status == REG_VALID) > { > if (register_size (gdbarch, S390_PSWA_REGNUM) == 4) > val = (val >> 12) & 3; Oops :) Alex
On 09/12/2012 08:54 AM, Alexander Graf wrote:
> So the CC pseudo-register is never written to?
They do handle a write to cc_regnum in s390_pseudo_register_write.
They modify psw.mask as one would expect.
r~
diff --git a/gdbstub.c b/gdbstub.c index 5d37dd9..6aed0b4 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -1499,27 +1499,35 @@ static int cpu_gdb_write_register(CPUAlphaState *env, uint8_t *mem_buf, int n) } #elif defined (TARGET_S390X) -#define NUM_CORE_REGS S390_NUM_TOTAL_REGS +#define NUM_CORE_REGS S390_NUM_REGS static int cpu_gdb_read_register(CPUS390XState *env, uint8_t *mem_buf, int n) { + uint64_t val; + int cc_op; + switch (n) { - case S390_PSWM_REGNUM: GET_REGL(env->psw.mask); break; - case S390_PSWA_REGNUM: GET_REGL(env->psw.addr); break; - case S390_R0_REGNUM ... S390_R15_REGNUM: - GET_REGL(env->regs[n-S390_R0_REGNUM]); break; - case S390_A0_REGNUM ... S390_A15_REGNUM: - GET_REG32(env->aregs[n-S390_A0_REGNUM]); break; - case S390_FPC_REGNUM: GET_REG32(env->fpc); break; - case S390_F0_REGNUM ... S390_F15_REGNUM: - /* XXX */ - break; - case S390_PC_REGNUM: GET_REGL(env->psw.addr); break; - case S390_CC_REGNUM: - env->cc_op = calc_cc(env, env->cc_op, env->cc_src, env->cc_dst, - env->cc_vr); - GET_REG32(env->cc_op); - break; + case S390_PSWM_REGNUM: + val = env->psw.mask & ~(3ull << 13); + cc_op = calc_cc(env, env->cc_op, env->cc_src, env->cc_dst, env->cc_vr); + val |= cc_op << 13; + GET_REGL(val); + break; + case S390_PSWA_REGNUM: + GET_REGL(env->psw.addr); + break; + case S390_R0_REGNUM ... S390_R15_REGNUM: + GET_REGL(env->regs[n-S390_R0_REGNUM]); + break; + case S390_A0_REGNUM ... S390_A15_REGNUM: + GET_REG32(env->aregs[n-S390_A0_REGNUM]); + break; + case S390_FPC_REGNUM: + GET_REG32(env->fpc); + break; + case S390_F0_REGNUM ... S390_F15_REGNUM: + GET_REG64(env->fregs[n-S390_F0_REGNUM].ll); + break; } return 0; @@ -1534,20 +1542,30 @@ static int cpu_gdb_write_register(CPUS390XState *env, uint8_t *mem_buf, int n) tmp32 = ldl_p(mem_buf); switch (n) { - case S390_PSWM_REGNUM: env->psw.mask = tmpl; break; - case S390_PSWA_REGNUM: env->psw.addr = tmpl; break; - case S390_R0_REGNUM ... S390_R15_REGNUM: - env->regs[n-S390_R0_REGNUM] = tmpl; break; - case S390_A0_REGNUM ... S390_A15_REGNUM: - env->aregs[n-S390_A0_REGNUM] = tmp32; r=4; break; - case S390_FPC_REGNUM: env->fpc = tmp32; r=4; break; - case S390_F0_REGNUM ... S390_F15_REGNUM: - /* XXX */ - break; - case S390_PC_REGNUM: env->psw.addr = tmpl; break; - case S390_CC_REGNUM: env->cc_op = tmp32; r=4; break; + case S390_PSWM_REGNUM: + env->psw.mask = tmpl; + env->cc_op = (tmpl >> 13) & 3; + break; + case S390_PSWA_REGNUM: + env->psw.addr = tmpl; + break; + case S390_R0_REGNUM ... S390_R15_REGNUM: + env->regs[n-S390_R0_REGNUM] = tmpl; + break; + case S390_A0_REGNUM ... S390_A15_REGNUM: + env->aregs[n-S390_A0_REGNUM] = tmp32; + r = 4; + break; + case S390_FPC_REGNUM: + env->fpc = tmp32; + r = 4; + break; + case S390_F0_REGNUM ... S390_F15_REGNUM: + env->fregs[n-S390_F0_REGNUM].ll = tmpl; + break; + default: + return 0; } - return r; } #elif defined (TARGET_LM32) diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h index ed81af3..471fb91 100644 --- a/target-s390x/cpu.h +++ b/target-s390x/cpu.h @@ -430,79 +430,6 @@ static inline void cpu_set_tls(CPUS390XState *env, target_ulong newtls) /* Total. */ #define S390_NUM_REGS 51 -/* Pseudo registers -- PC and condition code. */ -#define S390_PC_REGNUM S390_NUM_REGS -#define S390_CC_REGNUM (S390_NUM_REGS+1) -#define S390_NUM_PSEUDO_REGS 2 -#define S390_NUM_TOTAL_REGS (S390_NUM_REGS+2) - - - -/* Program Status Word. */ -#define S390_PSWM_REGNUM 0 -#define S390_PSWA_REGNUM 1 -/* General Purpose Registers. */ -#define S390_R0_REGNUM 2 -#define S390_R1_REGNUM 3 -#define S390_R2_REGNUM 4 -#define S390_R3_REGNUM 5 -#define S390_R4_REGNUM 6 -#define S390_R5_REGNUM 7 -#define S390_R6_REGNUM 8 -#define S390_R7_REGNUM 9 -#define S390_R8_REGNUM 10 -#define S390_R9_REGNUM 11 -#define S390_R10_REGNUM 12 -#define S390_R11_REGNUM 13 -#define S390_R12_REGNUM 14 -#define S390_R13_REGNUM 15 -#define S390_R14_REGNUM 16 -#define S390_R15_REGNUM 17 -/* Access Registers. */ -#define S390_A0_REGNUM 18 -#define S390_A1_REGNUM 19 -#define S390_A2_REGNUM 20 -#define S390_A3_REGNUM 21 -#define S390_A4_REGNUM 22 -#define S390_A5_REGNUM 23 -#define S390_A6_REGNUM 24 -#define S390_A7_REGNUM 25 -#define S390_A8_REGNUM 26 -#define S390_A9_REGNUM 27 -#define S390_A10_REGNUM 28 -#define S390_A11_REGNUM 29 -#define S390_A12_REGNUM 30 -#define S390_A13_REGNUM 31 -#define S390_A14_REGNUM 32 -#define S390_A15_REGNUM 33 -/* Floating Point Control Word. */ -#define S390_FPC_REGNUM 34 -/* Floating Point Registers. */ -#define S390_F0_REGNUM 35 -#define S390_F1_REGNUM 36 -#define S390_F2_REGNUM 37 -#define S390_F3_REGNUM 38 -#define S390_F4_REGNUM 39 -#define S390_F5_REGNUM 40 -#define S390_F6_REGNUM 41 -#define S390_F7_REGNUM 42 -#define S390_F8_REGNUM 43 -#define S390_F9_REGNUM 44 -#define S390_F10_REGNUM 45 -#define S390_F11_REGNUM 46 -#define S390_F12_REGNUM 47 -#define S390_F13_REGNUM 48 -#define S390_F14_REGNUM 49 -#define S390_F15_REGNUM 50 -/* Total. */ -#define S390_NUM_REGS 51 - -/* Pseudo registers -- PC and condition code. */ -#define S390_PC_REGNUM S390_NUM_REGS -#define S390_CC_REGNUM (S390_NUM_REGS+1) -#define S390_NUM_PSEUDO_REGS 2 -#define S390_NUM_TOTAL_REGS (S390_NUM_REGS+2) - /* CC optimization */ enum cc_op {
The real gdb protocol doesn't split out pc or cc as real registers. Those are pseudos that are extracted as needed from the PSW. Don't modify env->cc_op during read -- that way lies heisenbugs. Fill in the XXX for the fp registers. Remove duplicated defines in cpu.h. Signed-off-by: Richard Henderson <rth@twiddle.net> --- gdbstub.c | 78 +++++++++++++++++++++++++++++++++--------------------- target-s390x/cpu.h | 73 -------------------------------------------------- 2 files changed, 48 insertions(+), 103 deletions(-)