On 9/15/2014 10:03 AM, Paolo Bonzini wrote: > New functions to put together all 32 CR bits. Avoids easy > off-by-one mistakes such as the one fixed by commit f13f529 > (ppc: fix monitor access to CR, 2014-08-28). > > Signed-off-by: Paolo Bonzini <pbonz...@redhat.com> > --- > v1->v2: replaces "ppc: use ARRAY_SIZE in gdbstub.c" > > linux-user/elfload.c | 4 +--- > linux-user/signal.c | 8 ++------ > monitor.c | 9 +-------- > target-ppc/cpu.h | 20 ++++++++++++++++++++ > target-ppc/gdbstub.c | 42 ++++++++---------------------------------- > target-ppc/kvm.c | 11 ++--------- > 6 files changed, 34 insertions(+), 60 deletions(-) > > diff --git a/linux-user/elfload.c b/linux-user/elfload.c > index bea803b..a7d1714 100644 > --- a/linux-user/elfload.c > +++ b/linux-user/elfload.c > @@ -857,9 +857,7 @@ static void elf_core_copy_regs(target_elf_gregset_t > *regs, const CPUPPCState *en > (*regs)[36] = tswapreg(env->lr); > (*regs)[37] = tswapreg(env->xer); > > - for (i = 0; i < ARRAY_SIZE(env->crf); i++) { > - ccr |= env->crf[i] << (32 - ((i + 1) * 4)); > - } > + ccr = ppc_get_cr(env); > (*regs)[38] = tswapreg(ccr); > } > > diff --git a/linux-user/signal.c b/linux-user/signal.c > index e11b208..97c3107 100644 > --- a/linux-user/signal.c > +++ b/linux-user/signal.c > @@ -4534,9 +4534,7 @@ static void save_user_regs(CPUPPCState *env, struct > target_mcontext *frame) > __put_user(env->lr, &frame->mc_gregs[TARGET_PT_LNK]); > __put_user(env->xer, &frame->mc_gregs[TARGET_PT_XER]); > > - for (i = 0; i < ARRAY_SIZE(env->crf); i++) { > - ccr |= env->crf[i] << (32 - ((i + 1) * 4)); > - } > + ccr = ppc_get_cr(env); > __put_user(ccr, &frame->mc_gregs[TARGET_PT_CCR]); > > /* Save Altivec registers if necessary. */ > @@ -4616,9 +4614,7 @@ static void restore_user_regs(CPUPPCState *env, > __get_user(env->xer, &frame->mc_gregs[TARGET_PT_XER]); > __get_user(ccr, &frame->mc_gregs[TARGET_PT_CCR]); > > - for (i = 0; i < ARRAY_SIZE(env->crf); i++) { > - env->crf[i] = (ccr >> (32 - ((i + 1) * 4))) & 0xf; > - } > + ppc_set_cr(env, ccr); > > if (!sig) { > env->gpr[2] = save_r2; > diff --git a/monitor.c b/monitor.c > index ec73dd4..80acf25 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -2963,14 +2963,7 @@ static target_long monitor_get_pc (const struct > MonitorDef *md, int val) > static target_long monitor_get_ccr (const struct MonitorDef *md, int val) > { > CPUArchState *env = mon_get_cpu(); > - unsigned int u; > - int i; > - > - u = 0; > - for (i = 0; i < 8; i++) > - u |= env->crf[i] << (32 - (4 * (i + 1))); > - > - return u; > + return ppc_get_cr(env); > } > > static target_long monitor_get_msr (const struct MonitorDef *md, int val) > diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h > index c29ce3b..0c0196d 100644 > --- a/target-ppc/cpu.h > +++ b/target-ppc/cpu.h > @@ -1197,6 +1197,26 @@ void ppc_tlb_invalidate_one (CPUPPCState *env, > target_ulong addr); > > void store_fpscr(CPUPPCState *env, uint64_t arg, uint32_t mask); > > +static inline uint32_t ppc_get_cr(const CPUPPCState *env) > +{ > + uint32_t cr = 0; > + int i; > + > + for (i = 0; i < ARRAY_SIZE(env->crf); i++) { > + cr |= env->crf[i] << (32 - ((i + 1) * 4)); > + } > + return cr; > +} > + > +static inline void ppc_set_cr(CPUPPCState *env, uint32_t cr) > +{ > + int i; > + > + for (i = 0; i < ARRAY_SIZE(env->crf); i++) { > + env->crf[i] = (cr >> (32 - ((i + 1) * 4))) & 0xF; > + } > +} > + > static inline uint64_t ppc_dump_gpr(CPUPPCState *env, int gprn) > { > uint64_t gprv; > diff --git a/target-ppc/gdbstub.c b/target-ppc/gdbstub.c > index 14675f4..11d138e 100644 > --- a/target-ppc/gdbstub.c > +++ b/target-ppc/gdbstub.c > @@ -135,15 +135,8 @@ int ppc_cpu_gdb_read_register(CPUState *cs, uint8_t > *mem_buf, int n) > gdb_get_regl(mem_buf, env->msr); > break; > case 66: > - { > - uint32_t cr = 0; > - int i; > - for (i = 0; i < 8; i++) { > - cr |= env->crf[i] << (32 - ((i + 1) * 4)); > - } > - gdb_get_reg32(mem_buf, cr); > - break; > - } > + gdb_get_reg32(mem_buf, ppc_get_cr(env)); > + break; > case 67: > gdb_get_regl(mem_buf, env->lr); > break; > @@ -191,15 +184,8 @@ int ppc_cpu_gdb_read_register_apple(CPUState *cs, > uint8_t *mem_buf, int n) > gdb_get_reg64(mem_buf, env->msr); > break; > case 66 + 32: > - { > - uint32_t cr = 0; > - int i; > - for (i = 0; i < 8; i++) { > - cr |= env->crf[i] << (32 - ((i + 1) * 4)); > - } > - gdb_get_reg32(mem_buf, cr); > - break; > - } > + gdb_get_reg32(mem_buf, ppc_get_cr(env)); > + break; > case 67 + 32: > gdb_get_reg64(mem_buf, env->lr); > break; > @@ -243,14 +229,8 @@ int ppc_cpu_gdb_write_register(CPUState *cs, uint8_t > *mem_buf, int n) > ppc_store_msr(env, ldtul_p(mem_buf)); > break; > case 66: > - { > - uint32_t cr = ldl_p(mem_buf); > - int i; > - for (i = 0; i < 8; i++) { > - env->crf[i] = (cr >> (32 - ((i + 1) * 4))) & 0xF; > - } > - break; > - } > + ppc_set_cr(env, ldl_p(mem_buf)); > + break; > case 67: > env->lr = ldtul_p(mem_buf); > break; > @@ -293,14 +273,8 @@ int ppc_cpu_gdb_write_register_apple(CPUState *cs, > uint8_t *mem_buf, int n) > ppc_store_msr(env, ldq_p(mem_buf)); > break; > case 66 + 32: > - { > - uint32_t cr = ldl_p(mem_buf); > - int i; > - for (i = 0; i < 8; i++) { > - env->crf[i] = (cr >> (32 - ((i + 1) * 4))) & 0xF; > - } > - break; > - } > + ppc_set_cr(env, ldl_p(mem_buf)); > + break; > case 67 + 32: > env->lr = ldq_p(mem_buf); > break; > diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c > index 9c23c6b..e541b9e 100644 > --- a/target-ppc/kvm.c > +++ b/target-ppc/kvm.c > @@ -831,10 +831,7 @@ int kvm_arch_put_registers(CPUState *cs, int level) > for (i = 0;i < 32; i++) > regs.gpr[i] = env->gpr[i]; > > - regs.cr = 0; > - for (i = 0; i < 8; i++) { > - regs.cr |= (env->crf[i] & 15) << (4 * (7 - i)); > - } > + regs.cr = ppc_get_cr(env); > > ret = kvm_vcpu_ioctl(cs, KVM_SET_REGS, ®s); > if (ret < 0) > @@ -955,11 +952,7 @@ int kvm_arch_get_registers(CPUState *cs) > if (ret < 0) > return ret; > > - cr = regs.cr; > - for (i = 7; i >= 0; i--) { > - env->crf[i] = cr & 15; > - cr >>= 4; > - } > + ppc_set_cr(env, regs.cr); > > env->ctr = regs.ctr; > env->lr = regs.lr; >
One minor issue with this patch: CC ppc64-softmmu/target-ppc/kvm.o /bghome/tmusta/powerisa/qemu/qemu/target-ppc/kvm.c: In function ?kvm_arch_get_registers?: /bghome/tmusta/powerisa/qemu/qemu/target-ppc/kvm.c:948: warning: unused variable ?cr? which, of course, can be fixed like this: > git diff diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c index e541b9e..74c1324 100644 --- a/target-ppc/kvm.c +++ b/target-ppc/kvm.c @@ -945,7 +945,6 @@ int kvm_arch_get_registers(CPUState *cs) CPUPPCState *env = &cpu->env; struct kvm_regs regs; struct kvm_sregs sregs; - uint32_t cr; int i, ret; ret = kvm_vcpu_ioctl(cs, KVM_GET_REGS, ®s); Otherwise ... Reviewed-by: Tom Musta <tommu...@gmail.com>