Re: [RFC PATCH v2 4/6] KVM: SVM: Save shadow stack host state on VMRUN

From: Yang, Weijiang
Date: Tue Aug 01 2023 - 22:19:10 EST

On 8/2/2023 1:03 AM, John Allen wrote:
On Tue, Aug 01, 2023 at 09:28:11AM -0700, Sean Christopherson wrote:
On Tue, Aug 01, 2023, John Allen wrote:
On Fri, Jun 23, 2023 at 02:11:46PM -0700, Sean Christopherson wrote:
On Wed, May 24, 2023, John Allen wrote:
As for the values themselves, the kernel doesn't support Supervisor Shadow Stacks
(SSS), so PL0-2_SSP are guaranteed to be zero. And if/when SSS support is added,
I doubt the kernel will ever use PL1_SSP or PL2_SSP, so those can probably be
ignored entirely, and PL0_SSP might be constant per task? In other words, I don't
see any reason to try and track the host values for support that doesn't exist,
just do what VMX does for BNDCFGS and yell if the MSRs are non-zero. Though for
SSS it probably makes sense for KVM to refuse to load (KVM continues on for BNDCFGS
because it's a pretty safe assumption that the kernel won't regain MPX supported).

E.g. in rough pseudocode

if (boot_cpu_has(X86_FEATURE_SHSTK)) {
rdmsrl(MSR_IA32_PLx_SSP, host_plx_ssp);

if (WARN_ON_ONCE(host_pl0_ssp || host_pl1_ssp || host_pl2_ssp))
return -EIO;
The function in question returns void and wouldn't be able to return a
failure code to callers. We would have to rework this path in order to
fail in this way. Is it sufficient to just WARN_ON_ONCE here or is there
some other way we can cause KVM to fail to load here?
Sorry, I should have been more explicit than "it probably make sense for KVM to
refuse to load". The above would go somewhere in __kvm_x86_vendor_init().
I see, in that case that change should probably go up with:
"KVM:x86: Enable CET virtualization for VMX and advertise to userspace"
in Weijiang Yang's series with the rest of the changes to
__kvm_x86_vendor_init(). Though I can tack it on in my series if

The downside with above WARN_ON check is, KVM has to clear PL{0,1,2}_SSP for all CPUs when

SVM/VMX module is unloaded given guest would use them, otherwise, it may hit the check next

time the module is reloaded.

Can we add  check as below to make it easier?

@@ -9616,6 +9618,24 @@ static int __kvm_x86_vendor_init(struct kvm_x86_init_ops *ops)
                return -EIO;

+       if (boot_cpu_has(X86_FEATURE_SHSTK)) {
+               rdmsrl(MSR_IA32_S_CET, host_s_cet);
+               if (host_s_cet & CET_SHSTK_EN) {
+                       /*
+                        * Current CET KVM solution assumes host supervisor
+                        * shadow stack is always disable. If it's enabled
+                        * on host side, the guest supervisor states would
+                        * conflict with that of host's. When host supervisor
+                        * shadow stack is enabled one day, part of guest CET
+                        * enabling code should be refined to make both parties
+                        * work properly. Right now stop KVM module loading
+                        * once host supervisor shadow stack is detected on.
+                        */
+                       pr_err("Host supervisor shadow stack is not compatible with KVM!\n");
+                       return -EIO;
+               }
+       }
        x86_emulator_cache = kvm_alloc_emulator_cache();
        if (!x86_emulator_cache) {
                pr_err("failed to allocate cache for x86 emulator\n");

Anyway, these PLx_SSP only takes effect when SSS is enabled on host, otherwise,

they can used as scratch registers when SHSTK is available.