target/i386/cpu.c | 4 ++-- target/i386/cpu.h | 2 ++ target/i386/kvm/kvm-cpu.c | 19 ++++++++++++------- 3 files changed, 16 insertions(+), 9 deletions(-)
Some AMD processors expose the PKRU extended save state even if they do not have
the related PKU feature in CPUID. Worse, when they do they report a size of
64, whereas the expected size of the PKRU extended save state is 8, therefore
the esa->size == eax assertion does not hold.
The state is already ignored by KVM_GET_SUPPORTED_CPUID because it
was not enabled in the host XCR0. However, QEMU kvm_cpu_xsave_init()
runs before QEMU invokes arch_prctl() to enable dynamically-enabled
save states such as XTILEDATA, and KVM_GET_SUPPORTED_CPUID hides save
states that have yet to be enabled. Therefore, kvm_cpu_xsave_init()
needs to consult the host CPUID instead of KVM_GET_SUPPORTED_CPUID,
and dies with an assertion failure.
When setting up the ExtSaveArea array to match the host, ignore features that
KVM does not report as supported. This will cause QEMU to skip the incorrect
CPUID leaf instead of tripping the assertion.
Reported-by: Daniel P. Berrangé <berrange@redhat.com>
Analyzed-by: Yang Zhong <yang.zhong@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/cpu.c | 4 ++--
target/i386/cpu.h | 2 ++
target/i386/kvm/kvm-cpu.c | 19 ++++++++++++-------
3 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index a88d6554c8..ec3b50bf6e 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4981,8 +4981,8 @@ CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
return cpu_list;
}
-static uint64_t x86_cpu_get_supported_feature_word(FeatureWord w,
- bool migratable_only)
+uint64_t x86_cpu_get_supported_feature_word(FeatureWord w,
+ bool migratable_only)
{
FeatureWordInfo *wi = &feature_word_info[w];
uint64_t r = 0;
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 5e406088a9..e31e6bd8b8 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -606,6 +606,8 @@ typedef enum FeatureWord {
} FeatureWord;
typedef uint64_t FeatureWordArray[FEATURE_WORDS];
+uint64_t x86_cpu_get_supported_feature_word(FeatureWord w,
+ bool migratable_only);
/* cpuid_features bits */
#define CPUID_FP87 (1U << 0)
diff --git a/target/i386/kvm/kvm-cpu.c b/target/i386/kvm/kvm-cpu.c
index a35a1bf9fe..5eb955ce9a 100644
--- a/target/i386/kvm/kvm-cpu.c
+++ b/target/i386/kvm/kvm-cpu.c
@@ -99,13 +99,18 @@ static void kvm_cpu_xsave_init(void)
for (i = XSTATE_SSE_BIT + 1; i < XSAVE_STATE_AREA_COUNT; i++) {
ExtSaveArea *esa = &x86_ext_save_areas[i];
- if (esa->size) {
- host_cpuid(0xd, i, &eax, &ebx, &ecx, &edx);
- if (eax != 0) {
- assert(esa->size == eax);
- esa->offset = ebx;
- esa->ecx = ecx;
- }
+ if (!esa->size) {
+ continue;
+ }
+ if ((x86_cpu_get_supported_feature_word(esa->feature, false) & esa->bits)
+ != esa->bits) {
+ continue;
+ }
+ host_cpuid(0xd, i, &eax, &ebx, &ecx, &edx);
+ if (eax != 0) {
+ assert(esa->size == eax);
+ esa->offset = ebx;
+ esa->ecx = ecx;
}
}
}
--
2.35.1
On Wed, Mar 23, 2022 at 12:43:15PM +0100, Paolo Bonzini wrote: > Some AMD processors expose the PKRU extended save state even if they do not have > the related PKU feature in CPUID. Worse, when they do they report a size of > 64, whereas the expected size of the PKRU extended save state is 8, therefore > the esa->size == eax assertion does not hold. > > The state is already ignored by KVM_GET_SUPPORTED_CPUID because it > was not enabled in the host XCR0. However, QEMU kvm_cpu_xsave_init() > runs before QEMU invokes arch_prctl() to enable dynamically-enabled > save states such as XTILEDATA, and KVM_GET_SUPPORTED_CPUID hides save > states that have yet to be enabled. Therefore, kvm_cpu_xsave_init() > needs to consult the host CPUID instead of KVM_GET_SUPPORTED_CPUID, > and dies with an assertion failure. > > When setting up the ExtSaveArea array to match the host, ignore features that > KVM does not report as supported. This will cause QEMU to skip the incorrect > CPUID leaf instead of tripping the assertion. Closes: https://gitlab.com/qemu-project/qemu/-/issues/916 > > Reported-by: Daniel P. Berrangé <berrange@redhat.com> Also credit Reported-by: Peter Krempa <pkrempa@redhat.com> > Analyzed-by: Yang Zhong <yang.zhong@intel.com> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > target/i386/cpu.c | 4 ++-- > target/i386/cpu.h | 2 ++ > target/i386/kvm/kvm-cpu.c | 19 ++++++++++++------- > 3 files changed, 16 insertions(+), 9 deletions(-) Tested-by: Daniel P. Berrangé <berrange@redhat.com> no longer crashes on the AMD machine I have to hand. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Wed, Mar 23, 2022 at 12:43:15 +0100, Paolo Bonzini wrote: > Some AMD processors expose the PKRU extended save state even if they do not have > the related PKU feature in CPUID. Worse, when they do they report a size of > 64, whereas the expected size of the PKRU extended save state is 8, therefore > the esa->size == eax assertion does not hold. > > The state is already ignored by KVM_GET_SUPPORTED_CPUID because it > was not enabled in the host XCR0. However, QEMU kvm_cpu_xsave_init() > runs before QEMU invokes arch_prctl() to enable dynamically-enabled > save states such as XTILEDATA, and KVM_GET_SUPPORTED_CPUID hides save > states that have yet to be enabled. Therefore, kvm_cpu_xsave_init() > needs to consult the host CPUID instead of KVM_GET_SUPPORTED_CPUID, > and dies with an assertion failure. > > When setting up the ExtSaveArea array to match the host, ignore features that > KVM does not report as supported. This will cause QEMU to skip the incorrect > CPUID leaf instead of tripping the assertion. > > Reported-by: Daniel P. Berrangé <berrange@redhat.com> > Analyzed-by: Yang Zhong <yang.zhong@intel.com> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > target/i386/cpu.c | 4 ++-- > target/i386/cpu.h | 2 ++ > target/i386/kvm/kvm-cpu.c | 19 ++++++++++++------- > 3 files changed, 16 insertions(+), 9 deletions(-) Tested-by: Peter Krempa <pkrempa@redhat.com> With this patch it no longer abort()s on my Ryzen 3900X
On Wed, Mar 23, 2022 at 12:43:15PM +0100, Paolo Bonzini wrote: > Some AMD processors expose the PKRU extended save state even if they do not have > the related PKU feature in CPUID. Worse, when they do they report a size of > 64, whereas the expected size of the PKRU extended save state is 8, therefore > the esa->size == eax assertion does not hold. > > The state is already ignored by KVM_GET_SUPPORTED_CPUID because it > was not enabled in the host XCR0. However, QEMU kvm_cpu_xsave_init() > runs before QEMU invokes arch_prctl() to enable dynamically-enabled > save states such as XTILEDATA, and KVM_GET_SUPPORTED_CPUID hides save > states that have yet to be enabled. Therefore, kvm_cpu_xsave_init() > needs to consult the host CPUID instead of KVM_GET_SUPPORTED_CPUID, > and dies with an assertion failure. > > When setting up the ExtSaveArea array to match the host, ignore features that > KVM does not report as supported. This will cause QEMU to skip the incorrect > CPUID leaf instead of tripping the assertion. > > Reported-by: Daniel P. Berrangé <berrange@redhat.com> > Analyzed-by: Yang Zhong <yang.zhong@intel.com> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > target/i386/cpu.c | 4 ++-- > target/i386/cpu.h | 2 ++ > target/i386/kvm/kvm-cpu.c | 19 ++++++++++++------- > 3 files changed, 16 insertions(+), 9 deletions(-) Verified this patch on AMD EPYC 7402P, no crash issue now. thanks! Yang
© 2016 - 2024 Red Hat, Inc.