| Message ID | 20191206122247.7507-1-alex.bennee@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | target/arm: don't bother with id_aa64pfr0_read for USER_ONLY | expand |
On Fri, 6 Dec 2019 at 12:22, Alex Bennée <alex.bennee@linaro.org> wrote: > > For system emulation we need to check the state of the GIC before we > report the value. However this isn't relevant to exporting of the > value to linux-user and indeed breaks the exported value as set by > modify_arm_cp_regs. > > [AJB: the other option would be just to set reset value anyway and not > ifdef out the readfn as the register will become const anyway] If you want it to be const it would be clearer to define it with ARM_CP_CONST... I'm not sure what an ARM_CP_NO_RAW without a readfn or a fieldoffset will do on reads. thanks -- PMM
On 12/6/19 7:29 AM, Peter Maydell wrote: > On Fri, 6 Dec 2019 at 12:22, Alex Bennée <alex.bennee@linaro.org> wrote: >> >> For system emulation we need to check the state of the GIC before we >> report the value. However this isn't relevant to exporting of the >> value to linux-user and indeed breaks the exported value as set by >> modify_arm_cp_regs. >> >> [AJB: the other option would be just to set reset value anyway and not >> ifdef out the readfn as the register will become const anyway] > > If you want it to be const it would be clearer to define it > with ARM_CP_CONST... I'm not sure what an ARM_CP_NO_RAW without > a readfn or a fieldoffset will do on reads. Yep, and the accessfn should be ifdefed away as well. r~
Peter Maydell <peter.maydell@linaro.org> writes: > On Fri, 6 Dec 2019 at 12:22, Alex Bennée <alex.bennee@linaro.org> wrote: >> >> For system emulation we need to check the state of the GIC before we >> report the value. However this isn't relevant to exporting of the >> value to linux-user and indeed breaks the exported value as set by >> modify_arm_cp_regs. >> >> [AJB: the other option would be just to set reset value anyway and not >> ifdef out the readfn as the register will become const anyway] > > If you want it to be const it would be clearer to define it > with ARM_CP_CONST... I'm not sure what an ARM_CP_NO_RAW without > a readfn or a fieldoffset will do on reads. Well the modify_arm_cp_regs ensures it is ARM_CP_CONST when it changes the definition. It's just ensuring the reset value is set so it can be masked/fixed. However the ifdef approach does reduce the amount of unused stuff in the linux-user build. > > thanks > -- PMM -- Alex Bennée
diff --git a/target/arm/helper.c b/target/arm/helper.c index f78dd3b5fe3..489c31504a6 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -5867,6 +5867,7 @@ static uint64_t id_pfr1_read(CPUARMState *env, const ARMCPRegInfo *ri) return pfr1; } +#ifndef CONFIG_USER_ONLY static uint64_t id_aa64pfr0_read(CPUARMState *env, const ARMCPRegInfo *ri) { ARMCPU *cpu = env_archcpu(env); @@ -5877,6 +5878,7 @@ static uint64_t id_aa64pfr0_read(CPUARMState *env, const ARMCPRegInfo *ri) } return pfr0; } +#endif /* Shared logic between LORID and the rest of the LOR* registers. * Secure state has already been delt with. @@ -6297,16 +6299,22 @@ void register_cp_regs_for_features(ARMCPU *cpu) * define new registers here. */ ARMCPRegInfo v8_idregs[] = { - /* ID_AA64PFR0_EL1 is not a plain ARM_CP_CONST because we don't - * know the right value for the GIC field until after we - * define these regs. + /* + * ID_AA64PFR0_EL1 is not a plain ARM_CP_CONST in system + * emulation because we don't know the right value for the + * GIC field until after we define these regs. */ { .name = "ID_AA64PFR0_EL1", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 4, .opc2 = 0, .access = PL1_R, .type = ARM_CP_NO_RAW, .accessfn = access_aa64_tid3, +#ifdef CONFIG_USER_ONLY + .resetvalue = cpu->isar.id_aa64pfr0 +#else .readfn = id_aa64pfr0_read, - .writefn = arm_cp_write_ignore }, + .writefn = arm_cp_write_ignore +#endif + }, { .name = "ID_AA64PFR1_EL1", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 4, .opc2 = 1, .access = PL1_R, .type = ARM_CP_CONST,
For system emulation we need to check the state of the GIC before we report the value. However this isn't relevant to exporting of the value to linux-user and indeed breaks the exported value as set by modify_arm_cp_regs. [AJB: the other option would be just to set reset value anyway and not ifdef out the readfn as the register will become const anyway] Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- target/arm/helper.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) -- 2.20.1