[PATCH v3] hw/arm/virt: KVM: Enable PAuth when supported by the host

Marc Zyngier posted 1 patch 2 years, 2 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20220107150154.2490308-1-maz@kernel.org
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Peter Maydell <peter.maydell@linaro.org>
docs/system/arm/cpu-features.rst |  4 ----
target/arm/cpu.c                 | 12 +++---------
target/arm/cpu.h                 |  1 +
target/arm/cpu64.c               | 31 +++++++++++++++++++++++++++----
target/arm/kvm64.c               | 21 +++++++++++++++++++++
5 files changed, 52 insertions(+), 17 deletions(-)
[PATCH v3] hw/arm/virt: KVM: Enable PAuth when supported by the host
Posted by Marc Zyngier 2 years, 2 months ago
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. The 'pauth' comment is removed from cpu-features.rst,
as it is now common to both TCG and KVM.

Tested on an Apple M1 running 5.16-rc6.

Cc: Eric Auger <eric.auger@redhat.com>
Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
* From v2:
  - Fixed indentation and spelling
  - Slightly reworked the KVM handling in arm_cpu_pauth_finalize
    (no functional changes)
  - Picked Andrew's RB tag

 docs/system/arm/cpu-features.rst |  4 ----
 target/arm/cpu.c                 | 12 +++---------
 target/arm/cpu.h                 |  1 +
 target/arm/cpu64.c               | 31 +++++++++++++++++++++++++++----
 target/arm/kvm64.c               | 21 +++++++++++++++++++++
 5 files changed, 52 insertions(+), 17 deletions(-)

diff --git a/docs/system/arm/cpu-features.rst b/docs/system/arm/cpu-features.rst
index 584eb17097..3e626c4b68 100644
--- a/docs/system/arm/cpu-features.rst
+++ b/docs/system/arm/cpu-features.rst
@@ -217,10 +217,6 @@ TCG VCPU Features
 TCG VCPU features are CPU features that are specific to TCG.
 Below is the list of TCG VCPU features and their descriptions.
 
-  pauth                    Enable or disable ``FEAT_Pauth``, pointer
-                           authentication.  By default, the feature is
-                           enabled with ``-cpu max``.
-
   pauth-impdef             When ``FEAT_Pauth`` is enabled, either the
                            *impdef* (Implementation Defined) algorithm
                            is enabled or the *architected* QARMA algorithm
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index a211804fd3..f3c09931e4 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1380,17 +1380,10 @@ void arm_cpu_finalize_features(ARMCPU *cpu, Error **errp)
             return;
         }
 
-        /*
-         * KVM does not support modifications to this feature.
-         * We have not registered the cpu properties when KVM
-         * is in use, so the user will not be able to set them.
-         */
-        if (!kvm_enabled()) {
-            arm_cpu_pauth_finalize(cpu, &local_err);
-            if (local_err != NULL) {
+        arm_cpu_pauth_finalize(cpu, &local_err);
+        if (local_err != NULL) {
                 error_propagate(errp, local_err);
                 return;
-            }
         }
     }
 
@@ -2091,6 +2084,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..8786be7783 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -630,6 +630,15 @@ void arm_cpu_pauth_finalize(ARMCPU *cpu, Error **errp)
     int arch_val = 0, impdef_val = 0;
     uint64_t t;
 
