diff mbox series

[v3,05/15] i386/pc_sysfw: Ensure sysfw flash configuration does not conflict with IGVM

Message ID 97a1d5af646b0d6c3d1fe30022bcb61a16e46d95.1718979106.git.roy.hopkins@suse.com
State New
Headers show
Series Introduce support for IGVM files | expand

Commit Message

Roy Hopkins June 21, 2024, 2:29 p.m. UTC
When using an IGVM file the configuration of the system firmware is
defined by IGVM directives contained in the file. In this case the user
should not configure any pflash devices.

This commit skips initialization of the ROM mode when pflash0 is not set
then checks to ensure no pflash devices have been configured when using
IGVM, exiting with an error message if this is not the case.

Signed-off-by: Roy Hopkins <roy.hopkins@suse.com>
---
 hw/i386/pc_sysfw.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

Comments

Stefano Garzarella June 27, 2024, 12:38 p.m. UTC | #1
On Fri, Jun 21, 2024 at 03:29:08PM GMT, Roy Hopkins wrote:
>When using an IGVM file the configuration of the system firmware is
>defined by IGVM directives contained in the file. In this case the user
>should not configure any pflash devices.
>
>This commit skips initialization of the ROM mode when pflash0 is not set
>then checks to ensure no pflash devices have been configured when using
>IGVM, exiting with an error message if this is not the case.
>
>Signed-off-by: Roy Hopkins <roy.hopkins@suse.com>
>---
> hw/i386/pc_sysfw.c | 23 +++++++++++++++++++++--
> 1 file changed, 21 insertions(+), 2 deletions(-)
>
>diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
>index ef80281d28..39e94ce144 100644
>--- a/hw/i386/pc_sysfw.c
>+++ b/hw/i386/pc_sysfw.c
>@@ -239,8 +239,13 @@ void pc_system_firmware_init(PCMachineState *pcms,
>     }
>
>     if (!pflash_blk[0]) {
>-        /* Machine property pflash0 not set, use ROM mode */
>-        x86_bios_rom_init(X86_MACHINE(pcms), "bios.bin", rom_memory, false);

We have the same call, a few lines above if `pci_enabled` is false, 
should we make the same change there as well?

>+        /*
>+         * Machine property pflash0 not set, use ROM mode unless using 
>IGVM,
>+         * in which case the firmware must be provided by the IGVM file.
>+         */
>+        if (!MACHINE(pcms)->igvm) {
>+            x86_bios_rom_init(X86_MACHINE(pcms), "bios.bin", rom_memory, false);
>+        }
>     } else {
>         if (kvm_enabled() && !kvm_readonly_mem_enabled()) {
>             /*
>@@ -256,6 +261,20 @@ void pc_system_firmware_init(PCMachineState *pcms,
>     }
>
>     pc_system_flash_cleanup_unused(pcms);
>+
>+    /*
>+     * The user should not have specified any pflash devices when using IGVM
>+     * to configure the guest.
>+     */
>+    if (MACHINE(pcms)->igvm) {
>+        for (i = 0; i < ARRAY_SIZE(pcms->flash); i++) {
>+            if (pcms->flash[i]) {
>+                error_report("pflash devices cannot be configured when "
>+                             "using IGVM");
>+                exit(1);
>+            }
>+        }
>+    }
> }
>
> void x86_firmware_configure(hwaddr gpa, void *ptr, int size)
>-- 
>2.43.0
>
Roy Hopkins June 28, 2024, 11:10 a.m. UTC | #2
On Thu, 2024-06-27 at 14:38 +0200, Stefano Garzarella wrote:
> On Fri, Jun 21, 2024 at 03:29:08PM GMT, Roy Hopkins wrote:
> > When using an IGVM file the configuration of the system firmware is
> > defined by IGVM directives contained in the file. In this case the user
> > should not configure any pflash devices.
> > 
> > This commit skips initialization of the ROM mode when pflash0 is not set
> > then checks to ensure no pflash devices have been configured when using
> > IGVM, exiting with an error message if this is not the case.
> > 
> > Signed-off-by: Roy Hopkins <roy.hopkins@suse.com>
> > ---
> > hw/i386/pc_sysfw.c | 23 +++++++++++++++++++++--
> > 1 file changed, 21 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
> > index ef80281d28..39e94ce144 100644
> > --- a/hw/i386/pc_sysfw.c
> > +++ b/hw/i386/pc_sysfw.c
> > @@ -239,8 +239,13 @@ void pc_system_firmware_init(PCMachineState *pcms,
> >     }
> > 
> >     if (!pflash_blk[0]) {
> > -        /* Machine property pflash0 not set, use ROM mode */
> > -        x86_bios_rom_init(X86_MACHINE(pcms), "bios.bin", rom_memory,
> > false);
> 
> We have the same call, a few lines above if `pci_enabled` is false, 
> should we make the same change there as well?

Yes. I'll add the same change there.

> 
> > +        /*
> > +         * Machine property pflash0 not set, use ROM mode unless using 
> > IGVM,
> > +         * in which case the firmware must be provided by the IGVM file.
> > +         */
> > +        if (!MACHINE(pcms)->igvm) {
> > +            x86_bios_rom_init(X86_MACHINE(pcms), "bios.bin", rom_memory,
> > false);
> > +        }
> >     } else {
> >         if (kvm_enabled() && !kvm_readonly_mem_enabled()) {
> >             /*
> > @@ -256,6 +261,20 @@ void pc_system_firmware_init(PCMachineState *pcms,
> >     }
> > 
> >     pc_system_flash_cleanup_unused(pcms);
> > +
> > +    /*
> > +     * The user should not have specified any pflash devices when using
> > IGVM
> > +     * to configure the guest.
> > +     */
> > +    if (MACHINE(pcms)->igvm) {
> > +        for (i = 0; i < ARRAY_SIZE(pcms->flash); i++) {
> > +            if (pcms->flash[i]) {
> > +                error_report("pflash devices cannot be configured when "
> > +                             "using IGVM");
> > +                exit(1);
> > +            }
> > +        }
> > +    }
> > }
> > 
> > void x86_firmware_configure(hwaddr gpa, void *ptr, int size)
> > -- 
> > 2.43.0
> > 
>
diff mbox series

Patch

diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
index ef80281d28..39e94ce144 100644
--- a/hw/i386/pc_sysfw.c
+++ b/hw/i386/pc_sysfw.c
@@ -239,8 +239,13 @@  void pc_system_firmware_init(PCMachineState *pcms,
     }
 
     if (!pflash_blk[0]) {
-        /* Machine property pflash0 not set, use ROM mode */
-        x86_bios_rom_init(X86_MACHINE(pcms), "bios.bin", rom_memory, false);
+        /*
+         * Machine property pflash0 not set, use ROM mode unless using IGVM,
+         * in which case the firmware must be provided by the IGVM file.
+         */
+        if (!MACHINE(pcms)->igvm) {
+            x86_bios_rom_init(X86_MACHINE(pcms), "bios.bin", rom_memory, false);
+        }
     } else {
         if (kvm_enabled() && !kvm_readonly_mem_enabled()) {
             /*
@@ -256,6 +261,20 @@  void pc_system_firmware_init(PCMachineState *pcms,
     }
 
     pc_system_flash_cleanup_unused(pcms);
+
+    /*
+     * The user should not have specified any pflash devices when using IGVM
+     * to configure the guest.
+     */
+    if (MACHINE(pcms)->igvm) {
+        for (i = 0; i < ARRAY_SIZE(pcms->flash); i++) {
+            if (pcms->flash[i]) {
+                error_report("pflash devices cannot be configured when "
+                             "using IGVM");
+                exit(1);
+            }
+        }
+    }
 }
 
 void x86_firmware_configure(hwaddr gpa, void *ptr, int size)