The list of CPU features we probe from various MSR grew significantly
over time and the CPU map currently mentions 11 distinct MSR indexes.
But the code for directly probing host CPU features was still reading
only the original 0x10a index. Thus the CPU model in host capabilities
was missing a lot of features.

Instead of specifying a static list of indexes to read (which we would
forget to update in the future), let's just read all indexes found in
the CPU map.

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
---
 src/cpu/cpu_x86.c | 62 ++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 53 insertions(+), 9 deletions(-)

diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 6d72d446c9..1ce97d9dcf 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -2836,15 +2836,61 @@ cpuidSet(uint32_t base, virCPUData *data)
 }
 
 
+static size_t
+virCPUx86ListMSRs(virCPUx86Map *map,
+                  unsigned long **msrs)
+{
+    size_t n = 0;
+    size_t i, j, k;
+
+    /* These three nested loops look horrible, but data->len is always 1 here
+     * and thanks to grouping features in the CPU map by their MSR index the
+     * third loop will run more than one iteration only once for each distinct
+     * index.
+     */
+    for (i = 0; i < map->nfeatures; i++) {
+        virCPUx86Data *data = &map->features[i]->data;
+
+        for (j = 0; j < data->len; j++) {
+            virCPUx86DataItem *item = &data->items[j];
+            bool found = false;
+            unsigned long msr;
+
+            if (item->type != VIR_CPU_X86_DATA_MSR)
+                continue;
+
+            msr = item->data.msr.index;
+
+            for (k = n; k > 0; k--) {
+                if ((*msrs)[k - 1] == msr) {
+                    found = true;
+                    break;
+                }
+            }
+
+            if (!found)
+                VIR_APPEND_ELEMENT(*msrs, n, msr);
+        }
+    }
+
+    return n;
+}
+
+
 static int
 virCPUx86GetHost(virCPUDef *cpu,
                  virDomainCapsCPUModels *models)
 {
     g_autoptr(virCPUData) cpuData = NULL;
     unsigned int addrsz;
+    virCPUx86Map *map;
+    g_autofree unsigned long *msrs = NULL;
+    size_t nmsrs;
+    uint64_t msr;
+    size_t i;
     int ret;
 
-    if (virCPUx86DriverInitialize() < 0)
+    if (!(map = virCPUx86GetMap()))
         return -1;
 
     if (!(cpuData = virCPUDataNew(archs[0])))
@@ -2854,18 +2900,16 @@ virCPUx86GetHost(virCPUDef *cpu,
         cpuidSet(CPUX86_EXTENDED, cpuData) < 0)
         return -1;
 
-    /* Read the IA32_ARCH_CAPABILITIES MSR (0x10a) if supported.
-     * This is best effort since there might be no way to read the MSR
-     * when we are not running as root. */
-    if (virCPUx86DataCheckFeature(cpuData, "arch-capabilities") == 1) {
-        uint64_t msr;
-        unsigned long index = 0x10a;
+    nmsrs = virCPUx86ListMSRs(map, &msrs);
 
-        if (virHostCPUGetMSR(index, &msr) == 0) {
+    /* This is best effort since there might be no way to read the MSR
+     * when we are not running as root. */
+    for (i = 0; i < nmsrs; i++) {
+        if (virHostCPUGetMSR(msrs[i], &msr) == 0) {
             virCPUx86DataItem item = {
                 .type = VIR_CPU_X86_DATA_MSR,
                 .data.msr = {
-                    .index = index,
+                    .index = msrs[i],
                     .eax = msr & 0xffffffff,
                     .edx = msr >> 32,
                 },
-- 
2.47.1

On 1/16/25 14:05, Jiri Denemark wrote:
> The list of CPU features we probe from various MSR grew significantly
> over time and the CPU map currently mentions 11 distinct MSR indexes.
> But the code for directly probing host CPU features was still reading
> only the original 0x10a index. Thus the CPU model in host capabilities
> was missing a lot of features.
> 
> Instead of specifying a static list of indexes to read (which we would
> forget to update in the future), let's just read all indexes found in
> the CPU map.
> 
> Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
> ---
>  src/cpu/cpu_x86.c | 62 ++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 53 insertions(+), 9 deletions(-)

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal

On Thu, Jan 16, 2025 at 14:38:26 +0100, Michal Prívozník wrote:
> On 1/16/25 14:05, Jiri Denemark wrote:
> > The list of CPU features we probe from various MSR grew significantly
> > over time and the CPU map currently mentions 11 distinct MSR indexes.
> > But the code for directly probing host CPU features was still reading
> > only the original 0x10a index. Thus the CPU model in host capabilities
> > was missing a lot of features.
> > 
> > Instead of specifying a static list of indexes to read (which we would
> > forget to update in the future), let's just read all indexes found in
> > the CPU map.
> > 
> > Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
> > ---
> >  src/cpu/cpu_x86.c | 62 ++++++++++++++++++++++++++++++++++++++++-------
> >  1 file changed, 53 insertions(+), 9 deletions(-)
> 
> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Thanks, I was reminded declaring more variables on a single line is bad
so I squashed the following trivial patch before pushing.

Jirka

@@ -2841,7 +2841,7 @@ virCPUx86ListMSRs(virCPUx86Map *map,
                   unsigned long **msrs)
 {
     size_t n = 0;
-    size_t i, j, k;
+    size_t i;

     /* These three nested loops look horrible, but data->len is always 1 here
      * and thanks to grouping features in the CPU map by their MSR index the
@@ -2850,11 +2850,13 @@ virCPUx86ListMSRs(virCPUx86Map *map,
      */
     for (i = 0; i < map->nfeatures; i++) {
         virCPUx86Data *data = &map->features[i]->data;
+        size_t j;

         for (j = 0; j < data->len; j++) {
             virCPUx86DataItem *item = &data->items[j];
             bool found = false;
             unsigned long msr;
+            size_t k;

             if (item->type != VIR_CPU_X86_DATA_MSR)
                 continue;