[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 1/2] hw/i386/pc: pc_system_ovmf_table_find: Assert that fl
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH v2 1/2] hw/i386/pc: pc_system_ovmf_table_find: Assert that flash was parsed |
Date: |
Wed, 30 Jun 2021 16:35:18 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 |
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;
>> }
>>
>