hw/i386/ovmf: check if ovmf is supported before calling ovmf parsing code

[PATCH] hw/i386/ovmf: check if ovmf is supported before calling ovmf parsing code

Posted by Ani Sinha 1 month ago

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.

Signed-off-by: Ani Sinha <anisinha@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(-)

This patch is built on top of
https://mail.gnu.org/archive/html/qemu-devel/2025-02/msg06005.html

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

Re: [PATCH] hw/i386/ovmf: check if ovmf is supported before calling ovmf parsing code

Posted by Gerd Hoffmann 1 month ago

On Fri, Feb 28, 2025 at 10:34:34PM +0530, Ani Sinha wrote:
> 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.
> 
> Signed-off-by: Ani Sinha <anisinha@redhat.com>

Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>

take care,
  Gerd

Re: [PATCH] hw/i386/ovmf: check if ovmf is supported before calling ovmf parsing code

Posted by Ani Sinha 1 month ago

On Fri, Feb 28, 2025 at 10:34 PM Ani Sinha <anisinha@redhat.com> wrote:
>
> 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.

This patch passes the CI pipeline.
https://gitlab.com/anisinha/qemu/-/pipelines/1693838556
on branch
https://gitlab.com/anisinha/qemu/-/commits/fuki-hyperface

>
> Signed-off-by: Ani Sinha <anisinha@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(-)
>
> This patch is built on top of
> https://mail.gnu.org/archive/html/qemu-devel/2025-02/msg06005.html
>
> 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
>