Re: [PATCH v2 1/2] hw/i386/pc: pc_system_ovmf_table_find: Assert that flash was parsed

[Tom Lendacky]

On 6/30/21 12:46 AM, Dov Murik wrote:
> Add assertion in pc_system_ovmf_table_find that verifies that the flash
> was indeed previously parsed (looking for the OVMF table) by
> pc_system_parse_ovmf_flash.
> 
> Now pc_system_ovmf_table_find distinguishes between "no one called
> pc_system_parse_ovmf_flash" (which will abort due to assertion failure)
> and "the flash was parsed but no OVMF table was found, or it is invalid"
> (which will return false).
> 
> Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>

Does the qemu coding style prefer not initializing the bool to false since
it will default to that? Otherwise,

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---
>  hw/i386/pc_sysfw.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
> index 6ce37a2b05..9eac36b830 100644
> --- a/hw/i386/pc_sysfw.c
> +++ b/hw/i386/pc_sysfw.c
> @@ -126,6 +126,7 @@ void pc_system_flash_cleanup_unused(PCMachineState *pcms)
>  
>  #define OVMF_TABLE_FOOTER_GUID "96b582de-1fb2-45f7-baea-a366c55a082d"
>  
> +static bool ovmf_flash_parsed = false;
>  static uint8_t *ovmf_table;
>  static int ovmf_table_len;
>  
> @@ -136,10 +137,12 @@ static void pc_system_parse_ovmf_flash(uint8_t 
> *flash_ptr, size_t flash_size)
>      int tot_len;
>  
>      /* should only be called once */
> -    if (ovmf_table) {
> +    if (ovmf_flash_parsed) {
>          return;
>      }
>  
> +    ovmf_flash_parsed = true;
> +
>      if (flash_size < TARGET_PAGE_SIZE) {
>          return;
>      }
> @@ -183,6 +186,8 @@ bool pc_system_ovmf_table_find(const char *entry, uint8_t 
> **data,
>      int tot_len = ovmf_table_len;
>      QemuUUID entry_guid;
>  
> +    assert(ovmf_flash_parsed);
> +
>      if (qemu_uuid_parse(entry, &entry_guid) < 0) {
>          return false;
>      }
>