+    /* Exit early if PAuth is enabled, and fall through to disable it */
+    if (kvm_enabled() && cpu->prop_pauth) {
+        if (!cpu_isar_feature(aa64_pauth, cpu)) {
+            error_setg(errp, "'pauth' feature not supported by KVM on this host");
+        }
+
+        return;
+    }
+
     /* TODO: Handle HaveEnhancedPAC, HaveEnhancedPAC2, HaveFPAC. */
     if (cpu->prop_pauth) {
         if (cpu->prop_pauth_impdef) {
@@ -655,6 +664,23 @@ static Property arm_cpu_pauth_property =
 static Property arm_cpu_pauth_impdef_property =
     DEFINE_PROP_BOOL("pauth-impdef", ARMCPU, prop_pauth_impdef, false);
 
+void aarch64_add_pauth_properties(Object *obj)
+{
+    ARMCPU *cpu = ARM_CPU(obj);
+
+    /* Default to PAUTH on, with the architected algorithm on TCG. */
+    qdev_property_add_static(DEVICE(obj), &arm_cpu_pauth_property);
+    if (kvm_enabled()) {
+        /*
+         * Mirror PAuth support from the probed sysregs back into the
+         * property for KVM. Is it just a bit backward? Yes it is!
+         */
+        cpu->prop_pauth = cpu_isar_feature(aa64_pauth, cpu);
+    } else {
+        qdev_property_add_static(DEVICE(obj), &arm_cpu_pauth_impdef_property);
+    }
+}
+
 /* -cpu max: if KVM is enabled, like -cpu host (best possible with this host);
  * otherwise, a CPU with as many features enabled as our emulation supports.
  * The version of '-cpu max' for qemu-system-arm is defined in cpu.c;
@@ -829,13 +855,10 @@ static void aarch64_max_initfn(Object *obj)
         cpu->dcz_blocksize = 7; /*  512 bytes */
 #endif
 
-        /* Default to PAUTH on, with the architected algorithm. */
-        qdev_property_add_static(DEVICE(obj), &arm_cpu_pauth_property);
-        qdev_property_add_static(DEVICE(obj), &arm_cpu_pauth_impdef_property);
-
         bitmap_fill(cpu->sve_vq_supported, ARM_MAX_VQ);
     }
 
+    aarch64_add_pauth_properties(obj);
     aarch64_add_sve_properties(obj);
     object_property_add(obj, "sve-max-vq", "uint32", cpu_max_get_sve_max_vq,
                         cpu_max_set_sve_max_vq, NULL, NULL);
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index e790d6c9a5..71c3ca6971 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -491,6 +491,12 @@ static int read_sys_reg64(int fd, uint64_t *pret, uint64_t id)
     return ioctl(fd, KVM_GET_ONE_REG, &idreg);
 }
 
+static 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_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
 {
     /* Identify the feature bits corresponding to the host CPU, and
@@ -521,6 +527,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 synthesising 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;
     }
@@ -865,6 +882,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);
-- 
2.30.2


Re: [PATCH v3] hw/arm/virt: KVM: Enable PAuth when supported by the host
Posted by Richard Henderson 2 years, 2 months ago
On 1/7/22 7:01 AM, Marc Zyngier wrote:
> @@ -1380,17 +1380,10 @@ void arm_cpu_finalize_features(ARMCPU *cpu, Error **errp)
>               return;
>           }
>   
> -        /*
> -         * KVM does not support modifications to this feature.
> -         * We have not registered the cpu properties when KVM
> -         * is in use, so the user will not be able to set them.
> -         */
> -        if (!kvm_enabled()) {
> -            arm_cpu_pauth_finalize(cpu, &local_err);
> -            if (local_err != NULL) {
> +        arm_cpu_pauth_finalize(cpu, &local_err);
> +        if (local_err != NULL) {
>                   error_propagate(errp, local_err);
>                   return;
> -            }
>           }

Indentation is still off -- error + return should be out-dented one level.

Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH v3] hw/arm/virt: KVM: Enable PAuth when supported by the host
Posted by Marc Zyngier 2 years, 2 months ago
On 2022-01-07 20:23, Richard Henderson wrote:
> On 1/7/22 7:01 AM, Marc Zyngier wrote:
>> @@ -1380,17 +1380,10 @@ void arm_cpu_finalize_features(ARMCPU *cpu, 
>> Error **errp)
>>               return;
>>           }
>>   -        /*
>> -         * KVM does not support modifications to this feature.
>> -         * We have not registered the cpu properties when KVM
>> -         * is in use, so the user will not be able to set them.
>> -         */
>> -        if (!kvm_enabled()) {
>> -            arm_cpu_pauth_finalize(cpu, &local_err);
>> -            if (local_err != NULL) {
>> +        arm_cpu_pauth_finalize(cpu, &local_err);
>> +        if (local_err != NULL) {
>>                   error_propagate(errp, local_err);
>>                   return;
>> -            }
>>           }
> 
> Indentation is still off -- error + return should be out-dented one 
> level.
> 

Duh. Clearly, my brain can't spot these. Apologies for the extra noise.

> Otherwise,
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Thanks. I'll repost a version shortly, unless someone shouts.

         M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH v3] hw/arm/virt: KVM: Enable PAuth when supported by the host
