Re: [PATCH] KVM: x86: workaround invalid CPUID[0xD, 9] info on some AMD processors

2022-03-23 Thread Peter Krempa
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é 
> Analyzed-by: Yang Zhong 
> Signed-off-by: Paolo Bonzini 
> ---
>  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 

With this patch it no longer abort()s on my Ryzen 3900X




Re: [PATCH] KVM: x86: workaround invalid CPUID[0xD,9] info on some AMD processors

2022-03-23 Thread Yang Zhong
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é 
> Analyzed-by: Yang Zhong 
> Signed-off-by: Paolo Bonzini 
> ---
>  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



Re: [PATCH] KVM: x86: workaround invalid CPUID[0xD,9] info on some AMD processors

2022-03-23 Thread Daniel P . Berrangé
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é 

Also credit

  Reported-by: Peter Krempa 

> Analyzed-by: Yang Zhong 
> Signed-off-by: Paolo Bonzini 
> ---
>  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é 

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 :|