[PATCH] smbios: arm64: Allow table to be written at a fixed addr

[PATCH] smbios: arm64: Allow table to be written at a fixed addr

[Simon Glass]

U-Boot typically sets up its malloc() pool near the top of memory. On
ARM64 systems this can result in an SMBIOS table above 4GB which is
not supported by SMBIOSv2.

Work around this problem by providing a new option to choose an address
just below 4GB, if needed.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 lib/Kconfig                 | 17 +++++++++++++++++
 lib/efi_loader/efi_smbios.c | 12 ++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/lib/Kconfig b/lib/Kconfig
index f6ca559897e7..a77f2f3e9089 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -994,6 +994,23 @@ config GENERATE_SMBIOS_TABLE
 	  See also SMBIOS_SYSINFO which allows SMBIOS values to be provided in
 	  the devicetree.
 
+config SMBIOS_TABLE_FIXED
+	bool "Place the SMBIOS table at a special address"
+	depends on GENERATE_SMBIOS_TABLE && ARM64 && SMBIOS && EFI_LOADER
+	default y
+	help
+	  Use this option to place the SMBIOS table at a fixed address.
+
+	  U-Boot typically sets up its malloc() pool near the top of memory. On
+	  ARM64 systems this can result in an SMBIOS table above 4GB which is
+	  not supported by SMBIOSv2. This option works around this problem by
+	  chosing an address just below 4GB, if needed.
+
+config SMBIOS_TABLE_FIXED_ADDR
+	hex "Fixed address for use for SMBIOS table"
+	depends on SMBIOS_TABLE_FIXED
+	default 0xffff0000
+
 endmenu
 
 config LIB_RATIONAL
diff --git a/lib/efi_loader/efi_smbios.c b/lib/efi_loader/efi_smbios.c
index 48446f654d9b..84ea027ea48c 100644
--- a/lib/efi_loader/efi_smbios.c
+++ b/lib/efi_loader/efi_smbios.c
@@ -61,6 +61,18 @@ static int install_smbios_table(void)
 		return log_msg_ret("mem", -ENOMEM);
 
 	addr = map_to_sysmem(buf);
