On Wed, Nov 05, 2025 at 14:41:50 +0100, Jiri Denemark wrote:
On Sat, May 31, 2025 at 00:30:23 +0200, Hector Cao wrote:
A model specific register (msr) is an 64-bit register. It can be read with the RDMSR instruction and the register value is returned via two registers EDX:EAX. EDX holds the 32 higher bits and EAX holds the 32 lower bits.
In the x86_features.xml file, some vmx-* features are wrongly defined as bits in the EAX register. For example, for the MSR 0x48B, the feature vmx-apicv-xapic is currently specified as the first bit of the EAX register but in the Intel specification [1], this feature is represented by the first bit of the EDX register (higher 32 bits).
This is the list of affected msrs that need to be fixed:
0x48B : IA32_VMX_PROCBASED_CTLS2 0x48D : IA32_VMX_TRUE_PINBASED_CTLS 0x48E : IA32_VMX_TRUE_PROCBASED_CTLS 0x48F : IA32_VMX_TRUE_EXIT_CTLS 0x490 : IA32_VMX_TRUE_ENTRY_CTLS
[1] Appendix A.3 Intel® 64 and IA-32 Architectures Software Developers Manual Volume 3 (3A, 3B, 3C & 3D): System Programming Guide
So technically the values don't matter much as long as they are distinct. They are only used for detecting host CPU capabilities found in the capabilities XML, which is not used by libvirt for anything and our documentation says the host CPU definition in the capabilities XML is only marginally useful. For all other CPU model related things we use feature names, translate the names to bits according to our map, do some bit operations, and translate the bits back to names. In this case the mapping has to be bijective, but the actual values are irrelevant.
Nevertheless we should keep the values correct to avoid lying in the capabilities XML more than we have to. Luckily since the values are only used by a single process, we can change the values anytime without any impact on backward compatibility.
Signed-off-by: Hector Cao <hector.cao@canonical.com> --- src/cpu_map/x86_features.xml | 136 +++++++++++++++++------------------ 1 file changed, 68 insertions(+), 68 deletions(-)
... Overall the changes look OK, but as you noticed in your reply, you need to fix sync_qemu_features_i386.py script. The script should be changed first in a separate patch and then followed by a patch created by running the modified script. Just beware to only include changes to existing features.
And I completely forgot that after changing the feature definitions you should do what sync_qemu_features_i386.py suggests and run tests/cputestdata/cpu-data.py diff tests/cputestdata/x86_64-cpuid-*.json and if it generates any changes, you also need to regenerate cpu test data VIR_TEST_REGENERATE_OUTPUT=1 tests/cputest Jirka