* [PATCH] hw/arm/virt: KVM: Enable PAuth when supported by the host @ 2021-12-28 18:23 Marc Zyngier 2022-01-03 13:46 ` Andrew Jones 0 siblings, 1 reply; 4+ messages in thread From: Marc Zyngier @ 2021-12-28 18:23 UTC (permalink / raw) To: qemu-devel Cc: Peter Maydell, Andrew Jones, kvm, Richard Henderson, Eric Auger, kernel-team, kvmarm Add basic support for Pointer Authentication when running a KVM guest and that the host supports it, loosely based on the SVE support. Although the feature is enabled by default when the host advertises it, it is possible to disable it by setting the 'pauth=off' CPU property. Tested on an Apple M1 running 5.16-rc6. Cc: Eric Auger <eric.auger@redhat.com> Cc: Andrew Jones <drjones@redhat.com> Cc: Richard Henderson <richard.henderson@linaro.org> Cc: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Marc Zyngier <maz@kernel.org> --- docs/system/arm/cpu-features.rst | 5 +++++ target/arm/cpu.c | 1 + target/arm/cpu.h | 1 + target/arm/cpu64.c | 36 ++++++++++++++++++++++++++++++++ target/arm/kvm.c | 13 ++++++++++++ target/arm/kvm64.c | 10 +++++++++ target/arm/kvm_arm.h | 7 +++++++ 7 files changed, 73 insertions(+) diff --git a/docs/system/arm/cpu-features.rst b/docs/system/arm/cpu-features.rst index 584eb17097..c9e39546a5 100644 --- a/docs/system/arm/cpu-features.rst +++ b/docs/system/arm/cpu-features.rst @@ -211,6 +211,11 @@ the list of KVM VCPU features and their descriptions. influence the guest scheduler behavior and/or be exposed to the guest userspace. + pauth Enable or disable ``FEAT_Pauth``, pointer + authentication. By default, the feature is enabled + with ``-cpu host`` if supported by both the host + kernel and the hardware. + TCG VCPU Features ================= diff --git a/target/arm/cpu.c b/target/arm/cpu.c index a211804fd3..68b09cbc6a 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -2091,6 +2091,7 @@ static void arm_host_initfn(Object *obj) kvm_arm_set_cpu_features_from_host(cpu); if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) { aarch64_add_sve_properties(obj); + aarch64_add_pauth_properties(obj); } #else hvf_arm_set_cpu_features_from_host(cpu); diff --git a/target/arm/cpu.h b/target/arm/cpu.h index e33f37b70a..c6a4d50e82 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -1076,6 +1076,7 @@ void aarch64_sve_narrow_vq(CPUARMState *env, unsigned vq); void aarch64_sve_change_el(CPUARMState *env, int old_el, int new_el, bool el0_a64); void aarch64_add_sve_properties(Object *obj); +void aarch64_add_pauth_properties(Object *obj); /* * SVE registers are encoded in KVM's memory in an endianness-invariant format. diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c index 15245a60a8..305c0e19fe 100644 --- a/target/arm/cpu64.c +++ b/target/arm/cpu64.c @@ -625,6 +625,42 @@ void aarch64_add_sve_properties(Object *obj) #endif } +static bool cpu_arm_get_pauth(Object *obj, Error **errp) +{ + ARMCPU *cpu = ARM_CPU(obj); + return cpu_isar_feature(aa64_pauth, cpu); +} + +static void cpu_arm_set_pauth(Object *obj, bool value, Error **errp) +{ + ARMCPU *cpu = ARM_CPU(obj); + uint64_t t; + + if (value) { + if (!kvm_arm_pauth_supported()) { + error_setg(errp, "'pauth' feature not supported by KVM on this host"); + } + + return; + } + + /* + * If the host supports PAuth, we only end-up here if the user has + * disabled the support, and value is false. + */ + t = cpu->isar.id_aa64isar1; + t = FIELD_DP64(t, ID_AA64ISAR1, APA, value); + t = FIELD_DP64(t, ID_AA64ISAR1, GPA, value); + t = FIELD_DP64(t, ID_AA64ISAR1, API, value); + t = FIELD_DP64(t, ID_AA64ISAR1, GPI, value); + cpu->isar.id_aa64isar1 = t; +} + +void aarch64_add_pauth_properties(Object *obj) +{ + object_property_add_bool(obj, "pauth", cpu_arm_get_pauth, cpu_arm_set_pauth); +} + void arm_cpu_pauth_finalize(ARMCPU *cpu, Error **errp) { int arch_val = 0, impdef_val = 0; diff --git a/target/arm/kvm.c b/target/arm/kvm.c index bbf1ce7ba3..71e2f46ce8 100644 --- a/target/arm/kvm.c +++ b/target/arm/kvm.c @@ -84,6 +84,7 @@ bool kvm_arm_create_scratch_host_vcpu(const uint32_t *cpus_to_try, if (vmfd < 0) { goto err; } + cpufd = ioctl(vmfd, KVM_CREATE_VCPU, 0); if (cpufd < 0) { goto err; @@ -94,6 +95,18 @@ bool kvm_arm_create_scratch_host_vcpu(const uint32_t *cpus_to_try, goto finish; } + /* + * Ask for Pointer Authentication if supported. We can't play the + * SVE trick of synthetising the ID reg as KVM won't tell us + * whether we have the architected or IMPDEF version of PAuth, so + * we have to use the actual ID regs. + */ + if (ioctl(vmfd, KVM_CHECK_EXTENSION, KVM_CAP_ARM_PTRAUTH_ADDRESS) > 0 && + ioctl(vmfd, KVM_CHECK_EXTENSION, KVM_CAP_ARM_PTRAUTH_GENERIC) > 0) { + init->features[0] |= (1 << KVM_ARM_VCPU_PTRAUTH_ADDRESS | + 1 << KVM_ARM_VCPU_PTRAUTH_GENERIC); + } + if (init->target == -1) { struct kvm_vcpu_init preferred; diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c index e790d6c9a5..95b6902ca0 100644 --- a/target/arm/kvm64.c +++ b/target/arm/kvm64.c @@ -725,6 +725,12 @@ bool kvm_arm_sve_supported(void) return kvm_check_extension(kvm_state, KVM_CAP_ARM_SVE); } +bool kvm_arm_pauth_supported(void) +{ + return (kvm_check_extension(kvm_state, KVM_CAP_ARM_PTRAUTH_ADDRESS) && + kvm_check_extension(kvm_state, KVM_CAP_ARM_PTRAUTH_GENERIC)); +} + bool kvm_arm_steal_time_supported(void) { return kvm_check_extension(kvm_state, KVM_CAP_STEAL_TIME); @@ -865,6 +871,10 @@ int kvm_arch_init_vcpu(CPUState *cs) assert(kvm_arm_sve_supported()); cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_SVE; } + if (cpu_isar_feature(aa64_pauth, cpu)) { + cpu->kvm_init_features[0] |= (1 << KVM_ARM_VCPU_PTRAUTH_ADDRESS | + 1 << KVM_ARM_VCPU_PTRAUTH_GENERIC); + } /* Do KVM_ARM_VCPU_INIT ioctl */ ret = kvm_arm_vcpu_init(cs); diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h index b7f78b5215..c26acf7866 100644 --- a/target/arm/kvm_arm.h +++ b/target/arm/kvm_arm.h @@ -306,6 +306,13 @@ bool kvm_arm_pmu_supported(void); */ bool kvm_arm_sve_supported(void); +/** + * kvm_arm_pauth_supported: + * + * Returns true if KVM can enable Pointer Authentication and false otherwise. + */ +bool kvm_arm_pauth_supported(void); + /** * kvm_arm_get_max_vm_ipa_size: * @ms: Machine state handle -- 2.30.2 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] hw/arm/virt: KVM: Enable PAuth when supported by the host 2021-12-28 18:23 [PATCH] hw/arm/virt: KVM: Enable PAuth when supported by the host Marc Zyngier @ 2022-01-03 13:46 ` Andrew Jones 2022-01-03 18:05 ` Marc Zyngier 0 siblings, 1 reply; 4+ messages in thread From: Andrew Jones @ 2022-01-03 13:46 UTC (permalink / raw) To: Marc Zyngier Cc: Peter Maydell, kvm, Richard Henderson, qemu-devel, Eric Auger, kernel-team, kvmarm Hi Marc, On Tue, Dec 28, 2021 at 06:23:47PM +0000, Marc Zyngier wrote: > Add basic support for Pointer Authentication when running a KVM > guest and that the host supports it, loosely based on the SVE > support. > > Although the feature is enabled by default when the host advertises > it, it is possible to disable it by setting the 'pauth=off' CPU > property. > > Tested on an Apple M1 running 5.16-rc6. > > Cc: Eric Auger <eric.auger@redhat.com> > Cc: Andrew Jones <drjones@redhat.com> > Cc: Richard Henderson <richard.henderson@linaro.org> > Cc: Peter Maydell <peter.maydell@linaro.org> > Signed-off-by: Marc Zyngier <maz@kernel.org> > --- > docs/system/arm/cpu-features.rst | 5 +++++ > target/arm/cpu.c | 1 + > target/arm/cpu.h | 1 + > target/arm/cpu64.c | 36 ++++++++++++++++++++++++++++++++ > target/arm/kvm.c | 13 ++++++++++++ > target/arm/kvm64.c | 10 +++++++++ > target/arm/kvm_arm.h | 7 +++++++ > 7 files changed, 73 insertions(+) > > diff --git a/docs/system/arm/cpu-features.rst b/docs/system/arm/cpu-features.rst > index 584eb17097..c9e39546a5 100644 > --- a/docs/system/arm/cpu-features.rst > +++ b/docs/system/arm/cpu-features.rst > @@ -211,6 +211,11 @@ the list of KVM VCPU features and their descriptions. > influence the guest scheduler behavior and/or be > exposed to the guest userspace. > > + pauth Enable or disable ``FEAT_Pauth``, pointer > + authentication. By default, the feature is enabled > + with ``-cpu host`` if supported by both the host > + kernel and the hardware. > + Thanks for considering a documentation update. In this case, though, I think we should delete the "TCG VCPU Features" pauth paragraph, rather than add a new "KVM VCPU Features" pauth paragraph. We don't need to document each CPU feature. We just document complex ones, like sve*, KVM specific ones (kvm-*), and TCG specific ones (now only pauth-impdef). > TCG VCPU Features > ================= > > diff --git a/target/arm/cpu.c b/target/arm/cpu.c > index a211804fd3..68b09cbc6a 100644 > --- a/target/arm/cpu.c > +++ b/target/arm/cpu.c > @@ -2091,6 +2091,7 @@ static void arm_host_initfn(Object *obj) > kvm_arm_set_cpu_features_from_host(cpu); > if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) { > aarch64_add_sve_properties(obj); > + aarch64_add_pauth_properties(obj); > } > #else > hvf_arm_set_cpu_features_from_host(cpu); > diff --git a/target/arm/cpu.h b/target/arm/cpu.h > index e33f37b70a..c6a4d50e82 100644 > --- a/target/arm/cpu.h > +++ b/target/arm/cpu.h > @@ -1076,6 +1076,7 @@ void aarch64_sve_narrow_vq(CPUARMState *env, unsigned vq); > void aarch64_sve_change_el(CPUARMState *env, int old_el, > int new_el, bool el0_a64); > void aarch64_add_sve_properties(Object *obj); > +void aarch64_add_pauth_properties(Object *obj); > > /* > * SVE registers are encoded in KVM's memory in an endianness-invariant format. > diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c > index 15245a60a8..305c0e19fe 100644 > --- a/target/arm/cpu64.c > +++ b/target/arm/cpu64.c > @@ -625,6 +625,42 @@ void aarch64_add_sve_properties(Object *obj) > #endif > } > > +static bool cpu_arm_get_pauth(Object *obj, Error **errp) > +{ > + ARMCPU *cpu = ARM_CPU(obj); > + return cpu_isar_feature(aa64_pauth, cpu); > +} > + > +static void cpu_arm_set_pauth(Object *obj, bool value, Error **errp) > +{ > + ARMCPU *cpu = ARM_CPU(obj); > + uint64_t t; > + > + if (value) { > + if (!kvm_arm_pauth_supported()) { > + error_setg(errp, "'pauth' feature not supported by KVM on this host"); > + } > + > + return; > + } > + > + /* > + * If the host supports PAuth, we only end-up here if the user has > + * disabled the support, and value is false. > + */ > + t = cpu->isar.id_aa64isar1; > + t = FIELD_DP64(t, ID_AA64ISAR1, APA, value); > + t = FIELD_DP64(t, ID_AA64ISAR1, GPA, value); > + t = FIELD_DP64(t, ID_AA64ISAR1, API, value); > + t = FIELD_DP64(t, ID_AA64ISAR1, GPI, value); > + cpu->isar.id_aa64isar1 = t; > +} > + > +void aarch64_add_pauth_properties(Object *obj) > +{ > + object_property_add_bool(obj, "pauth", cpu_arm_get_pauth, cpu_arm_set_pauth); > +} I think we should try to merge as much as possible between the TCG and KVM pauth property handling. I think we just need to move the qdev_property_add_static(DEVICE(obj), &arm_cpu_pauth_property) call into KVM/TCG common code and then modify arm_cpu_pauth_finalize() to handle checking KVM for support when KVM is enabled and also to not look at the impdef property. > + > void arm_cpu_pauth_finalize(ARMCPU *cpu, Error **errp) > { > int arch_val = 0, impdef_val = 0; > diff --git a/target/arm/kvm.c b/target/arm/kvm.c > index bbf1ce7ba3..71e2f46ce8 100644 > --- a/target/arm/kvm.c > +++ b/target/arm/kvm.c > @@ -84,6 +84,7 @@ bool kvm_arm_create_scratch_host_vcpu(const uint32_t *cpus_to_try, > if (vmfd < 0) { > goto err; > } > + > cpufd = ioctl(vmfd, KVM_CREATE_VCPU, 0); > if (cpufd < 0) { > goto err; > @@ -94,6 +95,18 @@ bool kvm_arm_create_scratch_host_vcpu(const uint32_t *cpus_to_try, > goto finish; > } > > + /* > + * Ask for Pointer Authentication if supported. We can't play the > + * SVE trick of synthetising the ID reg as KVM won't tell us > + * whether we have the architected or IMPDEF version of PAuth, so > + * we have to use the actual ID regs. > + */ > + if (ioctl(vmfd, KVM_CHECK_EXTENSION, KVM_CAP_ARM_PTRAUTH_ADDRESS) > 0 && > + ioctl(vmfd, KVM_CHECK_EXTENSION, KVM_CAP_ARM_PTRAUTH_GENERIC) > 0) { > + init->features[0] |= (1 << KVM_ARM_VCPU_PTRAUTH_ADDRESS | > + 1 << KVM_ARM_VCPU_PTRAUTH_GENERIC); > + } > + I think kvm_init() is called prior to kvm_arm_get_host_cpu_features(), which means we can do this instead diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c index e790d6c9a573..d1512035ac5b 100644 --- a/target/arm/kvm64.c +++ b/target/arm/kvm64.c @@ -521,6 +521,17 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) */ struct kvm_vcpu_init init = { .target = -1, }; + /* + * Ask for Pointer Authentication if supported. We can't play the + * SVE trick of synthetising the ID reg as KVM won't tell us + * whether we have the architected or IMPDEF version of PAuth, so + * we have to use the actual ID regs. + */ + if (kvm_arm_pauth_supported()) { + init->features[0] |= (1 << KVM_ARM_VCPU_PTRAUTH_ADDRESS | + 1 << KVM_ARM_VCPU_PTRAUTH_GENERIC); + } + if (!kvm_arm_create_scratch_host_vcpu(cpus_to_try, fdarray, &init)) { return false; } Assuming I'm right about the call order, then I think I'd like that more. > if (init->target == -1) { > struct kvm_vcpu_init preferred; > > diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c > index e790d6c9a5..95b6902ca0 100644 > --- a/target/arm/kvm64.c > +++ b/target/arm/kvm64.c > @@ -725,6 +725,12 @@ bool kvm_arm_sve_supported(void) > return kvm_check_extension(kvm_state, KVM_CAP_ARM_SVE); > } > > +bool kvm_arm_pauth_supported(void) > +{ > + return (kvm_check_extension(kvm_state, KVM_CAP_ARM_PTRAUTH_ADDRESS) && > + kvm_check_extension(kvm_state, KVM_CAP_ARM_PTRAUTH_GENERIC)); > +} > + > bool kvm_arm_steal_time_supported(void) > { > return kvm_check_extension(kvm_state, KVM_CAP_STEAL_TIME); > @@ -865,6 +871,10 @@ int kvm_arch_init_vcpu(CPUState *cs) > assert(kvm_arm_sve_supported()); > cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_SVE; > } > + if (cpu_isar_feature(aa64_pauth, cpu)) { > + cpu->kvm_init_features[0] |= (1 << KVM_ARM_VCPU_PTRAUTH_ADDRESS | > + 1 << KVM_ARM_VCPU_PTRAUTH_GENERIC); > + } > > /* Do KVM_ARM_VCPU_INIT ioctl */ > ret = kvm_arm_vcpu_init(cs); > diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h > index b7f78b5215..c26acf7866 100644 > --- a/target/arm/kvm_arm.h > +++ b/target/arm/kvm_arm.h > @@ -306,6 +306,13 @@ bool kvm_arm_pmu_supported(void); > */ > bool kvm_arm_sve_supported(void); > > +/** > + * kvm_arm_pauth_supported: > + * > + * Returns true if KVM can enable Pointer Authentication and false otherwise. > + */ > +bool kvm_arm_pauth_supported(void); > + If we merge more of the pauth property handling with the TCG code, then we'll also need a stub in the !CONFIG_KVM section for compiling without kvm support. > /** > * kvm_arm_get_max_vm_ipa_size: > * @ms: Machine state handle > -- > 2.30.2 > Thanks, drew ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] hw/arm/virt: KVM: Enable PAuth when supported by the host 2022-01-03 13:46 ` Andrew Jones @ 2022-01-03 18:05 ` Marc Zyngier 2022-01-05 16:25 ` Andrew Jones 0 siblings, 1 reply; 4+ messages in thread From: Marc Zyngier @ 2022-01-03 18:05 UTC (permalink / raw) To: Andrew Jones Cc: Peter Maydell, kvm, Richard Henderson, qemu-devel, Eric Auger, kernel-team, kvmarm Hi Andrew, On Mon, 03 Jan 2022 13:46:01 +0000, Andrew Jones <drjones@redhat.com> wrote: > > Hi Marc, > > On Tue, Dec 28, 2021 at 06:23:47PM +0000, Marc Zyngier wrote: > > Add basic support for Pointer Authentication when running a KVM > > guest and that the host supports it, loosely based on the SVE > > support. > > > > Although the feature is enabled by default when the host advertises > > it, it is possible to disable it by setting the 'pauth=off' CPU > > property. > > > > Tested on an Apple M1 running 5.16-rc6. > > > > Cc: Eric Auger <eric.auger@redhat.com> > > Cc: Andrew Jones <drjones@redhat.com> > > Cc: Richard Henderson <richard.henderson@linaro.org> > > Cc: Peter Maydell <peter.maydell@linaro.org> > > Signed-off-by: Marc Zyngier <maz@kernel.org> > > --- > > docs/system/arm/cpu-features.rst | 5 +++++ > > target/arm/cpu.c | 1 + > > target/arm/cpu.h | 1 + > > target/arm/cpu64.c | 36 ++++++++++++++++++++++++++++++++ > > target/arm/kvm.c | 13 ++++++++++++ > > target/arm/kvm64.c | 10 +++++++++ > > target/arm/kvm_arm.h | 7 +++++++ > > 7 files changed, 73 insertions(+) > > > > diff --git a/docs/system/arm/cpu-features.rst b/docs/system/arm/cpu-features.rst > > index 584eb17097..c9e39546a5 100644 > > --- a/docs/system/arm/cpu-features.rst > > +++ b/docs/system/arm/cpu-features.rst > > @@ -211,6 +211,11 @@ the list of KVM VCPU features and their descriptions. > > influence the guest scheduler behavior and/or be > > exposed to the guest userspace. > > > > + pauth Enable or disable ``FEAT_Pauth``, pointer > > + authentication. By default, the feature is enabled > > + with ``-cpu host`` if supported by both the host > > + kernel and the hardware. > > + > > Thanks for considering a documentation update. In this case, though, I > think we should delete the "TCG VCPU Features" pauth paragraph, rather > than add a new "KVM VCPU Features" pauth paragraph. We don't need to > document each CPU feature. We just document complex ones, like sve*, > KVM specific ones (kvm-*), and TCG specific ones (now only pauth-impdef). Sure, works for me. Do we need to keep a trace of the available options? I'm not sure how a user is supposed to find out about those (I always end-up grepping through the code base, and something tells me I'm doing it wrong...). The QMP stuff flies way over my head. > > TCG VCPU Features > > ================= > > > > diff --git a/target/arm/cpu.c b/target/arm/cpu.c > > index a211804fd3..68b09cbc6a 100644 > > --- a/target/arm/cpu.c > > +++ b/target/arm/cpu.c > > @@ -2091,6 +2091,7 @@ static void arm_host_initfn(Object *obj) > > kvm_arm_set_cpu_features_from_host(cpu); > > if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) { > > aarch64_add_sve_properties(obj); > > + aarch64_add_pauth_properties(obj); > > } > > #else > > hvf_arm_set_cpu_features_from_host(cpu); > > diff --git a/target/arm/cpu.h b/target/arm/cpu.h > > index e33f37b70a..c6a4d50e82 100644 > > --- a/target/arm/cpu.h > > +++ b/target/arm/cpu.h > > @@ -1076,6 +1076,7 @@ void aarch64_sve_narrow_vq(CPUARMState *env, unsigned vq); > > void aarch64_sve_change_el(CPUARMState *env, int old_el, > > int new_el, bool el0_a64); > > void aarch64_add_sve_properties(Object *obj); > > +void aarch64_add_pauth_properties(Object *obj); > > > > /* > > * SVE registers are encoded in KVM's memory in an endianness-invariant format. > > diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c > > index 15245a60a8..305c0e19fe 100644 > > --- a/target/arm/cpu64.c > > +++ b/target/arm/cpu64.c > > @@ -625,6 +625,42 @@ void aarch64_add_sve_properties(Object *obj) > > #endif > > } > > > > +static bool cpu_arm_get_pauth(Object *obj, Error **errp) > > +{ > > + ARMCPU *cpu = ARM_CPU(obj); > > + return cpu_isar_feature(aa64_pauth, cpu); > > +} > > + > > +static void cpu_arm_set_pauth(Object *obj, bool value, Error **errp) > > +{ > > + ARMCPU *cpu = ARM_CPU(obj); > > + uint64_t t; > > + > > + if (value) { > > + if (!kvm_arm_pauth_supported()) { > > + error_setg(errp, "'pauth' feature not supported by KVM on this host"); > > + } > > + > > + return; > > + } > > + > > + /* > > + * If the host supports PAuth, we only end-up here if the user has > > + * disabled the support, and value is false. > > + */ > > + t = cpu->isar.id_aa64isar1; > > + t = FIELD_DP64(t, ID_AA64ISAR1, APA, value); > > + t = FIELD_DP64(t, ID_AA64ISAR1, GPA, value); > > + t = FIELD_DP64(t, ID_AA64ISAR1, API, value); > > + t = FIELD_DP64(t, ID_AA64ISAR1, GPI, value); > > + cpu->isar.id_aa64isar1 = t; > > +} > > + > > +void aarch64_add_pauth_properties(Object *obj) > > +{ > > + object_property_add_bool(obj, "pauth", cpu_arm_get_pauth, cpu_arm_set_pauth); > > +} > > I think we should try to merge as much as possible between the TCG and KVM > pauth property handling. I think we just need to move the > qdev_property_add_static(DEVICE(obj), &arm_cpu_pauth_property) call into > KVM/TCG common code and then modify arm_cpu_pauth_finalize() to > handle checking KVM for support when KVM is enabled and also to not > look at the impdef property. Happy to merge things more, though using qdev_property_add_static() feels a bit odd here (I have to forcefully replicate the probed state into the cpu->prop_pauth property in order to have a sensible default on KVM). Anyway, I'll post something with this hack, and we add another coat of paint to the bike shed! ;-) > > > + > > void arm_cpu_pauth_finalize(ARMCPU *cpu, Error **errp) > > { > > int arch_val = 0, impdef_val = 0; > > diff --git a/target/arm/kvm.c b/target/arm/kvm.c > > index bbf1ce7ba3..71e2f46ce8 100644 > > --- a/target/arm/kvm.c > > +++ b/target/arm/kvm.c > > @@ -84,6 +84,7 @@ bool kvm_arm_create_scratch_host_vcpu(const uint32_t *cpus_to_try, > > if (vmfd < 0) { > > goto err; > > } > > + > > cpufd = ioctl(vmfd, KVM_CREATE_VCPU, 0); > > if (cpufd < 0) { > > goto err; > > @@ -94,6 +95,18 @@ bool kvm_arm_create_scratch_host_vcpu(const uint32_t *cpus_to_try, > > goto finish; > > } > > > > + /* > > + * Ask for Pointer Authentication if supported. We can't play the > > + * SVE trick of synthetising the ID reg as KVM won't tell us > > + * whether we have the architected or IMPDEF version of PAuth, so > > + * we have to use the actual ID regs. > > + */ > > + if (ioctl(vmfd, KVM_CHECK_EXTENSION, KVM_CAP_ARM_PTRAUTH_ADDRESS) > 0 && > > + ioctl(vmfd, KVM_CHECK_EXTENSION, KVM_CAP_ARM_PTRAUTH_GENERIC) > 0) { > > + init->features[0] |= (1 << KVM_ARM_VCPU_PTRAUTH_ADDRESS | > > + 1 << KVM_ARM_VCPU_PTRAUTH_GENERIC); > > + } > > + > > I think kvm_init() is called prior to kvm_arm_get_host_cpu_features(), > which means we can do this instead > > diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c > index e790d6c9a573..d1512035ac5b 100644 > --- a/target/arm/kvm64.c > +++ b/target/arm/kvm64.c > @@ -521,6 +521,17 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) > */ > struct kvm_vcpu_init init = { .target = -1, }; > > + /* > + * Ask for Pointer Authentication if supported. We can't play the > + * SVE trick of synthetising the ID reg as KVM won't tell us > + * whether we have the architected or IMPDEF version of PAuth, so > + * we have to use the actual ID regs. > + */ > + if (kvm_arm_pauth_supported()) { > + init->features[0] |= (1 << KVM_ARM_VCPU_PTRAUTH_ADDRESS | > + 1 << KVM_ARM_VCPU_PTRAUTH_GENERIC); > + } > + > if (!kvm_arm_create_scratch_host_vcpu(cpus_to_try, fdarray, &init)) { > return false; > } > > Assuming I'm right about the call order, then I think I'd like that more. Yup, works nicely, and allows for some further cleanups. > > > > if (init->target == -1) { > > struct kvm_vcpu_init preferred; > > > > diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c > > index e790d6c9a5..95b6902ca0 100644 > > --- a/target/arm/kvm64.c > > +++ b/target/arm/kvm64.c > > @@ -725,6 +725,12 @@ bool kvm_arm_sve_supported(void) > > return kvm_check_extension(kvm_state, KVM_CAP_ARM_SVE); > > } > > > > +bool kvm_arm_pauth_supported(void) > > +{ > > + return (kvm_check_extension(kvm_state, KVM_CAP_ARM_PTRAUTH_ADDRESS) && > > + kvm_check_extension(kvm_state, KVM_CAP_ARM_PTRAUTH_GENERIC)); > > +} > > + > > bool kvm_arm_steal_time_supported(void) > > { > > return kvm_check_extension(kvm_state, KVM_CAP_STEAL_TIME); > > @@ -865,6 +871,10 @@ int kvm_arch_init_vcpu(CPUState *cs) > > assert(kvm_arm_sve_supported()); > > cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_SVE; > > } > > + if (cpu_isar_feature(aa64_pauth, cpu)) { > > + cpu->kvm_init_features[0] |= (1 << KVM_ARM_VCPU_PTRAUTH_ADDRESS | > > + 1 << KVM_ARM_VCPU_PTRAUTH_GENERIC); > > + } > > > > /* Do KVM_ARM_VCPU_INIT ioctl */ > > ret = kvm_arm_vcpu_init(cs); > > diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h > > index b7f78b5215..c26acf7866 100644 > > --- a/target/arm/kvm_arm.h > > +++ b/target/arm/kvm_arm.h > > @@ -306,6 +306,13 @@ bool kvm_arm_pmu_supported(void); > > */ > > bool kvm_arm_sve_supported(void); > > > > +/** > > + * kvm_arm_pauth_supported: > > + * > > + * Returns true if KVM can enable Pointer Authentication and false otherwise. > > + */ > > +bool kvm_arm_pauth_supported(void); > > + > > If we merge more of the pauth property handling with the TCG code, then > we'll also need a stub in the !CONFIG_KVM section for compiling without > kvm support. Actually, this can go altogether, as it can now be made static in kvm64.c and not be visible outside of the KVM code at all. Thanks a lot for the review and the guidance! M. -- Without deviation from the norm, progress is not possible. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] hw/arm/virt: KVM: Enable PAuth when supported by the host 2022-01-03 18:05 ` Marc Zyngier @ 2022-01-05 16:25 ` Andrew Jones 0 siblings, 0 replies; 4+ messages in thread From: Andrew Jones @ 2022-01-05 16:25 UTC (permalink / raw) To: Marc Zyngier Cc: Peter Maydell, kvm, Richard Henderson, qemu-devel, Eric Auger, kernel-team, kvmarm On Mon, Jan 03, 2022 at 06:05:41PM +0000, Marc Zyngier wrote: > Andrew Jones <drjones@redhat.com> wrote: > > > > Thanks for considering a documentation update. In this case, though, I > > think we should delete the "TCG VCPU Features" pauth paragraph, rather > > than add a new "KVM VCPU Features" pauth paragraph. We don't need to > > document each CPU feature. We just document complex ones, like sve*, > > KVM specific ones (kvm-*), and TCG specific ones (now only pauth-impdef). > > Sure, works for me. Do we need to keep a trace of the available > options? For arm we need to extend target/arm/helper.c:arm_cpu_list() to output the possible flags like x86 does. On x86 doing this qemu-system-x86_64 -cpu help not only gives us a list of cpu types, but also a list of flags we can provide to the cpus (although not all flags will work on all cpus...) On arm doing this qemu-system-aarch64 -cpu help only gives us a list of cpu types. > I'm not sure how a user is supposed to find out about those > (I always end-up grepping through the code base, and something tells > me I'm doing it wrong...). The QMP stuff flies way over my head. > Indeed, currently grepping is less awkward than probing with QMP. With an extension to target/arm/helper.c:arm_cpu_list() we can avoid grepping too. I've just added this to my TODO [again]. It was there once already, but fell off the bottom... Thanks, drew ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-01-05 16:26 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-12-28 18:23 [PATCH] hw/arm/virt: KVM: Enable PAuth when supported by the host Marc Zyngier 2022-01-03 13:46 ` Andrew Jones 2022-01-03 18:05 ` Marc Zyngier 2022-01-05 16:25 ` Andrew Jones
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).