+
+	/*
+	 * Deal with a fixed address if needed. For simplicity we assume that
+	 * the SMBIOS-table size is <64KB and that the malloc region does not
+	 * straddle the 4GB boundary.
+	 */
+	if (IS_ENABLED(CONFIG_SMBIOS_TABLE_FIXED) && addr >= SZ_4G - SZ_64K) {
+		addr = IF_ENABLED_INT(CONFIG_SMBIOS_TABLE_FIXED,
+				      CONFIG_SMBIOS_TABLE_FIXED_ADDR);
+		log_warning("Forcing SMBIOS table to address %lx\n", addr);
+	}
+
 	if (!write_smbios_table(addr)) {
 		log_err("Failed to write SMBIOS table\n");
 		return log_msg_ret("smbios", -EINVAL);
-- 
2.42.0.655.g421f12c284-goog

Re: [PATCH] smbios: arm64: Allow table to be written at a fixed addr

[Caleb Connolly]

Hi Simon,

On 21/10/2023 01:45, Simon Glass wrote:
> U-Boot typically sets up its malloc() pool near the top of memory. On
> ARM64 systems this can result in an SMBIOS table above 4GB which is
> not supported by SMBIOSv2.
> 
> Work around this problem by providing a new option to choose an address
> just below 4GB, if needed.
I may well be missing something here, but I don't quite understand the 
justification behind the original patch [1] which caused this regression.

I'm currently preparing support for a few different Qualcomm boards 
which have different memory layouts, so far it's been possible to avoid 
having any fixed addresses, so the same u-boot image can be used on all 
of them (with just a different DTB).

As I mentioned before, efi_allocate_pages() seems to work fine from 
install_smbios_table(), I haven't delved into the code too much but I 
wonder if this could be an acceptable solution?

Barring that, could we use an environment variable to pass this address 
in? Although I think that might not be a very desirable solution.

I appreciate you taking the time to prepare this patch and CC me. If 
nobody else objects to this patch then I'd prefer to avoid blocking it. 
In either case, I'd greatly appreciate any input on the above 
suggestions so that I can implement a solution for my boards.

Thanks and regards,

[1]: 
https://lore.kernel.org/u-boot/20230920030027.1385833-16-sjg@chromium.org/
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>   lib/Kconfig                 | 17 +++++++++++++++++
>   lib/efi_loader/efi_smbios.c | 12 ++++++++++++
>   2 files changed, 29 insertions(+)
> 
> diff --git a/lib/Kconfig b/lib/Kconfig
> index f6ca559897e7..a77f2f3e9089 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -994,6 +994,23 @@ config GENERATE_SMBIOS_TABLE
>   	  See also SMBIOS_SYSINFO which allows SMBIOS values to be provided in
>   	  the devicetree.
>   
> +config SMBIOS_TABLE_FIXED
> +	bool "Place the SMBIOS table at a special address"
> +	depends on GENERATE_SMBIOS_TABLE && ARM64 && SMBIOS && EFI_LOADER
> +	default y
> +	help
> +	  Use this option to place the SMBIOS table at a fixed address.
> +
> +	  U-Boot typically sets up its malloc() pool near the top of memory. On
> +	  ARM64 systems this can result in an SMBIOS table above 4GB which is
> +	  not supported by SMBIOSv2. This option works around this problem by
> +	  chosing an address just below 4GB, if needed.
> +
> +config SMBIOS_TABLE_FIXED_ADDR
> +	hex "Fixed address for use for SMBIOS table"
> +	depends on SMBIOS_TABLE_FIXED
> +	default 0xffff0000
> +
>   endmenu
>   
>   config LIB_RATIONAL
> diff --git a/lib/efi_loader/efi_smbios.c b/lib/efi_loader/efi_smbios.c
> index 48446f654d9b..84ea027ea48c 100644
> --- a/lib/efi_loader/efi_smbios.c
> +++ b/lib/efi_loader/efi_smbios.c
> @@ -61,6 +61,18 @@ static int install_smbios_table(void)
>   		return log_msg_ret("mem", -ENOMEM);
>   
>   	addr = map_to_sysmem(buf);
> +
> +	/*
> +	 * Deal with a fixed address if needed. For simplicity we assume that
> +	 * the SMBIOS-table size is <64KB and that the malloc region does not
> +	 * straddle the 4GB boundary.
> +	 */
> +	if (IS_ENABLED(CONFIG_SMBIOS_TABLE_FIXED) && addr >= SZ_4G - SZ_64K) {
> +		addr = IF_ENABLED_INT(CONFIG_SMBIOS_TABLE_FIXED,
> +				      CONFIG_SMBIOS_TABLE_FIXED_ADDR);
> +		log_warning("Forcing SMBIOS table to address %lx\n", addr);
> +	}
> +
>   	if (!write_smbios_table(addr)) {
>   		log_err("Failed to write SMBIOS table\n");
>   		return log_msg_ret("smbios", -EINVAL);

-- 
// Caleb (they/them)

Re: [PATCH] smbios: arm64: Allow table to be written at a fixed addr

[Simon Glass]

Hi Caleb,

On Sat, 21 Oct 2023 at 01:43, Caleb Connolly <caleb.connolly@linaro.org> wrote:
>
> Hi Simon,
>
> On 21/10/2023 01:45, Simon Glass wrote:
> > U-Boot typically sets up its malloc() pool near the top of memory. On
> > ARM64 systems this can result in an SMBIOS table above 4GB which is
> > not supported by SMBIOSv2.
> >
> > Work around this problem by providing a new option to choose an address
> > just below 4GB, if needed.
> I may well be missing something here, but I don't quite understand the
> justification behind the original patch [1] which caused this regression.
>
> I'm currently preparing support for a few different Qualcomm boards
> which have different memory layouts, so far it's been possible to avoid
> having any fixed addresses, so the same u-boot image can be used on all
> of them (with just a different DTB).

What sort of memory layout do you have? I could enhance the patch to
find the top of a memory bank below 4GB, perhaps? E.g. see the
'bdinfo' command.

>
> As I mentioned before, efi_allocate_pages() seems to work fine from
> install_smbios_table(), I haven't delved into the code too much but I
> wonder if this could be an acceptable solution?

Unfortunately that function is EFI-specific. The function can only be
called once EFI is inited. We only want to do that if booting with
EFI.

The tables are written at the start of U-Boot, partly because that is
how it is done on x86 and partly so the 'acpi' command actually works.

The EFI code ended up writing a separate set of tables, which is of
course not correct.

I did look at creating an API in U-Boot to alloc memory below 4GB but
then decided that for just this one use case it might not be worth it.

>
> Barring that, could we use an environment variable to pass this address
> in? Although I think that might not be a very desirable solution.
>
> I appreciate you taking the time to prepare this patch and CC me. If
> nobody else objects to this patch then I'd prefer to avoid blocking it.
> In either case, I'd greatly appreciate any input on the above
> suggestions so that I can implement a solution for my boards.

I would like it to be automatic. I assumed on ARM64 that there is
memory available at 3.99GB if U-Boot has relocated to above 4GB, but
as above I could make this more clever.

Regards,
Simon


>
> Thanks and regards,
>
> [1]:
> https://lore.kernel.org/u-boot/20230920030027.1385833-16-sjg@chromium.org/
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >   lib/Kconfig                 | 17 +++++++++++++++++
> >   lib/efi_loader/efi_smbios.c | 12 ++++++++++++
> >   2 files changed, 29 insertions(+)
> >
> > diff --git a/lib/Kconfig b/lib/Kconfig
> > index f6ca559897e7..a77f2f3e9089 100644
> > --- a/lib/Kconfig
> > +++ b/lib/Kconfig
> > @@ -994,6 +994,23 @@ config GENERATE_SMBIOS_TABLE
> >         See also SMBIOS_SYSINFO which allows SMBIOS values to be provided in
> >         the devicetree.
> >
> > +config SMBIOS_TABLE_FIXED
> > +     bool "Place the SMBIOS table at a special address"
> > +     depends on GENERATE_SMBIOS_TABLE && ARM64 && SMBIOS && EFI_LOADER
> > +     default y
> > +     help
> > +       Use this option to place the SMBIOS table at a fixed address.
> > +
> > +       U-Boot typically sets up its malloc() pool near the top of memory. On
> > +       ARM64 systems this can result in an SMBIOS table above 4GB which is
> > +       not supported by SMBIOSv2. This option works around this problem by
> > +       chosing an address just below 4GB, if needed.
> > +
> > +config SMBIOS_TABLE_FIXED_ADDR
> > +     hex "Fixed address for use for SMBIOS table"
> > +     depends on SMBIOS_TABLE_FIXED
> > +     default 0xffff0000
> > +
> >   endmenu
> >
> >   config LIB_RATIONAL
> > diff --git a/lib/efi_loader/efi_smbios.c b/lib/efi_loader/efi_smbios.c
> > index 48446f654d9b..84ea027ea48c 100644
> > --- a/lib/efi_loader/efi_smbios.c
> > +++ b/lib/efi_loader/efi_smbios.c
> > @@ -61,6 +61,18 @@ static int install_smbios_table(void)
> >               return log_msg_ret("mem", -ENOMEM);
> >
> >       addr = map_to_sysmem(buf);
> > +
> > +     /*
> > +      * Deal with a fixed address if needed. For simplicity we assume that
> > +      * the SMBIOS-table size is <64KB and that the malloc region does not
> > +      * straddle the 4GB boundary.
> > +      */
> > +     if (IS_ENABLED(CONFIG_SMBIOS_TABLE_FIXED) && addr >= SZ_4G - SZ_64K) {
> > +             addr = IF_ENABLED_INT(CONFIG_SMBIOS_TABLE_FIXED,
> > +                                   CONFIG_SMBIOS_TABLE_FIXED_ADDR);
> > +             log_warning("Forcing SMBIOS table to address %lx\n", addr);
> > +     }
> > +
> >       if (!write_smbios_table(addr)) {
> >               log_err("Failed to write SMBIOS table\n");
> >               return log_msg_ret("smbios", -EINVAL);
>
> --
> // Caleb (they/them)

Re: [PATCH] smbios: arm64: Allow table to be written at a fixed addr

[Caleb Connolly]

On 23/10/2023 08:04, Simon Glass wrote:
> Hi Caleb,
> 
> On Sat, 21 Oct 2023 at 01:43, Caleb Connolly <caleb.connolly@linaro.org> wrote:
>>
>> Hi Simon,
>>
>> On 21/10/2023 01:45, Simon Glass wrote:
>>> U-Boot typically sets up its malloc() pool near the top of memory. On
>>> ARM64 systems this can result in an SMBIOS table above 4GB which is
>>> not supported by SMBIOSv2.
>>>
>>> Work around this problem by providing a new option to choose an address
>>> just below 4GB, if needed.
>> I may well be missing something here, but I don't quite understand the
>> justification behind the original patch [1] which caused this regression.
>>
>> I'm currently preparing support for a few different Qualcomm boards
>> which have different memory layouts, so far it's been possible to avoid
>> having any fixed addresses, so the same u-boot image can be used on all
>> of them (with just a different DTB).
> 
> What sort of memory layout do you have? I could enhance the patch to
> find the top of a memory bank below 4GB, perhaps? E.g. see the
> 'bdinfo' command.

I think that would probably be sensible (afaict this is more of less
what the EFI function did).

One of my boards has RAM starting at 0x40000000 and ending at
0xC0000000, frustratingly not quite managing to use the entire 4G
address space. There is also a hole in RAM, so u-boot relocates itself
to the end of the first memory bank

=> bdinfo
boot_params = 0x0000000000000000
DRAM bank   = 0x0000000000000000
-> start    = 0x0000000040000000
-> size     = 0x000000003eb00000
DRAM bank   = 0x0000000000000001
-> start    = 0x0000000080000000
-> size     = 0x0000000040000000
flashstart  = 0x0000000000000000
flashsize   = 0x0000000000000000
flashoffset = 0x0000000000000000
baudrate    = 115200 bps
relocaddr   = 0x000000007ea12000
reloc off   = 0x000000007ea12000
Build       = 64-bit
fdt_blob    = 0x000000007e5ec5c0
new_fdt     = 0x000000007e5ec5c0
fdt_size    = 0x0000000000006780
lmb_dump_all:
 memory.cnt = 0x2 / max = 0x40
 memory[0]	[0x40000000-0x7eafffff], 0x3eb00000 bytes flags: 0
 memory[1]	[0x80000000-0xbfffffff], 0x40000000 bytes flags: 0
 reserved.cnt = 0x8 / max = 0x40
 reserved[0]	[0x45700000-0x45cfffff], 0x00600000 bytes flags: 4
 reserved[1]	[0x45e00000-0x45f3ffff], 0x00140000 bytes flags: 4
 reserved[2]	[0x45fff000-0x461fffff], 0x00201000 bytes flags: 4
 reserved[3]	[0x4ab00000-0x53616fff], 0x08b17000 bytes flags: 4
 reserved[4]	[0x5c000000-0x5cffffff], 0x01000000 bytes flags: 4
 reserved[5]	[0x60000000-0x638fffff], 0x03900000 bytes flags: 4
 reserved[6]	[0x7d5e8000-0x7eafffff], 0x01518000 bytes flags: 0
 reserved[7]	[0x89b01000-0x89d00fff], 0x00200000 bytes flags: 4
> 
>>
>> As I mentioned before, efi_allocate_pages() seems to work fine from
>> install_smbios_table(), I haven't delved into the code too much but I
>> wonder if this could be an acceptable solution?
> 
> Unfortunately that function is EFI-specific. The function can only be
> called once EFI is inited. We only want to do that if booting with
> EFI.
> 
> The tables are written at the start of U-Boot, partly because that is
> how it is done on x86 and partly so the 'acpi' command actually works.
> 
> The EFI code ended up writing a separate set of tables, which is of
> course not correct.
> 
> I did look at creating an API in U-Boot to alloc memory below 4GB but
> then decided that for just this one use case it might not be worth it.

The LMB allocator can already do this with lmb_alloc_base(), would this
be an ok solution?

I have tested it and it seems to work as intended, allocating the first
free 32-bit address, and avoiding reserved regions.

It seems like the LMB allocator doesn't have a global pool, so I'm not
sure how it avoids conflicts, although maybe that just isn't really an
issue in reality.
> 
>>
>> Barring that, could we use an environment variable to pass this address
>> in? Although I think that might not be a very desirable solution.
>>
>> I appreciate you taking the time to prepare this patch and CC me. If
>> nobody else objects to this patch then I'd prefer to avoid blocking it.
>> In either case, I'd greatly appreciate any input on the above
>> suggestions so that I can implement a solution for my boards.
> 
> I would like it to be automatic. I assumed on ARM64 that there is
> memory available at 3.99GB if U-Boot has relocated to above 4GB, but
> as above I could make this more clever.
> 
> Regards,
> Simon
> 
> 
>>
>> Thanks and regards,
>>
>> [1]:
>> https://lore.kernel.org/u-boot/20230920030027.1385833-16-sjg@chromium.org/
>>>
>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>> ---
>>>
>>>   lib/Kconfig                 | 17 +++++++++++++++++
>>>   lib/efi_loader/efi_smbios.c | 12 ++++++++++++
>>>   2 files changed, 29 insertions(+)
>>>
>>> diff --git a/lib/Kconfig b/lib/Kconfig
>>> index f6ca559897e7..a77f2f3e9089 100644
>>> --- a/lib/Kconfig
>>> +++ b/lib/Kconfig
>>> @@ -994,6 +994,23 @@ config GENERATE_SMBIOS_TABLE
>>>         See also SMBIOS_SYSINFO which allows SMBIOS values to be provided in
>>>         the devicetree.
>>>
>>> +config SMBIOS_TABLE_FIXED
>>> +     bool "Place the SMBIOS table at a special address"
>>> +     depends on GENERATE_SMBIOS_TABLE && ARM64 && SMBIOS && EFI_LOADER
>>> +     default y
>>> +     help
>>> +       Use this option to place the SMBIOS table at a fixed address.
>>> +
>>> +       U-Boot typically sets up its malloc() pool near the top of memory. On
>>> +       ARM64 systems this can result in an SMBIOS table above 4GB which is
>>> +       not supported by SMBIOSv2. This option works around this problem by
>>> +       chosing an address just below 4GB, if needed.
>>> +
>>> +config SMBIOS_TABLE_FIXED_ADDR
>>> +     hex "Fixed address for use for SMBIOS table"
>>> +     depends on SMBIOS_TABLE_FIXED
>>> +     default 0xffff0000
>>> +
>>>   endmenu
>>>
>>>   config LIB_RATIONAL
>>> diff --git a/lib/efi_loader/efi_smbios.c b/lib/efi_loader/efi_smbios.c
>>> index 48446f654d9b..84ea027ea48c 100644
>>> --- a/lib/efi_loader/efi_smbios.c
>>> +++ b/lib/efi_loader/efi_smbios.c
>>> @@ -61,6 +61,18 @@ static int install_smbios_table(void)
>>>               return log_msg_ret("mem", -ENOMEM);
>>>
>>>       addr = map_to_sysmem(buf);
>>> +
>>> +     /*
>>> +      * Deal with a fixed address if needed. For simplicity we assume that
>>> +      * the SMBIOS-table size is <64KB and that the malloc region does not
>>> +      * straddle the 4GB boundary.
>>> +      */
>>> +     if (IS_ENABLED(CONFIG_SMBIOS_TABLE_FIXED) && addr >= SZ_4G - SZ_64K) {
>>> +             addr = IF_ENABLED_INT(CONFIG_SMBIOS_TABLE_FIXED,
>>> +                                   CONFIG_SMBIOS_TABLE_FIXED_ADDR);
>>> +             log_warning("Forcing SMBIOS table to address %lx\n", addr);
>>> +     }
>>> +
>>>       if (!write_smbios_table(addr)) {
>>>               log_err("Failed to write SMBIOS table\n");
>>>               return log_msg_ret("smbios", -EINVAL);
>>
>> --
>> // Caleb (they/them)

-- 
// Caleb (they/them)

Re: [PATCH] smbios: arm64: Allow table to be written at a fixed addr

[Mark Kettenis]

> From: Simon Glass <sjg@chromium.org>
> Date: Mon, 23 Oct 2023 00:04:14 -0700
> 
> Hi Caleb,
> 
> On Sat, 21 Oct 2023 at 01:43, Caleb Connolly <caleb.connolly@linaro.org> wrote:
> >
> > Hi Simon,
> >
> > On 21/10/2023 01:45, Simon Glass wrote:
> > > U-Boot typically sets up its malloc() pool near the top of memory. On
> > > ARM64 systems this can result in an SMBIOS table above 4GB which is
> > > not supported by SMBIOSv2.
> > >
> > > Work around this problem by providing a new option to choose an address
> > > just below 4GB, if needed.
> > I may well be missing something here, but I don't quite understand the
> > justification behind the original patch [1] which caused this regression.
> >
> > I'm currently preparing support for a few different Qualcomm boards
> > which have different memory layouts, so far it's been possible to avoid
> > having any fixed addresses, so the same u-boot image can be used on all
> > of them (with just a different DTB).
> 
> What sort of memory layout do you have? I could enhance the patch to
> find the top of a memory bank below 4GB, perhaps? E.g. see the
> 'bdinfo' command.
> 
> >
> > As I mentioned before, efi_allocate_pages() seems to work fine from
> > install_smbios_table(), I haven't delved into the code too much but I
> > wonder if this could be an acceptable solution?
> 
> Unfortunately that function is EFI-specific. The function can only be
> called once EFI is inited. We only want to do that if booting with
> EFI.
> 
> The tables are written at the start of U-Boot, partly because that is
> how it is done on x86 and partly so the 'acpi' command actually works.
> 
> The EFI code ended up writing a separate set of tables, which is of
> course not correct.
> 
> I did look at creating an API in U-Boot to alloc memory below 4GB but
> then decided that for just this one use case it might not be worth it.
> 
> >
> > Barring that, could we use an environment variable to pass this address
> > in? Although I think that might not be a very desirable solution.
> >
> > I appreciate you taking the time to prepare this patch and CC me. If
> > nobody else objects to this patch then I'd prefer to avoid blocking it.
> > In either case, I'd greatly appreciate any input on the above
> > suggestions so that I can implement a solution for my boards.
> 
> I would like it to be automatic. I assumed on ARM64 that there is
> memory available at 3.99GB if U-Boot has relocated to above 4GB, but
> as above I could make this more clever.

There is absolutely no guarantee that arm64 machines have memory below
4GB.  Examples of SoCs that have no memory below 4GB are AMD's Opteron
A1100 SoC and all the recent Apple SoCs.

Cheers,

Mark

Re: [PATCH] smbios: arm64: Allow table to be written at a fixed addr

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1173 bytes --]

On Mon, Oct 23, 2023 at 05:31:19PM +0200, Mark Kettenis wrote:
> > From: Simon Glass <sjg@chromium.org>
> > Date: Mon, 23 Oct 2023 00:04:14 -0700
> > 
> > Hi Caleb,
> > 
> > On Sat, 21 Oct 2023 at 01:43, Caleb Connolly <caleb.connolly@linaro.org> wrote:
> > >
> > > Hi Simon,
> > >
> > > On 21/10/2023 01:45, Simon Glass wrote:
> > > > U-Boot typically sets up its malloc() pool near the top of memory. On
> > > > ARM64 systems this can result in an SMBIOS table above 4GB which is
> > > > not supported by SMBIOSv2.
[snip]
> There is absolutely no guarantee that arm64 machines have memory below
> 4GB.  Examples of SoCs that have no memory below 4GB are AMD's Opteron
> A1100 SoC and all the recent Apple SoCs.

So one thing to resolve here is where does that requirement about the
SMBIOS table needing to be below 4GB come from (standards wise), and in
turn is that obeyed by consumers like say Linux or OpenBSD? Answering my
own question, maybe in part, https://www.dmtf.org/standards/smbios reads
to me like there's a v3 and maybe we should be doing what we need to
support / identify as that, if it doesn't have that restriction?

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH] smbios: arm64: Allow table to be written at a fixed addr

[Simon Glass]

Hi Tom,

On Tue, 24 Oct 2023 at 15:34, Tom Rini <trini@konsulko.com> wrote:
>
> On Mon, Oct 23, 2023 at 05:31:19PM +0200, Mark Kettenis wrote:
> > > From: Simon Glass <sjg@chromium.org>
> > > Date: Mon, 23 Oct 2023 00:04:14 -0700
> > >
> > > Hi Caleb,
> > >
> > > On Sat, 21 Oct 2023 at 01:43, Caleb Connolly <caleb.connolly@linaro.org> wrote:
> > > >
> > > > Hi Simon,
> > > >
> > > > On 21/10/2023 01:45, Simon Glass wrote:
> > > > > U-Boot typically sets up its malloc() pool near the top of memory. On
> > > > > ARM64 systems this can result in an SMBIOS table above 4GB which is
> > > > > not supported by SMBIOSv2.
> [snip]
> > There is absolutely no guarantee that arm64 machines have memory below
> > 4GB.  Examples of SoCs that have no memory below 4GB are AMD's Opteron
> > A1100 SoC and all the recent Apple SoCs.
>
> So one thing to resolve here is where does that requirement about the
> SMBIOS table needing to be below 4GB come from (standards wise), and in
> turn is that obeyed by consumers like say Linux or OpenBSD? Answering my
> own question, maybe in part, https://www.dmtf.org/standards/smbios reads
> to me like there's a v3 and maybe we should be doing what we need to
> support / identify as that, if it doesn't have that restriction?

Yes that was my previous patch. However 1) we apparently don't want to
use SMBIOS3 and 2) my patch had some sort of bug so that it wasn't
read correctly.

