[PULL 16/16] target/i386/SEV: implement mask_cpuid_features

Paolo Bonzini posted 16 patches 4 months, 3 weeks ago
Maintainers: "Alex Bennée" <alex.bennee@linaro.org>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Thomas Huth <thuth@redhat.com>, Wainer dos Santos Moschetta <wainersm@redhat.com>, Beraldo Leal <bleal@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, Alexandre Iooss <erdnaxe@crans.org>, Mahmoud Mandour <ma.mandourr@gmail.com>, Pierrick Bouvier <pierrick.bouvier@linaro.org>, Marcelo Tosatti <mtosatti@redhat.com>, Laurent Vivier <lvivier@redhat.com>
[PULL 16/16] target/i386/SEV: implement mask_cpuid_features
Posted by Paolo Bonzini 4 months, 3 weeks ago
Drop features that are listed as "BitMask" in the PPR and currently
not supported by AMD processors.  The only ones that may become useful
in the future are TSC deadline timer and x2APIC, everything else is
not needed for SEV-SNP guests (e.g. VIRT_SSBD) or would require
processor support (e.g. TSC_ADJUST).

This allows running SEV-SNP guests with "-cpu host".

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/cpu.h |  4 ++++
 target/i386/sev.c | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 0d5624355e4..c43ac01c794 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -812,6 +812,8 @@ uint64_t x86_cpu_get_supported_feature_word(X86CPU *cpu, FeatureWord w);
 
 /* Support RDFSBASE/RDGSBASE/WRFSBASE/WRGSBASE */
 #define CPUID_7_0_EBX_FSGSBASE          (1U << 0)
+/* Support TSC adjust MSR */
+#define CPUID_7_0_EBX_TSC_ADJUST        (1U << 1)
 /* Support SGX */
 #define CPUID_7_0_EBX_SGX               (1U << 2)
 /* 1st Group of Advanced Bit Manipulation Extensions */
@@ -1002,6 +1004,8 @@ uint64_t x86_cpu_get_supported_feature_word(X86CPU *cpu, FeatureWord w);
 #define CPUID_8000_0008_EBX_STIBP_ALWAYS_ON    (1U << 17)
 /* Speculative Store Bypass Disable */
 #define CPUID_8000_0008_EBX_AMD_SSBD    (1U << 24)
+/* Paravirtualized Speculative Store Bypass Disable MSR */
+#define CPUID_8000_0008_EBX_VIRT_SSBD   (1U << 25)
 /* Predictive Store Forwarding Disable */
 #define CPUID_8000_0008_EBX_AMD_PSFD    (1U << 28)
 
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 2f3dbe289f4..2ba5f517228 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -945,6 +945,38 @@ out:
     return ret;
 }
 
+static uint32_t
+sev_snp_mask_cpuid_features(X86ConfidentialGuest *cg, uint32_t feature, uint32_t index,
+                            int reg, uint32_t value)
+{
+    switch (feature) {
+    case 1:
+        if (reg == R_ECX) {
+            return value & ~CPUID_EXT_TSC_DEADLINE_TIMER;
+        }
+        break;
+    case 7:
+        if (index == 0 && reg == R_EBX) {
+            return value & ~CPUID_7_0_EBX_TSC_ADJUST;
+        }
+        if (index == 0 && reg == R_EDX) {
+            return value & ~(CPUID_7_0_EDX_SPEC_CTRL |
+                             CPUID_7_0_EDX_STIBP |
+                             CPUID_7_0_EDX_FLUSH_L1D |
+                             CPUID_7_0_EDX_ARCH_CAPABILITIES |
+                             CPUID_7_0_EDX_CORE_CAPABILITY |
+                             CPUID_7_0_EDX_SPEC_CTRL_SSBD);
+        }
+        break;
+    case 0x80000008:
+        if (reg == R_EBX) {
+            return value & ~CPUID_8000_0008_EBX_VIRT_SSBD;
+        }
+        break;
+    }
+    return value;
+}
+
 static int
 sev_launch_update_data(SevCommonState *sev_common, hwaddr gpa,
                        uint8_t *addr, size_t len)
