| Message ID | 1391183143-30724-13-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | New |
| Headers | show |
cc Alistair, this may conflict with his timer work. On Sat, Feb 1, 2014 at 1:45 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > Convert the performance monitor reginfo definitions to use > an accessfn rather than returning EXCP_UDEF from read and > write functions. This also allows us to fix a couple of XXX > cases where we weren't imposing the access restrictions on > RAZ/WI or constant registers. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > target-arm/helper.c | 70 +++++++++++++++++++++-------------------------------- > 1 file changed, 28 insertions(+), 42 deletions(-) > > diff --git a/target-arm/helper.c b/target-arm/helper.c > index bd78350..9adaeb9 100644 > --- a/target-arm/helper.c > +++ b/target-arm/helper.c > @@ -485,26 +485,20 @@ static const ARMCPRegInfo v6_cp_reginfo[] = { > REGINFO_SENTINEL > }; > > - > -static int pmreg_read(CPUARMState *env, const ARMCPRegInfo *ri, > - uint64_t *value) > +static CPAccessResult pmreg_access(CPUARMState *env, const ARMCPRegInfo *ri) > { > - /* Generic performance monitor register read function for where > - * user access may be allowed by PMUSERENR. > + /* Perfomance monitor registers user accessibility is controlled > + * by PMUSERENR. > */ > if (arm_current_pl(env) == 0 && !env->cp15.c9_pmuserenr) { > - return EXCP_UDEF; > + return CP_ACCESS_TRAP; > } > - *value = CPREG_FIELD32(env, ri); > - return 0; > + return CP_ACCESS_OK; > } > > static int pmcr_write(CPUARMState *env, const ARMCPRegInfo *ri, > uint64_t value) > { > - if (arm_current_pl(env) == 0 && !env->cp15.c9_pmuserenr) { > - return EXCP_UDEF; > - } > /* only the DP, X, D and E bits are writable */ > env->cp15.c9_pmcr &= ~0x39; > env->cp15.c9_pmcr |= (value & 0x39); > @@ -514,9 +508,6 @@ static int pmcr_write(CPUARMState *env, const ARMCPRegInfo *ri, > static int pmcntenset_write(CPUARMState *env, const ARMCPRegInfo *ri, > uint64_t value) > { > - if (arm_current_pl(env) == 0 && !env->cp15.c9_pmuserenr) { > - return EXCP_UDEF; > - } > value &= (1 << 31); > env->cp15.c9_pmcnten |= value; > return 0; > @@ -525,9 +516,6 @@ static int pmcntenset_write(CPUARMState *env, const ARMCPRegInfo *ri, > static int pmcntenclr_write(CPUARMState *env, const ARMCPRegInfo *ri, > uint64_t value) > { > - if (arm_current_pl(env) == 0 && !env->cp15.c9_pmuserenr) { > - return EXCP_UDEF; > - } > value &= (1 << 31); > env->cp15.c9_pmcnten &= ~value; > return 0; > @@ -536,9 +524,6 @@ static int pmcntenclr_write(CPUARMState *env, const ARMCPRegInfo *ri, > static int pmovsr_write(CPUARMState *env, const ARMCPRegInfo *ri, > uint64_t value) > { > - if (arm_current_pl(env) == 0 && !env->cp15.c9_pmuserenr) { > - return EXCP_UDEF; > - } > env->cp15.c9_pmovsr &= ~value; > return 0; > } > @@ -546,9 +531,6 @@ static int pmovsr_write(CPUARMState *env, const ARMCPRegInfo *ri, > static int pmxevtyper_write(CPUARMState *env, const ARMCPRegInfo *ri, > uint64_t value) > { > - if (arm_current_pl(env) == 0 && !env->cp15.c9_pmuserenr) { > - return EXCP_UDEF; > - } > env->cp15.c9_pmxevtyper = value & 0xff; > return 0; > } > @@ -624,37 +606,41 @@ static const ARMCPRegInfo v7_cp_reginfo[] = { > { .name = "PMCNTENSET", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 1, > .access = PL0_RW, .resetvalue = 0, > .fieldoffset = offsetof(CPUARMState, cp15.c9_pmcnten), > - .readfn = pmreg_read, .writefn = pmcntenset_write, > - .raw_readfn = raw_read, .raw_writefn = raw_write }, > + .writefn = pmcntenset_write, > + .accessfn = pmreg_access, > + .raw_writefn = raw_write }, A nit but, You're field ordering scheme is inconsistent, here you go, write -> access -> raw_write .... > { .name = "PMCNTENCLR", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 2, > .access = PL0_RW, .fieldoffset = offsetof(CPUARMState, cp15.c9_pmcnten), > - .readfn = pmreg_read, .writefn = pmcntenclr_write, > + .accessfn = pmreg_access, > + .writefn = pmcntenclr_write, > .type = ARM_CP_NO_MIGRATE }, > { .name = "PMOVSR", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 3, > .access = PL0_RW, .fieldoffset = offsetof(CPUARMState, cp15.c9_pmovsr), > - .readfn = pmreg_read, .writefn = pmovsr_write, > - .raw_readfn = raw_read, .raw_writefn = raw_write }, > - /* Unimplemented so WI. Strictly speaking write accesses in PL0 should > - * respect PMUSERENR. > - */ > + .accessfn = pmreg_access, > + .writefn = pmovsr_write, > + .raw_writefn = raw_write }, ... and this is access -> write -> raw_write. Is there a prescribed order to keep new (or gradually refactored) code consistent? Regards, Peter > + /* Unimplemented so WI. */ > { .name = "PMSWINC", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 4, > - .access = PL0_W, .type = ARM_CP_NOP }, > + .access = PL0_W, .accessfn = pmreg_access, .type = ARM_CP_NOP }, > /* Since we don't implement any events, writing to PMSELR is UNPREDICTABLE. > - * We choose to RAZ/WI. XXX should respect PMUSERENR. > + * We choose to RAZ/WI. > */ > { .name = "PMSELR", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 5, > - .access = PL0_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, > - /* Unimplemented, RAZ/WI. XXX PMUSERENR */ > + .access = PL0_RW, .type = ARM_CP_CONST, .resetvalue = 0, > + .accessfn = pmreg_access }, > + /* Unimplemented, RAZ/WI. */ > { .name = "PMCCNTR", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 = 0, > - .access = PL0_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, > + .access = PL0_RW, .type = ARM_CP_CONST, .resetvalue = 0, > + .accessfn = pmreg_access }, > { .name = "PMXEVTYPER", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 = 1, > .access = PL0_RW, > .fieldoffset = offsetof(CPUARMState, cp15.c9_pmxevtyper), > - .readfn = pmreg_read, .writefn = pmxevtyper_write, > - .raw_readfn = raw_read, .raw_writefn = raw_write }, > - /* Unimplemented, RAZ/WI. XXX PMUSERENR */ > + .accessfn = pmreg_access, .writefn = pmxevtyper_write, > + .raw_writefn = raw_write }, > + /* Unimplemented, RAZ/WI. */ > { .name = "PMXEVCNTR", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 = 2, > - .access = PL0_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, > + .access = PL0_RW, .type = ARM_CP_CONST, .resetvalue = 0, > + .accessfn = pmreg_access }, > { .name = "PMUSERENR", .cp = 15, .crn = 9, .crm = 14, .opc1 = 0, .opc2 = 0, > .access = PL0_R | PL1_RW, > .fieldoffset = offsetof(CPUARMState, cp15.c9_pmuserenr), > @@ -1708,8 +1694,8 @@ void register_cp_regs_for_features(ARMCPU *cpu) > .name = "PMCR", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 0, > .access = PL0_RW, .resetvalue = cpu->midr & 0xff000000, > .fieldoffset = offsetof(CPUARMState, cp15.c9_pmcr), > - .readfn = pmreg_read, .writefn = pmcr_write, > - .raw_readfn = raw_read, .raw_writefn = raw_write, > + .accessfn = pmreg_access, .writefn = pmcr_write, > + .raw_writefn = raw_write, > }; > ARMCPRegInfo clidr = { > .name = "CLIDR", .cp = 15, .crn = 0, .crm = 0, .opc1 = 1, .opc2 = 1, > -- > 1.8.5 > >
On 5 February 2014 06:59, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote: > cc Alistair, this may conflict with his timer work. > > On Sat, Feb 1, 2014 at 1:45 AM, Peter Maydell <peter.maydell@linaro.org> wrote: >> @@ -624,37 +606,41 @@ static const ARMCPRegInfo v7_cp_reginfo[] = { >> { .name = "PMCNTENSET", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 1, >> .access = PL0_RW, .resetvalue = 0, >> .fieldoffset = offsetof(CPUARMState, cp15.c9_pmcnten), >> - .readfn = pmreg_read, .writefn = pmcntenset_write, >> - .raw_readfn = raw_read, .raw_writefn = raw_write }, >> + .writefn = pmcntenset_write, >> + .accessfn = pmreg_access, >> + .raw_writefn = raw_write }, > > A nit but, > > You're field ordering scheme is inconsistent, here you go, write -> > access -> raw_write .... > >> { .name = "PMCNTENCLR", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 2, >> .access = PL0_RW, .fieldoffset = offsetof(CPUARMState, cp15.c9_pmcnten), >> - .readfn = pmreg_read, .writefn = pmcntenclr_write, >> + .accessfn = pmreg_access, >> + .writefn = pmcntenclr_write, >> .type = ARM_CP_NO_MIGRATE }, >> { .name = "PMOVSR", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 3, >> .access = PL0_RW, .fieldoffset = offsetof(CPUARMState, cp15.c9_pmovsr), >> - .readfn = pmreg_read, .writefn = pmovsr_write, >> - .raw_readfn = raw_read, .raw_writefn = raw_write }, >> - /* Unimplemented so WI. Strictly speaking write accesses in PL0 should >> - * respect PMUSERENR. >> - */ >> + .accessfn = pmreg_access, >> + .writefn = pmovsr_write, >> + .raw_writefn = raw_write }, > > ... and this is access -> write -> raw_write. Is there a prescribed > order to keep new (or gradually refactored) code consistent? No, I've just been going for "whatever looks like it fits reasonably neatly onto not too many lines and is a fairly minimal change to existing structs", mostly. The thing I have been trying to be consistent with is the order for crn/crm/opc1/opc2 fields, which for new registers I've been making be op0/op1/crn/crm/op2, because that's the order the ARM ARM seems to have settled on. Unfortunately a lot of our existing definitions are crn/crm/op1/op2, because at the time there was variance in the ARM docs and that order seemed sensible to me. For the other fields, I think "name first; type/state/encoding second; access related fields; read and write accessors; reset stuff" is probably not a bad order. But the nice thing about having named fields is it doesn't actually matter what order things go in. thanks -- PMM
On Wed, Feb 5, 2014 at 9:01 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 5 February 2014 06:59, Peter Crosthwaite > <peter.crosthwaite@xilinx.com> wrote: >> cc Alistair, this may conflict with his timer work. It seems fine to me. None of Peter's code changes conflict with mine, except for adding the ".accessfn = pmreg_access," to the PMCCNTR register. My patch never implemented the return EXCP_UDEF; so no action is needed there >> >> On Sat, Feb 1, 2014 at 1:45 AM, Peter Maydell <peter.maydell@linaro.org> wrote: >>> @@ -624,37 +606,41 @@ static const ARMCPRegInfo v7_cp_reginfo[] = { >>> { .name = "PMCNTENSET", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 1, >>> .access = PL0_RW, .resetvalue = 0, >>> .fieldoffset = offsetof(CPUARMState, cp15.c9_pmcnten), >>> - .readfn = pmreg_read, .writefn = pmcntenset_write, >>> - .raw_readfn = raw_read, .raw_writefn = raw_write }, >>> + .writefn = pmcntenset_write, >>> + .accessfn = pmreg_access, >>> + .raw_writefn = raw_write }, >> >> A nit but, >> >> You're field ordering scheme is inconsistent, here you go, write -> >> access -> raw_write .... >> >>> { .name = "PMCNTENCLR", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 2, >>> .access = PL0_RW, .fieldoffset = offsetof(CPUARMState, cp15.c9_pmcnten), >>> - .readfn = pmreg_read, .writefn = pmcntenclr_write, >>> + .accessfn = pmreg_access, >>> + .writefn = pmcntenclr_write, >>> .type = ARM_CP_NO_MIGRATE }, >>> { .name = "PMOVSR", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 3, >>> .access = PL0_RW, .fieldoffset = offsetof(CPUARMState, cp15.c9_pmovsr), >>> - .readfn = pmreg_read, .writefn = pmovsr_write, >>> - .raw_readfn = raw_read, .raw_writefn = raw_write }, >>> - /* Unimplemented so WI. Strictly speaking write accesses in PL0 should >>> - * respect PMUSERENR. >>> - */ >>> + .accessfn = pmreg_access, >>> + .writefn = pmovsr_write, >>> + .raw_writefn = raw_write }, >> >> ... and this is access -> write -> raw_write. Is there a prescribed >> order to keep new (or gradually refactored) code consistent? > > No, I've just been going for "whatever looks like it fits reasonably > neatly onto not too many lines and is a fairly minimal change to existing > structs", mostly. The thing I have been trying to be consistent with is > the order for crn/crm/opc1/opc2 fields, which for new registers I've been > making be op0/op1/crn/crm/op2, because that's the order the ARM ARM > seems to have settled on. Unfortunately a lot of our existing definitions > are crn/crm/op1/op2, because at the time there was variance in the > ARM docs and that order seemed sensible to me. > > For the other fields, I think "name first; type/state/encoding second; > access related fields; read and write accessors; reset stuff" is probably > not a bad order. But the nice thing about having named fields is it > doesn't actually matter what order things go in. > > thanks > -- PMM >
On Wed, Feb 5, 2014 at 9:01 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 5 February 2014 06:59, Peter Crosthwaite > <peter.crosthwaite@xilinx.com> wrote: >> cc Alistair, this may conflict with his timer work. >> >> On Sat, Feb 1, 2014 at 1:45 AM, Peter Maydell <peter.maydell@linaro.org> wrote: >>> @@ -624,37 +606,41 @@ static const ARMCPRegInfo v7_cp_reginfo[] = { >>> { .name = "PMCNTENSET", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 1, >>> .access = PL0_RW, .resetvalue = 0, >>> .fieldoffset = offsetof(CPUARMState, cp15.c9_pmcnten), >>> - .readfn = pmreg_read, .writefn = pmcntenset_write, >>> - .raw_readfn = raw_read, .raw_writefn = raw_write }, >>> + .writefn = pmcntenset_write, >>> + .accessfn = pmreg_access, >>> + .raw_writefn = raw_write }, >> >> A nit but, >> >> You're field ordering scheme is inconsistent, here you go, write -> >> access -> raw_write .... >> >>> { .name = "PMCNTENCLR", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 2, >>> .access = PL0_RW, .fieldoffset = offsetof(CPUARMState, cp15.c9_pmcnten), >>> - .readfn = pmreg_read, .writefn = pmcntenclr_write, >>> + .accessfn = pmreg_access, >>> + .writefn = pmcntenclr_write, >>> .type = ARM_CP_NO_MIGRATE }, >>> { .name = "PMOVSR", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 3, >>> .access = PL0_RW, .fieldoffset = offsetof(CPUARMState, cp15.c9_pmovsr), >>> - .readfn = pmreg_read, .writefn = pmovsr_write, >>> - .raw_readfn = raw_read, .raw_writefn = raw_write }, >>> - /* Unimplemented so WI. Strictly speaking write accesses in PL0 should >>> - * respect PMUSERENR. >>> - */ >>> + .accessfn = pmreg_access, >>> + .writefn = pmovsr_write, >>> + .raw_writefn = raw_write }, >> >> ... and this is access -> write -> raw_write. Is there a prescribed >> order to keep new (or gradually refactored) code consistent? > > No, I've just been going for "whatever looks like it fits reasonably > neatly onto not too many lines and is a fairly minimal change to existing > structs", mostly. I agree that we should minimise diff. Existing code trumps any new rules we come up with here. But I picked on this one because its inconsistent just within the new changes. The thing I have been trying to be consistent with is > the order for crn/crm/opc1/opc2 fields, which for new registers I've been > making be op0/op1/crn/crm/op2, because that's the order the ARM ARM > seems to have settled on. Unfortunately a lot of our existing definitions > are crn/crm/op1/op2, because at the time there was variance in the > ARM docs and that order seemed sensible to me. > > For the other fields, I think "name first; type/state/encoding second; > access related fields; read and write accessors; reset stuff" is probably > not a bad order. Agreed. With diff-correction based on this scheme: Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> But the nice thing about having named fields is it > doesn't actually matter what order things go in. > But for similar entries its much more readable if they are self-consistent. Regards, Peter > thanks > -- PMM >
On 9 February 2014 02:59, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote: > Agreed. With diff-correction based on this scheme: > > Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> > > But the nice thing about having named fields is it >> doesn't actually matter what order things go in. >> > > But for similar entries its much more readable if they are self-consistent. Yes; this patch in particular was rather inconsistent and I have fixed it. thanks -- PMM
diff --git a/target-arm/helper.c b/target-arm/helper.c index bd78350..9adaeb9 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -485,26 +485,20 @@ static const ARMCPRegInfo v6_cp_reginfo[] = { REGINFO_SENTINEL }; - -static int pmreg_read(CPUARMState *env, const ARMCPRegInfo *ri, - uint64_t *value) +static CPAccessResult pmreg_access(CPUARMState *env, const ARMCPRegInfo *ri) { - /* Generic performance monitor register read function for where - * user access may be allowed by PMUSERENR. + /* Perfomance monitor registers user accessibility is controlled + * by PMUSERENR. */ if (arm_current_pl(env) == 0 && !env->cp15.c9_pmuserenr) { - return EXCP_UDEF; + return CP_ACCESS_TRAP; } - *value = CPREG_FIELD32(env, ri); - return 0; + return CP_ACCESS_OK; } static int pmcr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value) { - if (arm_current_pl(env) == 0 && !env->cp15.c9_pmuserenr) { - return EXCP_UDEF; - } /* only the DP, X, D and E bits are writable */ env->cp15.c9_pmcr &= ~0x39; env->cp15.c9_pmcr |= (value & 0x39); @@ -514,9 +508,6 @@ static int pmcr_write(CPUARMState *env, const ARMCPRegInfo *ri, static int pmcntenset_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value) { - if (arm_current_pl(env) == 0 && !env->cp15.c9_pmuserenr) { - return EXCP_UDEF; - } value &= (1 << 31); env->cp15.c9_pmcnten |= value; return 0; @@ -525,9 +516,6 @@ static int pmcntenset_write(CPUARMState *env, const ARMCPRegInfo *ri, static int pmcntenclr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value) { - if (arm_current_pl(env) == 0 && !env->cp15.c9_pmuserenr) { - return EXCP_UDEF; - } value &= (1 << 31); env->cp15.c9_pmcnten &= ~value; return 0; @@ -536,9 +524,6 @@ static int pmcntenclr_write(CPUARMState *env, const ARMCPRegInfo *ri, static int pmovsr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value) { - if (arm_current_pl(env) == 0 && !env->cp15.c9_pmuserenr) { - return EXCP_UDEF; - } env->cp15.c9_pmovsr &= ~value; return 0; } @@ -546,9 +531,6 @@ static int pmovsr_write(CPUARMState *env, const ARMCPRegInfo *ri, static int pmxevtyper_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value) { - if (arm_current_pl(env) == 0 && !env->cp15.c9_pmuserenr) { - return EXCP_UDEF; - } env->cp15.c9_pmxevtyper = value & 0xff; return 0; } @@ -624,37 +606,41 @@ static const ARMCPRegInfo v7_cp_reginfo[] = { { .name = "PMCNTENSET", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 1, .access = PL0_RW, .resetvalue = 0, .fieldoffset = offsetof(CPUARMState, cp15.c9_pmcnten), - .readfn = pmreg_read, .writefn = pmcntenset_write, - .raw_readfn = raw_read, .raw_writefn = raw_write }, + .writefn = pmcntenset_write, + .accessfn = pmreg_access, + .raw_writefn = raw_write }, { .name = "PMCNTENCLR", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 2, .access = PL0_RW, .fieldoffset = offsetof(CPUARMState, cp15.c9_pmcnten), - .readfn = pmreg_read, .writefn = pmcntenclr_write, + .accessfn = pmreg_access, + .writefn = pmcntenclr_write, .type = ARM_CP_NO_MIGRATE }, { .name = "PMOVSR", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 3, .access = PL0_RW, .fieldoffset = offsetof(CPUARMState, cp15.c9_pmovsr), - .readfn = pmreg_read, .writefn = pmovsr_write, - .raw_readfn = raw_read, .raw_writefn = raw_write }, - /* Unimplemented so WI. Strictly speaking write accesses in PL0 should - * respect PMUSERENR. - */ + .accessfn = pmreg_access, + .writefn = pmovsr_write, + .raw_writefn = raw_write }, + /* Unimplemented so WI. */ { .name = "PMSWINC", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 4, - .access = PL0_W, .type = ARM_CP_NOP }, + .access = PL0_W, .accessfn = pmreg_access, .type = ARM_CP_NOP }, /* Since we don't implement any events, writing to PMSELR is UNPREDICTABLE. - * We choose to RAZ/WI. XXX should respect PMUSERENR. + * We choose to RAZ/WI. */ { .name = "PMSELR", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 5, - .access = PL0_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, - /* Unimplemented, RAZ/WI. XXX PMUSERENR */ + .access = PL0_RW, .type = ARM_CP_CONST, .resetvalue = 0, + .accessfn = pmreg_access }, + /* Unimplemented, RAZ/WI. */ { .name = "PMCCNTR", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 = 0, - .access = PL0_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, + .access = PL0_RW, .type = ARM_CP_CONST, .resetvalue = 0, + .accessfn = pmreg_access }, { .name = "PMXEVTYPER", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 = 1, .access = PL0_RW, .fieldoffset = offsetof(CPUARMState, cp15.c9_pmxevtyper), - .readfn = pmreg_read, .writefn = pmxevtyper_write, - .raw_readfn = raw_read, .raw_writefn = raw_write }, - /* Unimplemented, RAZ/WI. XXX PMUSERENR */ + .accessfn = pmreg_access, .writefn = pmxevtyper_write, + .raw_writefn = raw_write }, + /* Unimplemented, RAZ/WI. */ { .name = "PMXEVCNTR", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 = 2, - .access = PL0_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, + .access = PL0_RW, .type = ARM_CP_CONST, .resetvalue = 0, + .accessfn = pmreg_access }, { .name = "PMUSERENR", .cp = 15, .crn = 9, .crm = 14, .opc1 = 0, .opc2 = 0, .access = PL0_R | PL1_RW, .fieldoffset = offsetof(CPUARMState, cp15.c9_pmuserenr), @@ -1708,8 +1694,8 @@ void register_cp_regs_for_features(ARMCPU *cpu) .name = "PMCR", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 0, .access = PL0_RW, .resetvalue = cpu->midr & 0xff000000, .fieldoffset = offsetof(CPUARMState, cp15.c9_pmcr), - .readfn = pmreg_read, .writefn = pmcr_write, - .raw_readfn = raw_read, .raw_writefn = raw_write, + .accessfn = pmreg_access, .writefn = pmcr_write, + .raw_writefn = raw_write, }; ARMCPRegInfo clidr = { .name = "CLIDR", .cp = 15, .crn = 0, .crm = 0, .opc1 = 1, .opc2 = 1,
Convert the performance monitor reginfo definitions to use an accessfn rather than returning EXCP_UDEF from read and write functions. This also allows us to fix a couple of XXX cases where we weren't imposing the access restrictions on RAZ/WI or constant registers. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target-arm/helper.c | 70 +++++++++++++++++++++-------------------------------- 1 file changed, 28 insertions(+), 42 deletions(-)