mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Dave Hansen <dave.hansen@intel.com>
To: Paolo Bonzini <pbonzini@redhat.com>,
	Yang Weijiang <weijiang.yang@intel.com>,
	"Gao, Chao" <chao.gao@intel.com>,
	"Edgecombe, Rick P" <rick.p.edgecombe@intel.com>,
	Maxim Levitsky <mlevitsk@redhat.com>,
	John Allen <john.allen@amd.com>,
	LKML <linux-kernel@vger.kernel.org>,
	peterz@infradead.org, kvm@vger.kernel.org,
	binbin.wu@linux.intel.com,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	"Bae, Chang Seok" <chang.seok.bae@intel.com>,
	"Mehta, Sohil" <sohil.mehta@intel.com>
Subject: Re: Kernel Shadow Stacks Versus XFEATURE_MASK_GUEST_SUPERVISOR
Date: Fri, 23 Oct 2026 14:21:20 -0700	[thread overview]
Message-ID: <5c74f231-46a7-4f37-8fcd-bff84842a923@intel.com> (raw)
In-Reply-To: <bd608c7c-676a-4b65-97dd-626d0e8a5514@redhat.com>

On 10/23/26 14:00, Paolo Bonzini wrote:
> On 10/23/26 21:03, Dave Hansen wrote:
...
> If you want to quickly work around for your !CONFIG_KVM kernel, I would do
> 
> #ifdef CONFIG_KVM
> /* Supervisor features which are enabled only in guest FPUs */
> #define XFEATURE_MASK_GUEST_SUPERVISOR  XFEATURE_MASK_CET_KERNEL
> #else
> #define XFEATURE_MASK_GUEST_SUPERVISOR  0
> #endif

Yup, that'd work for sure. Thanks!

>> Any thoughts on what we should do?
>>
>> Actually context switching XFEATURE_CET_KERNEL seems kinda silly in a
>> lot of ways. PL0_SSP can always be derived _and_ it isn't actually even
>> needed at context switch time, only when running userspace.
> 
> True but, just to understand, what is the performance like if you
> actually use XSAVES/XRSTORS to update SSP0 on context switches instead
> of WRMSR+cache?  Hopefully the microcode does a bunch of optimizations
> as well.

First, we can use WRMSRNS for this MSR, so it's faster than usual.
Second, on kernel=>kernel context switches that don't go out to
userspace, it doesn't need to be written at all. It's kinda like the
user FPU state. Third, I'm actually reusing the fred_ssp0 cache, so
further reduces the incremental cost. Fourth, it's not _really_
context-switched. There's never a need to read the register. It can
always be conjured out of thin air. It's possible to only write the
register, never read it.

I can benchmark it, but I seriously doubt that an XSAVE/XRSTOR at
context switch of three fields is going to beat an WRMSRNS of one.

>> I could just aggressively blow away my SSP0 cache any time an XRSTOR
>> touches XFEATURE_CET_KERNEL. That would be easy. But it also seems silly
>> that XFEATURE_CET_KERNEL is being blown away at _all_ during normal
>> context switches. Heck, it goes further than that because:
>>
>> fpregs_restore_userregs() does:
>>
>>     restore_fpregs_from_fpstate(fpu->fpstate,
>>                     XFEATURE_MASK_FPSTATE);
>>
>> Where XFEATURE_MASK_FPSTATE contains XFEATURE_MASK_SUPERVISOR_SUPPORTED.
> 
> The actual mask used by restore_fpregs_from_fpstate is tighter though,
> it's "fpu_kernel_cfg.max_features & XFEATURE_MASK_FPSTATE".
> 
> Can you replace fpu_kernel_cfg.max_features with some fpstate->xyz
> (sorry too late to think it through :)) that has CET_KERNEL for the
> guest FPU and not for the regular one?

Sure I can give that a shot.

There's one minor issue in that we need to init dynamic features which
_aren't_ in fpstate->xfeatures when context switching. There's a little
note about this in restore_fpregs_from_fpstate().

I think what you're getting at is that we need _something_ to say that
KVM fpstates should muck with XFEATURE_CET_KERNEL and task fpstates
should not, even though the task ones have space for XFEATURE_CET_KERNEL.

That's certainly doable.

>> So a "restore_userregs" function is blowing away supervisor state.
>>
>> IOW, I think we're using XFEATURE_MASK_FPSTATE a bit too broadly.
>>
>> I'd like to get to a place where only the KVM code even has
>> XFEATURE_CET_KERNEL in its RFBM.
> That I agree, but I think it can be handled in {save,restore}
> _fpregs_to_fpstate().

I think I understand what you're going after at least. I'll poke at it a
bit more.

Thanks for the thoughts!

      reply	other threads:[~2026-10-23 21:21 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-23 19:03 Dave Hansen
2026-10-23 21:00 ` Paolo Bonzini
2026-10-23 21:21   ` Dave Hansen [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5c74f231-46a7-4f37-8fcd-bff84842a923@intel.com \
    --to=dave.hansen@intel.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=binbin.wu@linux.intel.com \
    --cc=chang.seok.bae@intel.com \
    --cc=chao.gao@intel.com \
    --cc=john.allen@amd.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mlevitsk@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rick.p.edgecombe@intel.com \
    --cc=sohil.mehta@intel.com \
    --cc=tglx@linutronix.de \
    --cc=weijiang.yang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®