So my next version is going to be along the lines of what was discussed here.

Of course, we cannot solve Mark's problem with SMBIOS2, but I suppose
that is obvious. Anyway, those platforms probably don't need SMBIOS.

Regards,
Simon

Re: [PATCH] smbios: arm64: Allow table to be written at a fixed addr

[Heinrich Schuchardt]

Am 25. Oktober 2023 01:28:10 MESZ schrieb Simon Glass <sjg@chromium.org>:
>Hi Tom,
>
>On Tue, 24 Oct 2023 at 15:34, Tom Rini <trini@konsulko.com> wrote:
>>
>> On Mon, Oct 23, 2023 at 05:31:19PM +0200, Mark Kettenis wrote:
>> > > From: Simon Glass <sjg@chromium.org>
>> > > Date: Mon, 23 Oct 2023 00:04:14 -0700
>> > >
>> > > Hi Caleb,
>> > >
>> > > On Sat, 21 Oct 2023 at 01:43, Caleb Connolly <caleb.connolly@linaro.org> wrote:
>> > > >
>> > > > Hi Simon,
>> > > >
>> > > > On 21/10/2023 01:45, Simon Glass wrote:
>> > > > > U-Boot typically sets up its malloc() pool near the top of memory. On
>> > > > > ARM64 systems this can result in an SMBIOS table above 4GB which is
>> > > > > not supported by SMBIOSv2.
>> [snip]
>> > There is absolutely no guarantee that arm64 machines have memory below
>> > 4GB.  Examples of SoCs that have no memory below 4GB are AMD's Opteron
>> > A1100 SoC and all the recent Apple SoCs.
>>
>> So one thing to resolve here is where does that requirement about the
>> SMBIOS table needing to be below 4GB come from (standards wise), and in
>> turn is that obeyed by consumers like say Linux or OpenBSD? Answering my
>> own question, maybe in part, https://www.dmtf.org/standards/smbios reads
>> to me like there's a v3 and maybe we should be doing what we need to
>> support / identify as that, if it doesn't have that restriction?
>
>Yes that was my previous patch. However 1) we apparently don't want to
>use SMBIOS3 and 2) my patch had some sort of bug so that it wasn't
>read correctly.
>
>So my next version is going to be along the lines of what was discussed here.
>
>Of course, we cannot solve Mark's problem with SMBIOS2, but I suppose
>that is obvious. Anyway, those platforms probably don't need SMBIOS.

