linux-kernel - Re: [PATCH v5 3/7] x86/fpu/xstate: Differentiate default features for host and guest FPUs

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <aBJMxGLjXY9Ffv5M@google.com>
Date: Wed, 30 Apr 2025 09:20:08 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Rick P Edgecombe <rick.p.edgecombe@...el.com>
Cc: Chang Seok Bae <chang.seok.bae@...el.com>, "ebiggers@...gle.com" <ebiggers@...gle.com>, 
	"kvm@...r.kernel.org" <kvm@...r.kernel.org>, Dave Hansen <dave.hansen@...el.com>, 
	"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>, Stanislav Spassov <stanspas@...zon.de>, 
	"levymitchell0@...il.com" <levymitchell0@...il.com>, 
	"samuel.holland@...ive.com" <samuel.holland@...ive.com>, Xin3 Li <xin3.li@...el.com>, 
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "mingo@...hat.com" <mingo@...hat.com>, 
	"vigbalas@....com" <vigbalas@....com>, "pbonzini@...hat.com" <pbonzini@...hat.com>, 
	"tglx@...utronix.de" <tglx@...utronix.de>, "john.allen@....com" <john.allen@....com>, 
	"mlevitsk@...hat.com" <mlevitsk@...hat.com>, Weijiang Yang <weijiang.yang@...el.com>, 
	"hpa@...or.com" <hpa@...or.com>, "peterz@...radead.org" <peterz@...radead.org>, 
	"aruna.ramakrishna@...cle.com" <aruna.ramakrishna@...cle.com>, Chao Gao <chao.gao@...el.com>, 
	"bp@...en8.de" <bp@...en8.de>, "x86@...nel.org" <x86@...nel.org>
Subject: Re: [PATCH v5 3/7] x86/fpu/xstate: Differentiate default features for
 host and guest FPUs

On Wed, Apr 30, 2025, Rick P Edgecombe wrote:
> On Wed, 2025-04-30 at 08:01 -0700, Chang S. Bae wrote:
> > On 4/28/2025 8:36 PM, Edgecombe, Rick P wrote:
> > > 
> > > KVM_GET_XSAVE is part of KVM's API. It uses fields configured in struct
> > > fpu_guest. If fpu_user_cfg.default_features changes value (in the current code)
> > > it would change KVM's uABI. 
> > 
> > Not quite. The ABI reflects the XSAVE format directly. The XSAVE header 
> > indicates which feature states are present, so while the _contents_ of 
> > the buffer may vary depending on the feature set, the _format_ itself 
> > remains unchanged. That doesn't constitute a uABI change.
> 
> Heh, ok sure.

Hmm, it's a valid point that format isn't changing, and that host userspace and
guests will inevitably have different state in the XSAVE buffer.

That said, it's still an ABI change in the sense that once support for CET_S is
added, userspace can rely on KVM_{G,S}ET_XSAVE(2) to save/restore CET_S state,
and dropping that support would clearly break userspace.

> > > It should be simple. Two new configuration fields are added in this patch that
> > > match the existing concept and values of existing configurations fields. Per
> > > Sean, there are no plans to have them diverge. So why add them. 
> > 
> > I'm fine with dropping them -- as long as the resulting code remains 
> > clear and avoids unnecessary complexity around VCPU allocation.
> > 
> > Here are some of the considerations that led me to suggest them in the 
> > first place:
> > 
> >   * The guest-only feature model should be established in a clean and
> >     structured way.
> >   * The initialization logic should stay explicit -- especially to make
> >     it clear what constitutes guest features, even when they match host
> >     features. That naturally led to introducing a dedicated data
> >     structure.
> >   * Since the VCPU FPU container includes struct fpstate, it felt
> >     appropriate to mirror relevant fields where useful.
> >   * Including user_size and user_xfeatures made the VCPU allocation logic
> >     more straightforward and self-contained.
> > 
> > And to clarify -- this addition doesn’t necessarily imply divergence 
> > from fpu_guest_cfg. Its usage is local to setting up the guest fpstate, 
> > and nothing more.
> 
> I'd like to close this out. I see there there is currently one concept of user
> features and size, and per Sean, KVM intends to stay consistent with the rest of
> the kernel - leaving it at one concept. This was new info since you suggested
> the fields. So why don't you propose a resolution here and we'll just go with
> it.

I'm not totally opposed to diverging, but IMO there needs to be strong motivation
to do so.  Sharing code and ABI between KVM and the kernel is mutually beneficial
on multiple fronts.

Unless I've missed something, KVM will need to provide save/restore support via
MSRs for all CET_S state anyways, so I don't see any motivation whatsoever in this
particular case.