[PATCH v3 0/2] hw/i386/pc: Clarify pc_system_ovmf_table_find usage

[PATCH v3 0/2] hw/i386/pc: Clarify pc_system_ovmf_table_find usage

[Dov Murik]

Add assertion to verify that the flash was parsed (looking for the OVMF table),
and add documentation for pc_system_ovmf_table_find.

v3:
 - [style] remove static initalization to 'false'

v2:
 - add assertion (insert patch 1/2)

Dov Murik (2):
  hw/i386/pc: pc_system_ovmf_table_find: Assert that flash was parsed
  hw/i386/pc: Document pc_system_ovmf_table_find

 hw/i386/pc_sysfw.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

-- 
2.25.1

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

[Dov Murik]

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>
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..e353f2a4e9 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;
 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;
     }
-- 
2.25.1

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

[Philippe Mathieu-Daudé]

On 7/1/21 7:27 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>
> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

For obvious / checkpatch fixes you can keep the tags ;)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  hw/i386/pc_sysfw.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)

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

[Dov Murik]

On 01/07/2021 11:54, Philippe Mathieu-Daudé wrote:
> On 7/1/21 7:27 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>
>> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
> 
> For obvious / checkpatch fixes you can keep the tags ;)
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> 

Thanks Phil.

-Dov

[PATCH v3 2/2] hw/i386/pc: Document pc_system_ovmf_table_find

[Dov Murik]

Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/i386/pc_sysfw.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
index e353f2a4e9..6ddce92a86 100644
--- a/hw/i386/pc_sysfw.c
+++ b/hw/i386/pc_sysfw.c
@@ -179,6 +179,17 @@ static void pc_system_parse_ovmf_flash(uint8_t *flash_ptr, size_t flash_size)
     ovmf_table += tot_len;
 }
 
+/**
+ * pc_system_ovmf_table_find - Find the data associated with an entry in OVMF's
+ * reset vector GUIDed table.
+ *
+ * @entry: GUID string of the entry to lookup
+ * @data: Filled with a pointer to the entry's value (if not NULL)
+ * @data_len: Filled with the length of the entry's value (if not NULL). Pass
+ *            NULL here if the length of data is known.
+ *
+ * Return: true if the entry was found in the OVMF table; false otherwise.
+ */
 bool pc_system_ovmf_table_find(const char *entry, uint8_t **data,
                                int *data_len)
 {
-- 
2.25.1

Re: [PATCH v3 0/2] hw/i386/pc: Clarify pc_system_ovmf_table_find usage

[Philippe Mathieu-Daudé]

On 7/1/21 7:27 AM, Dov Murik wrote:
> Add assertion to verify that the flash was parsed (looking for the OVMF table),
> and add documentation for pc_system_ovmf_table_find.

> Dov Murik (2):
>   hw/i386/pc: pc_system_ovmf_table_find: Assert that flash was parsed
>   hw/i386/pc: Document pc_system_ovmf_table_find
> 
>  hw/i386/pc_sysfw.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 

Thanks, queued to fw-edk2 tree.