Posted by Peter Maydell 2 years, 2 months ago
On Sat, 8 Jan 2022 at 13:42, Marc Zyngier <maz@kernel.org> wrote:
>
> On 2022-01-07 20:23, Richard Henderson wrote:
> > On 1/7/22 7:01 AM, Marc Zyngier wrote:
> >> @@ -1380,17 +1380,10 @@ void arm_cpu_finalize_features(ARMCPU *cpu,
> >> Error **errp)
> >>               return;
> >>           }
> >>   -        /*
> >> -         * KVM does not support modifications to this feature.
> >> -         * We have not registered the cpu properties when KVM
> >> -         * is in use, so the user will not be able to set them.
> >> -         */
> >> -        if (!kvm_enabled()) {
> >> -            arm_cpu_pauth_finalize(cpu, &local_err);
> >> -            if (local_err != NULL) {
> >> +        arm_cpu_pauth_finalize(cpu, &local_err);
> >> +        if (local_err != NULL) {
> >>                   error_propagate(errp, local_err);
> >>                   return;
> >> -            }
> >>           }
> >
> > Indentation is still off -- error + return should be out-dented one
> > level.
> >
>
> Duh. Clearly, my brain can't spot these. Apologies for the extra noise.
>
> > Otherwise,
> > Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>
> Thanks. I'll repost a version shortly, unless someone shouts.

Don't worry about it -- I've applied this to target-arm.next and
fixed the indent there.

-- PMM

Re: [PATCH v3] hw/arm/virt: KVM: Enable PAuth when supported by the host
Posted by Marc Zyngier 2 years, 2 months ago
On Tue, 11 Jan 2022 13:58:49 +0000,
Peter Maydell <peter.maydell@linaro.org> wrote:
> 
> On Sat, 8 Jan 2022 at 13:42, Marc Zyngier <maz@kernel.org> wrote:
> >
> > On 2022-01-07 20:23, Richard Henderson wrote:
> > > On 1/7/22 7:01 AM, Marc Zyngier wrote:
> > >> @@ -1380,17 +1380,10 @@ void arm_cpu_finalize_features(ARMCPU *cpu,
> > >> Error **errp)
> > >>               return;
> > >>           }
> > >>   -        /*
> > >> -         * KVM does not support modifications to this feature.
> > >> -         * We have not registered the cpu properties when KVM
> > >> -         * is in use, so the user will not be able to set them.
> > >> -         */
> > >> -        if (!kvm_enabled()) {
> > >> -            arm_cpu_pauth_finalize(cpu, &local_err);
> > >> -            if (local_err != NULL) {
> > >> +        arm_cpu_pauth_finalize(cpu, &local_err);
> > >> +        if (local_err != NULL) {
> > >>                   error_propagate(errp, local_err);
> > >>                   return;
> > >> -            }
> > >>           }
> > >
> > > Indentation is still off -- error + return should be out-dented one
> > > level.
> > >
> >
> > Duh. Clearly, my brain can't spot these. Apologies for the extra noise.
> >
> > > Otherwise,
> > > Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> >
> > Thanks. I'll repost a version shortly, unless someone shouts.
> 
> Don't worry about it -- I've applied this to target-arm.next and
> fixed the indent there.

Awesome, thanks Peter.

	M.

-- 
Without deviation from the norm, progress is not possible.