Re: [PATCH v4 4/7] smbios: Use SMBIOS 3.0 to support an address above 4GB

2023-12-31 Thread Simon Glass
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

2023-12-28 Thread Tom Rini
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

2023-12-28 Thread Mark Kettenis
> 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

2023-12-28 Thread Simon Glass
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

2023-12-27 Thread Ilias Apalodimas
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
>