Re: [PATCH v4 4/7] smbios: Use SMBIOS 3.0 to support an address above 4GB
Hi Tom, On Thu, Dec 28, 2023 at 10:57 AM Tom Rini wrote: > > On Thu, Dec 28, 2023 at 03:46:09PM +0100, Mark Kettenis wrote: > > > From: Simon Glass > > > Date: Thu, 28 Dec 2023 13:37:03 + > > > > > > Hi Ilias, > > > > > > On Wed, Dec 27, 2023 at 11:05 AM Ilias Apalodimas > > > wrote: > > > > > > > > Hi Simon, > > > > > > > > On Wed, 27 Dec 2023 at 09:40, 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. > > > > > > > > Maybe I missed this on the previous revisions, but is there a reason > > > > we don't always use SMBIOS3 now? > > > > > > I am not sure...there was some comment about it not being supported in > > > some cases, so I have tried to accommodate that. > > > > > > > And perhaps try to install SMBIOS2 if > > > > 1. we fail > > > > > > due to? > > > > > > > 2. and the address is < 4GB > > > > > > We could, I suppose. Effectively we would drop generation of SMBIOS2. > > > > > > I really don't mind. This whole SMBIOS thing is a bit ridiculous, if > > > you ask me. > > > > Linux added support for the SMBIOS 3.0 64-bit entry point in 2014. I > > doubt anyone who cares about SMBIOS cares about kernels that old. > > > > So if it simplifies things, I'd drop support for the 32-bit SMBIOS > > entry point. > > I agree, lets just provide SMBIOS3 tables. OK...I would like to do this as a followup patch, so we still have the SMBIOS2 in the git history. I will take a look. Regards, Simon
Re: [PATCH v4 4/7] smbios: Use SMBIOS 3.0 to support an address above 4GB
On Thu, Dec 28, 2023 at 03:46:09PM +0100, Mark Kettenis wrote: > > From: Simon Glass > > Date: Thu, 28 Dec 2023 13:37:03 + > > > > Hi Ilias, > > > > On Wed, Dec 27, 2023 at 11:05 AM Ilias Apalodimas > > wrote: > > > > > > Hi Simon, > > > > > > On Wed, 27 Dec 2023 at 09:40, 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. > > > > > > Maybe I missed this on the previous revisions, but is there a reason > > > we don't always use SMBIOS3 now? > > > > I am not sure...there was some comment about it not being supported in > > some cases, so I have tried to accommodate that. > > > > > And perhaps try to install SMBIOS2 if > > > 1. we fail > > > > due to? > > > > > 2. and the address is < 4GB > > > > We could, I suppose. Effectively we would drop generation of SMBIOS2. > > > > I really don't mind. This whole SMBIOS thing is a bit ridiculous, if > > you ask me. > > Linux added support for the SMBIOS 3.0 64-bit entry point in 2014. I > doubt anyone who cares about SMBIOS cares about kernels that old. > > So if it simplifies things, I'd drop support for the 32-bit SMBIOS > entry point. I agree, lets just provide SMBIOS3 tables. -- Tom signature.asc Description: PGP signature
Re: [PATCH v4 4/7] smbios: Use SMBIOS 3.0 to support an address above 4GB
> From: Simon Glass > Date: Thu, 28 Dec 2023 13:37:03 + > > Hi Ilias, > > On Wed, Dec 27, 2023 at 11:05 AM Ilias Apalodimas > wrote: > > > > Hi Simon, > > > > On Wed, 27 Dec 2023 at 09:40, 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. > > > > Maybe I missed this on the previous revisions, but is there a reason > > we don't always use SMBIOS3 now? > > I am not sure...there was some comment about it not being supported in > some cases, so I have tried to accommodate that. > > > And perhaps try to install SMBIOS2 if > > 1. we fail > > due to? > > > 2. and the address is < 4GB > > We could, I suppose. Effectively we would drop generation of SMBIOS2. > > I really don't mind. This whole SMBIOS thing is a bit ridiculous, if > you ask me. Linux added support for the SMBIOS 3.0 64-bit entry point in 2014. I doubt anyone who cares about SMBIOS cares about kernels that old. So if it simplifies things, I'd drop support for the 32-bit SMBIOS entry point.
Re: [PATCH v4 4/7] smbios: Use SMBIOS 3.0 to support an address above 4GB
Hi Ilias, On Wed, Dec 27, 2023 at 11:05 AM Ilias Apalodimas wrote: > > Hi Simon, > > On Wed, 27 Dec 2023 at 09:40, 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. > > Maybe I missed this on the previous revisions, but is there a reason > we don't always use SMBIOS3 now? I am not sure...there was some comment about it not being supported in some cases, so I have tried to accommodate that. > And perhaps try to install SMBIOS2 if > 1. we fail due to? > 2. and the address is < 4GB We could, I suppose. Effectively we would drop generation of SMBIOS2. I really don't mind. This whole SMBIOS thing is a bit ridiculous, if you ask me. Regards, Simon
Re: [PATCH v4 4/7] smbios: Use SMBIOS 3.0 to support an address above 4GB
Hi Simon, On Wed, 27 Dec 2023 at 09:40, 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. Maybe I missed this on the previous revisions, but is there a reason we don't always use SMBIOS3 now? And perhaps try to install SMBIOS2 if 1. we fail 2. and the address is < 4GB Thanks /Ilias > > 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 > --- > > 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 >