Re: [PATCH v5 08/19] KVM:x86: Report KVM supported CET MSRs as to-be-saved

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 8/4/2023 1:51 PM, Chao Gao wrote:
On Fri, Aug 04, 2023 at 11:13:36AM +0800, Yang, Weijiang wrote:
@@ -7214,6 +7217,13 @@ static void kvm_probe_msr_to_save(u32 msr_index)
		if (!kvm_caps.supported_xss)
			return;
		break;
+	case MSR_IA32_U_CET:
+	case MSR_IA32_S_CET:
+	case MSR_KVM_GUEST_SSP:
+	case MSR_IA32_PL0_SSP ... MSR_IA32_INT_SSP_TAB:
+		if (!kvm_is_cet_supported())
shall we consider the case where IBT is supported while SS isn't
(e.g., in L1 guest)?
Yes, but userspace should be able to access SHSTK MSRs even only IBT is exposed to guest so
far as KVM can support SHSTK MSRs.
Why should userspace be allowed to access SHSTK MSRs in this case? L1 may not
even enumerate SHSTK (qemu removes -shstk explicitly but keeps IBT), how KVM in
L1 can allow its userspace to do that?
Hold on until host_initiated access is finalized.
+static inline bool kvm_is_cet_supported(void)
+{
+	return (kvm_caps.supported_xss & CET_XSTATE_MASK) == CET_XSTATE_MASK;
why not just check if SHSTK or IBT is supported explicitly, i.e.,

	return kvm_cpu_cap_has(X86_FEATURE_SHSTK) ||
	       kvm_cpu_cap_has(X86_FEATURE_IBT);

this is straightforward. And strictly speaking, the support of a feature and
the support of managing a feature's state via XSAVE(S) are two different things.x
I think using exiting check implies two things:
1. Platform/KVM can support CET features.
2. CET user mode MSRs are backed by host thus are guaranteed to be valid.
i.e., the purpose is to check guest CET dependencies instead of features' availability.
When KVM claims a feature is supported, it should ensure all its dependencies are
met. that's, KVM's support of a feature also imples all dependencies are met.
Function-wise, the two approaches have no difference. I just think checking
KVM's support of SHSTK/IBT is more clear because the function name is
kvm_is_cet_supported() rather than e.g., kvm_is_cet_state_managed_by_xsave().
OK, maybe the helper is not necessary anymore, I will remove it, thank you!
kvm_cpu_cap_has(X86_FEATURE_SHSTK) || kvm_cpu_cap_has(X86_FEATURE_IBT)

only tells at least one of the CET features is supported by KVM.

then patch 16 has no need to do

+	/*
+	 * If SHSTK and IBT are not available in KVM, clear CET user bit in
+	 * kvm_caps.supported_xss so that kvm_is_cet__supported() returns
+	 * false when called.
+	 */
+	if (!kvm_cpu_cap_has(X86_FEATURE_SHSTK) &&
+	    !kvm_cpu_cap_has(X86_FEATURE_IBT))
+		kvm_caps.supported_xss &= ~CET_XSTATE_MASK;




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux