Re: [U-Boot] [PATCH v2 01/16] efi: Update efi_smbios_register() to return error code
Hi Heinrich,
On 4 December 2017 at 15:15, Heinrich Schuchardt wrote:
>
>
> On 12/04/2017 10:28 PM, Simon Glass wrote:
>>
>> This function can fail but gives no indication of failure. Update it to
>> return an error when something goes wrong.
>>
>> Signed-off-by: Simon Glass
>> ---
>>
>> Changes in v2:
>> - Update return type of efi_smbios_register() to efi_status_t
>> - Use return value of efi_install_configuration_table
>>
>> include/efi_loader.h| 9 -
>> lib/efi_loader/efi_smbios.c | 7 ---
>> 2 files changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/efi_loader.h b/include/efi_loader.h
>> index 1b92edbd77..35f8f84401 100644
>> --- a/include/efi_loader.h
>> +++ b/include/efi_loader.h
>> @@ -164,7 +164,14 @@ int efi_gop_register(void);
>> /* Called by bootefi to make the network interface available */
>> int efi_net_register(void);
>> /* Called by bootefi to make SMBIOS tables available */
>> -void efi_smbios_register(void);
>> +/**
>> + * efi_smbios_register() - write out SMBIOS tables
>> + *
>> + * Called by bootefi to make SMBIOS tables available
>> + *
>> + * @return 0 if OK, -ENOMEM if no memory is available for the tables
>> + */
>> +efi_status_t efi_smbios_register(void);
>> struct efi_simple_file_system_protocol *
>> efi_fs_from_path(struct efi_device_path *fp);
>> diff --git a/lib/efi_loader/efi_smbios.c b/lib/efi_loader/efi_smbios.c
>> index ac412e7362..67f71892ca 100644
>> --- a/lib/efi_loader/efi_smbios.c
>> +++ b/lib/efi_loader/efi_smbios.c
>> @@ -13,7 +13,7 @@
>> static const efi_guid_t smbios_guid = SMBIOS_TABLE_GUID;
>> -void efi_smbios_register(void)
>> +efi_status_t efi_smbios_register(void)
>> {
>> /* Map within the low 32 bits, to allow for 32bit SMBIOS tables */
>> uint64_t dmi = 0x;
>> @@ -22,11 +22,12 @@ void efi_smbios_register(void)
>> int memtype = EFI_RUNTIME_SERVICES_DATA;
>> if (efi_allocate_pages(1, memtype, pages, &dmi) != EFI_SUCCESS)
>
>
> Please, return the value returned from efi_allocate_pages(). The function
> has a lot of different failure modes.
OK, will do. But looking at efi_allocate_pages() there is no
documentation as to the return value.
>
>> - return;
>> + return EFI_OUT_OF_RESOURCES;
>> /* Generate SMBIOS tables */
>> write_smbios_table(dmi);
>
> We should add a comment explaining why we do not use the return value of
> write_smbios_table(). My understanding is:
> write_smbios_table returns dmi rounded up to a multiple of 16.
> efi_allocate_pages returns a 4096 aligned address. So we do expect that the
> return value equals the parameter apssed to write_smbios_table().
OK will do.
Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 01/16] efi: Update efi_smbios_register() to return error code
On 12/04/2017 10:28 PM, Simon Glass wrote:
This function can fail but gives no indication of failure. Update it to
return an error when something goes wrong.
Signed-off-by: Simon Glass
---
Changes in v2:
- Update return type of efi_smbios_register() to efi_status_t
- Use return value of efi_install_configuration_table
include/efi_loader.h| 9 -
lib/efi_loader/efi_smbios.c | 7 ---
2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 1b92edbd77..35f8f84401 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -164,7 +164,14 @@ int efi_gop_register(void);
/* Called by bootefi to make the network interface available */
int efi_net_register(void);
/* Called by bootefi to make SMBIOS tables available */
-void efi_smbios_register(void);
+/**
+ * efi_smbios_register() - write out SMBIOS tables
+ *
+ * Called by bootefi to make SMBIOS tables available
+ *
+ * @return 0 if OK, -ENOMEM if no memory is available for the tables
+ */
+efi_status_t efi_smbios_register(void);
struct efi_simple_file_system_protocol *
efi_fs_from_path(struct efi_device_path *fp);
diff --git a/lib/efi_loader/efi_smbios.c b/lib/efi_loader/efi_smbios.c
index ac412e7362..67f71892ca 100644
--- a/lib/efi_loader/efi_smbios.c
+++ b/lib/efi_loader/efi_smbios.c
@@ -13,7 +13,7 @@
static const efi_guid_t smbios_guid = SMBIOS_TABLE_GUID;
-void efi_smbios_register(void)
+efi_status_t efi_smbios_register(void)
{
/* Map within the low 32 bits, to allow for 32bit SMBIOS tables */
uint64_t dmi = 0x;
@@ -22,11 +22,12 @@ void efi_smbios_register(void)
int memtype = EFI_RUNTIME_SERVICES_DATA;
if (efi_allocate_pages(1, memtype, pages, &dmi) != EFI_SUCCESS)
Please, return the value returned from efi_allocate_pages(). The
function has a lot of different failure modes.
- return;
+ return EFI_OUT_OF_RESOURCES;
/* Generate SMBIOS tables */
write_smbios_table(dmi);
We should add a comment explaining why we do not use the return value of
write_smbios_table(). My understanding is:
write_smbios_table returns dmi rounded up to a multiple of 16.
efi_allocate_pages returns a 4096 aligned address. So we do expect that
the return value equals the parameter apssed to write_smbios_table().
Best regards
Heinrich
/* And expose them to our EFI payload */
- efi_install_configuration_table(&smbios_guid, (void*)(uintptr_t)dmi);
+ return efi_install_configuration_table(&smbios_guid,
+ (void *)(uintptr_t)dmi);
}
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
