| Message ID | 20220331083549.749566-4-kraxel@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | i386: firmware parsing and sev setup for -bios loaded firmware | expand |
On Thu, Mar 31, 2022 at 10:35:49AM +0200, Gerd Hoffmann wrote: > Don't register firmware as rom, not needed (see comment). > Add x86_firmware_configure() call for proper sev initialization. > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > Tested-by: Xiaoyao Li <xiaoyao.li@intel.com> > --- > hw/i386/x86.c | 25 +++++++++++++++++++------ > 1 file changed, 19 insertions(+), 6 deletions(-) Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> I validated that I could validate the measurement of a SEV guest with -bios, and see the firmware start at least. Tested-by: Daniel P. Berrangé <berrange@redhat.com> > > diff --git a/hw/i386/x86.c b/hw/i386/x86.c > index b2e801a8720e..f98483c7fe83 100644 > --- a/hw/i386/x86.c > +++ b/hw/i386/x86.c > @@ -1116,12 +1116,25 @@ void x86_bios_rom_init(MachineState *ms, const char *default_firmware, > } > bios = g_malloc(sizeof(*bios)); > memory_region_init_ram(bios, NULL, "pc.bios", bios_size, &error_fatal); > - if (!isapc_ram_fw) { > - memory_region_set_readonly(bios, true); > - } > - ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), -1); > - if (ret != 0) { > - goto bios_error; > + if (sev_enabled()) { > + /* > + * The concept of a "reset" simply doesn't exist for > + * confidential computing guests, we have to destroy and > + * re-launch them instead. So there is no need to register > + * the firmware as rom to properly re-initialize on reset. > + * Just go for a straight file load instead. > + */ > + void *ptr = memory_region_get_ram_ptr(bios); > + load_image_size(filename, ptr, bios_size); > + x86_firmware_configure(ptr, bios_size); > + } else { > + if (!isapc_ram_fw) { > + memory_region_set_readonly(bios, true); > + } > + ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), -1); > + if (ret != 0) { > + goto bios_error; > + } > } > g_free(filename); > > -- > 2.35.1 > > With regards, Daniel
On Thu, Mar 31, 2022 at 02:10:04PM +0100, Daniel P. Berrangé wrote: > On Thu, Mar 31, 2022 at 10:35:49AM +0200, Gerd Hoffmann wrote: > > Don't register firmware as rom, not needed (see comment). > > Add x86_firmware_configure() call for proper sev initialization. > > > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > > Tested-by: Xiaoyao Li <xiaoyao.li@intel.com> > > --- > > hw/i386/x86.c | 25 +++++++++++++++++++------ > > 1 file changed, 19 insertions(+), 6 deletions(-) > > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> > > I validated that I could validate the measurement of a SEV > guest with -bios, and see the firmware start at least. > > Tested-by: Daniel P. Berrangé <berrange@redhat.com> Nice. With that in place we should be pretty close to get sev going with microvm. Probably needs revert of edk2 commit 06fa1f1931e9 ("OvmfPkg/Microvm: no sev"), with some luck direct kernel boot works without that though. take care, Gerd
On 31/3/22 10:35, Gerd Hoffmann wrote: > Don't register firmware as rom, not needed (see comment). > Add x86_firmware_configure() call for proper sev initialization. > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > Tested-by: Xiaoyao Li <xiaoyao.li@intel.com> > --- > hw/i386/x86.c | 25 +++++++++++++++++++------ > 1 file changed, 19 insertions(+), 6 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
diff --git a/hw/i386/x86.c b/hw/i386/x86.c index b2e801a8720e..f98483c7fe83 100644 --- a/hw/i386/x86.c +++ b/hw/i386/x86.c @@ -1116,12 +1116,25 @@ void x86_bios_rom_init(MachineState *ms, const char *default_firmware, } bios = g_malloc(sizeof(*bios)); memory_region_init_ram(bios, NULL, "pc.bios", bios_size, &error_fatal); - if (!isapc_ram_fw) { - memory_region_set_readonly(bios, true); - } - ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), -1); - if (ret != 0) { - goto bios_error; + if (sev_enabled()) { + /* + * The concept of a "reset" simply doesn't exist for + * confidential computing guests, we have to destroy and + * re-launch them instead. So there is no need to register + * the firmware as rom to properly re-initialize on reset. + * Just go for a straight file load instead. + */ + void *ptr = memory_region_get_ram_ptr(bios); + load_image_size(filename, ptr, bios_size); + x86_firmware_configure(ptr, bios_size); + } else { + if (!isapc_ram_fw) { + memory_region_set_readonly(bios, true); + } + ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), -1); + if (ret != 0) { + goto bios_error; + } } g_free(filename);