Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37;
Date:   Fri, 13 Oct 2023 17:31:53 -0700
In-Reply-To: <20231010200220.897953-7-john.allen@amd.com>
Mime-Version: 1.0
References: <20231010200220.897953-1-john.allen@amd.com> <20231010200220.897953-7-john.allen@amd.com>
Message-ID: <ZSnheYDyzhZ1DOW5@google.com>
Subject: Re: [PATCH 6/9] KVM: SVM: Add MSR_IA32_XSS to the GHCB for hypervisor kernel
From:   Sean Christopherson <seanjc@google.com>
To:     John Allen <john.allen@amd.com>
Cc:     kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
        pbonzini@redhat.com, weijiang.yang@intel.com,
        rick.p.edgecombe@intel.com, x86@kernel.org,
        thomas.lendacky@amd.com, bp@alien8.de
Content-Type: text/plain; charset="us-ascii"
Precedence: bulk

On Tue, Oct 10, 2023, John Allen wrote:
> When a guest issues a cpuid instruction for Fn0000000D_x0B
> (CetUserOffset), KVM will intercept and need to access the guest
> MSR_IA32_XSS value. For SEV-ES, this is encrypted and needs to be
> included in the GHCB to be visible to the hypervisor.
> 
> Signed-off-by: John Allen <john.allen@amd.com>
> ---
> @@ -3032,6 +3037,9 @@ static void sev_es_init_vmcb(struct vcpu_svm *svm)
>  		if (guest_cpuid_has(&svm->vcpu, X86_FEATURE_RDTSCP))
>  			svm_clr_intercept(svm, INTERCEPT_RDTSCP);
>  	}
> +
> +	if (kvm_caps.supported_xss)
> +		set_msr_interception(vcpu, svm->msrpm, MSR_IA32_XSS, 1, 1);

This creates a giant gaping virtualization hole for the guest to walk through.
Want to hide shadow stacks from the guest because hardware is broken?  Too bad,
the guest can still set any and all XFeature bits it wants!  I realize "KVM"
already creates such a hole by disabling interception of XSETBV, but that doesn't
make it right.  In quotes because it's not like KVM has a choice for SEV-ES guests.

This is another example of SEV-SNP and beyond clearly being designed to work with
a paravisor, SVM_VMGEXIT_AP_CREATE being the other big one that comes to mind.

KVM should at least block CET by killing the VM if the guest illegally sets
CR4.CET.  Ugh, and is that even possible with SVM_VMGEXIT_AP_CREATE?  The guest
can shove in whatever CR4 it wants, so long as it the value is supported by
hardware.

At what point do we bite the bullet and require a paravisor?  Because the more I
see of SNP, the more I'm convinced that it's not entirely safe to run untrusted
guest code at VMPL0.