Message ID | 1395715231-4217-1-git-send-email-anton@samba.org |
---|---|
State | New |
Headers | show |
Anton Blanchard <anton@samba.org> writes: > Add MSR_LE to the msr_mask for POWER8. > <snip> > - pcc->msr_mask = 0x800000000284FF36ULL; > + pcc->msr_mask = 0x800000000284FF37ULL; <snip> Should we be adding some #define's for the various bit positions on this mask? Looking at the current code it looks like a big ream of magic numbers. Cheers,
Am 25.03.2014 08:29, schrieb Alex Bennée: > > Anton Blanchard <anton@samba.org> writes: > >> Add MSR_LE to the msr_mask for POWER8. >> > <snip> >> - pcc->msr_mask = 0x800000000284FF36ULL; >> + pcc->msr_mask = 0x800000000284FF37ULL; > <snip> > > Should we be adding some #define's for the various bit positions on this > mask? Looking at the current code it looks like a big ream of magic > numbers. In general I concur that defines would be nice, however for 2.0 that's too risky for me as temporary maintainer and I'm not sure if these values are being pieced together by contributors or whether this is coming directly from the manual? The other issue has been that adding a new family, even after the initial round of cleanups, still requires a chunk of code to be copied, which seems prone to forgetting little bits on the new one, then maybe fixing up the original template but not the derived models, etc. Cheers, Andreas
Hi Anton, Am 25.03.2014 03:40, schrieb Anton Blanchard: > Add MSR_LE to the msr_mask for POWER8. > > Signed-off-by: Anton Blanchard <anton@samba.org> > Signed-off-by: Cédric Le Goater <clg@fr.ibm.com> Something went wrong here - you're sending patches without placing your Sob last, which breaks the DCO chain. Cédric indicated to me on IRC to use Reviewed-by and/or Tested-by for him, fixed. Please remember to include a cover letter next time. I've picked up patches 1-4 for 2.0, tweaking two of the commit messages: https://github.com/afaerber/qemu-cpu/commits/ppc-next The SPR patches were too big for me to review right now, and it was not clear to me from commit messages what those patches were fixing (test case). Thanks, Andreas
On 3/27/2014 9:58 AM, Andreas Färber wrote: > Am 25.03.2014 08:29, schrieb Alex Bennée: >> >> Anton Blanchard <anton@samba.org> writes: >> >>> Add MSR_LE to the msr_mask for POWER8. >>> >> <snip> >>> - pcc->msr_mask = 0x800000000284FF36ULL; >>> + pcc->msr_mask = 0x800000000284FF37ULL; >> <snip> >> >> Should we be adding some #define's for the various bit positions on this >> mask? Looking at the current code it looks like a big ream of magic >> numbers. > > In general I concur that defines would be nice, however for 2.0 that's > too risky for me as temporary maintainer and I'm not sure if these > values are being pieced together by contributors or whether this is > coming directly from the manual? There are defines in target-ppc/cpu.h that identify the bit position. So something like this could/should be done (in 2.1): pcc->msr_mask = (1ULL << MSR_SF) | (1ULL << MSR_TAG) | (1ULL << MSR_ISF) | (1ULL << MSR_DR /* MISSING */ ) | (1ULL << MSR_IR /* MISSING */ ) | (1ULL << MSR_FE1 /* MISSING */ ) | (1ULL << MSR_BE /* MISSING */ ) | (1ULL << MSR_ME /* MISSING */ ) | (1ULL << MSR_SE /* MISSING */ ) | (1ULL << MSR_FE0 /* MISSING */ ) | (1ULL << MSR_FP /* MISSING */ ) | (1ULL << MSR_PR /* MISSING */ ) | (1ULL << MSR_EE /* MISSING */ ); The set of defines is incomplete -- the items marked MISSING are not currently there. They could, of course, easily be added. Unfortunately, this behavior has been repeated over 50 times in the target-ppc/translate_init.c file and some MSR bit positions have multiple meanings based on processor family and era. So a thorough and accurate cleanup would provide a nice challenge :) I'm willing to tackle this after I get done with some decimal floating point work (probably 2 weeks). > The other issue has been that adding a new family, even after the > initial round of cleanups, still requires a chunk of code to be copied, > which seems prone to forgetting little bits on the new one, then maybe > fixing up the original template but not the derived models, etc.
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c index 7f53c33..a82c8f9 100644 --- a/target-ppc/translate_init.c +++ b/target-ppc/translate_init.c @@ -7175,7 +7175,7 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data) PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 | PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 | PPC2_ISA205 | PPC2_ISA207S; - pcc->msr_mask = 0x800000000284FF36ULL; + pcc->msr_mask = 0x800000000284FF37ULL; pcc->mmu_model = POWERPC_MMU_2_06; #if defined(CONFIG_SOFTMMU) pcc->handle_mmu_fault = ppc_hash64_handle_mmu_fault;