Re: [PATCH 6/9] i386/pc: Skip initialization of system FW when using IGVM

[Roy Hopkins]

On Fri, 2024-03-01 at 16:54 +0000, Daniel P. Berrangé wrote:
> On Tue, Feb 27, 2024 at 02:50:12PM +0000, Roy Hopkins wrote:
> > When using an IGVM file the configuration of the system firmware is
> > defined by IGVM directives contained in the file. Therefore the default
> > system firmware should not be initialized when an IGVM file has been
> > provided.
> > 
> > This commit checks to see if an IGVM file has been provided and, if it
> > has then the standard system firmware initialization is skipped and any
> > prepared flash devices are cleaned up.
> > 
> > Signed-off-by: Roy Hopkins <roy.hopkins@suse.com>
> > ---
> >  hw/i386/pc.c | 12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index f8eb684a49..17bb211708 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -63,6 +63,7 @@
> >  #include "e820_memory_layout.h"
> >  #include "trace.h"
> >  #include CONFIG_DEVICES
> > +#include "exec/confidential-guest-support.h"
> >  
> >  #ifdef CONFIG_XEN_EMU
> >  #include "hw/xen/xen-legacy-backend.h"
> > @@ -1023,8 +1024,15 @@ void pc_memory_init(PCMachineState *pcms,
> >          }
> >      }
> >  
> > -    /* Initialize PC system firmware */
> > -    pc_system_firmware_init(pcms, rom_memory);
> > +    /*
> > +     * If this is a confidential guest configured using IGVM then the IGVM
> > +     * configuration will include the system firmware. In this case do not
> > +     * initialise PC system firmware.
> > +     */
> > +    if (!cgs_is_igvm(machine->cgs)) {
> > +        /* Initialize PC system firmware */
> > +        pc_system_firmware_init(pcms, rom_memory);
> > +    }
> 
> If the user has given explicit pflash config I think we should be
> reporting an error for the invalid configuration, rather than
> silently ignoring their mistake.
> 

Ok. In that case, I'll replace this patch and move the check into
pc_system_firmware_init() in "pc_sysfw.c". I can then check for the presence of
an IGVM file after the firmware configuration has completed, generating an error
if pflash has been configured when an IGVM file is provided.

> >  
> >      option_rom_mr = g_malloc(sizeof(*option_rom_mr));
> >      memory_region_init_ram(option_rom_mr, NULL, "pc.rom", PC_ROM_SIZE,
> > -- 
> > 2.43.0
> > 
> > 
> 
> With regards,
> Daniel


Many thanks,
Roy