You are heading in the wrong direction. We need SMBIOS 3. SMBIOS 2 is only a fallback for outdated tools.

Upcoming mainline RISC-V platforms will also have memory starting above 4GiB.

Best regards

Heinrich

>
>Regards,
>Simon

Re: [PATCH] smbios: arm64: Allow table to be written at a fixed addr

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2656 bytes --]

On Wed, Oct 25, 2023 at 02:19:59AM +0200, Heinrich Schuchardt wrote:
> 
> 
> Am 25. Oktober 2023 01:28:10 MESZ schrieb Simon Glass <sjg@chromium.org>:
> >Hi Tom,
> >
> >On Tue, 24 Oct 2023 at 15:34, Tom Rini <trini@konsulko.com> wrote:
> >>
> >> On Mon, Oct 23, 2023 at 05:31:19PM +0200, Mark Kettenis wrote:
> >> > > From: Simon Glass <sjg@chromium.org>
> >> > > Date: Mon, 23 Oct 2023 00:04:14 -0700
> >> > >
> >> > > Hi Caleb,
> >> > >
> >> > > On Sat, 21 Oct 2023 at 01:43, Caleb Connolly <caleb.connolly@linaro.org> wrote:
> >> > > >
> >> > > > Hi Simon,
> >> > > >
> >> > > > On 21/10/2023 01:45, Simon Glass wrote:
> >> > > > > U-Boot typically sets up its malloc() pool near the top of memory. On
> >> > > > > ARM64 systems this can result in an SMBIOS table above 4GB which is
> >> > > > > not supported by SMBIOSv2.
> >> [snip]
> >> > There is absolutely no guarantee that arm64 machines have memory below
> >> > 4GB.  Examples of SoCs that have no memory below 4GB are AMD's Opteron
> >> > A1100 SoC and all the recent Apple SoCs.
> >>
> >> So one thing to resolve here is where does that requirement about the
> >> SMBIOS table needing to be below 4GB come from (standards wise), and in
> >> turn is that obeyed by consumers like say Linux or OpenBSD? Answering my
> >> own question, maybe in part, https://www.dmtf.org/standards/smbios reads
> >> to me like there's a v3 and maybe we should be doing what we need to
> >> support / identify as that, if it doesn't have that restriction?
> >
> >Yes that was my previous patch. However 1) we apparently don't want to
> >use SMBIOS3 and 2) my patch had some sort of bug so that it wasn't
> >read correctly.
> >
> >So my next version is going to be along the lines of what was discussed here.
> >
> >Of course, we cannot solve Mark's problem with SMBIOS2, but I suppose
> >that is obvious. Anyway, those platforms probably don't need SMBIOS.
> 
> You are heading in the wrong direction. We need SMBIOS 3. SMBIOS 2 is
> only a fallback for outdated tools.
> 
> Upcoming mainline RISC-V platforms will also have memory starting above 4GiB.

I want to echo this because yes, SMBIOS if I'm recalling things right is
one of those tools used to provide user friendly information about the
system, so "everyone" wants it, or at least platforms like ones Mark is
concerned about that have more human users than embedded system users,
would like to show the information.  Maybe part of the answer moving
forward is to allow being specific about SMBIOS2 or SMBIOS3 (or none) so
that the issue you had to fix can also be addressed minimally?

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH] smbios: arm64: Allow table to be written at a fixed addr

[Simon Glass]

Hi Tom,

On Tue, 24 Oct 2023 at 17:44, Tom Rini <trini@konsulko.com> wrote:
>
> On Wed, Oct 25, 2023 at 02:19:59AM +0200, Heinrich Schuchardt wrote:
> >
> >
> > Am 25. Oktober 2023 01:28:10 MESZ schrieb Simon Glass <sjg@chromium.org>:
> > >Hi Tom,
> > >
> > >On Tue, 24 Oct 2023 at 15:34, Tom Rini <trini@konsulko.com> wrote:
> > >>
> > >> On Mon, Oct 23, 2023 at 05:31:19PM +0200, Mark Kettenis wrote:
> > >> > > From: Simon Glass <sjg@chromium.org>
> > >> > > Date: Mon, 23 Oct 2023 00:04:14 -0700
> > >> > >
> > >> > > Hi Caleb,
> > >> > >
> > >> > > On Sat, 21 Oct 2023 at 01:43, Caleb Connolly <caleb.connolly@linaro.org> wrote:
> > >> > > >
> > >> > > > Hi Simon,
> > >> > > >
> > >> > > > On 21/10/2023 01:45, Simon Glass wrote:
> > >> > > > > U-Boot typically sets up its malloc() pool near the top of memory. On
> > >> > > > > ARM64 systems this can result in an SMBIOS table above 4GB which is
> > >> > > > > not supported by SMBIOSv2.
> > >> [snip]
> > >> > There is absolutely no guarantee that arm64 machines have memory below
> > >> > 4GB.  Examples of SoCs that have no memory below 4GB are AMD's Opteron
> > >> > A1100 SoC and all the recent Apple SoCs.
> > >>
> > >> So one thing to resolve here is where does that requirement about the
> > >> SMBIOS table needing to be below 4GB come from (standards wise), and in
> > >> turn is that obeyed by consumers like say Linux or OpenBSD? Answering my
> > >> own question, maybe in part, https://www.dmtf.org/standards/smbios reads
> > >> to me like there's a v3 and maybe we should be doing what we need to
> > >> support / identify as that, if it doesn't have that restriction?
> > >
> > >Yes that was my previous patch. However 1) we apparently don't want to
> > >use SMBIOS3 and 2) my patch had some sort of bug so that it wasn't
> > >read correctly.
> > >
> > >So my next version is going to be along the lines of what was discussed here.
> > >
> > >Of course, we cannot solve Mark's problem with SMBIOS2, but I suppose
> > >that is obvious. Anyway, those platforms probably don't need SMBIOS.
> >
> > You are heading in the wrong direction. We need SMBIOS 3. SMBIOS 2 is
> > only a fallback for outdated tools.
> >
> > Upcoming mainline RISC-V platforms will also have memory starting above 4GiB.
>
> I want to echo this because yes, SMBIOS if I'm recalling things right is
> one of those tools used to provide user friendly information about the
> system, so "everyone" wants it, or at least platforms like ones Mark is
> concerned about that have more human users than embedded system users,
> would like to show the information.  Maybe part of the answer moving
> forward is to allow being specific about SMBIOS2 or SMBIOS3 (or none) so
> that the issue you had to fix can also be addressed minimally?

OK, so perhaps fixing up this patch would work? [1]

I got the impression that we wanted to continue to use SMBIOS2 unless
we couldn't. So this patch provides for that. It is conceptually
similar to the way things worked before, so provides the smallest
possible change. The move to SMBIOS3 is really a separate issue, isn't
it?

In fact, how can writing SMBIOS2 have ever worked on machines without
memory below 4GB? That seems like a 'feature request' rather than a
bug fix...and the merge window is closed.

So...?

Regards,
Simon

[1] https://patchwork.ozlabs.org/project/uboot/patch/20231015024511.3995580-4-sjg@chromium.org/

Re: [PATCH] smbios: arm64: Allow table to be written at a fixed addr

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 4389 bytes --]

