diff mbox series

hw/arm/virt: KVM: Enable PAuth when supported by the host

Message ID 20211228182347.1025501-1-maz@kernel.org (mailing list archive)
State New, archived
Headers show
Series hw/arm/virt: KVM: Enable PAuth when supported by the host | expand

Commit Message

Marc Zyngier Dec. 28, 2021, 6:23 p.m. UTC
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(+)

Comments

Andrew Jones Jan. 3, 2022, 1:46 p.m. UTC | #1
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
Marc Zyngier Jan. 3, 2022, 6:05 p.m. UTC | #2
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.
Andrew Jones Jan. 5, 2022, 4:25 p.m. UTC | #3
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
diff mbox series

Patch

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