* [PATCH v2 0/2] hw/i386/pc: Clarify pc_system_ovmf_table_find usage
@ 2021-06-30 5:46 Dov Murik
2021-06-30 5:46 ` [PATCH v2 1/2] hw/i386/pc: pc_system_ovmf_table_find: Assert that flash was parsed Dov Murik
2021-06-30 5:46 ` [PATCH v2 2/2] hw/i386/pc: Document pc_system_ovmf_table_find Dov Murik
0 siblings, 2 replies; 8+ messages in thread
From: Dov Murik @ 2021-06-30 5:46 UTC (permalink / raw)
To: qemu-devel
Cc: Tom Lendacky, Eduardo Habkost, Michael S. Tsirkin,
Richard Henderson, Dov Murik, Paolo Bonzini,
Philippe Mathieu-Daudé
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(-)
--
2.25.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/2] hw/i386/pc: pc_system_ovmf_table_find: Assert that flash was parsed
2021-06-30 5:46 [PATCH v2 0/2] hw/i386/pc: Clarify pc_system_ovmf_table_find usage Dov Murik
@ 2021-06-30 5:46 ` Dov Murik
2021-06-30 8:23 ` Philippe Mathieu-Daudé
2021-06-30 13:38 ` Tom Lendacky
2021-06-30 5:46 ` [PATCH v2 2/2] hw/i386/pc: Document pc_system_ovmf_table_find Dov Murik
1 sibling, 2 replies; 8+ messages in thread
From: Dov Murik @ 2021-06-30 5:46 UTC (permalink / raw)
To: qemu-devel
Cc: Tom Lendacky, Eduardo Habkost, Michael S. Tsirkin,
Richard Henderson, Dov Murik, Paolo Bonzini,
Philippe Mathieu-Daudé
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>
---
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;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 2/2] hw/i386/pc: Document pc_system_ovmf_table_find
2021-06-30 5:46 [PATCH v2 0/2] hw/i386/pc: Clarify pc_system_ovmf_table_find usage Dov Murik
2021-06-30 5:46 ` [PATCH v2 1/2] hw/i386/pc: pc_system_ovmf_table_find: Assert that flash was parsed Dov Murik
@ 2021-06-30 5:46 ` Dov Murik
1 sibling, 0 replies; 8+ messages in thread
From: Dov Murik @ 2021-06-30 5:46 UTC (permalink / raw)
To: qemu-devel
Cc: Tom Lendacky, Eduardo Habkost, Michael S. Tsirkin,
Richard Henderson, Dov Murik, Paolo Bonzini,
Philippe Mathieu-Daudé
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 9eac36b830..d02f1b2361 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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] hw/i386/pc: pc_system_ovmf_table_find: Assert that flash was parsed
2021-06-30 5:46 ` [PATCH v2 1/2] hw/i386/pc: pc_system_ovmf_table_find: Assert that flash was parsed Dov Murik
@ 2021-06-30 8:23 ` Philippe Mathieu-Daudé
2021-06-30 11:08 ` Dov Murik
2021-06-30 13:38 ` Tom Lendacky
1 sibling, 1 reply; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-30 8:23 UTC (permalink / raw)
To: Dov Murik, qemu-devel
Cc: Tom Lendacky, Eduardo Habkost, Michael S. Tsirkin,
Richard Henderson, Paolo Bonzini
On 6/30/21 7: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>
> ---
> hw/i386/pc_sysfw.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Thank you Dov,
Phil.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] hw/i386/pc: pc_system_ovmf_table_find: Assert that flash was parsed
2021-06-30 8:23 ` Philippe Mathieu-Daudé
@ 2021-06-30 11:08 ` Dov Murik
0 siblings, 0 replies; 8+ messages in thread
From: Dov Murik @ 2021-06-30 11:08 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Tom Lendacky, Eduardo Habkost, Michael S. Tsirkin,
Richard Henderson, Dov Murik, Paolo Bonzini
On 30/06/2021 11:23, Philippe Mathieu-Daudé wrote:
> On 6/30/21 7: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>
>> ---
>> hw/i386/pc_sysfw.c | 7 ++++++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Thanks Phil for the constructive review process.
-Dov
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] hw/i386/pc: pc_system_ovmf_table_find: Assert that flash was parsed
2021-06-30 5:46 ` [PATCH v2 1/2] hw/i386/pc: pc_system_ovmf_table_find: Assert that flash was parsed Dov Murik
2021-06-30 8:23 ` Philippe Mathieu-Daudé
@ 2021-06-30 13:38 ` Tom Lendacky
2021-06-30 14:35 ` Philippe Mathieu-Daudé
1 sibling, 1 reply; 8+ messages in thread
From: Tom Lendacky @ 2021-06-30 13:38 UTC (permalink / raw)
To: Dov Murik, qemu-devel
Cc: Eduardo Habkost, Paolo Bonzini, Richard Henderson,
Marcel Apfelbaum, Philippe Mathieu-Daudé,
Michael S. Tsirkin
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;
> }
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] hw/i386/pc: pc_system_ovmf_table_find: Assert that flash was parsed
2021-06-30 13:38 ` Tom Lendacky
@ 2021-06-30 14:35 ` Philippe Mathieu-Daudé
2021-07-01 5:32 ` Dov Murik
0 siblings, 1 reply; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-30 14:35 UTC (permalink / raw)
To: Tom Lendacky, Dov Murik, qemu-devel
Cc: Paolo Bonzini, Richard Henderson, Eduardo Habkost, Michael S. Tsirkin
On 6/30/21 3:38 PM, Tom Lendacky wrote:
> 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?
Indeed, you are right, and checkpatch will block this patch:
ERROR: do not initialise statics to 0 or NULL
#33: FILE: hw/i386/pc_sysfw.c:129:
+static bool ovmf_flash_parsed = false;
total: 1 errors, 0 warnings, 28 lines checked
> 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;
>> }
>>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] hw/i386/pc: pc_system_ovmf_table_find: Assert that flash was parsed
2021-06-30 14:35 ` Philippe Mathieu-Daudé
@ 2021-07-01 5:32 ` Dov Murik
0 siblings, 0 replies; 8+ messages in thread
From: Dov Murik @ 2021-07-01 5:32 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Tom Lendacky, qemu-devel
Cc: Paolo Bonzini, Richard Henderson, Eduardo Habkost, Michael S. Tsirkin
On 30/06/2021 17:35, Philippe Mathieu-Daudé wrote:
> On 6/30/21 3:38 PM, Tom Lendacky wrote:
>> 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?
>
> Indeed, you are right, and checkpatch will block this patch:
>
> ERROR: do not initialise statics to 0 or NULL
> #33: FILE: hw/i386/pc_sysfw.c:129:
> +static bool ovmf_flash_parsed = false;
>
> total: 1 errors, 0 warnings, 28 lines checked
oops, missed that in my flow.
Sent a v3 series with this fix.
>
>> Otherwise,
>>
>> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Thanks, Tom!
-Dov
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-07-01 5:33 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-30 5:46 [PATCH v2 0/2] hw/i386/pc: Clarify pc_system_ovmf_table_find usage Dov Murik
2021-06-30 5:46 ` [PATCH v2 1/2] hw/i386/pc: pc_system_ovmf_table_find: Assert that flash was parsed Dov Murik
2021-06-30 8:23 ` Philippe Mathieu-Daudé
2021-06-30 11:08 ` Dov Murik
2021-06-30 13:38 ` Tom Lendacky
2021-06-30 14:35 ` Philippe Mathieu-Daudé
2021-07-01 5:32 ` Dov Murik
2021-06-30 5:46 ` [PATCH v2 2/2] hw/i386/pc: Document pc_system_ovmf_table_find Dov Murik
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).