On Tue, Oct 24, 2023 at 07:47:44PM -0700, Simon Glass wrote:
> Hi Tom,
> 
> On Tue, 24 Oct 2023 at 17:44, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Wed, Oct 25, 2023 at 02:19:59AM +0200, Heinrich Schuchardt wrote:
> > >
> > >
> > > Am 25. Oktober 2023 01:28:10 MESZ schrieb Simon Glass <sjg@chromium.org>:
> > > >Hi Tom,
> > > >
> > > >On Tue, 24 Oct 2023 at 15:34, Tom Rini <trini@konsulko.com> wrote:
> > > >>
> > > >> On Mon, Oct 23, 2023 at 05:31:19PM +0200, Mark Kettenis wrote:
> > > >> > > From: Simon Glass <sjg@chromium.org>
> > > >> > > Date: Mon, 23 Oct 2023 00:04:14 -0700
> > > >> > >
> > > >> > > Hi Caleb,
> > > >> > >
> > > >> > > On Sat, 21 Oct 2023 at 01:43, Caleb Connolly <caleb.connolly@linaro.org> wrote:
> > > >> > > >
> > > >> > > > Hi Simon,
> > > >> > > >
> > > >> > > > On 21/10/2023 01:45, Simon Glass wrote:
> > > >> > > > > U-Boot typically sets up its malloc() pool near the top of memory. On
> > > >> > > > > ARM64 systems this can result in an SMBIOS table above 4GB which is
> > > >> > > > > not supported by SMBIOSv2.
> > > >> [snip]
> > > >> > There is absolutely no guarantee that arm64 machines have memory below
> > > >> > 4GB.  Examples of SoCs that have no memory below 4GB are AMD's Opteron
> > > >> > A1100 SoC and all the recent Apple SoCs.
> > > >>
> > > >> So one thing to resolve here is where does that requirement about the
> > > >> SMBIOS table needing to be below 4GB come from (standards wise), and in
> > > >> turn is that obeyed by consumers like say Linux or OpenBSD? Answering my
> > > >> own question, maybe in part, https://www.dmtf.org/standards/smbios reads
> > > >> to me like there's a v3 and maybe we should be doing what we need to
> > > >> support / identify as that, if it doesn't have that restriction?
> > > >
> > > >Yes that was my previous patch. However 1) we apparently don't want to
> > > >use SMBIOS3 and 2) my patch had some sort of bug so that it wasn't
> > > >read correctly.
> > > >
> > > >So my next version is going to be along the lines of what was discussed here.
> > > >
> > > >Of course, we cannot solve Mark's problem with SMBIOS2, but I suppose
> > > >that is obvious. Anyway, those platforms probably don't need SMBIOS.
> > >
> > > You are heading in the wrong direction. We need SMBIOS 3. SMBIOS 2 is
> > > only a fallback for outdated tools.
> > >
> > > Upcoming mainline RISC-V platforms will also have memory starting above 4GiB.
> >
> > I want to echo this because yes, SMBIOS if I'm recalling things right is
> > one of those tools used to provide user friendly information about the
> > system, so "everyone" wants it, or at least platforms like ones Mark is
> > concerned about that have more human users than embedded system users,
> > would like to show the information.  Maybe part of the answer moving
> > forward is to allow being specific about SMBIOS2 or SMBIOS3 (or none) so
> > that the issue you had to fix can also be addressed minimally?
> 
> OK, so perhaps fixing up this patch would work? [1]
> 
> I got the impression that we wanted to continue to use SMBIOS2 unless
> we couldn't. So this patch provides for that. It is conceptually
> similar to the way things worked before, so provides the smallest
> possible change. The move to SMBIOS3 is really a separate issue, isn't
> it?
> 
> In fact, how can writing SMBIOS2 have ever worked on machines without
> memory below 4GB? That seems like a 'feature request' rather than a
> bug fix...and the merge window is closed.
> 
> So...?
> 
> Regards,
> Simon
> 
> [1] https://patchwork.ozlabs.org/project/uboot/patch/20231015024511.3995580-4-sjg@chromium.org/

This keeps bringing up more questions, sigh. Can we really, as we
seemingly do today, declare the table we're passing to be major_ver=3,
minor_ver=0 and not be using the struct [1] defines as the smbios3 table
format? anchor has a different size to start with.

I'm not familiar with where we decided we wanted to use SMBIOS2 unless
we couldn't, but maybe we need to re-evaluate that decision all the
same.

And in my mind, especially once we know the answer to my first question
here about struct vs version number, I see your patch in [1] is fixing a
problem, and adding whatever further information SMBIOS3 would let us
pass as future work for post-v2024.01.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH] smbios: arm64: Allow table to be written at a fixed addr

[Mark Kettenis]

> Date: Tue, 24 Oct 2023 18:34:05 -0400
> From: Tom Rini <trini@konsulko.com>
> 
> On Mon, Oct 23, 2023 at 05:31:19PM +0200, Mark Kettenis wrote:
> > > From: Simon Glass <sjg@chromium.org>
> > > Date: Mon, 23 Oct 2023 00:04:14 -0700
> > > 
> > > Hi Caleb,
> > > 
> > > On Sat, 21 Oct 2023 at 01:43, Caleb Connolly <caleb.connolly@linaro.org> wrote:
> > > >
> > > > Hi Simon,
> > > >
> > > > On 21/10/2023 01:45, Simon Glass wrote:
> > > > > U-Boot typically sets up its malloc() pool near the top of memory. On
> > > > > ARM64 systems this can result in an SMBIOS table above 4GB which is
> > > > > not supported by SMBIOSv2.
> [snip]
> > There is absolutely no guarantee that arm64 machines have memory below
> > 4GB.  Examples of SoCs that have no memory below 4GB are AMD's Opteron
> > A1100 SoC and all the recent Apple SoCs.
> 
> So one thing to resolve here is where does that requirement about the
> SMBIOS table needing to be below 4GB come from (standards wise), and in
> turn is that obeyed by consumers like say Linux or OpenBSD? Answering my
> own question, maybe in part, https://www.dmtf.org/standards/smbios reads
> to me like there's a v3 and maybe we should be doing what we need to
> support / identify as that, if it doesn't have that restriction?

Right.  U-Boot implements SMBIOS 2.x which is pretty much obsolete and
uses 32-bit addresses.  SMBIOS 3.x allows for 64-bit addresses.  So to
fix this U-Boot needs support for SMBIOS 3.x.

Personally I don't see why we'd need SMBIOS support on non-x86
platforms, which is why implementing this isn't a high priority for
me.  I simply disabled SMBIOS support for M1 for now.

Re: [PATCH] smbios: arm64: Allow table to be written at a fixed addr

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2168 bytes --]

On Wed, Oct 25, 2023 at 04:18:20PM +0200, Mark Kettenis wrote:
> > Date: Tue, 24 Oct 2023 18:34:05 -0400
> > From: Tom Rini <trini@konsulko.com>
> > 
> > On Mon, Oct 23, 2023 at 05:31:19PM +0200, Mark Kettenis wrote:
> > > > From: Simon Glass <sjg@chromium.org>
> > > > Date: Mon, 23 Oct 2023 00:04:14 -0700
> > > > 
> > > > Hi Caleb,
> > > > 
> > > > On Sat, 21 Oct 2023 at 01:43, Caleb Connolly <caleb.connolly@linaro.org> wrote:
> > > > >
> > > > > Hi Simon,
> > > > >
> > > > > On 21/10/2023 01:45, Simon Glass wrote:
> > > > > > U-Boot typically sets up its malloc() pool near the top of memory. On
> > > > > > ARM64 systems this can result in an SMBIOS table above 4GB which is
> > > > > > not supported by SMBIOSv2.
> > [snip]
> > > There is absolutely no guarantee that arm64 machines have memory below
> > > 4GB.  Examples of SoCs that have no memory below 4GB are AMD's Opteron
> > > A1100 SoC and all the recent Apple SoCs.
> > 
> > So one thing to resolve here is where does that requirement about the
> > SMBIOS table needing to be below 4GB come from (standards wise), and in
> > turn is that obeyed by consumers like say Linux or OpenBSD? Answering my
> > own question, maybe in part, https://www.dmtf.org/standards/smbios reads
> > to me like there's a v3 and maybe we should be doing what we need to
> > support / identify as that, if it doesn't have that restriction?
> 
> Right.  U-Boot implements SMBIOS 2.x which is pretty much obsolete and
> uses 32-bit addresses.  SMBIOS 3.x allows for 64-bit addresses.  So to
> fix this U-Boot needs support for SMBIOS 3.x.

OK, thanks.  And for clarity / repeating my confusion, at least right
now we also set the major/minor in the struct to 3.0.

> Personally I don't see why we'd need SMBIOS support on non-x86
> platforms, which is why implementing this isn't a high priority for
> me.  I simply disabled SMBIOS support for M1 for now.

This I think in turn (based on Heinrich's emails in v2 of this patch)
EFI_LOADER wants to pass SMBIOS (probably 3) information along so that
human user facing tools can see the kind of info that table provides.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH] smbios: arm64: Allow table to be written at a fixed addr

[Heinrich Schuchardt]

