Re: [libvirt PATCH] cpu_map: Add EPYC-Rome model

On Mon, Sep 28, 2020 at 13:17:22 +0200, Markus Schade wrote: 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., 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: The current changes to the following files should be dropped completely. 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