On Fri, Sep 12, 2025, Sean Christopherson wrote:
> On Fri, Sep 12, 2025, Xiaoyao Li wrote:
> > On 9/11/2025 6:42 PM, Chao Gao wrote:
> > > (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;
> }
On second thought, I think it's worth doing the CPL checks. Explaining why KVM
doesn't bother with checking privilege level is more work than just writing the
code.
/*
* Reject emulation if KVM might need to emulate shadow stack updates
* and/or indirect branch tracking enforcement, which the emulator
* doesn't support.
*/
if (opcode.flags & (ShadowStack | IndirBrnTrk) &&
ctxt->ops->get_cr(ctxt, 4) & X86_CR4_CET) {
u64 u_cet = 0, s_cet = 0;
/*
* Check both User and Supervisor on far transfers as inter-
* privilege level transfers are impacted by CET at the target
* privilege levels, and that is not known at this time. The
* the expectation is that the guest will not require emulation
* of any CET-affected instructions at any privilege level.
*/
if (!(opcode.flags & NearBranch)) {
u_cet = s_cet = CET_SHSTK_EN | CET_ENDBR_EN;
} else if (ctxt->ops->cpl(ctxt) == 3) {
u_cet = CET_SHSTK_EN | CET_ENDBR_EN;
} else {
s_cet = CET_SHSTK_EN | CET_ENDBR_EN;
}
if ((u_cet && ctxt->ops->get_msr(ctxt, MSR_IA32_U_CET, &u_cet)) ||
(s_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;
}
Side topic, has anyone actually tested that this works? I.e. that attempts to
emulate CET-affected instructions result in emulation failure? I'd love to have
a selftest for this (hint, hint), but presumably writing one is non-trivial due
to the need to get the selftest compiled with the necessary annotations, setup,
and whatnot.
