Re: [PATCH v10 24/27] KVM: x86: Enable CET virtualization for VMX and advertise to userspace

["Yang, Weijiang" <weijiang.yang@intel.com>]

On 5/22/2024 11:06 PM, Edgecombe, Rick P wrote:
> On Wed, 2024-05-22 at 17:03 +0800, Yang, Weijiang wrote:
>> Side topic:  would it be reasonable to enforce IBT dependency on
>> XFEATURE_CET_USER when *user* IBT
>> enabling patches are landing in kernel? Then guest kernel can play with user
>> IBT alone if VMM
>> userspace just wants to enable IBT for guest. Or when SHSTK is disabled for
>> whatever reason.
> I think earlier there was a comment that CET would be less likely to need to be
> disabled for security reasons, so there would not be utility for a system wide
> disable (that affects KVM). I recently remembered we actually already had a
> reason come up.
>
> The EDK2 SMI handler uses shadow stack and had a bug around saving and restoring
> CET state. Using IBT in the kernel was causing systems to hang. The temporary
> fix was to disable IBT.
>
> So the point is, let's not try to find a narrow way to get away with enabling as
> much as technically possible in KVM.
>
> The simple obviously correct solution would be:
> XFEATURE_CET_USER + XFEATURE_CET_KERNEL + X86_FEATURE_IBT = KVM IBT support
> XFEATURE_CET_USER + XFEATURE_CET_KERNEL + X86_FEATURE_SHSTK = KVM SHSTK support

Yes, I can easily achieve it by removing the raw cpuid check for KVM IBT. Host side CET xstate
support check is already there in this patch.

>
> It should be correct both with and without that patch to enable
> XFEATURE_CET_USER for X86_FEATURE_IBT.

IMHO, given the fact user IBT hasn't been enabled in kernel, it's not too bad just discarding the patch.
I can highlight the issue somewhere in this series.

>
> Then the two missing changes to expand support would be:
> 1. Fixing that ibt=off disables X86_FEATURE_IBT. The fix is to move to bool as
> peterz suggested.
> 2. Making XFEATURE_CET_USER also depend on X86_FEATURE_IBT (the patch in this
> series)
>
> We should do those, but in a later small series. Does it seem reasonable? Can we
> just do the simple obvious solution above for now?

It makes sense for me, but I want to hear x86 and KVM maintainers' voice for it.

Thanks!