| Message ID | 20210921000303.400537-6-seanjc@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86: Clean up RESET "emulation" | expand |
Sean Christopherson <seanjc@google.com> writes: > Drop code to initialize XCR0 during fx_init(), a.k.a. vCPU creation, as > XCR0 has been initialized during kvm_vcpu_reset() (for RESET) since > commit a554d207dc46 ("KVM: X86: Processor States following Reset or INIT"). > > Back when XCR0 support was added by commit 2acf923e38fb ("KVM: VMX: > Enable XSAVE/XRSTOR for guest"), KVM didn't differentiate between RESET > and INIT. Ignoring the fact that calling fx_init() for INIT is obviously > wrong, e.g. FPU state after INIT is not the same as after RESET, setting > XCR0 in fx_init() was correct. > > Eventually fx_init() got moved to kvm_arch_vcpu_init(), a.k.a. vCPU > creation (ignore the terrible name) by commit 0ee6a5172573 ("x86/fpu, > kvm: Simplify fx_init()"). Finally, commit 95a0d01eef7a ("KVM: x86: Move > all vcpu init code into kvm_arch_vcpu_create()") killed off > kvm_arch_vcpu_init(), Technically, empty kvm_arch_vcpu_init() was still alive for a few more commits and only ddd259c9aaba ("KVM: Drop kvm_arch_vcpu_init() and kvm_arch_vcpu_uninit()") killed it for real but a curious reader can find all these gory details himself) > leaving behind the oddity of redundant setting of > guest state during vCPU creation. > > No functional change intended. > > Signed-off-by: Sean Christopherson <seanjc@google.com> > --- > arch/x86/kvm/x86.c | 7 +------ > 1 file changed, 1 insertion(+), 6 deletions(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index e0bff5473813..6fd3fe21863e 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -993,7 +993,7 @@ static int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr) > /* > * Do not allow the guest to set bits that we do not support > * saving. However, xcr0 bit 0 is always set, even if the > - * emulated CPU does not support XSAVE (see fx_init). > + * emulated CPU does not support XSAVE (see kvm_vcpu_reset()). > */ > valid_bits = vcpu->arch.guest_supported_xcr0 | XFEATURE_MASK_FP; > if (xcr0 & ~valid_bits) > @@ -10623,11 +10623,6 @@ static void fx_init(struct kvm_vcpu *vcpu) > if (boot_cpu_has(X86_FEATURE_XSAVES)) > vcpu->arch.guest_fpu->state.xsave.header.xcomp_bv = > host_xcr0 | XSTATE_COMPACTION_ENABLED; > - > - /* > - * Ensure guest xcr0 is valid for loading > - */ > - vcpu->arch.xcr0 = XFEATURE_MASK_FP; > } > > void kvm_free_guest_fpu(struct kvm_vcpu *vcpu) Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index e0bff5473813..6fd3fe21863e 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -993,7 +993,7 @@ static int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr) /* * Do not allow the guest to set bits that we do not support * saving. However, xcr0 bit 0 is always set, even if the - * emulated CPU does not support XSAVE (see fx_init). + * emulated CPU does not support XSAVE (see kvm_vcpu_reset()). */ valid_bits = vcpu->arch.guest_supported_xcr0 | XFEATURE_MASK_FP; if (xcr0 & ~valid_bits) @@ -10623,11 +10623,6 @@ static void fx_init(struct kvm_vcpu *vcpu) if (boot_cpu_has(X86_FEATURE_XSAVES)) vcpu->arch.guest_fpu->state.xsave.header.xcomp_bv = host_xcr0 | XSTATE_COMPACTION_ENABLED; - - /* - * Ensure guest xcr0 is valid for loading - */ - vcpu->arch.xcr0 = XFEATURE_MASK_FP; } void kvm_free_guest_fpu(struct kvm_vcpu *vcpu)
Drop code to initialize XCR0 during fx_init(), a.k.a. vCPU creation, as XCR0 has been initialized during kvm_vcpu_reset() (for RESET) since commit a554d207dc46 ("KVM: X86: Processor States following Reset or INIT"). Back when XCR0 support was added by commit 2acf923e38fb ("KVM: VMX: Enable XSAVE/XRSTOR for guest"), KVM didn't differentiate between RESET and INIT. Ignoring the fact that calling fx_init() for INIT is obviously wrong, e.g. FPU state after INIT is not the same as after RESET, setting XCR0 in fx_init() was correct. Eventually fx_init() got moved to kvm_arch_vcpu_init(), a.k.a. vCPU creation (ignore the terrible name) by commit 0ee6a5172573 ("x86/fpu, kvm: Simplify fx_init()"). Finally, commit 95a0d01eef7a ("KVM: x86: Move all vcpu init code into kvm_arch_vcpu_create()") killed off kvm_arch_vcpu_init(), leaving behind the oddity of redundant setting of guest state during vCPU creation. No functional change intended. Signed-off-by: Sean Christopherson <seanjc@google.com> --- arch/x86/kvm/x86.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)