On Wed, Dec 22, 2021, Jing Liu wrote: > Statically enable all xfeatures allowed by guest perm in Statically isn't the right word. It's not dymanic with respect to running the vCPU, but it's certainly not static. I think you can just omit "Statically" entirely. > KVM_SET_CPUID2, with fpstate buffer sized accordingly. This avoids > run-time expansion in the emulation and restore path of XCR0 and > XFD MSR [1]. > > Change kvm_vcpu_after_set_cpuid() to return error given fpstate > reallocation may fail. > > [1] https://lore.kernel.org/all/20211214024948.048572883@xxxxxxxxxxxxx/ > > Suggested-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx> > Suggested-by: Paolo Bonzini <pbonzini@xxxxxxxxxx> > Signed-off-by: Jing Liu <jing2.liu@xxxxxxxxx> > --- > arch/x86/kvm/cpuid.c | 24 +++++++++++++++++------- > 1 file changed, 17 insertions(+), 7 deletions(-) > > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c > index a068373a7fbd..eb5a5070accb 100644 > --- a/arch/x86/kvm/cpuid.c > +++ b/arch/x86/kvm/cpuid.c > @@ -204,10 +204,12 @@ void kvm_update_cpuid_runtime(struct kvm_vcpu *vcpu) > } > EXPORT_SYMBOL_GPL(kvm_update_cpuid_runtime); > > -static void kvm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu) > +static int kvm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu) > { > struct kvm_lapic *apic = vcpu->arch.apic; > struct kvm_cpuid_entry2 *best; > + u64 xfeatures; > + int r; > > best = kvm_find_cpuid_entry(vcpu, 1, 0); > if (best && apic) { > @@ -222,9 +224,17 @@ static void kvm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu) > best = kvm_find_cpuid_entry(vcpu, 0xD, 0); > if (!best) > vcpu->arch.guest_supported_xcr0 = 0; > - else > - vcpu->arch.guest_supported_xcr0 = > - (best->eax | ((u64)best->edx << 32)) & supported_xcr0; > + else { > + xfeatures = best->eax | ((u64)best->edx << 32); > + > + vcpu->arch.guest_supported_xcr0 = xfeatures & supported_xcr0; > + > + if (xfeatures != vcpu->arch.guest_fpu.xfeatures) { > + r = fpu_update_guest_perm_features(&vcpu->arch.guest_fpu); > + if (r) > + return r; IMO, this should be done and check before "committing" state, otherwise KVM will set the vCPU's CPUID info and update a variety of state, but then tell userspace that it failed. The -EPERM case in particular falls squarely into the "check" category.
