A number of CPUID registers are exposed to userspace by modern Linux
kernels thanks to the "ARM64 CPU Feature Registers" ABI. For QEMU's
user-mode emulation we don't need to emulate the kernels trap but just
return the value the trap would have done. For this we use the PL0U_R
permission mask which allows this access in CONFIG_USER mode.
Some registers only return a subset of their contents so we need
specific CONFIG_USER_ONLY logic to do this.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
v4
- tweak commit message
- use PL0U_R instead of PL1U_R to be less confusing
- more CONFIG_USER logic for special cases
- mask a bunch of bits for some registers
---
target/arm/helper.c | 51 ++++++++++++++++++++++++++++++++-------------
1 file changed, 36 insertions(+), 15 deletions(-)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 42c1c0b144..68808e7293 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -3543,7 +3543,7 @@ static uint64_t mpidr_read(CPUARMState *env, const ARMCPRegInfo *ri)
static const ARMCPRegInfo mpidr_cp_reginfo[] = {
{ .name = "MPIDR", .state = ARM_CP_STATE_BOTH,
.opc0 = 3, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 5,
- .access = PL1_R, .readfn = mpidr_read, .type = ARM_CP_NO_RAW },
+ .access = PL0U_R, .readfn = mpidr_read, .type = ARM_CP_NO_RAW },
REGINFO_SENTINEL
};
@@ -5488,6 +5488,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 = arm_env_get_cpu(env);
@@ -5498,6 +5499,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.
@@ -5799,18 +5801,26 @@ 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 for system
+ * emulation because we don't know the right value for the
+ * GIC field until after we define these regs. For
+ * user-mode HWCAP_CPUID emulation the GIC bits are masked
+ * anyway.
*/
{ .name = "ID_AA64PFR0_EL1", .state = ARM_CP_STATE_AA64,
.opc0 = 3, .opc1 = 0, .crn = 0, .crm = 4, .opc2 = 0,
+#ifndef CONFIG_USER_ONLY
.access = PL1_R, .type = ARM_CP_NO_RAW,
.readfn = id_aa64pfr0_read,
- .writefn = arm_cp_write_ignore },
+ .writefn = arm_cp_write_ignore
+#else
+ .access = PL0U_R, .type = ARM_CP_CONST,
+ .resetvalue = cpu->isar.id_aa64pfr0 & 0x000f000f0ff0000ULL
+#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,
+ .access = PL0U_R, .type = ARM_CP_CONST,
.resetvalue = cpu->isar.id_aa64pfr1},
{ .name = "ID_AA64PFR2_EL1_RESERVED", .state = ARM_CP_STATE_AA64,
.opc0 = 3, .opc1 = 0, .crn = 0, .crm = 4, .opc2 = 2,
@@ -5839,11 +5849,11 @@ void register_cp_regs_for_features(ARMCPU *cpu)
.resetvalue = 0 },
{ .name = "ID_AA64DFR0_EL1", .state = ARM_CP_STATE_AA64,
.opc0 = 3, .opc1 = 0, .crn = 0, .crm = 5, .opc2 = 0,
- .access = PL1_R, .type = ARM_CP_CONST,
+ .access = PL0U_R, .type = ARM_CP_CONST,
.resetvalue = cpu->id_aa64dfr0 },
{ .name = "ID_AA64DFR1_EL1", .state = ARM_CP_STATE_AA64,
.opc0 = 3, .opc1 = 0, .crn = 0, .crm = 5, .opc2 = 1,
- .access = PL1_R, .type = ARM_CP_CONST,
+ .access = PL0U_R, .type = ARM_CP_CONST,
.resetvalue = cpu->id_aa64dfr1 },
{ .name = "ID_AA64DFR2_EL1_RESERVED", .state = ARM_CP_STATE_AA64,
.opc0 = 3, .opc1 = 0, .crn = 0, .crm = 5, .opc2 = 2,
@@ -5871,11 +5881,16 @@ void register_cp_regs_for_features(ARMCPU *cpu)
.resetvalue = 0 },
{ .name = "ID_AA64ISAR0_EL1", .state = ARM_CP_STATE_AA64,
.opc0 = 3, .opc1 = 0, .crn = 0, .crm = 6, .opc2 = 0,
- .access = PL1_R, .type = ARM_CP_CONST,
- .resetvalue = cpu->isar.id_aa64isar0 },
+ .access = PL0U_R, .type = ARM_CP_CONST,
+#ifdef CONFIG_USER_ONLY
+ .resetvalue = cpu->isar.id_aa64isar0 & 0x000fffffff0ffff0ULL
+#else
+ .resetvalue = cpu->isar.id_aa64isar0
+#endif
+ },
{ .name = "ID_AA64ISAR1_EL1", .state = ARM_CP_STATE_AA64,
.opc0 = 3, .opc1 = 0, .crn = 0, .crm = 6, .opc2 = 1,
- .access = PL1_R, .type = ARM_CP_CONST,
+ .access = PL0U_R, .type = ARM_CP_CONST,
.resetvalue = cpu->isar.id_aa64isar1 },
{ .name = "ID_AA64ISAR2_EL1_RESERVED", .state = ARM_CP_STATE_AA64,
.opc0 = 3, .opc1 = 0, .crn = 0, .crm = 6, .opc2 = 2,
@@ -5903,11 +5918,11 @@ void register_cp_regs_for_features(ARMCPU *cpu)
.resetvalue = 0 },
{ .name = "ID_AA64MMFR0_EL1", .state = ARM_CP_STATE_AA64,
.opc0 = 3, .opc1 = 0, .crn = 0, .crm = 7, .opc2 = 0,
- .access = PL1_R, .type = ARM_CP_CONST,
+ .access = PL0U_R, .type = ARM_CP_CONST,
.resetvalue = cpu->isar.id_aa64mmfr0 },
{ .name = "ID_AA64MMFR1_EL1", .state = ARM_CP_STATE_AA64,
.opc0 = 3, .opc1 = 0, .crn = 0, .crm = 7, .opc2 = 1,
- .access = PL1_R, .type = ARM_CP_CONST,
+ .access = PL0U_R, .type = ARM_CP_CONST,
.resetvalue = cpu->isar.id_aa64mmfr1 },
{ .name = "ID_AA64MMFR2_EL1_RESERVED", .state = ARM_CP_STATE_AA64,
.opc0 = 3, .opc1 = 0, .crn = 0, .crm = 7, .opc2 = 2,
@@ -6211,7 +6226,7 @@ void register_cp_regs_for_features(ARMCPU *cpu)
ARMCPRegInfo id_v8_midr_cp_reginfo[] = {
{ .name = "MIDR_EL1", .state = ARM_CP_STATE_BOTH,
.opc0 = 3, .opc1 = 0, .crn = 0, .crm = 0, .opc2 = 0,
- .access = PL1_R, .type = ARM_CP_NO_RAW, .resetvalue = cpu->midr,
+ .access = PL0U_R, .type = ARM_CP_NO_RAW, .resetvalue = cpu->midr,
.fieldoffset = offsetof(CPUARMState, cp15.c0_cpuid),
.readfn = midr_read },
/* crn = 0 op1 = 0 crm = 0 op2 = 4,7 : AArch32 aliases of MIDR */
@@ -6223,7 +6238,13 @@ void register_cp_regs_for_features(ARMCPU *cpu)
.access = PL1_R, .resetvalue = cpu->midr },
{ .name = "REVIDR_EL1", .state = ARM_CP_STATE_BOTH,
.opc0 = 3, .opc1 = 0, .crn = 0, .crm = 0, .opc2 = 6,
- .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = cpu->revidr },
+#ifdef CONFIG_USER_ONLY
+ .access = PL0U_R, .type = ARM_CP_CONST,
+ .resetvalue = 0 /* HW_CPUID IMPDEF fields are 0 */ },
+#else
+ .access = PL1_R, .type = ARM_CP_CONST,
+ .resetvalue = cpu->revidr },
+#endif
REGINFO_SENTINEL
};
ARMCPRegInfo id_cp_reginfo[] = {
--
2.17.1
On Mon, 28 Jan 2019 at 17:39, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> A number of CPUID registers are exposed to userspace by modern Linux
> kernels thanks to the "ARM64 CPU Feature Registers" ABI. For QEMU's
> user-mode emulation we don't need to emulate the kernels trap but just
> return the value the trap would have done. For this we use the PL0U_R
> permission mask which allows this access in CONFIG_USER mode.
>
> Some registers only return a subset of their contents so we need
> specific CONFIG_USER_ONLY logic to do this.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> ---
> v4
> - tweak commit message
> - use PL0U_R instead of PL1U_R to be less confusing
> - more CONFIG_USER logic for special cases
> - mask a bunch of bits for some registers
> ---
> target/arm/helper.c | 51 ++++++++++++++++++++++++++++++++-------------
> 1 file changed, 36 insertions(+), 15 deletions(-)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 42c1c0b144..68808e7293 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -3543,7 +3543,7 @@ static uint64_t mpidr_read(CPUARMState *env, const ARMCPRegInfo *ri)
> static const ARMCPRegInfo mpidr_cp_reginfo[] = {
> { .name = "MPIDR", .state = ARM_CP_STATE_BOTH,
> .opc0 = 3, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 5,
> - .access = PL1_R, .readfn = mpidr_read, .type = ARM_CP_NO_RAW },
> + .access = PL0U_R, .readfn = mpidr_read, .type = ARM_CP_NO_RAW },
> REGINFO_SENTINEL
> };
>
> @@ -5488,6 +5488,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 = arm_env_get_cpu(env);
> @@ -5498,6 +5499,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.
> @@ -5799,18 +5801,26 @@ 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 for system
> + * emulation because we don't know the right value for the
> + * GIC field until after we define these regs. For
> + * user-mode HWCAP_CPUID emulation the GIC bits are masked
> + * anyway.
> */
> { .name = "ID_AA64PFR0_EL1", .state = ARM_CP_STATE_AA64,
> .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 4, .opc2 = 0,
> +#ifndef CONFIG_USER_ONLY
> .access = PL1_R, .type = ARM_CP_NO_RAW,
> .readfn = id_aa64pfr0_read,
> - .writefn = arm_cp_write_ignore },
> + .writefn = arm_cp_write_ignore
> +#else
> + .access = PL0U_R, .type = ARM_CP_CONST,
> + .resetvalue = cpu->isar.id_aa64pfr0 & 0x000f000f0ff0000ULL
> +#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,
> + .access = PL0U_R, .type = ARM_CP_CONST,
> .resetvalue = cpu->isar.id_aa64pfr1},
> { .name = "ID_AA64PFR2_EL1_RESERVED", .state = ARM_CP_STATE_AA64,
> .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 4, .opc2 = 2,
> @@ -5839,11 +5849,11 @@ void register_cp_regs_for_features(ARMCPU *cpu)
> .resetvalue = 0 },
> { .name = "ID_AA64DFR0_EL1", .state = ARM_CP_STATE_AA64,
> .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 5, .opc2 = 0,
> - .access = PL1_R, .type = ARM_CP_CONST,
> + .access = PL0U_R, .type = ARM_CP_CONST,
> .resetvalue = cpu->id_aa64dfr0 },
This doesn't seem right -- I think ID_AA64DFR0_EL1 should be exposed
as zero, not whatever the underlying register value is.
More generally, this ifdeffing of "maybe mask the values" doesn't
seem very future-proof. If we add something to one of the cpu-id_*
values, that should be masked out to zero by default for linux-user,
not defaulting to passed through.
thanks
-- PMM
Peter Maydell <peter.maydell@linaro.org> writes:
> On Mon, 28 Jan 2019 at 17:39, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> A number of CPUID registers are exposed to userspace by modern Linux
>> kernels thanks to the "ARM64 CPU Feature Registers" ABI. For QEMU's
>> user-mode emulation we don't need to emulate the kernels trap but just
>> return the value the trap would have done. For this we use the PL0U_R
>> permission mask which allows this access in CONFIG_USER mode.
>>
>> Some registers only return a subset of their contents so we need
>> specific CONFIG_USER_ONLY logic to do this.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>
>> ---
>> v4
>> - tweak commit message
>> - use PL0U_R instead of PL1U_R to be less confusing
>> - more CONFIG_USER logic for special cases
>> - mask a bunch of bits for some registers
>> ---
>> target/arm/helper.c | 51 ++++++++++++++++++++++++++++++++-------------
>> 1 file changed, 36 insertions(+), 15 deletions(-)
>>
>> diff --git a/target/arm/helper.c b/target/arm/helper.c
>> index 42c1c0b144..68808e7293 100644
>> --- a/target/arm/helper.c
>> +++ b/target/arm/helper.c
>> @@ -3543,7 +3543,7 @@ static uint64_t mpidr_read(CPUARMState *env, const ARMCPRegInfo *ri)
>> static const ARMCPRegInfo mpidr_cp_reginfo[] = {
>> { .name = "MPIDR", .state = ARM_CP_STATE_BOTH,
>> .opc0 = 3, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 5,
>> - .access = PL1_R, .readfn = mpidr_read, .type = ARM_CP_NO_RAW },
>> + .access = PL0U_R, .readfn = mpidr_read, .type = ARM_CP_NO_RAW },
>> REGINFO_SENTINEL
>> };
>>
>> @@ -5488,6 +5488,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 = arm_env_get_cpu(env);
>> @@ -5498,6 +5499,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.
>> @@ -5799,18 +5801,26 @@ 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 for system
>> + * emulation because we don't know the right value for the
>> + * GIC field until after we define these regs. For
>> + * user-mode HWCAP_CPUID emulation the GIC bits are masked
>> + * anyway.
>> */
>> { .name = "ID_AA64PFR0_EL1", .state = ARM_CP_STATE_AA64,
>> .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 4, .opc2 = 0,
>> +#ifndef CONFIG_USER_ONLY
>> .access = PL1_R, .type = ARM_CP_NO_RAW,
>> .readfn = id_aa64pfr0_read,
>> - .writefn = arm_cp_write_ignore },
>> + .writefn = arm_cp_write_ignore
>> +#else
>> + .access = PL0U_R, .type = ARM_CP_CONST,
>> + .resetvalue = cpu->isar.id_aa64pfr0 & 0x000f000f0ff0000ULL
>> +#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,
>> + .access = PL0U_R, .type = ARM_CP_CONST,
>> .resetvalue = cpu->isar.id_aa64pfr1},
>> { .name = "ID_AA64PFR2_EL1_RESERVED", .state = ARM_CP_STATE_AA64,
>> .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 4, .opc2 = 2,
>> @@ -5839,11 +5849,11 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>> .resetvalue = 0 },
>> { .name = "ID_AA64DFR0_EL1", .state = ARM_CP_STATE_AA64,
>> .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 5, .opc2 = 0,
>> - .access = PL1_R, .type = ARM_CP_CONST,
>> + .access = PL0U_R, .type = ARM_CP_CONST,
>> .resetvalue = cpu->id_aa64dfr0 },
>
> This doesn't seem right -- I think ID_AA64DFR0_EL1 should be exposed
> as zero, not whatever the underlying register value is.
Under a kernel I'm seeing numbers reported so I don't think it's totally
squashed:
id_aa64dfr0_el1 : 0x0000000000000006
id_aa64dfr1_el1 : 0x0000000000000000
Confusingly the kernel does:
static const struct arm64_ftr_bits ftr_id_aa64dfr0[] = {
ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, 36, 28, 0),
ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64DFR0_PMSVER_SHIFT, 4, 0),
ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64DFR0_CTX_CMPS_SHIFT, 4, 0),
ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64DFR0_WRPS_SHIFT, 4, 0),
ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64DFR0_BRPS_SHIFT, 4, 0),
/*
* We can instantiate multiple PMU instances with different levels
* of support.
*/
S_ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_EXACT, ID_AA64DFR0_PMUVER_SHIFT, 4, 0),
ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, ID_AA64DFR0_TRACEVER_SHIFT, 4, 0),
ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, ID_AA64DFR0_DEBUGVER_SHIFT, 4, 0x6),
ARM64_FTR_END,
};
So I'm not sure if that is intentional as the line:
ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, ID_AA64DFR0_DEBUGVER_SHIFT, 4, 0x6)
implies the DEBUGVER is hidden but actually 0x6?
> More generally, this ifdeffing of "maybe mask the values" doesn't
> seem very future-proof. If we add something to one of the cpu-id_*
> values, that should be masked out to zero by default for linux-user,
> not defaulting to passed through.
Do you mean setting cpu->id_foo during cpu_init? I wasn't sure if that
might interact badly with the recent changes to base our features off
what's actually set in them.
>
> thanks
> -- PMM
--
Alex Bennée
On Mon, 4 Feb 2019 at 15:33, Alex Bennée <alex.bennee@linaro.org> wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
> > This doesn't seem right -- I think ID_AA64DFR0_EL1 should be exposed
> > as zero, not whatever the underlying register value is.
>
> Under a kernel I'm seeing numbers reported so I don't think it's totally
> squashed:
>
> id_aa64dfr0_el1 : 0x0000000000000006
> id_aa64dfr1_el1 : 0x0000000000000000
>
> Confusingly the kernel does:
>
> static const struct arm64_ftr_bits ftr_id_aa64dfr0[] = {
> ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, 36, 28, 0),
> ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64DFR0_PMSVER_SHIFT, 4, 0),
> ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64DFR0_CTX_CMPS_SHIFT, 4, 0),
> ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64DFR0_WRPS_SHIFT, 4, 0),
> ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64DFR0_BRPS_SHIFT, 4, 0),
> /*
> * We can instantiate multiple PMU instances with different levels
> * of support.
> */
> S_ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_EXACT, ID_AA64DFR0_PMUVER_SHIFT, 4, 0),
> ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, ID_AA64DFR0_TRACEVER_SHIFT, 4, 0),
> ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, ID_AA64DFR0_DEBUGVER_SHIFT, 4, 0x6),
> ARM64_FTR_END,
> };
>
> So I'm not sure if that is intentional as the line:
>
> ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_EXACT, ID_AA64DFR0_DEBUGVER_SHIFT, 4, 0x6)
>
> implies the DEBUGVER is hidden but actually 0x6?
I think this is because the field in the register is architecturally
defined to have a restricted set of permitted values, of which 0 is
not one. (6 means "ARMv8 debug architecture" so is the effective minimum.)
> > More generally, this ifdeffing of "maybe mask the values" doesn't
> > seem very future-proof. If we add something to one of the cpu-id_*
> > values, that should be masked out to zero by default for linux-user,
> > not defaulting to passed through.
>
> Do you mean setting cpu->id_foo during cpu_init? I wasn't sure if that
> might interact badly with the recent changes to base our features off
> what's actually set in them.
No, I meant having something where we either:
(a) have a separate set of cpreg structs for the userspace visible versions,
or (b) have some code which modifies the cpreg structs in v8_idregs[] (it's
not const) before handing it to define_arm_cp_regs().
(b) sounds better to me -- the ideal here I think would be something where we:
* mark all the regs in the space the kernel covers as PL0U_R with a bit
of code rather than by changing all the cpreg structs
* default them to "no bits of the real value are exposed, all bits 0"
* have a hopefully short list of overrides, probably defined in a special
purpose array with entries like
{ /* ID_AA64DFR0_EL1 */, .crm = 5, .opc2 = 0, .passbits = 0x0,
.fixedbits = 0x6 },
Or we could match the override entries with cpreg values by register name,
which would let you say
{ "ID_AA64DFR0_EL1", .passbits = 0x0, .fixedbits = 0x6 },
and avoid having to repeat the encoding values, which might be neater.
thanks
-- PMM
© 2016 - 2026 Red Hat, Inc.