Messages in this thread | ![/](/images/icornerl.gif) | | Date | Fri, 26 Jan 2024 19:48:52 +0100 | Subject | Re: [PATCH] KVM: x86: Give a hint when Win2016 might fail to boot due to XSAVES erratum | From | "Maciej S. Szmigiero" <> |
| |
On 26.01.2024 19:36, Paolo Bonzini wrote: > On Wed, Jan 24, 2024 at 9:18 PM Maciej S. Szmigiero > <mail@maciej.szmigiero.name> wrote: >> +static void kvm_hv_xsaves_xsavec_maybe_warn_unlocked(struct kvm_vcpu *vcpu) > > Calling this function "unlocked" is confusing (others would say > "locked" is confusing instead). The double-underscore convention is > more common. > >> +{ >> + struct kvm *kvm = vcpu->kvm; >> + struct kvm_hv *hv = to_kvm_hv(kvm); >> + >> + if (hv->xsaves_xsavec_warned) >> + return; >> + >> + if (!vcpu->arch.hyperv_enabled) >> + return; > > I think these two should be in kvm_hv_xsaves_xsavec_maybe_warn(), > though the former needs to be checked again under the lock. > >> + if ((hv->hv_guest_os_id & KVM_HV_WIN2016_GUEST_ID_MASK) != >> + KVM_HV_WIN2016_GUEST_ID) >> + return; > > At this point there is no need to return. You can set > xsaves_xsavec_warned and save the checks in the future. > >> + /* UP configurations aren't affected */ >> + if (atomic_read(&kvm->online_vcpus) < 2) >> + return; >> + >> + if (boot_cpu_has(X86_FEATURE_XSAVES) || >> + !guest_cpuid_has(vcpu, X86_FEATURE_XSAVEC)) >> + return; > > boot_cpu_has can also be done first to cull the whole check. > >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >> index 27e23714e960..db0a2c40d749 100644 >> --- a/arch/x86/kvm/x86.c >> +++ b/arch/x86/kvm/x86.c >> @@ -1782,6 +1782,10 @@ static int set_efer >> if ((efer ^ old_efer) & KVM_MMU_EFER_ROLE_BITS) >> kvm_mmu_reset_context(vcpu); >> >> + if (guest_cpuid_is_amd_or_hygon(vcpu) && >> + efer & EFER_SVME) >> + kvm_hv_xsaves_xsavec_maybe_warn(vcpu); >> + >> return 0; >> } > > Checking guest_cpuid_is_amd_or_hygon() is relatively expensive, it > should be done after "efer & EFER_SVME" but really the bug can happen > just as well on Intel as far as I understand? It's just less likely > due to the AMD erratum.
Yes, I've checked this guest on an Intel host and it also fails to boot in !XSAVES && XSAVEC configuration.
Only on Intel it's purely a theoretical problem as AFAIK there's no corresponding Intel errata that disables just XSAVES.
> > I'll send a v2. > > Paolo >
Thanks, Maciej
| ![\](/images/icornerr.gif) |