On 30 October 2014 21:28, Greg Bellows <greg.bell...@linaro.org> wrote: > Added CP register info entries for the ARMv7 MAIR0/1 secure banks. > > Signed-off-by: Greg Bellows <greg.bell...@linaro.org> > > --- > > v5 -> v6 > - Changed _el field variants to be array based > --- > target-arm/cpu.h | 12 +++++++++++- > target-arm/helper.c | 8 +++++--- > 2 files changed, 16 insertions(+), 4 deletions(-) > > diff --git a/target-arm/cpu.h b/target-arm/cpu.h > index 348ce73..1a76fc6 100644 > --- a/target-arm/cpu.h > +++ b/target-arm/cpu.h > @@ -305,7 +305,17 @@ typedef struct CPUARMState { > uint32_t c9_pmxevtyper; /* perf monitor event type */ > uint32_t c9_pmuserenr; /* perf monitor user enable */ > uint32_t c9_pminten; /* perf monitor interrupt enables */ > - uint64_t mair_el1; > + union { /* Memory attribute redirection */ > + struct { > + uint64_t _unused_mair_0; > + uint32_t mair0_ns; > + uint32_t mair1_ns; > + uint64_t _unused_mair_1; > + uint32_t mair0_s; > + uint32_t mair1_s;
Surely these need the big-endian ifdefs too if we're unioning uint32_ts with a uint64_t array? (Can you check the other patches for this too, please? I might have missed it when reviewing some of them, but a quick scan through the cpu.h file in your tree ought to catch any other cases.) > + }; > + uint64_t mair_el[4]; > + }; > union { /* vector base address register */ > struct { > uint64_t _unused_vbar; > diff --git a/target-arm/helper.c b/target-arm/helper.c > index d782897..fd5f074 100644 > --- a/target-arm/helper.c > +++ b/target-arm/helper.c > @@ -939,7 +939,7 @@ static const ARMCPRegInfo v7_cp_reginfo[] = { > */ > { .name = "MAIR_EL1", .state = ARM_CP_STATE_AA64, > .opc0 = 3, .opc1 = 0, .crn = 10, .crm = 2, .opc2 = 0, > - .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, cp15.mair_el1), > + .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, > cp15.mair_el[1]), > .resetvalue = 0 }, > /* For non-long-descriptor page tables these are PRRR and NMRR; > * regardless they still act as reads-as-written for QEMU. > @@ -948,11 +948,13 @@ static const ARMCPRegInfo v7_cp_reginfo[] = { > */ > { .name = "MAIR0", .state = ARM_CP_STATE_AA32, .type = ARM_CP_OVERRIDE, > .cp = 15, .opc1 = 0, .crn = 10, .crm = 2, .opc2 = 0, .access = PL1_RW, > - .fieldoffset = offsetoflow32(CPUARMState, cp15.mair_el1), > + .bank_fieldoffsets = { offsetof(CPUARMState, cp15.mair0_s), > + offsetof(CPUARMState, cp15.mair0_ns) }, > .resetfn = arm_cp_reset_ignore }, > { .name = "MAIR1", .state = ARM_CP_STATE_AA32, .type = ARM_CP_OVERRIDE, > .cp = 15, .opc1 = 0, .crn = 10, .crm = 2, .opc2 = 1, .access = PL1_RW, > - .fieldoffset = offsetofhigh32(CPUARMState, cp15.mair_el1), > + .bank_fieldoffsets = { offsetof(CPUARMState, cp15.mair1_s), > + offsetof(CPUARMState, cp15.mair1_ns) }, > .resetfn = arm_cp_reset_ignore }, > { .name = "ISR_EL1", .state = ARM_CP_STATE_BOTH, > .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 1, .opc2 = 0, > -- > 1.8.3.2 > Otherwise you can add my reviewed-by tag. -- PMM