[PATCH 0/1] cpu_map: fix vmx-* features wrong bitmaps
Hello, I'm unable to make virsh capabilities to detect the right CPU model on an Intel Granite Rapids platform. I would expect the get the CPU model defined in one of the src/cpu_map/x86_GraniteRapids*.xml files. The cause of the wrong detection is the fact that some vmx-* are not correctly detected and considered missing on this platform. When I take a look at the src/cpu_map/x86_features.xml file, I think that some of the vmx-* are wrongly defined. Taking as an example the vmx-apicv-xapic feature, it is defined as the first bit in the EAX register when we read the MSR 0x0000048b . But in Intel specification, this feature is represented as the first bit in the EDX register (32 higher bits). You can find in this submission the patch that fixes this issue, and this for all the affected MSRs. I tested this fix on my Granite Rapids platform and the CPU model is now correctly detected. Hector Cao (1): cpu_map: fix vmx-* features wrong bitmaps src/cpu_map/x86_features.xml | 136 +++++++++++++++++------------------ 1 file changed, 68 insertions(+), 68 deletions(-) -- 2.45.2
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 Signed-off-by: Hector Cao <hector.cao@canonical.com> --- src/cpu_map/x86_features.xml | 136 +++++++++++++++++------------------ 1 file changed, 68 insertions(+), 68 deletions(-) diff --git a/src/cpu_map/x86_features.xml b/src/cpu_map/x86_features.xml index d06d60e230..0f60adb388 100644 --- a/src/cpu_map/x86_features.xml +++ b/src/cpu_map/x86_features.xml @@ -922,67 +922,67 @@ <!-- msr 0x0000048b --> <feature name='vmx-apicv-xapic'> - <msr index='0x0000048b' edx='0x00000000' eax='0x00000001'/> + <msr index='0x0000048b' edx='0x00000001' eax='0x00000000'/> </feature> <feature name='vmx-ept'> - <msr index='0x0000048b' edx='0x00000000' eax='0x00000002'/> + <msr index='0x0000048b' edx='0x00000002' eax='0x00000000'/> </feature> <feature name='vmx-desc-exit'> - <msr index='0x0000048b' edx='0x00000000' eax='0x00000004'/> + <msr index='0x0000048b' edx='0x00000004 ' eax='0x00000000'/> </feature> <feature name='vmx-rdtscp-exit'> - <msr index='0x0000048b' edx='0x00000000' eax='0x00000008'/> + <msr index='0x0000048b' edx='0x00000008 ' eax='0x00000000'/> </feature> <feature name='vmx-apicv-x2apic'> - <msr index='0x0000048b' edx='0x00000000' eax='0x00000010'/> + <msr index='0x0000048b' edx='0x00000010 ' eax='0x00000000'/> </feature> <feature name='vmx-vpid'> - <msr index='0x0000048b' edx='0x00000000' eax='0x00000020'/> + <msr index='0x0000048b' edx='0x00000020 ' eax='0x00000000'/> </feature> <feature name='vmx-wbinvd-exit'> - <msr index='0x0000048b' edx='0x00000000' eax='0x00000040'/> + <msr index='0x0000048b' edx='0x00000040 ' eax='0x00000000'/> </feature> <feature name='vmx-unrestricted-guest'> - <msr index='0x0000048b' edx='0x00000000' eax='0x00000080'/> + <msr index='0x0000048b' edx='0x00000080 ' eax='0x00000000'/> </feature> <feature name='vmx-apicv-register'> - <msr index='0x0000048b' edx='0x00000000' eax='0x00000100'/> + <msr index='0x0000048b' edx='0x00000100 ' eax='0x00000000'/> </feature> <feature name='vmx-apicv-vid'> - <msr index='0x0000048b' edx='0x00000000' eax='0x00000200'/> + <msr index='0x0000048b' edx='0x00000200 ' eax='0x00000000'/> </feature> <feature name='vmx-ple'> - <msr index='0x0000048b' edx='0x00000000' eax='0x00000400'/> + <msr index='0x0000048b' edx='0x00000400 ' eax='0x00000000'/> </feature> <feature name='vmx-rdrand-exit'> - <msr index='0x0000048b' edx='0x00000000' eax='0x00000800'/> + <msr index='0x0000048b' edx='0x00000800 ' eax='0x00000000'/> </feature> <feature name='vmx-invpcid-exit'> - <msr index='0x0000048b' edx='0x00000000' eax='0x00001000'/> + <msr index='0x0000048b' edx='0x00001000 ' eax='0x00000000'/> </feature> <feature name='vmx-vmfunc'> - <msr index='0x0000048b' edx='0x00000000' eax='0x00002000'/> + <msr index='0x0000048b' edx='0x00002000 ' eax='0x00000000'/> </feature> <feature name='vmx-shadow-vmcs'> - <msr index='0x0000048b' edx='0x00000000' eax='0x00004000'/> + <msr index='0x0000048b' edx='0x00004000 ' eax='0x00000000'/> </feature> <feature name='vmx-encls-exit'> - <msr index='0x0000048b' edx='0x00000000' eax='0x00008000'/> + <msr index='0x0000048b' edx='0x00008000 ' eax='0x00000000'/> </feature> <feature name='vmx-rdseed-exit'> - <msr index='0x0000048b' edx='0x00000000' eax='0x00010000'/> + <msr index='0x0000048b' edx='0x00010000 ' eax='0x00000000'/> </feature> <feature name='vmx-pml'> - <msr index='0x0000048b' edx='0x00000000' eax='0x00020000'/> + <msr index='0x0000048b' edx='0x00020000 ' eax='0x00000000'/> </feature> <feature name='vmx-xsaves'> - <msr index='0x0000048b' edx='0x00000000' eax='0x00100000'/> + <msr index='0x0000048b' edx='0x00100000 ' eax='0x00000000'/> </feature> <feature name='vmx-tsc-scaling'> - <msr index='0x0000048b' edx='0x00000000' eax='0x02000000'/> + <msr index='0x0000048b' edx='0x02000000 ' eax='0x00000000'/> </feature> <feature name='vmx-enable-user-wait-pause'> - <msr index='0x0000048b' edx='0x00000000' eax='0x04000000'/> + <msr index='0x0000048b' edx='0x04000000 ' eax='0x00000000'/> </feature> <!-- msr 0x0000048c --> @@ -1041,151 +1041,151 @@ <!-- msr 0x0000048d --> <feature name='vmx-intr-exit'> - <msr index='0x0000048d' edx='0x00000000' eax='0x00000001'/> + <msr index='0x0000048d' edx='0x00000001 ' eax='0x00000000'/> </feature> <feature name='vmx-nmi-exit'> - <msr index='0x0000048d' edx='0x00000000' eax='0x00000008'/> + <msr index='0x0000048d' edx='0x00000008 ' eax='0x00000000'/> </feature> <feature name='vmx-vnmi'> - <msr index='0x0000048d' edx='0x00000000' eax='0x00000020'/> + <msr index='0x0000048d' edx='0x00000020 ' eax='0x00000000'/> </feature> <feature name='vmx-preemption-timer'> - <msr index='0x0000048d' edx='0x00000000' eax='0x00000040'/> + <msr index='0x0000048d' edx='0x00000040 ' eax='0x00000000'/> </feature> <feature name='vmx-posted-intr'> - <msr index='0x0000048d' edx='0x00000000' eax='0x00000080'/> + <msr index='0x0000048d' edx='0x00000080 ' eax='0x00000000'/> </feature> <!-- msr 0x0000048e --> <feature name='vmx-vintr-pending'> - <msr index='0x0000048e' edx='0x00000000' eax='0x00000004'/> + <msr index='0x0000048e' edx='0x00000004 ' eax='0x00000000'/> </feature> <feature name='vmx-tsc-offset'> - <msr index='0x0000048e' edx='0x00000000' eax='0x00000008'/> + <msr index='0x0000048e' edx='0x00000008 ' eax='0x00000000'/> </feature> <feature name='vmx-hlt-exit'> - <msr index='0x0000048e' edx='0x00000000' eax='0x00000080'/> + <msr index='0x0000048e' edx='0x00000080 ' eax='0x00000000'/> </feature> <feature name='vmx-invlpg-exit'> - <msr index='0x0000048e' edx='0x00000000' eax='0x00000200'/> + <msr index='0x0000048e' edx='0x00000200 ' eax='0x00000000'/> </feature> <feature name='vmx-mwait-exit'> - <msr index='0x0000048e' edx='0x00000000' eax='0x00000400'/> + <msr index='0x0000048e' edx='0x00000400 ' eax='0x00000000'/> </feature> <feature name='vmx-rdpmc-exit'> - <msr index='0x0000048e' edx='0x00000000' eax='0x00000800'/> + <msr index='0x0000048e' edx='0x00000800 ' eax='0x00000000'/> </feature> <feature name='vmx-rdtsc-exit'> - <msr index='0x0000048e' edx='0x00000000' eax='0x00001000'/> + <msr index='0x0000048e' edx='0x00001000 ' eax='0x00000000'/> </feature> <feature name='vmx-cr3-load-noexit'> - <msr index='0x0000048e' edx='0x00000000' eax='0x00008000'/> + <msr index='0x0000048e' edx='0x00008000 ' eax='0x00000000'/> </feature> <feature name='vmx-cr3-store-noexit'> - <msr index='0x0000048e' edx='0x00000000' eax='0x00010000'/> + <msr index='0x0000048e' edx='0x00010000 ' eax='0x00000000'/> </feature> <feature name='vmx-cr8-load-exit'> - <msr index='0x0000048e' edx='0x00000000' eax='0x00080000'/> + <msr index='0x0000048e' edx='0x00080000 ' eax='0x00000000'/> </feature> <feature name='vmx-cr8-store-exit'> - <msr index='0x0000048e' edx='0x00000000' eax='0x00100000'/> + <msr index='0x0000048e' edx='0x00100000 ' eax='0x00000000'/> </feature> <feature name='vmx-flexpriority'> - <msr index='0x0000048e' edx='0x00000000' eax='0x00200000'/> + <msr index='0x0000048e' edx='0x00200000 ' eax='0x00000000'/> </feature> <feature name='vmx-vnmi-pending'> - <msr index='0x0000048e' edx='0x00000000' eax='0x00400000'/> + <msr index='0x0000048e' edx='0x00400000 ' eax='0x00000000'/> </feature> <feature name='vmx-movdr-exit'> - <msr index='0x0000048e' edx='0x00000000' eax='0x00800000'/> + <msr index='0x0000048e' edx='0x00800000 ' eax='0x00000000'/> </feature> <feature name='vmx-io-exit'> - <msr index='0x0000048e' edx='0x00000000' eax='0x01000000'/> + <msr index='0x0000048e' edx='0x01000000 ' eax='0x00000000'/> </feature> <feature name='vmx-io-bitmap'> - <msr index='0x0000048e' edx='0x00000000' eax='0x02000000'/> + <msr index='0x0000048e' edx='0x02000000 ' eax='0x00000000'/> </feature> <feature name='vmx-mtf'> - <msr index='0x0000048e' edx='0x00000000' eax='0x08000000'/> + <msr index='0x0000048e' edx='0x08000000 ' eax='0x00000000'/> </feature> <feature name='vmx-msr-bitmap'> - <msr index='0x0000048e' edx='0x00000000' eax='0x10000000'/> + <msr index='0x0000048e' edx='0x10000000 ' eax='0x00000000'/> </feature> <feature name='vmx-monitor-exit'> - <msr index='0x0000048e' edx='0x00000000' eax='0x20000000'/> + <msr index='0x0000048e' edx='0x20000000 ' eax='0x00000000'/> </feature> <feature name='vmx-pause-exit'> - <msr index='0x0000048e' edx='0x00000000' eax='0x40000000'/> + <msr index='0x0000048e' edx='0x40000000 ' eax='0x00000000'/> </feature> <feature name='vmx-secondary-ctls'> - <msr index='0x0000048e' edx='0x00000000' eax='0x80000000'/> + <msr index='0x0000048e' edx='0x80000000 ' eax='0x00000000'/> </feature> <!-- msr 0x0000048f --> <feature name='vmx-exit-nosave-debugctl'> - <msr index='0x0000048f' edx='0x00000000' eax='0x00000004'/> + <msr index='0x0000048f' edx='0x00000004 ' eax='0x00000000'/> </feature> <feature name='vmx-exit-load-perf-global-ctrl'> - <msr index='0x0000048f' edx='0x00000000' eax='0x00001000'/> + <msr index='0x0000048f' edx='0x00001000 ' eax='0x00000000'/> </feature> <feature name='vmx-exit-ack-intr'> - <msr index='0x0000048f' edx='0x00000000' eax='0x00008000'/> + <msr index='0x0000048f' edx='0x00008000 ' eax='0x00000000'/> </feature> <feature name='vmx-exit-save-pat'> - <msr index='0x0000048f' edx='0x00000000' eax='0x00040000'/> + <msr index='0x0000048f' edx='0x00040000 ' eax='0x00000000'/> </feature> <feature name='vmx-exit-load-pat'> - <msr index='0x0000048f' edx='0x00000000' eax='0x00080000'/> + <msr index='0x0000048f' edx='0x00080000 ' eax='0x00000000'/> </feature> <feature name='vmx-exit-save-efer'> - <msr index='0x0000048f' edx='0x00000000' eax='0x00100000'/> + <msr index='0x0000048f' edx='0x00100000 ' eax='0x00000000'/> </feature> <feature name='vmx-exit-load-efer'> - <msr index='0x0000048f' edx='0x00000000' eax='0x00200000'/> + <msr index='0x0000048f' edx='0x00200000 ' eax='0x00000000'/> </feature> <feature name='vmx-exit-save-preemption-timer'> - <msr index='0x0000048f' edx='0x00000000' eax='0x00400000'/> + <msr index='0x0000048f' edx='0x00400000 ' eax='0x00000000'/> </feature> <feature name='vmx-exit-clear-bndcfgs'> - <msr index='0x0000048f' edx='0x00000000' eax='0x00800000'/> + <msr index='0x0000048f' edx='0x00800000 ' eax='0x00000000'/> </feature> <feature name='vmx-exit-clear-rtit-ctl'> - <msr index='0x0000048f' edx='0x00000000' eax='0x02000000'/> + <msr index='0x0000048f' edx='0x02000000 ' eax='0x00000000'/> </feature> <feature name='vmx-exit-load-pkrs'> - <msr index='0x0000048f' edx='0x00000000' eax='0x20000000'/> + <msr index='0x0000048f' edx='0x20000000 ' eax='0x00000000'/> </feature> <feature name='vmx-exit-secondary-ctls'> - <msr index='0x0000048f' edx='0x00000000' eax='0x80000000'/> + <msr index='0x0000048f' edx='0x80000000 ' eax='0x00000000'/> </feature> <!-- msr 0x00000490 --> <feature name='vmx-entry-noload-debugctl'> - <msr index='0x00000490' edx='0x00000000' eax='0x00000004'/> + <msr index='0x00000490' edx='0x00000004 ' eax='0x00000000'/> </feature> <feature name='vmx-entry-ia32e-mode'> - <msr index='0x00000490' edx='0x00000000' eax='0x00000200'/> + <msr index='0x00000490' edx='0x00000200 ' eax='0x00000000'/> </feature> <feature name='vmx-entry-load-perf-global-ctrl'> - <msr index='0x00000490' edx='0x00000000' eax='0x00002000'/> + <msr index='0x00000490' edx='0x00002000 ' eax='0x00000000'/> </feature> <feature name='vmx-entry-load-pat'> - <msr index='0x00000490' edx='0x00000000' eax='0x00004000'/> + <msr index='0x00000490' edx='0x00004000 ' eax='0x00000000'/> </feature> <feature name='vmx-entry-load-efer'> - <msr index='0x00000490' edx='0x00000000' eax='0x00008000'/> + <msr index='0x00000490' edx='0x00008000 ' eax='0x00000000'/> </feature> <feature name='vmx-entry-load-bndcfgs'> - <msr index='0x00000490' edx='0x00000000' eax='0x00010000'/> + <msr index='0x00000490' edx='0x00010000 ' eax='0x00000000'/> </feature> <feature name='vmx-entry-load-rtit-ctl'> - <msr index='0x00000490' edx='0x00000000' eax='0x00040000'/> + <msr index='0x00000490' edx='0x00040000 ' eax='0x00000000'/> </feature> <feature name='vmx-entry-load-pkrs'> - <msr index='0x00000490' edx='0x00000000' eax='0x00400000'/> + <msr index='0x00000490' edx='0x00400000 ' eax='0x00000000'/> </feature> <feature name='vmx-entry-load-fred'> - <msr index='0x00000490' edx='0x00000000' eax='0x00800000'/> + <msr index='0x00000490' edx='0x00800000 ' eax='0x00000000'/> </feature> <!-- msr 0x00000491 --> -- 2.45.2
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(-)
diff --git a/src/cpu_map/x86_features.xml b/src/cpu_map/x86_features.xml index d06d60e230..0f60adb388 100644 --- a/src/cpu_map/x86_features.xml +++ b/src/cpu_map/x86_features.xml @@ -922,67 +922,67 @@
<!-- msr 0x0000048b --> <feature name='vmx-apicv-xapic'> - <msr index='0x0000048b' edx='0x00000000' eax='0x00000001'/> + <msr index='0x0000048b' edx='0x00000001' eax='0x00000000'/> </feature> <feature name='vmx-ept'> - <msr index='0x0000048b' edx='0x00000000' eax='0x00000002'/> + <msr index='0x0000048b' edx='0x00000002' eax='0x00000000'/> </feature> <feature name='vmx-desc-exit'> - <msr index='0x0000048b' edx='0x00000000' eax='0x00000004'/> + <msr index='0x0000048b' edx='0x00000004 ' eax='0x00000000'/>
Extra space in the edx value here and in the rest of the patch. 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. Jirka
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
Thanks Jiří for this feedback, much appreciated. To summarize the next steps: 1) I will submit a patch to modify the script sync_qemu_features_i386.py The change aims to extract correctly the vmx-* features from the MSR registers (and ONLY those MSR registers, other MSR registers will be left untouched) that have special logic (control bits in the 32 lower bits). 2) Once 1) has been approved and merged, a) Run the script sync_qemu_features_i386.py to generate the x86 feature XML file. b) Run tests/cputestdata/cpu-data.py diff tests/cputestdata/x86_64-cpuid-*.json c) VIR_TEST_REGENERATE_OUTPUT=1 tests/cputest Submit a patch with the change resulting from the above actions. What do you think ? Best Hector On Thu, Nov 6, 2025 at 12:13 PM Jiří Denemark <jdenemar@redhat.com> wrote:
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
-- Hector CAO Software Engineer – Server Team / Virtualization hector.cao@canonical.com https://launc <https://launchpad.net/~hectorcao>hpad.net/~hectorcao <https://launchpad.net/~hectorcao> <https://launchpad.net/~hectorcao>
On Wed, Nov 12, 2025 at 14:47:53 +0100, Hector Cao wrote:
Thanks Jiří for this feedback, much appreciated.
To summarize the next steps:
1) I will submit a patch to modify the script sync_qemu_features_i386.py The change aims to extract correctly the vmx-* features from the MSR registers (and ONLY those MSR registers, other MSR registers will be left untouched) that have special logic (control bits in the 32 lower bits).
2) Once 1) has been approved and merged, a) Run the script sync_qemu_features_i386.py to generate the x86 feature XML file. b) Run tests/cputestdata/cpu-data.py diff tests/cputestdata/x86_64-cpuid-*.json c) VIR_TEST_REGENERATE_OUTPUT=1 tests/cputest Submit a patch with the change resulting from the above actions.
I suggest sending all this in a several patches in a single series (just make sure tests pass after each patch) as the changes to this script will be easier to review if we also see the result. Jirka
I just realized that I should not modify the x86_features.xml file directly. I have to fix this issue elsewhere, probably in the sync_qemu_features_i386.py script or in the libvirt code that reads the MSRs registers. QEMU specifies vmx-apicv-xapic as 1st bit of the register 0x48B (IA32_VMX_PROCBASED_CTLS2) but has the login to interpret this bit position correctly in the register raw value. Libvirt does not have this login and by consequence, does not get the right bit value. I would appreciate some feedback on how we can best tackle this. On Sat, May 31, 2025 at 12:31 AM Hector Cao <hector.cao@canonical.com> wrote:
Hello,
I'm unable to make virsh capabilities to detect the right CPU model on an Intel Granite Rapids platform. I would expect the get the CPU model defined in one of the src/cpu_map/x86_GraniteRapids*.xml files.
The cause of the wrong detection is the fact that some vmx-* are not correctly detected and considered missing on this platform.
When I take a look at the src/cpu_map/x86_features.xml file, I think that some of the vmx-* are wrongly defined.
Taking as an example the vmx-apicv-xapic feature, it is defined as the first bit in the EAX register when we read the MSR 0x0000048b . But in Intel specification, this feature is represented as the first bit in the EDX register (32 higher bits).
You can find in this submission the patch that fixes this issue, and this for all the affected MSRs.
I tested this fix on my Granite Rapids platform and the CPU model is now correctly detected.
Hector Cao (1): cpu_map: fix vmx-* features wrong bitmaps
src/cpu_map/x86_features.xml | 136 +++++++++++++++++------------------ 1 file changed, 68 insertions(+), 68 deletions(-)
-- 2.45.2
-- Hector CAO Software Engineer – Partner Engineering Team hector.cao@canonical.com https://launc <https://launchpad.net/~hectorcao>hpad.net/~hectorcao <https://launchpad.net/~hectorcao> <https://launchpad.net/~hectorcao>
Hello, A friendly ping, To fix this issue, I would like to propose this solution. Here is the draft patch, I can submit a proper one in a separate mail if we can reach an agreement on this solution. Thanks ! diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 0f7eb8f48b..570160c18f 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -2922,6 +2922,16 @@ virCPUx86GetHost(virCPUDef *cpu, }, }; + if ((item.data.msr.index == 0x48B) + || (item.data.msr.index == 0x48D) + || (item.data.msr.index == 0x48E) + || (item.data.msr.index == 0x48F) + || (item.data.msr.index == 0x490) + ) { + item.data.msr.eax = item.data.msr.edx & ~item.data.msr.eax; + item.data.msr.edx = 0; + } + virCPUx86DataAdd(cpuData, &item); } } On Sun, Jun 1, 2025 at 11:01 PM Hector Cao <hector.cao@canonical.com> wrote:
I just realized that I should not modify the x86_features.xml file directly.
I have to fix this issue elsewhere, probably in the sync_qemu_features_i386.py script or in the libvirt code that reads the MSRs registers.
QEMU specifies vmx-apicv-xapic as 1st bit of the register 0x48B (IA32_VMX_PROCBASED_CTLS2) but has the login to interpret this bit position correctly in the register raw value. Libvirt does not have this login and by consequence, does not get the right bit value.
I would appreciate some feedback on how we can best tackle this.
On Sat, May 31, 2025 at 12:31 AM Hector Cao <hector.cao@canonical.com> wrote:
Hello,
I'm unable to make virsh capabilities to detect the right CPU model on an Intel Granite Rapids platform. I would expect the get the CPU model defined in one of the src/cpu_map/x86_GraniteRapids*.xml files.
The cause of the wrong detection is the fact that some vmx-* are not correctly detected and considered missing on this platform.
When I take a look at the src/cpu_map/x86_features.xml file, I think that some of the vmx-* are wrongly defined.
Taking as an example the vmx-apicv-xapic feature, it is defined as the first bit in the EAX register when we read the MSR 0x0000048b . But in Intel specification, this feature is represented as the first bit in the EDX register (32 higher bits).
You can find in this submission the patch that fixes this issue, and this for all the affected MSRs.
I tested this fix on my Granite Rapids platform and the CPU model is now correctly detected.
Hector Cao (1): cpu_map: fix vmx-* features wrong bitmaps
src/cpu_map/x86_features.xml | 136 +++++++++++++++++------------------ 1 file changed, 68 insertions(+), 68 deletions(-)
-- 2.45.2
-- Hector CAO Software Engineer – Partner Engineering Team hector.cao@canonical.com https://launc <https://launchpad.net/~hectorcao>hpad.net/~hectorcao <https://launchpad.net/~hectorcao>
-- Hector CAO Software Engineer – Partner Engineering Team hector.cao@canonical.com https://launc <https://launchpad.net/~hectorcao>hpad.net/~hectorcao <https://launchpad.net/~hectorcao> <https://launchpad.net/~hectorcao>
On Mon, Jun 16, 2025 at 01:46:59 +0200, Hector Cao wrote:
Hello,
A friendly ping,
To fix this issue, I would like to propose this solution. Here is the draft patch, I can submit a proper one in a separate mail if we can reach an agreement on this solution.
Thanks !
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 0f7eb8f48b..570160c18f 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -2922,6 +2922,16 @@ virCPUx86GetHost(virCPUDef *cpu, }, };
+ if ((item.data.msr.index == 0x48B) + || (item.data.msr.index == 0x48D) + || (item.data.msr.index == 0x48E) + || (item.data.msr.index == 0x48F) + || (item.data.msr.index == 0x490) + ) { + item.data.msr.eax = item.data.msr.edx & ~item.data.msr.eax; + item.data.msr.edx = 0; + } + virCPUx86DataAdd(cpuData, &item); } }
Hi, I apologize for a late response. I need to study the details to understand what actually is the problem and mainly the consequences of changing it and the best place for such a change. In general, the conversion from raw bits to names and back is only limited to a single host and I think there is just one place where the actual values matter (more than being distinct) and the result is not used for anything serious. Thus we should be able to change the values in out CPU map without introducing real issues, but I need to think about it more. Jirka
Hello Jirka, Do you have time to think about this ? If you are ok, I can go ahead and propose a solution to this issue, Best regards, Hector
On Mon, Jun 16, 2025 at 01:46:59 +0200, Hector Cao wrote:
Hello,
A friendly ping,
To fix this issue, I would like to propose this solution. Here is the draft patch, I can submit a proper one in a separate mail if we can reach an agreement on this solution.
Thanks !
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 0f7eb8f48b..570160c18f 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -2922,6 +2922,16 @@ virCPUx86GetHost(virCPUDef *cpu, }, };
+ if ((item.data.msr.index == 0x48B) + || (item.data.msr.index == 0x48D) + || (item.data.msr.index == 0x48E) + || (item.data.msr.index == 0x48F) + || (item.data.msr.index == 0x490) + ) { + item.data.msr.eax = item.data.msr.edx & ~item.data.msr.eax; + item.data.msr.edx = 0; + } + virCPUx86DataAdd(cpuData, &item); } }
I think the change does not belong here. The logic should be implemented in sync_qemu_features_i386.py script to make sure our feature definitions are correct. Jirka
participants (4)
-
Hector Cao -
hector.cao＠canonical.com
-
Jiri Denemark -
Jiří Denemark