On 2/10/25 3:24 AM, Daniel P. Berrangé wrote:
On Fri, Feb 07, 2025 at 05:33:27PM -0600, Kim Phillips wrote:
The Allowed SEV Features feature allows the host kernel to control
which SEV features it does not want the guest to enable [1].

This has to be explicitly opted-in by the user because it has the
ability to break existing VMs if it were set automatically.

Currently, both the PmcVirtualization and SecureAvic features
require the Allowed SEV Features feature to be set.

Based on a similar patch written for Secure TSC [2].

[1] Section 15.36.20 "Allowed SEV Features", AMD64 Architecture
     Programmer's Manual, Pub. 24593 Rev. 3.42 - March 2024:
     https://bugzilla.kernel.org/attachment.cgi?id=306250

[2] https://github.com/qemu/qemu/commit/4b2288dc6025ba32519ee8d202ca72d565cbbab7

Despite that URL, that commit also does not appear to be merged into
the QEMU git repo, and indeed I can't find any record of it even being
posted as a patch for review on qemu-devel.

This is horribly misleading to reviewers, suggesting that the referenced
patch was already accepted :-(

Apologies, that was not the intent.  I'll remove it from the next version.

@@ -1524,6 +1552,20 @@ static int sev_common_kvm_init(ConfidentialGuestSupport 
*cgs, Error **errp)
      case KVM_X86_SNP_VM: {
          struct kvm_sev_init args = { 0 };
+ if (sev_es_enabled()) {
+            __u64 vmsa_features, supported_vmsa_features;
+
+            supported_vmsa_features = sev_supported_vmsa_features();
+            vmsa_features = sev_common->vmsa_features;
+            if ((vmsa_features & supported_vmsa_features) != vmsa_features) {
+                error_setg(errp, "%s: requested sev feature mask (0x%llx) "
+                           "contains bits not supported by the host kernel "
+                           " (0x%llx)", __func__, vmsa_features,
+                           supported_vmsa_features);

This logic is being applied unconditionally, and not connected to
the setting of the new 'allowed-sev-features' flag value. Is that
correct  ?

That's correct.

Will this end up breaking existing deployed guests, or is this a
scenario that would have been blocked with an error later on
regardless ?

It would have been blocked regardless by this check in kvm's __sev_guest_init:

https://elixir.bootlin.com/linux/v6.13.2/source/arch/x86/kvm/svm/sev.c#L418

I've addressed all your other comments.

Thank you for your review,

Kim

Reply via email to