[PULL 0/3] Some refactoring/cleanups for cpu versions on microvms

[PULL 0/3] Some refactoring/cleanups for cpu versions on microvms

[Ani Sinha]

The following changes since commit 661c2e1ab29cd9c4d268ae3f44712e8d421c0e56:

  scripts/checkpatch: Fix a typo (2025-03-04 09:30:26 +0800)

are available in the Git repository at:

  git@gitlab.com:anisinha/qemu.git tags/pull-master-20250305

for you to fetch changes up to 22550e8eaba43233f68a344bd5a3d4d2f62b23a4:

  microvm: do not use the lastest cpu version (2025-03-05 12:19:45 +0530)

----------------------------------------------------------------
Cleanup latest cpu model usage for microvms.
Some refactoring of code around ovmf metadata parsing.

----------------------------------------------------------------
Ani Sinha (3):
      hw/i386: introduce x86_firmware_reconfigure api
      hw/i386/ovmf: check if ovmf is supported before calling ovmf parsing code
      microvm: do not use the lastest cpu version

 hw/i386/microvm.c             |  2 +-
 hw/i386/pc_sysfw.c            | 30 ++++++++++++++++++++++--------
 hw/i386/pc_sysfw_ovmf-stubs.c | 10 ++++++++++
 hw/i386/pc_sysfw_ovmf.c       | 10 ++++++++++
 include/hw/i386/pc.h          |  2 ++
 include/hw/i386/x86.h         |  1 +
 target/i386/cpu.c             | 15 ---------------
 target/i386/cpu.h             |  4 ----
 8 files changed, 46 insertions(+), 28 deletions(-)

[PULL 1/3] hw/i386: introduce x86_firmware_reconfigure api

[Ani Sinha]

Normally, there is no need to perform firmware reconfiguration once the
virtual machine has started. Hence, currently ovmf firmware parsing happens only
once. However, if the firmware changes betweeen boots then reconfiguration needs
to happen again. Firmware can change if for example the guest brings its own
firmware bundle and installs it with the help of the hypervisor[1]. Therefore,
this change introduces a new api with which firmware configuration steps can
be forced again.
This is mostly refactoring work. No functional changes. CI pipeline does not
break with this change.

1) https://pretalx.com/kvm-forum-2024/talk/HJSKRQ/

Message-ID: <20250228114230.306852-1-anisinha@redhat.com>
Signed-off-by: Ani Sinha <anisinha@redhat.com>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/i386/pc_sysfw.c            | 26 ++++++++++++++++++--------
 hw/i386/pc_sysfw_ovmf-stubs.c |  5 +++++
 hw/i386/pc_sysfw_ovmf.c       |  5 +++++
 include/hw/i386/pc.h          |  1 +
 include/hw/i386/x86.h         |  1 +
 5 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
index 1eeb58ab37..a9943d95c8 100644
--- a/hw/i386/pc_sysfw.c
+++ b/hw/i386/pc_sysfw.c
@@ -258,16 +258,9 @@ void pc_system_firmware_init(PCMachineState *pcms,
     pc_system_flash_cleanup_unused(pcms);
 }
 
-void x86_firmware_configure(hwaddr gpa, void *ptr, int size)
+static void x86_firmware_configure_sev(hwaddr gpa, void *ptr, int size)
 {
     int ret;
-
-    /*
-     * OVMF places a GUIDed structures in the flash, so
-     * search for them
-     */
-    pc_system_parse_ovmf_flash(ptr, size);
-
     if (sev_enabled()) {
 
         /* Copy the SEV metadata table (if it exists) */
@@ -282,3 +275,20 @@ void x86_firmware_configure(hwaddr gpa, void *ptr, int size)
         sev_encrypt_flash(gpa, ptr, size, &error_fatal);
     }
 }
+
+void x86_firmware_configure(hwaddr gpa, void *ptr, int size)
+{
+    /*
+     * OVMF places a GUIDed structures in the flash, so
+     * search for them
+     */
+    pc_system_parse_ovmf_flash(ptr, size);
+    x86_firmware_configure_sev(gpa, ptr, size);
+}
+
+void x86_firmware_reconfigure(hwaddr gpa, void *ptr, int size)
+{
+    invalidate_ovmf_parsed_metadata();
+    pc_system_parse_ovmf_flash(ptr, size);
+    x86_firmware_configure_sev(gpa, ptr, size);
+}
diff --git a/hw/i386/pc_sysfw_ovmf-stubs.c b/hw/i386/pc_sysfw_ovmf-stubs.c
index aabe78b271..edf890a525 100644
--- a/hw/i386/pc_sysfw_ovmf-stubs.c
+++ b/hw/i386/pc_sysfw_ovmf-stubs.c
@@ -24,3 +24,8 @@ void pc_system_parse_ovmf_flash(uint8_t *flash_ptr, size_t flash_size)
 {
     g_assert_not_reached();
 }
