Re: [PATCH v4 0/4] target/i386: Add new CPU model SapphireRapids and new fast string op leaves

[Xiaoyao Li]

On 2/27/2023 6:13 PM, Paolo Bonzini wrote:
> Sapphire Rapids enablement patches got stuck on the doubts regarding
> properties for AMX support.  However, for now there is no need to have
> anything but hardcoded values, because all Intel processors with AMX
> currently support exactly the same palettes and TMUL limits.  Intel has
> also promised that palette formats will remain backwards compatible so
> the only worry is for the TMUL leaf, CPUID[1Eh].
>
> However, providing modifiable properties for AMX is premature.  
> Rather,
> the first step should be to_validate_  host CPUID values against the
> ones supported by QEMU.  

Paolo,

The validation of host CPUID values (kvm supported CPUIDs) against the 
ones supported by QEMU (the hardcoded value) is missing in current QEMU.

As for how to implement the validation, I have two options in mind:

a) special check in x86_cpu_filter_features() just like what did for 
Intel PT:

    if ((env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) &&
        kvm_enabled()) {
        KVMState *s = CPU(cpu)->kvm_state;
        uint32_t eax_0 = kvm_arch_get_supported_cpuid(s, 0x14, 0, R_EAX);
        uint32_t ebx_0 = kvm_arch_get_supported_cpuid(s, 0x14, 0, R_EBX);
        uint32_t ecx_0 = kvm_arch_get_supported_cpuid(s, 0x14, 0, R_ECX);
        uint32_t eax_1 = kvm_arch_get_supported_cpuid(s, 0x14, 1, R_EAX);
        uint32_t ebx_1 = kvm_arch_get_supported_cpuid(s, 0x14, 1, R_EBX);

        if (!eax_0 ||
           ((ebx_0 & INTEL_PT_MINIMAL_EBX) != INTEL_PT_MINIMAL_EBX) ||
           ((ecx_0 & INTEL_PT_MINIMAL_ECX) != INTEL_PT_MINIMAL_ECX) ||
           ((eax_1 & INTEL_PT_MTC_BITMAP) != INTEL_PT_MTC_BITMAP) ||
           ((eax_1 & INTEL_PT_ADDR_RANGES_NUM_MASK) <
                                           INTEL_PT_ADDR_RANGES_NUM) ||
           ((ebx_1 & (INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP)) !=
                (INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP)) ||
           ((ecx_0 & CPUID_14_0_ECX_LIP) !=
                (env->features[FEAT_14_0_ECX] & CPUID_14_0_ECX_LIP))) {
            /*
             * Processor Trace capabilities aren't configurable, so if the
             * host can't emulate the capabilities we report on
             * cpu_x86_cpuid(), intel-pt can't be enabled on the 
current host.
             */
            mark_unavailable_features(cpu, FEAT_7_0_EBX, 
CPUID_7_0_EBX_INTEL_PT, prefix);
        }
    }

This has flaws for leaf 0x1e, since its value might change on future 
production (Intel PT is facing this exact problem that SPR has less PT 
capabilities of CPUID(0x14,1):EBX[15:0] than ICX, and Intel PT cannot be 
enabled for guest on SPR machine). As well, if hardware reports 
different value of leaf 0x1e in the future, QEMU will fail to enable AMX 
for guest.

b) at least introduce FEAT_ for CPUID leaf 0x1E, so that it will be 
checked in x86_cpu_filter_features() automatically and "-cpu max/host" 
can pass through the host's value to guest. The additional work is that 
we might need MultiBitFeature framework introducing in 
https://lore.kernel.org/qemu-devel/20230106083826.5384-1-lei4.wang@intel.com/T/#t

Do you think it worths the effort to go for option b? or just option a 
for now is enough?

> So for now apply the simpler patch that only
> adds the new model.