On 25.10.23 16:28, Tom Rini wrote:
> On Wed, Oct 25, 2023 at 04:18:20PM +0200, Mark Kettenis wrote:
>>> Date: Tue, 24 Oct 2023 18:34:05 -0400
>>> From: Tom Rini <trini@konsulko.com>
>>>
>>> On Mon, Oct 23, 2023 at 05:31:19PM +0200, Mark Kettenis wrote:
>>>>> From: Simon Glass <sjg@chromium.org>
>>>>> Date: Mon, 23 Oct 2023 00:04:14 -0700
>>>>>
>>>>> Hi Caleb,
>>>>>
>>>>> On Sat, 21 Oct 2023 at 01:43, Caleb Connolly <caleb.connolly@linaro.org> wrote:
>>>>>>
>>>>>> Hi Simon,
>>>>>>
>>>>>> On 21/10/2023 01:45, Simon Glass wrote:
>>>>>>> U-Boot typically sets up its malloc() pool near the top of memory. On
>>>>>>> ARM64 systems this can result in an SMBIOS table above 4GB which is
>>>>>>> not supported by SMBIOSv2.
>>> [snip]
>>>> There is absolutely no guarantee that arm64 machines have memory below
>>>> 4GB.  Examples of SoCs that have no memory below 4GB are AMD's Opteron
>>>> A1100 SoC and all the recent Apple SoCs.
>>>
>>> So one thing to resolve here is where does that requirement about the
>>> SMBIOS table needing to be below 4GB come from (standards wise), and in
>>> turn is that obeyed by consumers like say Linux or OpenBSD? Answering my
>>> own question, maybe in part, https://www.dmtf.org/standards/smbios reads
>>> to me like there's a v3 and maybe we should be doing what we need to
>>> support / identify as that, if it doesn't have that restriction?
>>
>> Right.  U-Boot implements SMBIOS 2.x which is pretty much obsolete and
>> uses 32-bit addresses.  SMBIOS 3.x allows for 64-bit addresses.  So to
>> fix this U-Boot needs support for SMBIOS 3.x.
>
> OK, thanks.  And for clarity / repeating my confusion, at least right
> now we also set the major/minor in the struct to 3.0.
>
>> Personally I don't see why we'd need SMBIOS support on non-x86
>> platforms, which is why implementing this isn't a high priority for
>> me.  I simply disabled SMBIOS support for M1 for now.
>
> This I think in turn (based on Heinrich's emails in v2 of this patch)
> EFI_LOADER wants to pass SMBIOS (probably 3) information along so that
> human user facing tools can see the kind of info that table provides.
>

If you have to manage a fleet of devices, be it Apple laptops, small
servers or IoT devices, you want to be able to retrieve information
about these devices like installed firmware, serial numbers, etc. This
information should be presented by our SMBIOS implementation.

Other EFI architectures (ARM and RISC-V) target the same markets as x86.
The customers expect that they can use the same tools irrespective of
architecture.

Best regards

Heinrich

Re: [PATCH] smbios: arm64: Allow table to be written at a fixed addr

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 3435 bytes --]

On Wed, Oct 25, 2023 at 05:28:10PM +0200, Heinrich Schuchardt wrote:
> On 25.10.23 16:28, Tom Rini wrote:
> > On Wed, Oct 25, 2023 at 04:18:20PM +0200, Mark Kettenis wrote:
> > > > Date: Tue, 24 Oct 2023 18:34:05 -0400
> > > > From: Tom Rini <trini@konsulko.com>
> > > > 
> > > > On Mon, Oct 23, 2023 at 05:31:19PM +0200, Mark Kettenis wrote:
> > > > > > From: Simon Glass <sjg@chromium.org>
> > > > > > Date: Mon, 23 Oct 2023 00:04:14 -0700
> > > > > > 
> > > > > > Hi Caleb,
> > > > > > 
> > > > > > On Sat, 21 Oct 2023 at 01:43, Caleb Connolly <caleb.connolly@linaro.org> wrote:
> > > > > > > 
> > > > > > > Hi Simon,
> > > > > > > 
> > > > > > > On 21/10/2023 01:45, Simon Glass wrote:
> > > > > > > > U-Boot typically sets up its malloc() pool near the top of memory. On
> > > > > > > > ARM64 systems this can result in an SMBIOS table above 4GB which is
> > > > > > > > not supported by SMBIOSv2.
> > > > [snip]
> > > > > There is absolutely no guarantee that arm64 machines have memory below
> > > > > 4GB.  Examples of SoCs that have no memory below 4GB are AMD's Opteron
> > > > > A1100 SoC and all the recent Apple SoCs.
> > > > 
> > > > So one thing to resolve here is where does that requirement about the
> > > > SMBIOS table needing to be below 4GB come from (standards wise), and in
> > > > turn is that obeyed by consumers like say Linux or OpenBSD? Answering my
> > > > own question, maybe in part, https://www.dmtf.org/standards/smbios reads
> > > > to me like there's a v3 and maybe we should be doing what we need to
> > > > support / identify as that, if it doesn't have that restriction?
> > > 
> > > Right.  U-Boot implements SMBIOS 2.x which is pretty much obsolete and
> > > uses 32-bit addresses.  SMBIOS 3.x allows for 64-bit addresses.  So to
> > > fix this U-Boot needs support for SMBIOS 3.x.
> > 
> > OK, thanks.  And for clarity / repeating my confusion, at least right
> > now we also set the major/minor in the struct to 3.0.
> > 
> > > Personally I don't see why we'd need SMBIOS support on non-x86
> > > platforms, which is why implementing this isn't a high priority for
> > > me.  I simply disabled SMBIOS support for M1 for now.
> > 
> > This I think in turn (based on Heinrich's emails in v2 of this patch)
> > EFI_LOADER wants to pass SMBIOS (probably 3) information along so that
> > human user facing tools can see the kind of info that table provides.
> > 
> 
> If you have to manage a fleet of devices, be it Apple laptops, small
> servers or IoT devices, you want to be able to retrieve information
> about these devices like installed firmware, serial numbers, etc. This
> information should be presented by our SMBIOS implementation.
> 
> Other EFI architectures (ARM and RISC-V) target the same markets as x86.
> The customers expect that they can use the same tools irrespective of
> architecture.

Ah, we do have serial# being passed along that way (as well as other
methods which is why I didn't think about them for SMBIOS). But this
also doesn't answer my bigger concern which is that we're setting our
version major to 3, our minor to 0 and then filling in a struct that's
not the "smbios3" one that Simon's patch from another thread added. Is
what we're filling out today semantically valid? I'm hoping someone who
has already read over the spec / is familiar and I don't need to go do
that right now.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH] smbios: arm64: Allow table to be written at a fixed addr

[Heinrich Schuchardt]

On 10/25/23 19:09, Tom Rini wrote:
> On Wed, Oct 25, 2023 at 05:28:10PM +0200, Heinrich Schuchardt wrote:
>> On 25.10.23 16:28, Tom Rini wrote:
>>> On Wed, Oct 25, 2023 at 04:18:20PM +0200, Mark Kettenis wrote:
>>>>> Date: Tue, 24 Oct 2023 18:34:05 -0400
>>>>> From: Tom Rini <trini@konsulko.com>
>>>>>
>>>>> On Mon, Oct 23, 2023 at 05:31:19PM +0200, Mark Kettenis wrote:
>>>>>>> From: Simon Glass <sjg@chromium.org>
>>>>>>> Date: Mon, 23 Oct 2023 00:04:14 -0700
>>>>>>>
>>>>>>> Hi Caleb,
>>>>>>>
>>>>>>> On Sat, 21 Oct 2023 at 01:43, Caleb Connolly <caleb.connolly@linaro.org> wrote:
>>>>>>>>
>>>>>>>> Hi Simon,
>>>>>>>>
>>>>>>>> On 21/10/2023 01:45, Simon Glass wrote:
>>>>>>>>> U-Boot typically sets up its malloc() pool near the top of memory. On
>>>>>>>>> ARM64 systems this can result in an SMBIOS table above 4GB which is
>>>>>>>>> not supported by SMBIOSv2.
>>>>> [snip]
>>>>>> There is absolutely no guarantee that arm64 machines have memory below
>>>>>> 4GB.  Examples of SoCs that have no memory below 4GB are AMD's Opteron
>>>>>> A1100 SoC and all the recent Apple SoCs.
>>>>>
>>>>> So one thing to resolve here is where does that requirement about the
>>>>> SMBIOS table needing to be below 4GB come from (standards wise), and in
>>>>> turn is that obeyed by consumers like say Linux or OpenBSD? Answering my
>>>>> own question, maybe in part, https://www.dmtf.org/standards/smbios reads
>>>>> to me like there's a v3 and maybe we should be doing what we need to
>>>>> support / identify as that, if it doesn't have that restriction?
>>>>
>>>> Right.  U-Boot implements SMBIOS 2.x which is pretty much obsolete and
>>>> uses 32-bit addresses.  SMBIOS 3.x allows for 64-bit addresses.  So to
>>>> fix this U-Boot needs support for SMBIOS 3.x.
>>>
>>> OK, thanks.  And for clarity / repeating my confusion, at least right
>>> now we also set the major/minor in the struct to 3.0.
>>>
>>>> Personally I don't see why we'd need SMBIOS support on non-x86
>>>> platforms, which is why implementing this isn't a high priority for
>>>> me.  I simply disabled SMBIOS support for M1 for now.
>>>
>>> This I think in turn (based on Heinrich's emails in v2 of this patch)
>>> EFI_LOADER wants to pass SMBIOS (probably 3) information along so that
>>> human user facing tools can see the kind of info that table provides.
>>>
>>
>> If you have to manage a fleet of devices, be it Apple laptops, small
>> servers or IoT devices, you want to be able to retrieve information
>> about these devices like installed firmware, serial numbers, etc. This
>> information should be presented by our SMBIOS implementation.
>>
>> Other EFI architectures (ARM and RISC-V) target the same markets as x86.
>> The customers expect that they can use the same tools irrespective of
>> architecture.
>
> Ah, we do have serial# being passed along that way (as well as other
> methods which is why I didn't think about them for SMBIOS). But this
> also doesn't answer my bigger concern which is that we're setting our
> version major to 3, our minor to 0 and then filling in a struct that's
> not the "smbios3" one that Simon's patch from another thread added. Is
> what we're filling out today semantically valid? I'm hoping someone who
> has already read over the spec / is familiar and I don't need to go do
> that right now.
>

First of all there is a field "Anchor String". For a 32bit structure it
contains the value '_SM_', for a 64bit structure it contains the value
'_SM3_'. The different EFI GUIDs relate to these two different
structures. In our mails we simply referred to them as SMBIOS2 and
SMBIOS3 structures.

Next there are the fields 'SMBIOS Major Version' and 'SMBIOS Minor
Version'. These should refer to the standard version 3.7 if U-Boot
adheres to the current version.

As RISC-V was only fully added in SMBIOS version 3.4 we should not use
any older version of the specification.

See
https://www.dmtf.org/content/dmtf-releases-smbios-37

Best regards

Heinrich

Re: [PATCH] smbios: arm64: Allow table to be written at a fixed addr

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 4587 bytes --]

