| Message ID | 20230609172324.982888-2-aaron@os.amperecomputing.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Implement Most ARMv8.3 Pointer Authentication Features | expand |
On 6/9/23 10:23, Aaron Lindsay wrote: > --- a/target/arm/hvf/hvf.c > +++ b/target/arm/hvf/hvf.c > @@ -847,6 +847,7 @@ static bool hvf_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) > { HV_SYS_REG_ID_AA64DFR1_EL1, &host_isar.id_aa64dfr1 }, > { HV_SYS_REG_ID_AA64ISAR0_EL1, &host_isar.id_aa64isar0 }, > { HV_SYS_REG_ID_AA64ISAR1_EL1, &host_isar.id_aa64isar1 }, > + { HV_SYS_REG_ID_AA64ISAR2_EL1, &host_isar.id_aa64isar2 }, Sadly not defined for MacOSX13.1.sdk, and it's an enum so you can't #ifdef it either. You'll need a meson probe for it. Otherwise, looks good. r~
On Jun 09 13:49, Richard Henderson wrote: > On 6/9/23 10:23, Aaron Lindsay wrote: > > --- a/target/arm/hvf/hvf.c > > +++ b/target/arm/hvf/hvf.c > > @@ -847,6 +847,7 @@ static bool hvf_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) > > { HV_SYS_REG_ID_AA64DFR1_EL1, &host_isar.id_aa64dfr1 }, > > { HV_SYS_REG_ID_AA64ISAR0_EL1, &host_isar.id_aa64isar0 }, > > { HV_SYS_REG_ID_AA64ISAR1_EL1, &host_isar.id_aa64isar1 }, > > + { HV_SYS_REG_ID_AA64ISAR2_EL1, &host_isar.id_aa64isar2 }, > > Sadly not defined for MacOSX13.1.sdk, and it's an enum so you can't #ifdef it either. > > You'll need a meson probe for it. I'm not very familiar with HVF or meson - I am not sure I understand what you're suggesting here (and a few attempts to grep around for an example didn't turn up anything that looked helpful). Are you suggesting some sort of build-time auto-detection, a "dumb" configuration switch that a user could use to manually enable this, or something else? And/or is there an example you could point me to of what you're thinking? -Aaron
On Mon, 12 Jun 2023 at 14:18, Aaron Lindsay <aaron@os.amperecomputing.com> wrote: > > On Jun 09 13:49, Richard Henderson wrote: > > On 6/9/23 10:23, Aaron Lindsay wrote: > > > --- a/target/arm/hvf/hvf.c > > > +++ b/target/arm/hvf/hvf.c > > > @@ -847,6 +847,7 @@ static bool hvf_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) > > > { HV_SYS_REG_ID_AA64DFR1_EL1, &host_isar.id_aa64dfr1 }, > > > { HV_SYS_REG_ID_AA64ISAR0_EL1, &host_isar.id_aa64isar0 }, > > > { HV_SYS_REG_ID_AA64ISAR1_EL1, &host_isar.id_aa64isar1 }, > > > + { HV_SYS_REG_ID_AA64ISAR2_EL1, &host_isar.id_aa64isar2 }, > > > > Sadly not defined for MacOSX13.1.sdk, and it's an enum so you can't #ifdef it either. > > > > You'll need a meson probe for it. > > I'm not very familiar with HVF or meson - I am not sure I understand > what you're suggesting here (and a few attempts to grep around for an > example didn't turn up anything that looked helpful). Are you suggesting > some sort of build-time auto-detection, a "dumb" configuration switch > that a user could use to manually enable this, or something else? And/or > is there an example you could point me to of what you're thinking? So the first thing here is: where is HV_SYS_REG_ID_AA64ISAR2_EL1 defined in the first place? https://developer.apple.com/documentation/hypervisor/hv_sys_reg_t does not list it. If this is really the right name for the value, then: We do our build-time detection of stuff that might or might not be present using meson's facilities or that kind of thing. These are all in meson.build. In this case I think that what you want to use is has_header_symbol(), which checks for presence of some symbol in a given header. There's examples in meson.build, you want something like config_host_data.set('HAVE_HV_SYS_REG_ID_AA64ISAR2_EL1', cc.has_header_symbol('whatever.h', 'HV_SYS_REG_ID_AA64ISAR2_EL1')) which tells meson "if this header has this symbol then define this preprocessor value HAVE_...". Then you can #ifdef on that. (We're inconsistent about whether we use CONFIG_FOO or HAVE_FOO for this sort of test.) Or alternatively, since this is macos specific and Apple are quite careful about API versioning, it may be simpler to use macos's own version macros to ifdef things so we only try to use the enum value when building for a macos version that knows about it. Grepping for 'MAC_OS_VERSION' brings up examples of that approach. thanks -- PMM
On 6/19/23 12:51, Peter Maydell wrote: > On Mon, 12 Jun 2023 at 14:18, Aaron Lindsay > <aaron@os.amperecomputing.com> wrote: >> >> On Jun 09 13:49, Richard Henderson wrote: >>> On 6/9/23 10:23, Aaron Lindsay wrote: >>>> --- a/target/arm/hvf/hvf.c >>>> +++ b/target/arm/hvf/hvf.c >>>> @@ -847,6 +847,7 @@ static bool hvf_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) >>>> { HV_SYS_REG_ID_AA64DFR1_EL1, &host_isar.id_aa64dfr1 }, >>>> { HV_SYS_REG_ID_AA64ISAR0_EL1, &host_isar.id_aa64isar0 }, >>>> { HV_SYS_REG_ID_AA64ISAR1_EL1, &host_isar.id_aa64isar1 }, >>>> + { HV_SYS_REG_ID_AA64ISAR2_EL1, &host_isar.id_aa64isar2 }, >>> >>> Sadly not defined for MacOSX13.1.sdk, and it's an enum so you can't #ifdef it either. >>> >>> You'll need a meson probe for it. >> >> I'm not very familiar with HVF or meson - I am not sure I understand >> what you're suggesting here (and a few attempts to grep around for an >> example didn't turn up anything that looked helpful). Are you suggesting >> some sort of build-time auto-detection, a "dumb" configuration switch >> that a user could use to manually enable this, or something else? And/or >> is there an example you could point me to of what you're thinking? > > So the first thing here is: where is HV_SYS_REG_ID_AA64ISAR2_EL1 > defined in the first place? > https://developer.apple.com/documentation/hypervisor/hv_sys_reg_t > does not list it. > > If this is really the right name for the value, then: > > We do our build-time detection of stuff that might or might > not be present using meson's facilities or that kind of > thing. These are all in meson.build. In this case I think > that what you want to use is has_header_symbol(), which > checks for presence of some symbol in a given header. There's > examples in meson.build, you want something like > > config_host_data.set('HAVE_HV_SYS_REG_ID_AA64ISAR2_EL1', > cc.has_header_symbol('whatever.h', > 'HV_SYS_REG_ID_AA64ISAR2_EL1')) > > which tells meson "if this header has this symbol then define > this preprocessor value HAVE_...". Then you can #ifdef on that. > (We're inconsistent about whether we use CONFIG_FOO or HAVE_FOO > for this sort of test.) > > Or alternatively, since this is macos specific and Apple are > quite careful about API versioning, it may be simpler to use > macos's own version macros to ifdef things so we only try to > use the enum value when building for a macos version that knows > about it. Grepping for 'MAC_OS_VERSION' brings up examples of > that approach. AFAICS, there is no macos version that contains this register, because there is no Apple cpu that contains it. I think we should be fine, for now, in omitting any HVF change, letting the register default to 0. r~
diff --git a/target/arm/cpu.h b/target/arm/cpu.h index 36c608f0e6..df04c9a9ab 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -1023,6 +1023,7 @@ struct ArchCPU { uint32_t dbgdevid1; uint64_t id_aa64isar0; uint64_t id_aa64isar1; + uint64_t id_aa64isar2; uint64_t id_aa64pfr0; uint64_t id_aa64pfr1; uint64_t id_aa64mmfr0; diff --git a/target/arm/helper.c b/target/arm/helper.c index d4bee43bd0..4ced2f71e5 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -8204,11 +8204,11 @@ void register_cp_regs_for_features(ARMCPU *cpu) .access = PL1_R, .type = ARM_CP_CONST, .accessfn = access_aa64_tid3, .resetvalue = cpu->isar.id_aa64isar1 }, - { .name = "ID_AA64ISAR2_EL1_RESERVED", .state = ARM_CP_STATE_AA64, + { .name = "ID_AA64ISAR2_EL1", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 6, .opc2 = 2, .access = PL1_R, .type = ARM_CP_CONST, .accessfn = access_aa64_tid3, - .resetvalue = 0 }, + .resetvalue = cpu->isar.id_aa64isar2 }, { .name = "ID_AA64ISAR3_EL1_RESERVED", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 6, .opc2 = 3, .access = PL1_R, .type = ARM_CP_CONST, diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c index 8f72624586..bf567b24db 100644 --- a/target/arm/hvf/hvf.c +++ b/target/arm/hvf/hvf.c @@ -847,6 +847,7 @@ static bool hvf_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) { HV_SYS_REG_ID_AA64DFR1_EL1, &host_isar.id_aa64dfr1 }, { HV_SYS_REG_ID_AA64ISAR0_EL1, &host_isar.id_aa64isar0 }, { HV_SYS_REG_ID_AA64ISAR1_EL1, &host_isar.id_aa64isar1 }, + { HV_SYS_REG_ID_AA64ISAR2_EL1, &host_isar.id_aa64isar2 }, { HV_SYS_REG_ID_AA64MMFR0_EL1, &host_isar.id_aa64mmfr0 }, { HV_SYS_REG_ID_AA64MMFR1_EL1, &host_isar.id_aa64mmfr1 }, { HV_SYS_REG_ID_AA64MMFR2_EL1, &host_isar.id_aa64mmfr2 }, diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c index 94bbd9661f..e2d05d7fc0 100644 --- a/target/arm/kvm64.c +++ b/target/arm/kvm64.c @@ -306,6 +306,8 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) ARM64_SYS_REG(3, 0, 0, 6, 0)); err |= read_sys_reg64(fdarray[2], &ahcf->isar.id_aa64isar1, ARM64_SYS_REG(3, 0, 0, 6, 1)); + err |= read_sys_reg64(fdarray[2], &ahcf->isar.id_aa64isar2, + ARM64_SYS_REG(3, 0, 0, 6, 2)); err |= read_sys_reg64(fdarray[2], &ahcf->isar.id_aa64mmfr0, ARM64_SYS_REG(3, 0, 0, 7, 0)); err |= read_sys_reg64(fdarray[2], &ahcf->isar.id_aa64mmfr1,
Signed-off-by: Aaron Lindsay <aaron@os.amperecomputing.com> --- target/arm/cpu.h | 1 + target/arm/helper.c | 4 ++-- target/arm/hvf/hvf.c | 1 + target/arm/kvm64.c | 2 ++ 4 files changed, 6 insertions(+), 2 deletions(-)