Re: [pve-devel] [PATCH v4 qemu-server 1/1] cpuconfig: add new x86-64-vX models

[Fiona Ebner <f.ebner@proxmox.com>]

Am 06.06.23 um 15:36 schrieb DERUMIER, Alexandre:
> Le mardi 06 juin 2023 à 14:09 +0200, Fiona Ebner a écrit :
>> Am 02.06.23 um 12:05 schrieb Alexandre Derumier:
>>>
>>> "
>>> In 2020, AMD, Intel, Red Hat, and SUSE worked together to define
>>> three microarchitecture levels on top of the historical x86-64
>>> baseline:
>>>
>>>   * x86-64:    original x86_64 baseline instruction set
>>>   * x86-64-v2: vector instructions up to Streaming SIMD
>>>                Extensions 4.2 (SSE4.2)  and Supplemental
>>>                Streaming SIMD Extensions 3 (SSSE3), the
>>>                POPCNT instruction, and CMPXCHG16B
>>>   * x86-64-v3: vector instructions up to AVX2, MOVBE,
>>>                and additional bit-manipulation instructions.
>>>   * x86-64-v4: vector instructions from some of the
>>>                AVX-512 variants.
>>> "
>>
>> Can we also link to  because
>> table 3.1 in the PDF contains a bit more precise information?
>>
>> I used the following to test for some instructions, but feel free to
>> tell me something better ;)
>>
> I'm a pretty poor C developper, but that's seem right!
> (To be honest, I didn't have even thinked to test instructions one by
> one  )
> 

I just tested the example instruction from the PDF for the features that
did not obviously correspond to a certain flag.

>> Could you describe how you chose/tested the flags?
> 
> from the qemu doc generator
> https://github.com/qemu/qemu/commit/4e2f5f3a9db06e87a73eb60a7cc9754fc13596ee
> 
> 
> +# Mandatory CPUID features for each microarch ABI level
>> +levels = [
>> +    [ # x86-64 baseline
>> +        "cmov",
>> +        "cx8",
>> +        "fpu",
>> +        "fxsr",
>> +        "mmx",
>> +        "syscall",
>> +        "sse",
>> +        "sse2",
>> +    ],
>> +    [ # x86-64-v2
>> +        "cx16",
>> +        "lahf-lm",
>> +        "popcnt",
>> +        "pni",
>> +        "sse4.1",
>> +        "sse4.2",
>> +        "ssse3",
>> +    ],
>> +    [ # x86-64-v3
>> +        "avx",
>> +        "avx2",
>> +        "bmi1",
>> +        "bmi2",
>> +        "f16c",
>> +        "fma",
>> +        "abm",
>> +        "movbe",
>> +    ],
>> +    [ # x86-64-v4
>> +        "avx512f",
>> +        "avx512bw",
>> +        "avx512cd",
>> +        "avx512dq",
>> +        "avx512vl",
>> +    ],
>> +]
>>

Okay, great!

>>> x86-64-v3 : Derived from qemu64
>>> +aes;+popcnt;+pni;+sse4.1;+sse4.2;+ssse3;+avx;+avx2;+bmi1;+bmi2;+f1
>>> 6c;+fma;+abm;+movbe
>>
>> Again comparing with the table in the PDF all flags are clear (with
>> abm
>> adding the LZCNT feature/instruction).
>>
>> But isn't the OSXSAVE feature missing? At least if I try with my
>> little
>> test program above I get "illegal hardware instruction" for xgetbv
>> (that
>> is the example instruction for the OSXSAVE CPU Feature mentioned in
>> the
>> PDF) and the has_osxsave variable in the cpuid-dump2 program is also
>> false.
> 
> AFAIK, it's has been removed from qemu some years ago. (and I don't see
> reference in other qemu models)
> 
> https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1825195
> https://listman.redhat.com/archives/libvir-list/2019-May/msg00274.html

The commit removing it mentions that it is a dynamic feature

> commit f1a23522b03a569f13aad49294bb4c4b1a9500c7
> Author: Eduardo Habkost <ehabkost@redhat.com>
> Date:   Mon Jun 11 17:38:55 2018 -0300
> 
>     i386: Remove osxsave CPUID flag name
>     
>     OSXAVE is not a static feature flag: it changes dynamically at
>     runtime depending on CR4, and it was never configurable: KVM
>     never returned OSXSAVE on GET_SUPPORTED_CPUID, and it is not
>     included in TCG_EXT_FEATURES.
>     
>     Remove OSXSAVE from the feature name array so users don't try to
>     configure it manually.

which according to
https://www.gnu.org/software/libc/manual/html_node/X86.html needs to be
enabled by the OS:

> OSXSAVE – The OS has set CR4.OSXSAVE[bit 18] to enable XSETBV/XGETBV instructions to access XCR0 and to support processor extended state management using XSAVE/XRSTOR.
But my Debian 11 guest apparently doesn't do it when using x86-64-v3.
Then I get "illegal hardware instruction" for xgetbv. If I use CPU type
host on the other hand, I don't get that error.

So I checked in the kernel sources and found that apparently depends on
the xsave feature. In arch/x86/kernel/fpu/xstate.c:

> void fpu__init_cpu_xstate(void) 
> {   
>     if (!boot_cpu_has(X86_FEATURE_XSAVE) || !fpu_kernel_cfg.max_features)
>         return;
> 
>     cr4_set_bits(X86_CR4_OSXSAVE);

and sure enough, adding +xsave to the flags for x86-64-v3 makes the
"illegal hardware instruction" error for xgetbv go away.

So while the QEMU docs don't mention the xsave flag, it seems necessary
to be in-line with the ABI definition?

In the table "CPUID EAX=1: Feature Information in EDX and ECX" in
https://en.wikipedia.org/wiki/CPUID#EAX=1:_Processor_Info_and_Feature_Bits

bit 26 of ECX is xsave which does list the XGETBV instruction
bit 27 of ECX is osxsave which just says "XSAVE enabled by OS"

So again, sounds like the xsave feature is required for the osxsave
feature to even make sense.