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

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

On 5/21/2024 1:09 AM, Sean Christopherson wrote:
> On Mon, May 20, 2024, Weijiang Yang wrote:
>> On 5/17/2024 10:26 PM, Sean Christopherson wrote:
>>> On Fri, May 17, 2024, Thomas Gleixner wrote:
>>>> On Thu, May 16 2024 at 07:39, Sean Christopherson wrote:
>>>>> On Thu, May 16, 2024, Weijiang Yang wrote:
>>>>>> We synced the issue internally, and got conclusion that KVM should honor host
>>>>>> IBT config.  In this case IBT bit in boot_cpu_data should be honored.  With
>>>>>> this policy, it can avoid CPUID confusion to guest side due to host ibt=off
>>>>>> config.
>>>>> What was the reasoning?  CPUID confusion is a weak justification, e.g. it's not
>>>>> like the guest has visibility into the host kernel, and raw CPUID will still show
>>>>> IBT support in the host.
>>>>>
>>>>> On the other hand, I can definitely see folks wanting to expose IBT to guests
>>>>> when running non-complaint host kernels, especially when live migration is in
>>>>> play, i.e. when hiding IBT from the guest will actively cause problems.
>>>> I have to disagree here violently.
>>>>
>>>> If the exposure of a CPUID bit to a guest requires host side support,
>>>> e.g. in xstate handling, then exposing it to a guest is simply not
>>>> possible.
>>> Ya, I don't disagree, I just didn't realize that CET_USER would be cleared in the
>>> supported xfeatures mask.
>> For host side support, fortunately,  this patch already has some checks for
>> that. But for userspace CPUID config, it allows IBT to be exposed alone.
>>
>> IIUC, this series tries to tie IBT to SHSTK feature, i.e., IBT cannot be
>> exposed as an independent feature to guest without exposing SHSTK at the same
>> time. If it is, then below patch is not needed anymore:
>> https://lore.kernel.org/all/20240219074733.122080-3-weijiang.yang@intel.com/
> That's a question for the x86 maintainers.  Specifically, do they want to allow
> enabling XFEATURE_CET_USER even if userspace shadow stack support is disabled.
>
> I don't think it impacts KVM, at least not directly.  Regardless of what decision
> the kernel makes, KVM needs to disable IBT and SHSTK if CET_USER _or_ CET_KERNEL
> is missing, which KVM already does via:
>
> 	if ((kvm_caps.supported_xss & (XFEATURE_MASK_CET_USER |
> 	     XFEATURE_MASK_CET_KERNEL)) !=
> 	    (XFEATURE_MASK_CET_USER | XFEATURE_MASK_CET_KERNEL)) {
> 		kvm_cpu_cap_clear(X86_FEATURE_SHSTK);
> 		kvm_cpu_cap_clear(X86_FEATURE_IBT);
> 		kvm_caps.supported_xss &= ~(XFEATURE_MASK_CET_USER |
> 					    XFEATURE_MASK_CET_KERNEL);
> 	}
>
>> I'd check and clear IBT bit from CPUID when userspace enables only IBT via
>> KVM_SET_CPUID2.
> No.  It is userspace's responsibility to provide a sane CPUID model for the guest.
> KVM needs to ensure that *KVM* doesn't treat IBT as supported if the kernel doesn't
> allow XFEATURE_CET_USER, but userspace can advertise whatever it wants to the guest
> (and gets to keep the pieces if it does something funky).

OK, I think we can go ahead to keep KVM patches as-is given the fact user IBT is not enabled in Linux.
I only hope other OSes can enforce both SHSTK and IBT dependency on XFEATURE_CET_USER so
that user IBT can work well there.

Then IBT can be exposed to guest alone because guest *kernel* IBT only relies on S_CET MSR  which is
VMCS auto-saved/restored.

What's your thoughts?