Re: [PATCH v5 04/12] smbios: Use SMBIOS 3.0 to support an address above 4GB
Hi Heinrich, On Mon, Jan 1, 2024 at 10:34 AM Heinrich Schuchardt wrote: > > On 12/31/23 16:25, 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. > > > > - Use log_debug() for warning > > - Rebase on Heinrich's smbios.h patch > > - Set the checksum for SMBIOS3 > > > > Signed-off-by: Simon Glass > > --- > > > > (no changes since v4) > > > > Changes in v4: > > - Check the start of the table rather than the end > > > > Changes in v2: > > - Check the end of the table rather than the start. > > > > include/smbios.h | 6 +- > > lib/smbios.c | 30 +- > > 2 files changed, 30 insertions(+), 6 deletions(-) > > Applied to u-boot-dm/next, thanks!
Re: [PATCH v5 04/12] smbios: Use SMBIOS 3.0 to support an address above 4GB
Hi Heinrich,
On Mon, Jan 1, 2024 at 10:34 AM Heinrich Schuchardt wrote:
>
> On 12/31/23 16:25, 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.
> >
> > - Use log_debug() for warning
> > - Rebase on Heinrich's smbios.h patch
> > - Set the checksum for SMBIOS3
> >
> > Signed-off-by: Simon Glass
> > ---
> >
> > (no changes since v4)
> >
> > Changes in v4:
> > - Check the start of the table rather than the end
> >
> > Changes in v2:
> > - Check the end of the table rather than the start.
> >
> > include/smbios.h | 6 +-
> > lib/smbios.c | 30 +-
> > 2 files changed, 30 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/smbios.h b/include/smbios.h
> > index e601283d293..77be58887a2 100644
> > --- a/include/smbios.h
> > +++ b/include/smbios.h
> > @@ -12,7 +12,7 @@
> >
> > /* 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 */
> > @@ -80,6 +80,10 @@ struct __packed smbios3_entry {
> > u64 struct_table_address;
> > };
> >
> > +/* These two structures should use the same amount of 16-byte-aligned
> > space */
>
> I cannot see from where you take such a requirement.
> By chance it is fulfilled by the current definitions.
> If this a leftover from debugging we should remove it.
Oh, I just assumed it was a requirement, perhaps. Yes we can remove
this, if you like. In fact perhaps we should remove all SMBIOS2 stuff
as a follow-up?
Regards,
Simon
Re: [PATCH v5 04/12] smbios: Use SMBIOS 3.0 to support an address above 4GB
On 12/31/23 16:25, 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.
- Use log_debug() for warning
- Rebase on Heinrich's smbios.h patch
- Set the checksum for SMBIOS3
Signed-off-by: Simon Glass
---
(no changes since v4)
Changes in v4:
- Check the start of the table rather than the end
Changes in v2:
- Check the end of the table rather than the start.
include/smbios.h | 6 +-
lib/smbios.c | 30 +-
2 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/include/smbios.h b/include/smbios.h
index e601283d293..77be58887a2 100644
--- a/include/smbios.h
+++ b/include/smbios.h
@@ -12,7 +12,7 @@
/* 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 */
@@ -80,6 +80,10 @@ struct __packed smbios3_entry {
u64 struct_table_address;
};
+/* These two structures should use the same amount of 16-byte-aligned space */
I cannot see from where you take such a requirement.
By chance it is fulfilled by the current definitions.
If this a leftover from debugging we should remove it.
Best regards
Heinrich
+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 eea72670bd9..7f79d969c92 100644
--- a/lib/smbios.c
+++ b/lib/smbios.c
@@ -567,7 +567,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;
@@ -590,16 +594,32 @@ ulong write_smbios_table(ulong addr)
* 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.
+*
+* Check the address of the end of the tables. If it is above 4GB then
+* it is sensible to use SMBIOS3 even if the start of the table is below
+* 4GB (this case is very unlikely to happen in practice)
*/
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;
+ log_debug("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->doc_rev = 0;
+ se->entry_point_rev = 1;
+ se->max_struct_size = len;
+ se->struct_table_address = table_addr;
+ se->checksum = table_compute_checksum(se,
+ sizeof(struct smbios3_entry));
} else {
struct smbios_entry *se;
Re: [PATCH v5 04/12] smbios: Use SMBIOS 3.0 to support an address above 4GB
On 12/31/23 17:11, Heinrich Schuchardt wrote:
On 12/31/23 16:25, 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.
- Use log_debug() for warning
- Rebase on Heinrich's smbios.h patch
- Set the checksum for SMBIOS3
Signed-off-by: Simon Glass
---
(no changes since v4)
Changes in v4:
- Check the start of the table rather than the end
Changes in v2:
- Check the end of the table rather than the start.
include/smbios.h | 6 +-
lib/smbios.c | 30 +-
2 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/include/smbios.h b/include/smbios.h
index e601283d293..77be58887a2 100644
--- a/include/smbios.h
+++ b/include/smbios.h
@@ -12,7 +12,7 @@
/* 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 */
@@ -80,6 +80,10 @@ struct __packed smbios3_entry {
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 eea72670bd9..7f79d969c92 100644
--- a/lib/smbios.c
+++ b/lib/smbios.c
@@ -567,7 +567,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;
@@ -590,16 +594,32 @@ ulong write_smbios_table(ulong addr)
* 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.
+ *
+ * Check the address of the end of the tables. If it is above 4GB
then
+ * it is sensible to use SMBIOS3 even if the start of the table
is below
+ * 4GB (this case is very unlikely to happen in practice)
*/
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) {
All SMBIOS specifications since version 3.0.0, published 2015, allow
using a SMBIOS3 anchor in all cases. SMBIOS_MAJOR_VER == 3.
Our target is to keep the code size small. Why do you increase the code
size here?
You drop SMBIOS2 in patch 8.
Reviewed-by: Heinrich Schuchardt
Best regards
Heinrich
+ 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;
+ log_debug("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->doc_rev = 0;
+ se->entry_point_rev = 1;
+ se->max_struct_size = len;
+ se->struct_table_address = table_addr;
+ se->checksum = table_compute_checksum(se,
+ sizeof(struct smbios3_entry));
} else {
struct smbios_entry *se;
Re: [PATCH v5 04/12] smbios: Use SMBIOS 3.0 to support an address above 4GB
On 12/31/23 16:25, 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.
- Use log_debug() for warning
- Rebase on Heinrich's smbios.h patch
- Set the checksum for SMBIOS3
Signed-off-by: Simon Glass
---
(no changes since v4)
Changes in v4:
- Check the start of the table rather than the end
Changes in v2:
- Check the end of the table rather than the start.
include/smbios.h | 6 +-
lib/smbios.c | 30 +-
2 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/include/smbios.h b/include/smbios.h
index e601283d293..77be58887a2 100644
--- a/include/smbios.h
+++ b/include/smbios.h
@@ -12,7 +12,7 @@
/* 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 */
@@ -80,6 +80,10 @@ struct __packed smbios3_entry {
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 eea72670bd9..7f79d969c92 100644
--- a/lib/smbios.c
+++ b/lib/smbios.c
@@ -567,7 +567,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;
@@ -590,16 +594,32 @@ ulong write_smbios_table(ulong addr)
* 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.
+*
+* Check the address of the end of the tables. If it is above 4GB then
+* it is sensible to use SMBIOS3 even if the start of the table is below
+* 4GB (this case is very unlikely to happen in practice)
*/
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) {
All SMBIOS specifications since version 3.0.0, published 2015, allow
using a SMBIOS3 anchor in all cases. SMBIOS_MAJOR_VER == 3.
Our target is to keep the code size small. Why do you increase the code
size here?
Best regards
Heinrich
+ 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;
+ log_debug("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->doc_rev = 0;
+ se->entry_point_rev = 1;
+ se->max_struct_size = len;
+ se->struct_table_address = table_addr;
+ se->checksum = table_compute_checksum(se,
+ sizeof(struct smbios3_entry));
} else {
struct smbios_entry *se;
[PATCH v5 04/12] 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.
- Use log_debug() for warning
- Rebase on Heinrich's smbios.h patch
- Set the checksum for SMBIOS3
Signed-off-by: Simon Glass
---
(no changes since v4)
Changes in v4:
- Check the start of the table rather than the end
Changes in v2:
- Check the end of the table rather than the start.
include/smbios.h | 6 +-
lib/smbios.c | 30 +-
2 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/include/smbios.h b/include/smbios.h
index e601283d293..77be58887a2 100644
--- a/include/smbios.h
+++ b/include/smbios.h
@@ -12,7 +12,7 @@
/* 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 */
@@ -80,6 +80,10 @@ struct __packed smbios3_entry {
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 eea72670bd9..7f79d969c92 100644
--- a/lib/smbios.c
+++ b/lib/smbios.c
@@ -567,7 +567,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;
@@ -590,16 +594,32 @@ ulong write_smbios_table(ulong addr)
* 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.
+*
+* Check the address of the end of the tables. If it is above 4GB then
+* it is sensible to use SMBIOS3 even if the start of the table is below
+* 4GB (this case is very unlikely to happen in practice)
*/
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;
+ log_debug("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->doc_rev = 0;
+ se->entry_point_rev = 1;
+ se->max_struct_size = len;
+ se->struct_table_address = table_addr;
+ se->checksum = table_compute_checksum(se,
+ sizeof(struct smbios3_entry));
} else {
struct smbios_entry *se;
--
2.34.1
