Re: [PATCH v2 3/6] smbios: Use SMBIOS 3.0 to support an address above 4GB
Hi Heinrich, On Tue, Dec 26, 2023 at 11:01 AM Heinrich Schuchardt wrote: > > 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 > > --- > > > > 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_VER3 > > -#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.g...@gmx.de/ > > Otherwise looks good to me. Thanks, I added in your patch and resent the series. I tested it with dmicode and it seems to work OK. Regards, Simon
Re: [PATCH v2 3/6] smbios: Use SMBIOS 3.0 to support an address above 4GB
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 --- 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.g...@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;
Re: [PATCH v2 3/6] smbios: Use SMBIOS 3.0 to support an address above 4GB
Hi Tom, On Sat, 2 Dec 2023 at 12:03, Tom Rini wrote: > > On Sat, Dec 02, 2023 at 11:27:15AM -0700, Simon Glass wrote: > > Hi, > > > > On Tue, 21 Nov 2023 at 13:49, Tom Rini wrote: > > > > > > On Tue, Nov 21, 2023 at 10:40:39PM +0200, Ilias Apalodimas wrote: > > > > Hi Simon > > > > > > > > On Tue, 21 Nov 2023 at 04:17, Simon Glass wrote: > > > > > > > > > > Hi Heinrich, > > > > > > > > > > On Mon, 20 Nov 2023 at 19:11, Heinrich Schuchardt > > > > > wrote: > > > > > > > > > > > > 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 > > > > > > > --- > > > > > > > > > > > > > > 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_VER3 > > > > > > > -#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; > > > > > > > + > > > > > > > + u8 minor_ver; > > > > > > > + u8 docrev; > > > > > > > + u8 entry_point_rev; > > > > > > > + u8 reserved; > > > > > > > + u32 max_struct_size; > > > > > > > + > > > > > > > + 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) { > > > > > > > > > > > > You have to check the end of the the last SMBIOS structure not the > > > > > > start > > > > > > of the first SMBIOS structure against 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 lon
Re: [PATCH v2 3/6] smbios: Use SMBIOS 3.0 to support an address above 4GB
On Sat, Dec 02, 2023 at 11:27:15AM -0700, Simon Glass wrote: > Hi, > > On Tue, 21 Nov 2023 at 13:49, Tom Rini wrote: > > > > On Tue, Nov 21, 2023 at 10:40:39PM +0200, Ilias Apalodimas wrote: > > > Hi Simon > > > > > > On Tue, 21 Nov 2023 at 04:17, Simon Glass wrote: > > > > > > > > Hi Heinrich, > > > > > > > > On Mon, 20 Nov 2023 at 19:11, Heinrich Schuchardt > > > > wrote: > > > > > > > > > > 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 > > > > > > --- > > > > > > > > > > > > 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_VER3 > > > > > > -#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; > > > > > > + > > > > > > + u8 minor_ver; > > > > > > + u8 docrev; > > > > > > + u8 entry_point_rev; > > > > > > + u8 reserved; > > > > > > + u32 max_struct_size; > > > > > > + > > > > > > + 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) { > > > > > > > > > > You have to check the end of the the last SMBIOS structure not the > > > > > start > > > > > of the first SMBIOS structure against 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); > > > > > > > > > > This should be log_debug(). > > > > > > > > > > > + se = map_sysmem(start_addr, size
Re: [PATCH v2 3/6] smbios: Use SMBIOS 3.0 to support an address above 4GB
Hi, On Tue, 21 Nov 2023 at 13:49, Tom Rini wrote: > > On Tue, Nov 21, 2023 at 10:40:39PM +0200, Ilias Apalodimas wrote: > > Hi Simon > > > > On Tue, 21 Nov 2023 at 04:17, Simon Glass wrote: > > > > > > Hi Heinrich, > > > > > > On Mon, 20 Nov 2023 at 19:11, Heinrich Schuchardt > > > wrote: > > > > > > > > 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 > > > > > --- > > > > > > > > > > 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_VER3 > > > > > -#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; > > > > > + > > > > > + u8 minor_ver; > > > > > + u8 docrev; > > > > > + u8 entry_point_rev; > > > > > + u8 reserved; > > > > > + u32 max_struct_size; > > > > > + > > > > > + 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) { > > > > > > > > You have to check the end of the the last SMBIOS structure not the start > > > > of the first SMBIOS structure against 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); > > > > > > > > This should be log_debug(). > > > > > > > > > + 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; >
Re: [PATCH v2 3/6] smbios: Use SMBIOS 3.0 to support an address above 4GB
On Tue, Nov 21, 2023 at 10:40:39PM +0200, Ilias Apalodimas wrote: > Hi Simon > > On Tue, 21 Nov 2023 at 04:17, Simon Glass wrote: > > > > Hi Heinrich, > > > > On Mon, 20 Nov 2023 at 19:11, Heinrich Schuchardt > > wrote: > > > > > > 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 > > > > --- > > > > > > > > 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_VER3 > > > > -#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; > > > > + > > > > + u8 minor_ver; > > > > + u8 docrev; > > > > + u8 entry_point_rev; > > > > + u8 reserved; > > > > + u32 max_struct_size; > > > > + > > > > + 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) { > > > > > > You have to check the end of the the last SMBIOS structure not the start > > > of the first SMBIOS structure against 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); > > > > > > This should be log_debug(). > > > > > > > + 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; > > > > > > You must fill the checksum: > > > > > > se->checksum = table_compute_checksum(se, sizeof(struct smbios3_entry)); > > > > > > With the che
Re: [PATCH v2 3/6] smbios: Use SMBIOS 3.0 to support an address above 4GB
Hi Simon On Tue, 21 Nov 2023 at 04:17, Simon Glass wrote: > > Hi Heinrich, > > On Mon, 20 Nov 2023 at 19:11, Heinrich Schuchardt wrote: > > > > 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 > > > --- > > > > > > 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_VER3 > > > -#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; > > > + > > > + u8 minor_ver; > > > + u8 docrev; > > > + u8 entry_point_rev; > > > + u8 reserved; > > > + u32 max_struct_size; > > > + > > > + 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) { > > > > You have to check the end of the the last SMBIOS structure not the start > > of the first SMBIOS structure against 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); > > > > This should be log_debug(). > > > > > + 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; > > > > You must fill the checksum: > > > > se->checksum = table_compute_checksum(se, sizeof(struct smbios3_entry)); > > > > With the checksum filled I can use dmidecode in Linux based on the > > SMBIOS3 table: > > > > ubuntu@ubuntu:~$ sudo dmidecode > > # dmidecode 3.5 > > # SMBIOS3 entry point at 0x27ef53000 > > Found SMBIOS entry point in EFI, reading table from /dev/mem. > > SMBIOS 3.7.0 present. > > # SMBIOS implementations newer than version 3.5.0 are not > > # fully
Re: [PATCH v2 3/6] smbios: Use SMBIOS 3.0 to support an address above 4GB
Hi Heinrich, On Mon, 20 Nov 2023 at 19:11, Heinrich Schuchardt wrote: > > 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 > > --- > > > > 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_VER3 > > -#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; > > + > > + u8 minor_ver; > > + u8 docrev; > > + u8 entry_point_rev; > > + u8 reserved; > > + u32 max_struct_size; > > + > > + 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) { > > You have to check the end of the the last SMBIOS structure not the start > of the first SMBIOS structure against 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); > > This should be log_debug(). > > > + 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; > > You must fill the checksum: > > se->checksum = table_compute_checksum(se, sizeof(struct smbios3_entry)); > > With the checksum filled I can use dmidecode in Linux based on the > SMBIOS3 table: > > ubuntu@ubuntu:~$ sudo dmidecode > # dmidecode 3.5 > # SMBIOS3 entry point at 0x27ef53000 > Found SMBIOS entry point in EFI, reading table from /dev/mem. > SMBIOS 3.7.0 present. > # SMBIOS implementations newer than version 3.5.0 are not > # fully supported by this version of dmidecode. > Table at 0x27EF53020. > > Handle 0x, DMI type 0, 24 bytes > BIOS Information > Vendor: U-Boot OK, great, thank you for figuring this out. Do you think this might be a reasonable solution for -next ? Regards, Simon
Re: [PATCH v2 3/6] smbios: Use SMBIOS 3.0 to support an address above 4GB
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 --- 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; + + u8 minor_ver; + u8 docrev; + u8 entry_point_rev; + u8 reserved; + u32 max_struct_size; + + 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) { You have to check the end of the the last SMBIOS structure not the start of the first SMBIOS structure against 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); This should be log_debug(). + 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; You must fill the checksum: se->checksum = table_compute_checksum(se, sizeof(struct smbios3_entry)); With the checksum filled I can use dmidecode in Linux based on the SMBIOS3 table: ubuntu@ubuntu:~$ sudo dmidecode # dmidecode 3.5 # SMBIOS3 entry point at 0x27ef53000 Found SMBIOS entry point in EFI, reading table from /dev/mem. SMBIOS 3.7.0 present. # SMBIOS implementations newer than version 3.5.0 are not # fully supported by this version of dmidecode. Table at 0x27EF53020. Handle 0x, DMI type 0, 24 bytes BIOS Information Vendor: U-Boot ... Best regards Heinrich } else { struct smbios_entry *se;
[PATCH v2 3/6] smbios: Use SMBIOS 3.0 to support an address above 4GB
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 --- 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; + + u8 minor_ver; + u8 docrev; + u8 entry_point_rev; + u8 reserved; + u32 max_struct_size; + + 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; -- 2.42.0.655.g421f12c284-goog