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

["Daniel P. Berrangé" <berrange@redhat.com>]

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 :|