On Wed, Oct 25, 2023 at 10:52:48PM +0200, Heinrich Schuchardt wrote:
> On 10/25/23 19:09, Tom Rini wrote:
> > On Wed, Oct 25, 2023 at 05:28:10PM +0200, Heinrich Schuchardt wrote:
> > > On 25.10.23 16:28, Tom Rini wrote:
> > > > On Wed, Oct 25, 2023 at 04:18:20PM +0200, Mark Kettenis wrote:
> > > > > > Date: Tue, 24 Oct 2023 18:34:05 -0400
> > > > > > From: Tom Rini <trini@konsulko.com>
> > > > > > 
> > > > > > On Mon, Oct 23, 2023 at 05:31:19PM +0200, Mark Kettenis wrote:
> > > > > > > > From: Simon Glass <sjg@chromium.org>
> > > > > > > > Date: Mon, 23 Oct 2023 00:04:14 -0700
> > > > > > > > 
> > > > > > > > Hi Caleb,
> > > > > > > > 
> > > > > > > > On Sat, 21 Oct 2023 at 01:43, Caleb Connolly <caleb.connolly@linaro.org> wrote:
> > > > > > > > > 
> > > > > > > > > Hi Simon,
> > > > > > > > > 
> > > > > > > > > On 21/10/2023 01:45, Simon Glass wrote:
> > > > > > > > > > U-Boot typically sets up its malloc() pool near the top of memory. On
> > > > > > > > > > ARM64 systems this can result in an SMBIOS table above 4GB which is
> > > > > > > > > > not supported by SMBIOSv2.
> > > > > > [snip]
> > > > > > > There is absolutely no guarantee that arm64 machines have memory below
> > > > > > > 4GB.  Examples of SoCs that have no memory below 4GB are AMD's Opteron
> > > > > > > A1100 SoC and all the recent Apple SoCs.
> > > > > > 
> > > > > > So one thing to resolve here is where does that requirement about the
> > > > > > SMBIOS table needing to be below 4GB come from (standards wise), and in
> > > > > > turn is that obeyed by consumers like say Linux or OpenBSD? Answering my
> > > > > > own question, maybe in part, https://www.dmtf.org/standards/smbios reads
> > > > > > to me like there's a v3 and maybe we should be doing what we need to
> > > > > > support / identify as that, if it doesn't have that restriction?
> > > > > 
> > > > > Right.  U-Boot implements SMBIOS 2.x which is pretty much obsolete and
> > > > > uses 32-bit addresses.  SMBIOS 3.x allows for 64-bit addresses.  So to
> > > > > fix this U-Boot needs support for SMBIOS 3.x.
> > > > 
> > > > OK, thanks.  And for clarity / repeating my confusion, at least right
> > > > now we also set the major/minor in the struct to 3.0.
> > > > 
> > > > > Personally I don't see why we'd need SMBIOS support on non-x86
> > > > > platforms, which is why implementing this isn't a high priority for
> > > > > me.  I simply disabled SMBIOS support for M1 for now.
> > > > 
> > > > This I think in turn (based on Heinrich's emails in v2 of this patch)
> > > > EFI_LOADER wants to pass SMBIOS (probably 3) information along so that
> > > > human user facing tools can see the kind of info that table provides.
> > > > 
> > > 
> > > If you have to manage a fleet of devices, be it Apple laptops, small
> > > servers or IoT devices, you want to be able to retrieve information
> > > about these devices like installed firmware, serial numbers, etc. This
> > > information should be presented by our SMBIOS implementation.
> > > 
> > > Other EFI architectures (ARM and RISC-V) target the same markets as x86.
> > > The customers expect that they can use the same tools irrespective of
> > > architecture.
> > 
> > Ah, we do have serial# being passed along that way (as well as other
> > methods which is why I didn't think about them for SMBIOS). But this
> > also doesn't answer my bigger concern which is that we're setting our
> > version major to 3, our minor to 0 and then filling in a struct that's
> > not the "smbios3" one that Simon's patch from another thread added. Is
> > what we're filling out today semantically valid? I'm hoping someone who
> > has already read over the spec / is familiar and I don't need to go do
> > that right now.
> > 
> 
> First of all there is a field "Anchor String". For a 32bit structure it
> contains the value '_SM_', for a 64bit structure it contains the value
> '_SM3_'. The different EFI GUIDs relate to these two different
> structures. In our mails we simply referred to them as SMBIOS2 and
> SMBIOS3 structures.
> 
> Next there are the fields 'SMBIOS Major Version' and 'SMBIOS Minor
> Version'. These should refer to the standard version 3.7 if U-Boot
> adheres to the current version.
> 
> As RISC-V was only fully added in SMBIOS version 3.4 we should not use
> any older version of the specification.
> 
> See
> https://www.dmtf.org/content/dmtf-releases-smbios-37

That's helpful.  Can we use an SMBIOS2 structure with 'SMBIOS Major
Version' = 3 and be valid?

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH] smbios: arm64: Allow table to be written at a fixed addr

[Heinrich Schuchardt]

