Re: [PATCH 2/3] i386: factor out x86_firmware_configure()

[Philippe Mathieu-Daudé]

On 31/3/22 10:35, 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);
Can we remove the SEV check ...

> -                /*
> -                 * 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);

... making this code generic ...?

>               }
>           }
>       }
> @@ -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);
> +
> +    if (sev_enabled()) {

... because we are still checking SEV here.

> +        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);
> +    }
> +}