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
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,
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
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
> > #
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;