+
+void invalidate_ovmf_parsed_metadata(void)
+{
+    g_assert_not_reached();
+}
diff --git a/hw/i386/pc_sysfw_ovmf.c b/hw/i386/pc_sysfw_ovmf.c
index 07a4c267fa..3244c17a7d 100644
--- a/hw/i386/pc_sysfw_ovmf.c
+++ b/hw/i386/pc_sysfw_ovmf.c
@@ -36,6 +36,11 @@ static bool ovmf_flash_parsed;
 static uint8_t *ovmf_table;
 static int ovmf_table_len;
 
+void invalidate_ovmf_parsed_metadata(void)
+{
+    ovmf_flash_parsed = false;
+}
+
 void pc_system_parse_ovmf_flash(uint8_t *flash_ptr, size_t flash_size)
 {
     uint8_t *ptr;
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 103b54301f..7b0d0c54f5 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -211,6 +211,7 @@ void pc_system_firmware_init(PCMachineState *pcms, MemoryRegion *rom_memory);
 bool pc_system_ovmf_table_find(const char *entry, uint8_t **data,
                                int *data_len);
 void pc_system_parse_ovmf_flash(uint8_t *flash_ptr, size_t flash_size);
+void invalidate_ovmf_parsed_metadata(void);
 
 /* sgx.c */
 void pc_machine_init_sgx_epc(PCMachineState *pcms);
diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
index d43cb3908e..18c0d6851a 100644
--- a/include/hw/i386/x86.h
+++ b/include/hw/i386/x86.h
@@ -155,5 +155,6 @@ DeviceState *ioapic_init_secondary(GSIState *gsi_state);
 
 /* pc_sysfw.c */
 void x86_firmware_configure(hwaddr gpa, void *ptr, int size);
+void x86_firmware_reconfigure(hwaddr gpa, void *ptr, int size);
 
 #endif
-- 
2.42.0

[PULL 2/3] hw/i386/ovmf: check if ovmf is supported before calling ovmf parsing code

[Ani Sinha]

Currently call to x86_firmware_configure() -> pc_system_parse_ovmf_flash()
happens only when SEV is enabled. Fortunately, X86_FW_OVMF is turned on
automatically when SEV is enabled and therefore,  we never end up calling
pc_system_parse_ovmf_flash() when X86_FW_OVMF is turned off. In future,
it is possible that users call x86_firmware_configure() or
x86_firmware_reconfigure() without checking if SEV is enabled. Therefore,
x86_firmware_configure() or x86_firmware_reconfigure() need to check if
ovmf is supported before calling ovmf parsing code. Hence, this change
introduces an api ovmf_supported() that returns true wnen ovmf is enabled
and false otherwise. Ovmf parsing code is only called after checking if ovmf
is supported.

Message-ID: <20250228170434.317306-1-anisinha@redhat.com>
Signed-off-by: Ani Sinha <anisinha@redhat.com>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/i386/pc_sysfw.c            | 18 +++++++++++-------
 hw/i386/pc_sysfw_ovmf-stubs.c |  5 +++++
 hw/i386/pc_sysfw_ovmf.c       |  5 +++++
 include/hw/i386/pc.h          |  1 +
 4 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
index a9943d95c8..725d142606 100644
--- a/hw/i386/pc_sysfw.c
+++ b/hw/i386/pc_sysfw.c
@@ -278,17 +278,21 @@ static void x86_firmware_configure_sev(hwaddr gpa, void *ptr, int size)
 
 void x86_firmware_configure(hwaddr gpa, void *ptr, int size)
 {
-    /*
-     * OVMF places a GUIDed structures in the flash, so
-     * search for them
-     */
-    pc_system_parse_ovmf_flash(ptr, size);
+    if (ovmf_supported()) {
+        /*
+         * OVMF places a GUIDed structures in the flash, so
+         * search for them
+         */
+        pc_system_parse_ovmf_flash(ptr, size);
+    }
     x86_firmware_configure_sev(gpa, ptr, size);
 }
 
 void x86_firmware_reconfigure(hwaddr gpa, void *ptr, int size)
 {
-    invalidate_ovmf_parsed_metadata();
-    pc_system_parse_ovmf_flash(ptr, size);
+    if (ovmf_supported()) {
+        invalidate_ovmf_parsed_metadata();
+        pc_system_parse_ovmf_flash(ptr, size);
+    }
     x86_firmware_configure_sev(gpa, ptr, size);
 }
diff --git a/hw/i386/pc_sysfw_ovmf-stubs.c b/hw/i386/pc_sysfw_ovmf-stubs.c
index edf890a525..08ec18b9b7 100644
--- a/hw/i386/pc_sysfw_ovmf-stubs.c
+++ b/hw/i386/pc_sysfw_ovmf-stubs.c
@@ -15,6 +15,11 @@
 #include "qemu/osdep.h"
 #include "hw/i386/pc.h"
 
+bool ovmf_supported(void)
+{
+    return false;
+}
+
 bool pc_system_ovmf_table_find(const char *entry, uint8_t **data, int *data_len)
 {
     g_assert_not_reached();
diff --git a/hw/i386/pc_sysfw_ovmf.c b/hw/i386/pc_sysfw_ovmf.c
index 3244c17a7d..e6497fd7a7 100644
--- a/hw/i386/pc_sysfw_ovmf.c
+++ b/hw/i386/pc_sysfw_ovmf.c
@@ -36,6 +36,11 @@ static bool ovmf_flash_parsed;
 static uint8_t *ovmf_table;
 static int ovmf_table_len;
 
+bool ovmf_supported(void)
+{
+    return true;
+}
+
 void invalidate_ovmf_parsed_metadata(void)
 {
     ovmf_flash_parsed = false;
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 7b0d0c54f5..2e41ca8b05 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -212,6 +212,7 @@ bool pc_system_ovmf_table_find(const char *entry, uint8_t **data,
                                int *data_len);
 void pc_system_parse_ovmf_flash(uint8_t *flash_ptr, size_t flash_size);
 void invalidate_ovmf_parsed_metadata(void);
+bool ovmf_supported(void);
 
 /* sgx.c */
 void pc_machine_init_sgx_epc(PCMachineState *pcms);
-- 
2.42.0

[PULL 3/3] microvm: do not use the lastest cpu version

[Ani Sinha]

commit 0788a56bd1ae3 ("i386: Make unversioned CPU models be aliases")
introduced 'default_cpu_version' for PCMachineClass. This created three
categories of CPU models:
 - Most unversioned CPU models would use version 1 by default.
 - For machines 4.0.1 and older that do not support cpu model aliases, a
   special default_cpu_version value of CPU_VERSION_LEGACY is used.
 - It was thought that future machines would use the latest value of cpu
   versions corresponding to default_cpu_version value of
   CPU_VERSION_LATEST [1].

All pc machines still use the default cpu version of 1 for
unversioned cpu models. CPU_VERSION_LATEST is a moving target and
changes with time. Therefore, if machines use CPU_VERSION_LATEST, it would
mean that over a period of time, for the same versioned machine type,
the cpu version would be different depending on what the latest was at that
time. This would break guests even when they use a constant specific
versioned machine type.
Additionally, microvm machines are not versioned anyway and therefore
there is no requirement to use the latest cpu model by default.
Let microvms use the non-versioned cpu model and remove all references
to CPU_VERSION_LATEST as there are no other users (nor we anticipate
future consumers of CPU_VERSION_LATEST).

Those users who need spefific cpu versions can use explicit version in
the QEMU command line to select the specific cpu version desired.

CI pipline does not break with this change.

1) See commit dcafd1ef0af227 ("i386: Register versioned CPU models")

CC: imammedo@redhat.com
CC: zhao1.liu@intel.com
Message-ID: <20250220065326.312596-1-anisinha@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Sergio Lopez <slp@redhat.com>
Signed-off-by: Ani Sinha <anisinha@redhat.com>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
---
 hw/i386/microvm.c |  2 +-
 target/i386/cpu.c | 15 ---------------
 target/i386/cpu.h |  4 ----
 3 files changed, 1 insertion(+), 20 deletions(-)

diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
index d0a236c74f..a340a5fd39 100644
--- a/hw/i386/microvm.c
+++ b/hw/i386/microvm.c
@@ -491,7 +491,7 @@ static void microvm_machine_state_init(MachineState *machine)
 
     microvm_memory_init(mms);
 
-    x86_cpus_init(x86ms, CPU_VERSION_LATEST);
+    x86_cpus_init(x86ms, 1);
 
     microvm_devices_init(mms);
 }
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 0cd9b70938..2da2cf36fd 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5640,18 +5640,6 @@ void x86_cpu_set_default_version(X86CPUVersion version)
     default_cpu_version = version;
 }
 
