[PATCH v2 3/6] smbios: Use SMBIOS 3.0 to support an address above 4GB

[Heinrich Schuchardt]

On 10/15/23 04:45, Simon Glass wrote:
> When the SMBIOS table is written to an address above 4GB a 32-bit table
> address is not large enough.
>
> Use an SMBIOS3 table in that case.
>
> Note that we cannot use efi_allocate_pages() since this function has
> nothing to do with EFI. There is no equivalent function to allocate
> memory below 4GB in U-Boot. One solution would be to create a separate
> malloc() pool, or just always put the malloc() pool below 4GB.
>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
>
> Changes in v2:
> - Check the end of the table rather than the start.
>
>   include/smbios.h | 22 +++++++++++++++++++++-
>   lib/smbios.c     | 24 +++++++++++++++++++-----
>   2 files changed, 40 insertions(+), 6 deletions(-)
>
> diff --git a/include/smbios.h b/include/smbios.h
> index c9df2706f5a6..ddabb558299e 100644
> --- a/include/smbios.h
> +++ b/include/smbios.h
> @@ -12,7 +12,8 @@
>
>   /* SMBIOS spec version implemented */
>   #define SMBIOS_MAJOR_VER	3
> -#define SMBIOS_MINOR_VER	0
> +#define SMBIOS_MINOR_VER	7
> +
>
>   enum {
>   	SMBIOS_STR_MAX	= 64,	/* Maximum length allowed for a string */
> @@ -54,6 +55,25 @@ struct __packed smbios_entry {
>   	u8 bcd_rev;
>   };
>
> +struct __packed smbios3_entry {
> +	u8 anchor[5];
> +	u8 checksum;
> +	u8 length;
> +	u8 major_ver;
> +

This empty line is superfluous.

> +	u8 minor_ver;
> +	u8 docrev;

'doc_rev' would be more consistent with the other fields.

> +	u8 entry_point_rev;
> +	u8 reserved;
> +	u32 max_struct_size;
> +

This empty line is superfluous.

Please, consider copying the comments from my patch

[PATCH v2,1/2] smbios: SMBIOS 3.0 (64-bit) Entry Point structure
https://patchwork.ozlabs.org/project/uboot/patch/20231223010334.248291-2-xypron.glpk@gmx.de/

Otherwise looks good to me.

Heinrich

> +	u64 struct_table_address;
> +};
> +
> +/* These two structures should use the same amount of 16-byte-aligned space */
> +static_assert(ALIGN(16, sizeof(struct smbios_entry)) ==
> +	      ALIGN(16, sizeof(struct smbios3_entry)));
> +
>   /* BIOS characteristics */
>   #define BIOS_CHARACTERISTICS_PCI_SUPPORTED	(1 << 7)
>   #define BIOS_CHARACTERISTICS_UPGRADEABLE	(1 << 11)
> diff --git a/lib/smbios.c b/lib/smbios.c
> index c7a557bc9b7b..92e98388084f 100644
> --- a/lib/smbios.c
> +++ b/lib/smbios.c
> @@ -487,7 +487,11 @@ ulong write_smbios_table(ulong addr)
>   	addr = ALIGN(addr, 16);
>   	start_addr = addr;
>
> -	addr += sizeof(struct smbios_entry);
> +	/*
> +	 * So far we don't know which struct will be used, but they both end
> +	 * up using the same amount of 16-bit-aligned space
> +	 */
> +	addr += max(sizeof(struct smbios_entry), sizeof(struct smbios3_entry));
>   	addr = ALIGN(addr, 16);
>   	tables = addr;
>
> @@ -512,14 +516,24 @@ ulong write_smbios_table(ulong addr)
>   	 * sandbox's DRAM buffer.
>   	 */
>   	table_addr = (ulong)map_sysmem(tables, 0);
> -	if (sizeof(table_addr) > sizeof(u32) && table_addr > (ulong)UINT_MAX) {
> +	if (sizeof(table_addr) > sizeof(u32) && addr >= (ulong)UINT_MAX) {
> +		struct smbios3_entry *se;
>   		/*
>   		 * 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);
> -		addr = 0;
> +		printf("WARNING: Using SMBIOS3.0 due to table-address overflow %lx\n",
> +		       table_addr);
> +		se = map_sysmem(start_addr, sizeof(struct smbios_entry));
> +		memset(se, '\0', sizeof(struct smbios_entry));
> +		memcpy(se->anchor, "_SM3_", 5);
> +		se->length = sizeof(struct smbios3_entry);
> +		se->major_ver = SMBIOS_MAJOR_VER;
> +		se->minor_ver = SMBIOS_MINOR_VER;
> +		se->docrev = 0;
> +		se->entry_point_rev = 1;
> +		se->max_struct_size = len;
> +		se->struct_table_address = table_addr;
>   	} else {
>   		struct smbios_entry *se;
>