[U-Boot,v2,01/16] efi: Update efi_smbios_register() to return error code

Message ID 20171204212832.130100-2-sjg@chromium.org
State New
Delegated to: Alexander Graf
Headers show
Series
  • efi: Enable basic sandbox support for EFI loader
Related show

Commit Message

Simon Glass Dec. 4, 2017, 9:28 p.m.
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 <sjg@chromium.org>
---

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(-)

Comments

Heinrich Schuchardt Dec. 4, 2017, 10:15 p.m. | #1
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 <sjg@chromium.org>
> ---
> 
> 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 = 0xffffffff;
> @@ -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);
>   }
>

Patch

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 = 0xffffffff;
@@ -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)
-		return;
+		return EFI_OUT_OF_RESOURCES;
 
 	/* Generate SMBIOS tables */
 	write_smbios_table(dmi);
 
 	/* 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);
 }