[U-Boot] [PATCH v13 1/4] sandbox: smbios: Update to support sandbox
Alexander Graf
agraf at suse.de
Wed Nov 14 10:18:57 UTC 2018
On 11/14/2018 07:50 AM, Simon Glass wrote:
> At present this code casts addresses to pointers so cannot be used with
> sandbox. Update it to use mapmem instead.
>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
>
> Changes in v13:
> - Update code to deal with the struct_table_address member
>
> Changes in v12: None
> Changes in v11:
> - Fix the EFI code that has since been added and relies on broken behaviour
>
> Changes in v9: None
> Changes in v7: None
> Changes in v5: None
> Changes in v4: None
> Changes in v3:
> - Drop incorrect map_sysmem() in write_smbios_table()
>
> lib/efi_loader/efi_smbios.c | 20 +++++++++-----
> lib/smbios.c | 52 ++++++++++++++++++++++++++++++-------
> 2 files changed, 56 insertions(+), 16 deletions(-)
>
> diff --git a/lib/efi_loader/efi_smbios.c b/lib/efi_loader/efi_smbios.c
> index 38e42fa2432..a81488495e2 100644
> --- a/lib/efi_loader/efi_smbios.c
> +++ b/lib/efi_loader/efi_smbios.c
> @@ -7,6 +7,7 @@
>
> #include <common.h>
> #include <efi_loader.h>
> +#include <mapmem.h>
> #include <smbios.h>
>
> static const efi_guid_t smbios_guid = SMBIOS_TABLE_GUID;
> @@ -19,17 +20,19 @@ static const efi_guid_t smbios_guid = SMBIOS_TABLE_GUID;
> efi_status_t efi_smbios_register(void)
> {
> /* Map within the low 32 bits, to allow for 32bit SMBIOS tables */
> - u64 dmi = U32_MAX;
> + u64 dmi_addr = U32_MAX;
> efi_status_t ret;
> + void *dmi;
>
> /* Reserve 4kiB page for SMBIOS */
> ret = efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
> - EFI_RUNTIME_SERVICES_DATA, 1, &dmi);
> + EFI_RUNTIME_SERVICES_DATA, 1, &dmi_addr);
>
> if (ret != EFI_SUCCESS) {
> /* Could not find space in lowmem, use highmem instead */
> ret = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES,
> - EFI_RUNTIME_SERVICES_DATA, 1, &dmi);
> + EFI_RUNTIME_SERVICES_DATA, 1,
> + &dmi_addr);
>
> if (ret != EFI_SUCCESS)
> return ret;
> @@ -39,11 +42,14 @@ efi_status_t efi_smbios_register(void)
> * Generate SMBIOS tables - we know that efi_allocate_pages() returns
> * a 4k-aligned address, so it is safe to assume that
> * write_smbios_table() will write the table at that address.
> + *
> + * Note that on sandbox, efi_allocate_pages() unfortunately returns a
> + * pointer even though it uses a uint64_t type. Convert it.
> */
> - assert(!(dmi & 0xf));
> - write_smbios_table(dmi);
> + assert(!(dmi_addr & 0xf));
> + dmi = (void *)(uintptr_t)dmi_addr;
> + write_smbios_table(map_to_sysmem(dmi));
>
> /* And expose them to our EFI payload */
> - return efi_install_configuration_table(&smbios_guid,
> - (void *)(uintptr_t)dmi);
> + return efi_install_configuration_table(&smbios_guid, dmi);
> }
> diff --git a/lib/smbios.c b/lib/smbios.c
> index 326eb00230d..2cd08b5e16f 100644
> --- a/lib/smbios.c
> +++ b/lib/smbios.c
> @@ -6,6 +6,7 @@
> */
>
> #include <common.h>
> +#include <mapmem.h>
> #include <smbios.h>
> #include <tables_csum.h>
> #include <version.h>
> @@ -72,9 +73,10 @@ static int smbios_string_table_len(char *start)
>
> static int smbios_write_type0(ulong *current, int handle)
> {
> - struct smbios_type0 *t = (struct smbios_type0 *)*current;
> + struct smbios_type0 *t;
> int len = sizeof(struct smbios_type0);
>
> + t = map_sysmem(*current, len);
> memset(t, 0, sizeof(struct smbios_type0));
> fill_smbios_header(t, SMBIOS_BIOS_INFORMATION, len, handle);
> t->vendor = smbios_add_string(t->eos, "U-Boot");
> @@ -101,16 +103,18 @@ static int smbios_write_type0(ulong *current, int handle)
>
> len = t->length + smbios_string_table_len(t->eos);
> *current += len;
> + unmap_sysmem(t);
>
> return len;
> }
>
> static int smbios_write_type1(ulong *current, int handle)
> {
> - struct smbios_type1 *t = (struct smbios_type1 *)*current;
> + struct smbios_type1 *t;
> int len = sizeof(struct smbios_type1);
> char *serial_str = env_get("serial#");
>
> + t = map_sysmem(*current, len);
> memset(t, 0, sizeof(struct smbios_type1));
> fill_smbios_header(t, SMBIOS_SYSTEM_INFORMATION, len, handle);
> t->manufacturer = smbios_add_string(t->eos, CONFIG_SMBIOS_MANUFACTURER);
> @@ -122,15 +126,17 @@ static int smbios_write_type1(ulong *current, int handle)
>
> len = t->length + smbios_string_table_len(t->eos);
> *current += len;
> + unmap_sysmem(t);
>
> return len;
> }
>
> static int smbios_write_type2(ulong *current, int handle)
> {
> - struct smbios_type2 *t = (struct smbios_type2 *)*current;
> + struct smbios_type2 *t;
> int len = sizeof(struct smbios_type2);
>
> + t = map_sysmem(*current, len);
> memset(t, 0, sizeof(struct smbios_type2));
> fill_smbios_header(t, SMBIOS_BOARD_INFORMATION, len, handle);
> t->manufacturer = smbios_add_string(t->eos, CONFIG_SMBIOS_MANUFACTURER);
> @@ -140,15 +146,17 @@ static int smbios_write_type2(ulong *current, int handle)
>
> len = t->length + smbios_string_table_len(t->eos);
> *current += len;
> + unmap_sysmem(t);
>
> return len;
> }
>
> static int smbios_write_type3(ulong *current, int handle)
> {
> - struct smbios_type3 *t = (struct smbios_type3 *)*current;
> + struct smbios_type3 *t;
> int len = sizeof(struct smbios_type3);
>
> + t = map_sysmem(*current, len);
> memset(t, 0, sizeof(struct smbios_type3));
> fill_smbios_header(t, SMBIOS_SYSTEM_ENCLOSURE, len, handle);
> t->manufacturer = smbios_add_string(t->eos, CONFIG_SMBIOS_MANUFACTURER);
> @@ -160,6 +168,7 @@ static int smbios_write_type3(ulong *current, int handle)
>
> len = t->length + smbios_string_table_len(t->eos);
> *current += len;
> + unmap_sysmem(t);
>
> return len;
> }
> @@ -198,9 +207,10 @@ static void smbios_write_type4_dm(struct smbios_type4 *t)
>
> static int smbios_write_type4(ulong *current, int handle)
> {
> - struct smbios_type4 *t = (struct smbios_type4 *)*current;
> + struct smbios_type4 *t;
> int len = sizeof(struct smbios_type4);
>
> + t = map_sysmem(*current, len);
> memset(t, 0, sizeof(struct smbios_type4));
> fill_smbios_header(t, SMBIOS_PROCESSOR_INFORMATION, len, handle);
> t->processor_type = SMBIOS_PROCESSOR_TYPE_CENTRAL;
> @@ -214,32 +224,37 @@ static int smbios_write_type4(ulong *current, int handle)
>
> len = t->length + smbios_string_table_len(t->eos);
> *current += len;
> + unmap_sysmem(t);
>
> return len;
> }
>
> static int smbios_write_type32(ulong *current, int handle)
> {
> - struct smbios_type32 *t = (struct smbios_type32 *)*current;
> + struct smbios_type32 *t;
> int len = sizeof(struct smbios_type32);
>
> + t = map_sysmem(*current, len);
> memset(t, 0, sizeof(struct smbios_type32));
> fill_smbios_header(t, SMBIOS_SYSTEM_BOOT_INFORMATION, len, handle);
>
> *current += len;
> + unmap_sysmem(t);
>
> return len;
> }
>
> static int smbios_write_type127(ulong *current, int handle)
> {
> - struct smbios_type127 *t = (struct smbios_type127 *)*current;
> + struct smbios_type127 *t;
> int len = sizeof(struct smbios_type127);
>
> + t = map_sysmem(*current, len);
> memset(t, 0, sizeof(struct smbios_type127));
> fill_smbios_header(t, SMBIOS_END_OF_TABLE, len, handle);
>
> *current += len;
> + unmap_sysmem(t);
>
> return len;
> }
> @@ -257,6 +272,7 @@ static smbios_write_type smbios_write_funcs[] = {
> ulong write_smbios_table(ulong addr)
> {
> struct smbios_entry *se;
> + ulong table_addr;
> ulong tables;
> int len = 0;
> int max_struct_size = 0;
> @@ -268,7 +284,7 @@ ulong write_smbios_table(ulong addr)
> /* 16 byte align the table address */
> addr = ALIGN(addr, 16);
>
> - se = (struct smbios_entry *)(uintptr_t)addr;
> + se = map_sysmem(addr, sizeof(struct smbios_entry));
> memset(se, 0, sizeof(struct smbios_entry));
>
> addr += sizeof(struct smbios_entry);
> @@ -290,7 +306,24 @@ ulong write_smbios_table(ulong addr)
> se->max_struct_size = max_struct_size;
> memcpy(se->intermediate_anchor, "_DMI_", 5);
> se->struct_table_length = len;
> - se->struct_table_address = tables;
> +
> + /*
> + * We must use a pointer here so things work correctly on sandbox. The
> + * user of this table is not aware of the mapping of addresses to
> + * sandbox's DRAM buffer.
> + */
> + table_addr = (ulong)map_sysmem(tables, 0);
> + if (sizeof(table_addr) >= sizeof(u32) && table_addr >= (1ULL << 32)) {
sizeof(long) >= sizeof(u32) will always be true on any platform U-Boot
supports, no?
How about
if (sizeof(table_addr) > sizeof(u32) && table_addr > (ulong)UINT_MAX)
> + /*
> + * We need to put this >32-bit pointer into the table but the
> + * field is only 32 bits wide.
> + */
> + printf("WARNING: SMBIOS table_address overflow %llx\n",
> + (unsigned long long)table_addr);
> + table_addr = 0;
I'm also not sure what to do about the error case. IMHO ideally we
should propagate the error to the upper layers, but consider it non-fatal.
> + }
> + se->struct_table_address = table_addr;
> +
> se->struct_count = handle;
>
> /* calculate checksums */
> @@ -298,6 +331,7 @@ ulong write_smbios_table(ulong addr)
> isize = sizeof(struct smbios_entry) - SMBIOS_INTERMEDIATE_OFFSET;
> se->intermediate_checksum = table_compute_checksum(istart, isize);
> se->checksum = table_compute_checksum(se, sizeof(struct smbios_entry));
> + unmap_sysmem(se);
>
> return addr;
> }
More information about the U-Boot
mailing list