On Wed, Jul 03, 2024 at 12:05:43PM +0100, 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.hopk...@suse.com> > --- > hw/i386/pc_sysfw.c | 31 ++++++++++++++++++++++++++++--- > 1 file changed, 28 insertions(+), 3 deletions(-)
Reviewed-by: Daniel P. Berrangé <berra...@redhat.com> > > diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c > index ef80281d28..f5e40b3ef6 100644 > --- a/hw/i386/pc_sysfw.c > +++ b/hw/i386/pc_sysfw.c > @@ -219,7 +219,13 @@ void pc_system_firmware_init(PCMachineState *pcms, > BlockBackend *pflash_blk[ARRAY_SIZE(pcms->flash)]; > > if (!pcmc->pci_enabled) { > - x86_bios_rom_init(X86_MACHINE(pcms), "bios.bin", rom_memory, true); > + /* > + * If an IGVM file is specified then the firmware must be provided > + * in the IGVM file. > + */ > + if (!X86_MACHINE(pcms)->igvm) { > + x86_bios_rom_init(X86_MACHINE(pcms), "bios.bin", rom_memory, > true); > + } IIUC from looking at x86_bios_rom_init, the 'firmware' machine property will be NULL if no -bios arg is given, and non-NULL if -bios is set, so we can give an error message is -bios is set, while doing the right thing if unset. IOW I think this could look more like X86MachineState *x86ms = X86_MACHINE(pcms); if (x86ms->igvm) { if (x86ms->firmware) { error_report("Firmware ROM cannot be configured when " "using IGVM"); exit(1); } } else { x86_bios_rom_init(x86ms, "bios.bin", rom_memory, true); } > return; > } > > @@ -239,8 +245,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 (!X86_MACHINE(pcms)->igvm) { > + x86_bios_rom_init(X86_MACHINE(pcms), "bios.bin", rom_memory, > false); > + } Same as earlier > } else { > if (kvm_enabled() && !kvm_readonly_mem_enabled()) { > /* > @@ -256,6 +267,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 (X86_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 > With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|