| Message ID | 20220107150154.2490308-1-maz@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] hw/arm/virt: KVM: Enable PAuth when supported by the host | expand |
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~
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.
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
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.
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);