Re: [PATCH v14 15/22] KVM: x86: Don't emulate instructions guarded by CET

[Sean Christopherson <seanjc@google.com>]

On Fri, Sep 12, 2025, Xiaoyao Li wrote:
> On 9/11/2025 6:42 PM, Chao Gao wrote:
> > > > @@ -4941,6 +4947,24 @@ int x86_decode_insn(struct x86_emulate_ctxt *ctxt, void *insn, int insn_len, int
> > > >    	if (ctxt->d == 0)
> > > >    		return EMULATION_FAILED;
> > > > +	if (ctxt->ops->get_cr(ctxt, 4) & X86_CR4_CET) {
> > > > +		u64 u_cet, s_cet;
> > > > +		bool stop_em;
> > > > +
> > > > +		if (ctxt->ops->get_msr(ctxt, MSR_IA32_U_CET, &u_cet) ||
> > > > +		    ctxt->ops->get_msr(ctxt, MSR_IA32_S_CET, &s_cet))
> > > > +			return EMULATION_FAILED;
> > > > +
> > > > +		stop_em = ((u_cet & CET_SHSTK_EN) || (s_cet & CET_SHSTK_EN)) &&
> > > > +			  (opcode.flags & ShadowStack);
> > > > +
> > > > +		stop_em |= ((u_cet & CET_ENDBR_EN) || (s_cet & CET_ENDBR_EN)) &&
> > > > +			   (opcode.flags & IndirBrnTrk);
> > > 
> > > Why don't check CPL here? Just for simplicity?
> > 
> > I think so. This is a corner case and we don't want to make it very precise

Checking CPL here would not make the code more complex, e.g. naively it could be
something like:

		u64 cet;
		int r;

		if (ctxt->ops->cpl(ctxt) == 3)
			r = ctxt->ops->get_msr(ctxt, MSR_IA32_U_CET, &cet);
		else
			r = ctxt->ops->get_msr(ctxt, MSR_IA32_S_CET, &cet);
		if (r)
			return EMULATION_FAILED;

		if (cet & CET_SHSTK_EN && opcode.flags & ShadowStack)
			  return EMULATION_FAILED;

		if (cet & CET_ENDBR_EN && opcode.flags & IndirBrnTrk)
			  return EMULATION_FAILED;

> > (and thus complex). The reason is that no one had a strong opinion on whether
> > to do the CPL check or not. I asked the same question before [*], but I don't
> > have a strong opinion on this either.
> 
> I'm OK with it.

I have a strong opinion.  :-)

KVM must NOT check CPL, because inter-privilege level transfers could trigger
CET emulation and both levels.  E.g. a FAR CALL will be affected by both shadow
stacks and IBT at the target privilege level.

So this need more than just a changelog blurb, it needs a comment.  The code
can also be cleaned up and optimized.  Reading CR4 and two MSRs (via indirect
calls, i.e. potential retpolines) is wasteful for the vast majority of instructions,
and gathering "stop emulation" into a local variable when a positive test is fatal
is pointless.

	/*
	 * Reject emulation if KVM might need to emulate shadow stack updates
	 * and/or indirect branch tracking enforcement, which the emulator
	 * doesn't support.  Deliberately don't check CPL as inter-privilege
	 * level transfers can trigger emulation at both privilege levels, and
	 * the expectation is that the guest will not require emulation of any
	 * CET-affected instructions at any privilege level.
	 */
	if (opcode.flags & (ShadowStack | IndirBrnTrk) &&
	    ctxt->ops->get_cr(ctxt, 4) & X86_CR4_CET) {
		u64 u_cet, s_cet;

		if (ctxt->ops->get_msr(ctxt, MSR_IA32_U_CET, &u_cet) ||
		    ctxt->ops->get_msr(ctxt, MSR_IA32_S_CET, &s_cet))
			return EMULATION_FAILED;

		if ((u_cet | s_cet) & CET_SHSTK_EN && opcode.flags & ShadowStack)
			  return EMULATION_FAILED;

		if ((u_cet | s_cet) & CET_ENDBR_EN && opcode.flags & IndirBrnTrk)
			  return EMULATION_FAILED;
	}