On Mon, Sep 28, 2020 at 13:17:22 +0200, Markus Schade wrote:
> Signed-off-by: Markus Schade <markus.schade@xxxxxxxxxxx>
> ---
Thanks, I'm sorry I didn't say it last time (as I didn't realize it
would be needed in this case), but it's better to split the patch in
two. First add the new data files for EPYC-7502, i.e.,
> tests/cputest.c | 1 +
> ...86_64-cpuid-EPYC-7502-32-Core-disabled.xml | 9 +
> ...x86_64-cpuid-EPYC-7502-32-Core-enabled.xml | 11 +
> .../x86_64-cpuid-EPYC-7502-32-Core-guest.xml | 35 +
> .../x86_64-cpuid-EPYC-7502-32-Core-host.xml | 36 +
> .../x86_64-cpuid-EPYC-7502-32-Core-json.xml | 24 +
> .../x86_64-cpuid-EPYC-7502-32-Core.json | 1910 +++++++++++++++++
> .../x86_64-cpuid-EPYC-7502-32-Core.sig | 4 +
> .../x86_64-cpuid-EPYC-7502-32-Core.xml | 66 +
At this point, you'd have to run
VIR_TEST_REGENERATE_OUTPUT=1 tests/cputest
to update the computed CPU models to EPYC (as Rome is still unknown).
The second patch would add the new CPU model, which will result in some
CPUs to be detected as the new CPU model:
> src/cpu_map/index.xml | 1 +
> src/cpu_map/meson.build | 1 +
> src/cpu_map/x86_EPYC-Rome.xml | 81 +
The current changes to the following files should be dropped completely.
> ...4-cpuid-Ryzen-9-3900X-12-Core-disabled.xml | 1 -
> ...64-cpuid-Ryzen-9-3900X-12-Core-enabled.xml | 2 +-
> ...6_64-cpuid-Ryzen-9-3900X-12-Core-guest.xml | 9 +-
> ...86_64-cpuid-Ryzen-9-3900X-12-Core-host.xml | 9 +-
> ...86_64-cpuid-Ryzen-9-3900X-12-Core-json.xml | 8 +-
> .../x86_64-cpuid-Ryzen-9-3900X-12-Core.json | 23 +
> .../x86_64-cpuid-Ryzen-9-3900X-12-Core.xml | 12 +-
So after the new CPU model is introduced in patch 2, you would run
VIR_TEST_REGENERATE_OUTPUT=1 tests/cputest
This should change x86_64-cpuid-EPYC-7502-32-Core-{guest,host,json}.xml
files to use EPYC-Rome. And this correct change will be nicely visible
in the patch as it does not contain 2000 lines of boring generated data.
But as you noticed the cputest would still fail for Ryzen 9. Instead of
refreshing the data (which could be done as a separate patch before
introducing the new model, but it is not really needed) we can do two
things. Either copy just the new supported CPU models from
x86_64-cpuid-Ryzen-9-3900X-12-Core.json or change the following line in
tests/cputest.c
DO_TEST_CPUID(VIR_ARCH_X86_64, "Ryzen-9-3900X-12-Core", JSON_MODELS);
to
DO_TEST_CPUID(VIR_ARCH_X86_64, "Ryzen-9-3900X-12-Core", JSON_MODELS_REQUIRED);
After this change, compiling libvirt, and regenerating the output of
cputest all tests should pass again.
I guess the name of the flag is terrible and the documentation does not
really help either, but what it does is making sure the CPU model in
*-guest.xml has to be one of the models supported by QEMU (i.e., listed
in *.json file). This second option is easier and preferred. It will
also nicely show that *-{guest,json}.xml files remains unchanged, but
*-host.xml changes from EPYC-IBPB to EPYC-Rome, which is actually wrong
(and you can explicitly mention it in the commit message) because our
CPU decoding code does not work that well for AMD CPUs and we need to
fix it (but that's of course out of scope for this series).
Jirka