-static X86CPUVersion x86_cpu_model_last_version(const X86CPUModel *model)
-{
-    int v = 0;
-    const X86CPUVersionDefinition *vdef =
-        x86_cpu_def_get_versions(model->cpudef);
-    while (vdef->version) {
-        v = vdef->version;
-        vdef++;
-    }
-    return v;
-}
-
 /* Return the actual version being used for a specific CPU model */
 static X86CPUVersion x86_cpu_model_resolve_version(const X86CPUModel *model)
 {
@@ -5659,9 +5647,6 @@ static X86CPUVersion x86_cpu_model_resolve_version(const X86CPUModel *model)
     if (v == CPU_VERSION_AUTO) {
         v = default_cpu_version;
     }
-    if (v == CPU_VERSION_LATEST) {
-        return x86_cpu_model_last_version(model);
-    }
     return v;
 }
 
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 10ce019e3f..113cf10aea 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -2740,10 +2740,6 @@ void apic_handle_tpr_access_report(DeviceState *d, target_ulong ip,
                                    TPRAccess access);
 
 /* Special values for X86CPUVersion: */
-
-/* Resolve to latest CPU version */
-#define CPU_VERSION_LATEST -1
-
 /*
  * Resolve to version defined by current machine type.
  * See x86_cpu_set_default_version()
-- 
2.42.0