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.hopk...@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
