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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aQ5Dz4UznE9a0N2-@google.com>
Date: Fri, 7 Nov 2025 11:09:03 -0800
From: Sean Christopherson <seanjc@...gle.com>
To: Binbin Wu <binbin.wu@...ux.intel.com>
Cc: Chao Gao <chao.gao@...el.com>, pbonzini@...hat.com, kvm@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] KVM: x86: Add a help to dedup loading guest/host XCR0 and XSS

On Fri, Nov 07, 2025, Binbin Wu wrote:
> 
> 
> 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@...ux.intel.com>
> > Reviewed-by: Chao Gao <chao.gao@...el.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.

I don't love passing true/false, but I Xiaoyao does make a good a point that
subtly requiring the caller to pass vcpu->arch.xcr0 vs. kvm_host.xcr0 is weird
and confusing.

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

+100, I find your version much more intuitive.

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