Re: [PATCH] KVM: x86: Add a help to dedup loading guest/host XCR0 and XSS

[Binbin Wu <binbin.wu@linux.intel.com>]

On 11/7/2025 11:14 AM, Chao Gao wrote:
> s/help/helper in the subject.
>
> On Thu, Nov 06, 2025 at 06:11:38PM +0800, Binbin Wu wrote:
>> Add and use a helper, kvm_load_xfeatures(), to dedup the code that loads
>> guest/host xfeatures by passing XCR0 and XSS values accordingly.
>>
>> No functional change intended.
>>
>> Signed-off-by: Binbin Wu <binbin.wu@linux.intel.com>
> Reviewed-by: Chao Gao <chao.gao@intel.com>
>
> <snip>
>
>> @@ -11406,7 +11391,7 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
>> 	vcpu->mode = OUTSIDE_GUEST_MODE;
>> 	smp_wmb();
>>
>> -	kvm_load_host_xfeatures(vcpu);
>> +	kvm_load_xfeatures(vcpu, kvm_host.xcr0, kvm_host.xss);
> Nit: given that xcr0/xss are either guest or host values, would it be slightly
> better for this helper to accept a boolean (e.g., bool load_guest) to convey
> that the API loads guest (or host) values rather than arbitrary xcr0/xss
> values? like fpu_swap_kvm_fpstate().

Make sense.

>
> static void kvm_load_xfeatures(struct kvm_vcpu *vcpu, bool load_guest)
> {
> 	u64 xcr0 = load_guest ? vcpu->arch.xcr0 : kvm_host.xcr0;
> 	u64 xss  = load_guest ? vcpu->arch.ia32_xss : kvm_host.xss;

Since they are only used once, I even want to open code them as:

static void kvm_load_xfeatures(struct kvm_vcpu *vcpu, bool load_guest)
{
         if (vcpu->arch.guest_state_protected)
                 return;

         if (kvm_is_cr4_bit_set(vcpu, X86_CR4_OSXSAVE)) {
                 if (vcpu->arch.xcr0 != kvm_host.xcr0)
                         xsetbv(XCR_XFEATURE_ENABLED_MASK,
                                load_guest ? vcpu->arch.xcr0 : kvm_host.xcr0);

                 if (guest_cpu_cap_has(vcpu, X86_FEATURE_XSAVES) &&
                     vcpu->arch.ia32_xss != kvm_host.xss)
                         wrmsrq(MSR_IA32_XSS,
                                load_guest ? vcpu->arch.ia32_xss : kvm_host.xss);
         }
}

>
> 	if (vcpu->arch.guest_state_protected)
> 		return;
>
>> 	/*
>> 	 * Sync xfd before calling handle_exit_irqoff() which may
>>
>> base-commit: a996dd2a5e1ec54dcf7d7b93915ea3f97e14e68a
>> prerequisite-patch-id: 9aafd634f0ab2033d7b032e227d356777469e046
>> prerequisite-patch-id: 656ce1f5aa97c77a9cf6125713707a5007b2c7ba
>> prerequisite-patch-id: d6328b8c0fdb8593bb534ab7378821edcf9f639d
>> prerequisite-patch-id: c7f36d1cedc4ae6416223d2225460944629b3d4f
>> -- 
>> 2.46.0
>>
>>