@@ -2315,6 +2347,7 @@ sev_snp_guest_class_init(ObjectClass *oc, void *data)
     klass->launch_finish = sev_snp_launch_finish;
     klass->launch_update_data = sev_snp_launch_update_data;
     klass->kvm_init = sev_snp_kvm_init;
+    x86_klass->mask_cpuid_features = sev_snp_mask_cpuid_features;
     x86_klass->kvm_type = sev_snp_kvm_type;
 
     object_class_property_add(oc, "policy", "uint64",
-- 
2.45.2
Re: [PULL 16/16] target/i386/SEV: implement mask_cpuid_features
Posted by Daniel P. Berrangé 3 months, 2 weeks ago
On Thu, Jul 04, 2024 at 11:58:06AM +0200, Paolo Bonzini wrote:
> Drop features that are listed as "BitMask" in the PPR and currently
> not supported by AMD processors.  The only ones that may become useful
> in the future are TSC deadline timer and x2APIC, everything else is
> not needed for SEV-SNP guests (e.g. VIRT_SSBD) or would require
> processor support (e.g. TSC_ADJUST).
> 
> This allows running SEV-SNP guests with "-cpu host".

What CPU generation(s)/model(s)  did you test this with ?

I've been talking to a libvirt user (CC'd) who has tried host CPU
passthrough with SNP, using QEMU git master from yesterday (which
has this patch present) and they are still getting failures. 

They are using pc-q35-9.1 machine type and host CPU passthrough,
on a Milan 7003  host, and getting this validation failure

  SEV-SNP: CPUID validation failed for function 0x80000021, index: 0x0, provided: eax:0x18000245, ebx: 0x00000000, ecx: 0x00000000, edx: 0x00000000, expected: eax:0x18000045, ebx: 0x00000000, ecx: 0x00000000, edx: 0x00000000

I don't see any  0x0000_0200  bit defined for EAX with
the function 0x8000_0021 in target/i386/cpu.c, so there's
nothing we can mask out on the CLI, nor in your
sev_snp_mask_cpuid_features function. 

For that matter we've not defined the 0x1000_0000 or 0x0800_000
bits either, though SNP isn't complaining about those at least.

> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index 2f3dbe289f4..2ba5f517228 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -945,6 +945,38 @@ out:
>      return ret;
>  }
>  
> +static uint32_t
> +sev_snp_mask_cpuid_features(X86ConfidentialGuest *cg, uint32_t feature, uint32_t index,
> +                            int reg, uint32_t value)
> +{
> +    switch (feature) {
> +    case 1:
> +        if (reg == R_ECX) {
> +            return value & ~CPUID_EXT_TSC_DEADLINE_TIMER;
> +        }
> +        break;
> +    case 7:
> +        if (index == 0 && reg == R_EBX) {
> +            return value & ~CPUID_7_0_EBX_TSC_ADJUST;
> +        }
> +        if (index == 0 && reg == R_EDX) {
> +            return value & ~(CPUID_7_0_EDX_SPEC_CTRL |
> +                             CPUID_7_0_EDX_STIBP |
> +                             CPUID_7_0_EDX_FLUSH_L1D |
> +                             CPUID_7_0_EDX_ARCH_CAPABILITIES |
> +                             CPUID_7_0_EDX_CORE_CAPABILITY |
> +                             CPUID_7_0_EDX_SPEC_CTRL_SSBD);
> +        }
> +        break;
> +    case 0x80000008:
> +        if (reg == R_EBX) {
> +            return value & ~CPUID_8000_0008_EBX_VIRT_SSBD;
> +        }
> +        break;
> +    }
> +    return value;
> +}
> +
>  static int
>  sev_launch_update_data(SevCommonState *sev_common, hwaddr gpa,
>                         uint8_t *addr, size_t len)
> @@ -2315,6 +2347,7 @@ sev_snp_guest_class_init(ObjectClass *oc, void *data)
>      klass->launch_finish = sev_snp_launch_finish;
>      klass->launch_update_data = sev_snp_launch_update_data;
>      klass->kvm_init = sev_snp_kvm_init;
> +    x86_klass->mask_cpuid_features = sev_snp_mask_cpuid_features;
>      x86_klass->kvm_type = sev_snp_kvm_type;
>  
>      object_class_property_add(oc, "policy", "uint64",
> -- 
> 2.45.2
> 
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|