On 10/25/23 23:09, Tom Rini wrote:
> On Wed, Oct 25, 2023 at 10:52:48PM +0200, Heinrich Schuchardt wrote:
>> On 10/25/23 19:09, Tom Rini wrote:
>>> On Wed, Oct 25, 2023 at 05:28:10PM +0200, Heinrich Schuchardt wrote:
>>>> On 25.10.23 16:28, Tom Rini wrote:
>>>>> On Wed, Oct 25, 2023 at 04:18:20PM +0200, Mark Kettenis wrote:
>>>>>>> Date: Tue, 24 Oct 2023 18:34:05 -0400
>>>>>>> From: Tom Rini <trini@konsulko.com>
>>>>>>>
>>>>>>> On Mon, Oct 23, 2023 at 05:31:19PM +0200, Mark Kettenis wrote:
>>>>>>>>> From: Simon Glass <sjg@chromium.org>
>>>>>>>>> Date: Mon, 23 Oct 2023 00:04:14 -0700
>>>>>>>>>
>>>>>>>>> Hi Caleb,
>>>>>>>>>
>>>>>>>>> On Sat, 21 Oct 2023 at 01:43, Caleb Connolly <caleb.connolly@linaro.org> wrote:
>>>>>>>>>>
>>>>>>>>>> Hi Simon,
>>>>>>>>>>
>>>>>>>>>> On 21/10/2023 01:45, Simon Glass wrote:
>>>>>>>>>>> U-Boot typically sets up its malloc() pool near the top of memory. On
>>>>>>>>>>> ARM64 systems this can result in an SMBIOS table above 4GB which is
>>>>>>>>>>> not supported by SMBIOSv2.
>>>>>>> [snip]
>>>>>>>> There is absolutely no guarantee that arm64 machines have memory below
>>>>>>>> 4GB.  Examples of SoCs that have no memory below 4GB are AMD's Opteron
>>>>>>>> A1100 SoC and all the recent Apple SoCs.
>>>>>>>
>>>>>>> So one thing to resolve here is where does that requirement about the
>>>>>>> SMBIOS table needing to be below 4GB come from (standards wise), and in
>>>>>>> turn is that obeyed by consumers like say Linux or OpenBSD? Answering my
>>>>>>> own question, maybe in part, https://www.dmtf.org/standards/smbios reads
>>>>>>> to me like there's a v3 and maybe we should be doing what we need to
>>>>>>> support / identify as that, if it doesn't have that restriction?
>>>>>>
>>>>>> Right.  U-Boot implements SMBIOS 2.x which is pretty much obsolete and
>>>>>> uses 32-bit addresses.  SMBIOS 3.x allows for 64-bit addresses.  So to
>>>>>> fix this U-Boot needs support for SMBIOS 3.x.
>>>>>
>>>>> OK, thanks.  And for clarity / repeating my confusion, at least right
>>>>> now we also set the major/minor in the struct to 3.0.
>>>>>
>>>>>> Personally I don't see why we'd need SMBIOS support on non-x86
>>>>>> platforms, which is why implementing this isn't a high priority for
>>>>>> me.  I simply disabled SMBIOS support for M1 for now.
>>>>>
>>>>> This I think in turn (based on Heinrich's emails in v2 of this patch)
>>>>> EFI_LOADER wants to pass SMBIOS (probably 3) information along so that
>>>>> human user facing tools can see the kind of info that table provides.
>>>>>
>>>>
>>>> If you have to manage a fleet of devices, be it Apple laptops, small
>>>> servers or IoT devices, you want to be able to retrieve information
>>>> about these devices like installed firmware, serial numbers, etc. This
>>>> information should be presented by our SMBIOS implementation.
>>>>
>>>> Other EFI architectures (ARM and RISC-V) target the same markets as x86.
>>>> The customers expect that they can use the same tools irrespective of
>>>> architecture.
>>>
>>> Ah, we do have serial# being passed along that way (as well as other
>>> methods which is why I didn't think about them for SMBIOS). But this
>>> also doesn't answer my bigger concern which is that we're setting our
>>> version major to 3, our minor to 0 and then filling in a struct that's
>>> not the "smbios3" one that Simon's patch from another thread added. Is
>>> what we're filling out today semantically valid? I'm hoping someone who
>>> has already read over the spec / is familiar and I don't need to go do
>>> that right now.
>>>
>>
>> First of all there is a field "Anchor String". For a 32bit structure it
>> contains the value '_SM_', for a 64bit structure it contains the value
>> '_SM3_'. The different EFI GUIDs relate to these two different
>> structures. In our mails we simply referred to them as SMBIOS2 and
>> SMBIOS3 structures.
>>
>> Next there are the fields 'SMBIOS Major Version' and 'SMBIOS Minor
>> Version'. These should refer to the standard version 3.7 if U-Boot
>> adheres to the current version.
>>
>> As RISC-V was only fully added in SMBIOS version 3.4 we should not use
>> any older version of the specification.
>>
>> See
>> https://www.dmtf.org/content/dmtf-releases-smbios-37
>
> That's helpful.  Can we use an SMBIOS2 structure with 'SMBIOS Major
> Version' = 3 and be valid?
>

The SMBIOS specification 3.7 says:

"If an implementation provides both a 32-bit and a 64-bit entry point,
they must both report the same SMBIOS major.minor specification version"

This implies that SMBIOS Major Version = 3 is valid both in the 32- and
in the 64-bit structure.

Best regards

Heinrich

Re: [PATCH] smbios: arm64: Allow table to be written at a fixed addr

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 5506 bytes --]

On Thu, Oct 26, 2023 at 01:02:40AM +0200, Heinrich Schuchardt wrote:
> On 10/25/23 23:09, Tom Rini wrote:
> > On Wed, Oct 25, 2023 at 10:52:48PM +0200, Heinrich Schuchardt wrote:
> > > On 10/25/23 19:09, Tom Rini wrote:
> > > > On Wed, Oct 25, 2023 at 05:28:10PM +0200, Heinrich Schuchardt wrote:
> > > > > On 25.10.23 16:28, Tom Rini wrote:
> > > > > > On Wed, Oct 25, 2023 at 04:18:20PM +0200, Mark Kettenis wrote:
> > > > > > > > Date: Tue, 24 Oct 2023 18:34:05 -0400
> > > > > > > > From: Tom Rini <trini@konsulko.com>
> > > > > > > > 
> > > > > > > > On Mon, Oct 23, 2023 at 05:31:19PM +0200, Mark Kettenis wrote:
> > > > > > > > > > From: Simon Glass <sjg@chromium.org>
> > > > > > > > > > Date: Mon, 23 Oct 2023 00:04:14 -0700
> > > > > > > > > > 
> > > > > > > > > > Hi Caleb,
> > > > > > > > > > 
> > > > > > > > > > On Sat, 21 Oct 2023 at 01:43, Caleb Connolly <caleb.connolly@linaro.org> wrote:
> > > > > > > > > > > 
> > > > > > > > > > > Hi Simon,
> > > > > > > > > > > 
> > > > > > > > > > > On 21/10/2023 01:45, Simon Glass wrote:
> > > > > > > > > > > > U-Boot typically sets up its malloc() pool near the top of memory. On
> > > > > > > > > > > > ARM64 systems this can result in an SMBIOS table above 4GB which is
> > > > > > > > > > > > not supported by SMBIOSv2.
> > > > > > > > [snip]
> > > > > > > > > There is absolutely no guarantee that arm64 machines have memory below
> > > > > > > > > 4GB.  Examples of SoCs that have no memory below 4GB are AMD's Opteron
> > > > > > > > > A1100 SoC and all the recent Apple SoCs.
> > > > > > > > 
> > > > > > > > So one thing to resolve here is where does that requirement about the
> > > > > > > > SMBIOS table needing to be below 4GB come from (standards wise), and in
> > > > > > > > turn is that obeyed by consumers like say Linux or OpenBSD? Answering my
> > > > > > > > own question, maybe in part, https://www.dmtf.org/standards/smbios reads
> > > > > > > > to me like there's a v3 and maybe we should be doing what we need to
> > > > > > > > support / identify as that, if it doesn't have that restriction?
> > > > > > > 
> > > > > > > Right.  U-Boot implements SMBIOS 2.x which is pretty much obsolete and
> > > > > > > uses 32-bit addresses.  SMBIOS 3.x allows for 64-bit addresses.  So to
> > > > > > > fix this U-Boot needs support for SMBIOS 3.x.
> > > > > > 
> > > > > > OK, thanks.  And for clarity / repeating my confusion, at least right
> > > > > > now we also set the major/minor in the struct to 3.0.
> > > > > > 
> > > > > > > Personally I don't see why we'd need SMBIOS support on non-x86
> > > > > > > platforms, which is why implementing this isn't a high priority for
> > > > > > > me.  I simply disabled SMBIOS support for M1 for now.
> > > > > > 
> > > > > > This I think in turn (based on Heinrich's emails in v2 of this patch)
> > > > > > EFI_LOADER wants to pass SMBIOS (probably 3) information along so that
> > > > > > human user facing tools can see the kind of info that table provides.
> > > > > > 
> > > > > 
> > > > > If you have to manage a fleet of devices, be it Apple laptops, small
> > > > > servers or IoT devices, you want to be able to retrieve information
> > > > > about these devices like installed firmware, serial numbers, etc. This
> > > > > information should be presented by our SMBIOS implementation.
> > > > > 
> > > > > Other EFI architectures (ARM and RISC-V) target the same markets as x86.
> > > > > The customers expect that they can use the same tools irrespective of
> > > > > architecture.
> > > > 
> > > > Ah, we do have serial# being passed along that way (as well as other
> > > > methods which is why I didn't think about them for SMBIOS). But this
> > > > also doesn't answer my bigger concern which is that we're setting our
> > > > version major to 3, our minor to 0 and then filling in a struct that's
> > > > not the "smbios3" one that Simon's patch from another thread added. Is
> > > > what we're filling out today semantically valid? I'm hoping someone who
> > > > has already read over the spec / is familiar and I don't need to go do
> > > > that right now.
> > > > 
> > > 
> > > First of all there is a field "Anchor String". For a 32bit structure it
> > > contains the value '_SM_', for a 64bit structure it contains the value
> > > '_SM3_'. The different EFI GUIDs relate to these two different
> > > structures. In our mails we simply referred to them as SMBIOS2 and
> > > SMBIOS3 structures.
> > > 
> > > Next there are the fields 'SMBIOS Major Version' and 'SMBIOS Minor
> > > Version'. These should refer to the standard version 3.7 if U-Boot
> > > adheres to the current version.
> > > 
> > > As RISC-V was only fully added in SMBIOS version 3.4 we should not use
> > > any older version of the specification.
> > > 
> > > See
> > > https://www.dmtf.org/content/dmtf-releases-smbios-37
> > 
> > That's helpful.  Can we use an SMBIOS2 structure with 'SMBIOS Major
> > Version' = 3 and be valid?
> > 
> 
> The SMBIOS specification 3.7 says:
> 
> "If an implementation provides both a 32-bit and a 64-bit entry point,
> they must both report the same SMBIOS major.minor specification version"
> 
> This implies that SMBIOS Major Version = 3 is valid both in the 32- and
> in the 64-bit structure.

OK, so tooling isn't going to, or at least shouldn't, blow up because we
have Major Version 3 in a SMBIOS2 structure in 32bit address space,
thanks.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]