[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/3] i386: factor out x86_firmware_configure()
From: |
Daniel P . Berrangé |
Subject: |
Re: [PATCH 2/3] i386: factor out x86_firmware_configure() |
Date: |
Thu, 31 Mar 2022 14:07:36 +0100 |
User-agent: |
Mutt/2.1.5 (2021-12-30) |
On Thu, Mar 31, 2022 at 10:35:48AM +0200, Gerd Hoffmann wrote:
> move sev firmware setup to separate function so it can be used from
> other code paths. No functional change.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> Tested-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
> include/hw/i386/x86.h | 3 +++
> hw/i386/pc_sysfw.c | 36 ++++++++++++++++++++++--------------
> 2 files changed, 25 insertions(+), 14 deletions(-)
>
> diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
> index 916cc325eeb1..4841a49f86c0 100644
> --- a/include/hw/i386/x86.h
> +++ b/include/hw/i386/x86.h
> @@ -140,4 +140,7 @@ void gsi_handler(void *opaque, int n, int level);
> void ioapic_init_gsi(GSIState *gsi_state, const char *parent_name);
> DeviceState *ioapic_init_secondary(GSIState *gsi_state);
>
> +/* pc_sysfw.c */
> +void x86_firmware_configure(void *ptr, int size);
> +
> #endif
> diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
> index c8b17af95353..36b6121b77b9 100644
> --- a/hw/i386/pc_sysfw.c
> +++ b/hw/i386/pc_sysfw.c
> @@ -148,7 +148,6 @@ static void pc_system_flash_map(PCMachineState *pcms,
> MemoryRegion *flash_mem;
> void *flash_ptr;
> int flash_size;
> - int ret;
>
> assert(PC_MACHINE_GET_CLASS(pcms)->pci_enabled);
>
> @@ -196,19 +195,7 @@ static void pc_system_flash_map(PCMachineState *pcms,
> if (sev_enabled()) {
> flash_ptr = memory_region_get_ram_ptr(flash_mem);
> flash_size = memory_region_size(flash_mem);
> - /*
> - * OVMF places a GUIDed structures in the flash, so
> - * search for them
> - */
> - pc_system_parse_ovmf_flash(flash_ptr, flash_size);
> -
> - ret = sev_es_save_reset_vector(flash_ptr, flash_size);
> - if (ret) {
> - error_report("failed to locate and/or save reset
> vector");
> - exit(1);
> - }
> -
> - sev_encrypt_flash(flash_ptr, flash_size, &error_fatal);
> + x86_firmware_configure(flash_ptr, flash_size);
> }
> }
> }
> @@ -260,3 +247,24 @@ void pc_system_firmware_init(PCMachineState *pcms,
>
> pc_system_flash_cleanup_unused(pcms);
> }
> +
> +void x86_firmware_configure(void *ptr, int size)
> +{
> + int ret;
> +
> + /*
> + * OVMF places a GUIDed structures in the flash, so
> + * search for them
> + */
> + pc_system_parse_ovmf_flash(ptr, size);
Any reason you chose to put this outside the sev_enabled()
check when you moved it, as that is a functional change ?
It ought to be harmless in theory, unless someone figures
out a way to break pc_system_parse_ovmf_flash code with
unexpected input.
> +
> + if (sev_enabled()) {
> + ret = sev_es_save_reset_vector(ptr, size);
> + if (ret) {
> + error_report("failed to locate and/or save reset vector");
> + exit(1);
> + }
> +
> + sev_encrypt_flash(ptr, size, &error_fatal);
> + }
> +}
> --
> 2.35.1
>
>
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 :|