* [PATCH 0/3] target/arm: Fix kvm probe of ID_AA64ZFR0
@ 2022-07-25 18:14 Richard Henderson
2022-07-25 18:14 ` [PATCH 1/3] target/arm: Create kvm_arm_svm_supported Richard Henderson
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Richard Henderson @ 2022-07-25 18:14 UTC (permalink / raw)
To: qemu-devel; +Cc: yuzenghui, qemu-arm
Our probing of this SVE register was done within an incorrect
vCPU environment, so that the id register was always RAZ.
r~
Richard Henderson (3):
target/arm: Create kvm_arm_svm_supported
target/arm: Set KVM_ARM_VCPU_SVE while probing the host
target/arm: Move sve probe inside kvm >= 4.15 branch
target/arm/kvm64.c | 50 +++++++++++++++++++++++++---------------------
1 file changed, 27 insertions(+), 23 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/3] target/arm: Create kvm_arm_svm_supported
2022-07-25 18:14 [PATCH 0/3] target/arm: Fix kvm probe of ID_AA64ZFR0 Richard Henderson
@ 2022-07-25 18:14 ` Richard Henderson
2022-07-26 2:02 ` Zenghui Yu via
2022-07-25 18:14 ` [PATCH 2/3] target/arm: Set KVM_ARM_VCPU_SVE while probing the host Richard Henderson
2022-07-25 18:14 ` [PATCH 3/3] target/arm: Move sve probe inside kvm >= 4.15 branch Richard Henderson
2 siblings, 1 reply; 7+ messages in thread
From: Richard Henderson @ 2022-07-25 18:14 UTC (permalink / raw)
To: qemu-devel; +Cc: yuzenghui, qemu-arm
Indication for support for SVE will not depend on whether we
perform the query on the main kvm_state or the temp vcpu.
Mirror kvm_arm_pauth_supported.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/kvm64.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index d16d4ea250..36ff20c9e3 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -496,6 +496,11 @@ static bool kvm_arm_pauth_supported(void)
kvm_check_extension(kvm_state, KVM_CAP_ARM_PTRAUTH_GENERIC));
}
+static bool kvm_arm_svm_supported(void)
+{
+ return kvm_check_extension(kvm_state, KVM_CAP_ARM_SVE);
+}
+
bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
{
/* Identify the feature bits corresponding to the host CPU, and
@@ -675,7 +680,7 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
}
}
- sve_supported = ioctl(fdarray[0], KVM_CHECK_EXTENSION, KVM_CAP_ARM_SVE) > 0;
+ sve_supported = kvm_arm_svm_supported();
/* Add feature bits that can't appear until after VCPU init. */
if (sve_supported) {
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/3] target/arm: Set KVM_ARM_VCPU_SVE while probing the host
2022-07-25 18:14 [PATCH 0/3] target/arm: Fix kvm probe of ID_AA64ZFR0 Richard Henderson
2022-07-25 18:14 ` [PATCH 1/3] target/arm: Create kvm_arm_svm_supported Richard Henderson
@ 2022-07-25 18:14 ` Richard Henderson
2022-07-25 18:20 ` Richard Henderson
2022-07-25 18:14 ` [PATCH 3/3] target/arm: Move sve probe inside kvm >= 4.15 branch Richard Henderson
2 siblings, 1 reply; 7+ messages in thread
From: Richard Henderson @ 2022-07-25 18:14 UTC (permalink / raw)
To: qemu-devel; +Cc: yuzenghui, qemu-arm
Because we weren't setting this flag, our probe of ID_AA64ZFR0
was always returning zero. This also obviates the adjustment
of ID_AA64PFR0, which had sanitized the SVE field.
Reported-by: Zenghui Yu <yuzenghui@huawei.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/kvm64.c | 27 +++++++++++++--------------
1 file changed, 13 insertions(+), 14 deletions(-)
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 36ff20c9e3..8b2ae9948a 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -512,7 +512,6 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
bool sve_supported;
bool pmu_supported = false;
uint64_t features = 0;
- uint64_t t;
int err;
/* Old kernels may not know about the PREFERRED_TARGET ioctl: however
@@ -533,10 +532,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.
+ * Ask for SVE if supported, so that we can query ID_AA64ZFR0,
+ * which is otherwise RAZ.
+ */
+ sve_supported = kvm_arm_svm_supported();
+ if (sve_supported) {
+ init.features[0] |= 1 << KVM_ARM_VCPU_SVE;
+ }
+
+ /*
+ * Ask for Pointer Authentication if supported, so that we get
+ * the unsanitized field values for AA64ISAR1_EL1.
*/
if (kvm_arm_pauth_supported()) {
init.features[0] |= (1 << KVM_ARM_VCPU_PTRAUTH_ADDRESS |
@@ -680,20 +686,13 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
}
}
- sve_supported = kvm_arm_svm_supported();
-
- /* Add feature bits that can't appear until after VCPU init. */
if (sve_supported) {
- t = ahcf->isar.id_aa64pfr0;
- t = FIELD_DP64(t, ID_AA64PFR0, SVE, 1);
- ahcf->isar.id_aa64pfr0 = t;
-
/*
* There is a range of kernels between kernel commit 73433762fcae
* and f81cb2c3ad41 which have a bug where the kernel doesn't expose
* SYS_ID_AA64ZFR0_EL1 via the ONE_REG API unless the VM has enabled
- * SVE support, so we only read it here, rather than together with all
- * the other ID registers earlier.
+ * SVE support, which resulted in an error rather than RAZ.
+ * So only read the register if we set KVM_ARM_VCPU_SVE above.
*/
err |= read_sys_reg64(fdarray[2], &ahcf->isar.id_aa64zfr0,
ARM64_SYS_REG(3, 0, 0, 4, 4));
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] target/arm: Move sve probe inside kvm >= 4.15 branch
2022-07-25 18:14 [PATCH 0/3] target/arm: Fix kvm probe of ID_AA64ZFR0 Richard Henderson
2022-07-25 18:14 ` [PATCH 1/3] target/arm: Create kvm_arm_svm_supported Richard Henderson
2022-07-25 18:14 ` [PATCH 2/3] target/arm: Set KVM_ARM_VCPU_SVE while probing the host Richard Henderson
@ 2022-07-25 18:14 ` Richard Henderson
2 siblings, 0 replies; 7+ messages in thread
From: Richard Henderson @ 2022-07-25 18:14 UTC (permalink / raw)
To: qemu-devel; +Cc: yuzenghui, qemu-arm
The test for the IF block indicates no ID registers are exposed, much
less host support for SVE. Move the SVE probe into the ELSE block.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/kvm64.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 8b2ae9948a..bc3d7d9883 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -684,18 +684,18 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
err |= read_sys_reg64(fdarray[2], &ahcf->isar.reset_pmcr_el0,
ARM64_SYS_REG(3, 3, 9, 12, 0));
}
- }
- if (sve_supported) {
- /*
- * There is a range of kernels between kernel commit 73433762fcae
- * and f81cb2c3ad41 which have a bug where the kernel doesn't expose
- * SYS_ID_AA64ZFR0_EL1 via the ONE_REG API unless the VM has enabled
- * SVE support, which resulted in an error rather than RAZ.
- * So only read the register if we set KVM_ARM_VCPU_SVE above.
- */
- err |= read_sys_reg64(fdarray[2], &ahcf->isar.id_aa64zfr0,
- ARM64_SYS_REG(3, 0, 0, 4, 4));
+ if (sve_supported) {
+ /*
+ * There is a range of kernels between kernel commit 73433762fcae
+ * and f81cb2c3ad41 which have a bug where the kernel doesn't
+ * expose SYS_ID_AA64ZFR0_EL1 via the ONE_REG API unless the VM has
+ * enabled SVE support, which resulted in an error rather than RAZ.
+ * So only read the register if we set KVM_ARM_VCPU_SVE above.
+ */
+ err |= read_sys_reg64(fdarray[2], &ahcf->isar.id_aa64zfr0,
+ ARM64_SYS_REG(3, 0, 0, 4, 4));
+ }
}
kvm_arm_destroy_scratch_host_vcpu(fdarray);
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] target/arm: Set KVM_ARM_VCPU_SVE while probing the host
2022-07-25 18:14 ` [PATCH 2/3] target/arm: Set KVM_ARM_VCPU_SVE while probing the host Richard Henderson
@ 2022-07-25 18:20 ` Richard Henderson
0 siblings, 0 replies; 7+ messages in thread
From: Richard Henderson @ 2022-07-25 18:20 UTC (permalink / raw)
To: qemu-devel; +Cc: yuzenghui, qemu-arm
On 7/25/22 11:14, Richard Henderson wrote:
> Because we weren't setting this flag, our probe of ID_AA64ZFR0
> was always returning zero. This also obviates the adjustment
> of ID_AA64PFR0, which had sanitized the SVE field.
Oh, I meant to say here that this the effects of the bug are not visible, because the only
thing that ISAR.ID_AA64ZFR0 is used for within qemu at present is tcg translation. The
other tests for SVE within KVM are via ISAR.ID_AA64PFR0.SVE.
r~
>
> Reported-by: Zenghui Yu <yuzenghui@huawei.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/arm/kvm64.c | 27 +++++++++++++--------------
> 1 file changed, 13 insertions(+), 14 deletions(-)
>
> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index 36ff20c9e3..8b2ae9948a 100644
> --- a/target/arm/kvm64.c
> +++ b/target/arm/kvm64.c
> @@ -512,7 +512,6 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
> bool sve_supported;
> bool pmu_supported = false;
> uint64_t features = 0;
> - uint64_t t;
> int err;
>
> /* Old kernels may not know about the PREFERRED_TARGET ioctl: however
> @@ -533,10 +532,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.
> + * Ask for SVE if supported, so that we can query ID_AA64ZFR0,
> + * which is otherwise RAZ.
> + */
> + sve_supported = kvm_arm_svm_supported();
> + if (sve_supported) {
> + init.features[0] |= 1 << KVM_ARM_VCPU_SVE;
> + }
> +
> + /*
> + * Ask for Pointer Authentication if supported, so that we get
> + * the unsanitized field values for AA64ISAR1_EL1.
> */
> if (kvm_arm_pauth_supported()) {
> init.features[0] |= (1 << KVM_ARM_VCPU_PTRAUTH_ADDRESS |
> @@ -680,20 +686,13 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
> }
> }
>
> - sve_supported = kvm_arm_svm_supported();
> -
> - /* Add feature bits that can't appear until after VCPU init. */
> if (sve_supported) {
> - t = ahcf->isar.id_aa64pfr0;
> - t = FIELD_DP64(t, ID_AA64PFR0, SVE, 1);
> - ahcf->isar.id_aa64pfr0 = t;
> -
> /*
> * There is a range of kernels between kernel commit 73433762fcae
> * and f81cb2c3ad41 which have a bug where the kernel doesn't expose
> * SYS_ID_AA64ZFR0_EL1 via the ONE_REG API unless the VM has enabled
> - * SVE support, so we only read it here, rather than together with all
> - * the other ID registers earlier.
> + * SVE support, which resulted in an error rather than RAZ.
> + * So only read the register if we set KVM_ARM_VCPU_SVE above.
> */
> err |= read_sys_reg64(fdarray[2], &ahcf->isar.id_aa64zfr0,
> ARM64_SYS_REG(3, 0, 0, 4, 4));
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] target/arm: Create kvm_arm_svm_supported
2022-07-25 18:14 ` [PATCH 1/3] target/arm: Create kvm_arm_svm_supported Richard Henderson
@ 2022-07-26 2:02 ` Zenghui Yu via
2022-07-26 4:40 ` Richard Henderson
0 siblings, 1 reply; 7+ messages in thread
From: Zenghui Yu via @ 2022-07-26 2:02 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, qemu-arm
Hi Richard,
On 2022/7/26 2:14, Richard Henderson wrote:
> Indication for support for SVE will not depend on whether we
> perform the query on the main kvm_state or the temp vcpu.
> Mirror kvm_arm_pauth_supported.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/arm/kvm64.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index d16d4ea250..36ff20c9e3 100644
> --- a/target/arm/kvm64.c
> +++ b/target/arm/kvm64.c
> @@ -496,6 +496,11 @@ static bool kvm_arm_pauth_supported(void)
> kvm_check_extension(kvm_state, KVM_CAP_ARM_PTRAUTH_GENERIC));
> }
>
> +static bool kvm_arm_svm_supported(void)
> +{
> + return kvm_check_extension(kvm_state, KVM_CAP_ARM_SVE);
> +}
> +
Not sure if it's a typo. Maybe we should instead use
kvm_arm_sve_supported() which was introduced in commit 14e99e0fbbc6.
Zenghui
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] target/arm: Create kvm_arm_svm_supported
2022-07-26 2:02 ` Zenghui Yu via
@ 2022-07-26 4:40 ` Richard Henderson
0 siblings, 0 replies; 7+ messages in thread
From: Richard Henderson @ 2022-07-26 4:40 UTC (permalink / raw)
To: Zenghui Yu; +Cc: qemu-devel, qemu-arm
On 7/25/22 19:02, Zenghui Yu wrote:
> Hi Richard,
>
> On 2022/7/26 2:14, Richard Henderson wrote:
>> Indication for support for SVE will not depend on whether we
>> perform the query on the main kvm_state or the temp vcpu.
>> Mirror kvm_arm_pauth_supported.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>> target/arm/kvm64.c | 7 ++++++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
>> index d16d4ea250..36ff20c9e3 100644
>> --- a/target/arm/kvm64.c
>> +++ b/target/arm/kvm64.c
>> @@ -496,6 +496,11 @@ static bool kvm_arm_pauth_supported(void)
>> kvm_check_extension(kvm_state, KVM_CAP_ARM_PTRAUTH_GENERIC));
>> }
>>
>> +static bool kvm_arm_svm_supported(void)
>> +{
>> + return kvm_check_extension(kvm_state, KVM_CAP_ARM_SVE);
>> +}
>> +
>
> Not sure if it's a typo. Maybe we should instead use
> kvm_arm_sve_supported() which was introduced in commit 14e99e0fbbc6.
Oof, it certainly is.
r~
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-07-26 4:42 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-25 18:14 [PATCH 0/3] target/arm: Fix kvm probe of ID_AA64ZFR0 Richard Henderson
2022-07-25 18:14 ` [PATCH 1/3] target/arm: Create kvm_arm_svm_supported Richard Henderson
2022-07-26 2:02 ` Zenghui Yu via
2022-07-26 4:40 ` Richard Henderson
2022-07-25 18:14 ` [PATCH 2/3] target/arm: Set KVM_ARM_VCPU_SVE while probing the host Richard Henderson
2022-07-25 18:20 ` Richard Henderson
2022-07-25 18:14 ` [PATCH 3/3] target/arm: Move sve probe inside kvm >= 4.15 branch Richard Henderson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.