dt-bindings: chosen: Add "smbios3-entrypoint" property

[adrianicolae]

Some bootloaders like U-boot, particularly for the ARM architecture,
provide SMBIOS/DMI tables at a specific memory address. However, these
systems often do not boot using a full UEFI environment, which means the
kernel's standard EFI DMI scanner cannot find these tables.

This property allows a bootloader to pass the physical address of the
SMBIOSv3 entrypoint structure to the kernel via the /chosen node.

[ardbiesheuvel]

I am not fluent in dt-schema, so I am not sure this matters, but I'll raise it just in case: the address doesn't really need to be 64-bits. The SMBIOS3 entrypoint structure (which supersedes the SMBIOS entrypoint structure) was introduced to raise the max address of the SMBIOS table array beyond 4 GiB, but here we just need to capture the address of the entry point itself. IOW, maybe this should just be types.yaml#/definitions/address like the other fields?

[adrianicolae]

Thanks for noticing this, since it is similar to linux,initrd-start/end the address definition is better. I've updated that and also added maxItems: 1.

[sjg20]

How about smbios3-start or -base? This the start of the tables and 'entry point' sounds more like you have jump to it. Do we care about the size of the tables? I suppose people can decode the header when they want to find out, but it is common to include both an address and a size for properties which refer to memory (e.g. 'reg').

I assume that v3 is the only valid version at present and v2 is not really used anymore.

In a devicetree environment it would be better to include the information from the SMBIOS tables in a full devicetree schema, but that's a problem for another year :-)

[ardbiesheuvel]

How about smbios3-start or -base? This the start of the tables and 'entry point' sounds more like you have jump to it.

It is not the start of the tables. It is the address of a separate structure in memory that contains the version information and the actual address of the tables, and those could be somewhere else in memory entirely (for x86 legacy reasons). This structure is what the DMTF SMBIOS spec calls entrypoint and so I don't think we should be inventing our own terminology here.

Note that there is existing tooling such as dmidecode that expects to be able to access the entrypoint structure directly, and so omitting things or re-inventing them is going to be problematic for compatibility.

Do we care about the size of the tables? I suppose people can decode the header when they want to find out, but it is common to include both an address and a size for properties which refer to memory (e.g. 'reg').

The entrypoint structure has a fixed size.

I assume that v3 is the only valid version at present and v2 is not really used anymore.

There is no v2. It is called SMBIOS3 because SMBIOS 3.x was the first version to describe a variant of the entrypoint that can accommodate a 64-bit address for the table array.

In a devicetree environment it would be better to include the information from the SMBIOS tables in a full devicetree schema, but that's a problem for another year :-)

I don't see how that would be useful. There is already spec outside of DT that describes all of this - what would be the added value of duplicating this elsewhere?

[sjg20]

Yes, I understand this is a header and the tables could be elsewhere. So what are the semantics of that? Should we require that the tables be immediately after this 'entry header'? If the tables are elsewhere, does that mean we have a reserved-memory section?

Re the SMBIOS spec, having written a generator and parsers for it, it is pretty a pretty ugly and old-fashioned format which could be implemented nicely in devicetree.

[sjg20]

LGTM

[sjg20]

Yes, I understand this is a header and the tables could be elsewhere. So what are the semantics of that? Should we require that the tables be immediately after this 'entry header'? If the tables are elsewhere, does that mean we have a reserved-memory section?

Re the SMBIOS spec, having written a generator and parsers for it, it is pretty a pretty ugly and old-fashioned format which could be implemented nicely in devicetree.

[ardbiesheuvel]

Yes, I understand this is a header and the tables could be elsewhere. So what are the semantics of that? Should we require that the tables be immediately after this 'entry header'?

There are still two types of entry point structures, and so existing implementations that provide both might have to be updated to comply with this requirement. Given that there is no technical need for the structure and the tables to be adjacent in memory, I don't think it should be added as a requirement here.

If the tables are elsewhere, does that mean we have a reserved-memory section?

That is a good point, actually, but it applies to both the entry point structure and the tables.

When booting via EFI, the provisions in the EFI spec should be followed, which states that SMBIOS tables and the entry point should be in memory of type EfiRuntimeServicesData, which is implicitly reserved for firmware use. Linux today does not take any explicit measures to reserve this memory.

So when not booting via EFI, the memory does need to be marked as reserved in another way, presumably via the /reserved-memory/ node?

Re the SMBIOS spec, having written a generator and parsers for it, it is pretty a pretty ugly and old-fashioned format which could be implemented nicely in devicetree.

I don't disagree, but our personal views on SMBIOS aesthetics are not the point here. There is an existing corpus of tooling that people want to reuse, and re-inventing SMBIOS in DT is only going to result in more fragmentation than there already is.

[adrianicolae]

So when not booting via EFI, the memory does need to be marked as reserved in another way, presumably via the /reserved-memory/ node?

Yes, when testing this change, the device tree included a reserved-memory node to prevent the structure of being overwritten.

[robherring]

So when not booting via EFI, the memory does need to be marked as reserved in another way, presumably via the /reserved-memory/ node?

Yes, when testing this change, the device tree included a reserved-memory node to prevent the structure of being overwritten.

Humm, we shouldn't have the same information twice. Besides just being redundant, what if they disagree? Other things in /chosen (e.g. initrd) are not in /reserved-memory. It is up to the OS to move or reserve those regions (just like the DT itself). I think either way can work here. /chosen is more for things the prior boot stage sets up or picks one of multiple options (e.g. stdout-path). /reserved-memory is better if this is static forever once set up (e.g. kexec wouldn't touch it and just pass it thru to the next stage).

[ardbiesheuvel]

So when not booting via EFI, the memory does need to be marked as reserved in another way, presumably via the /reserved-memory/ node?

Yes, when testing this change, the device tree included a reserved-memory node to prevent the structure of being overwritten.

Humm, we shouldn't have the same information twice. Besides just being redundant, what if they disagree? Other things in /chosen (e.g. initrd) are not in /reserved-memory. It is up to the OS to move or reserve those regions (just like the DT itself). I think either way can work here. /chosen is more for things the prior boot stage sets up or picks one of multiple options (e.g. stdout-path). /reserved-memory is better if this is static forever once set up (e.g. kexec wouldn't touch it and just pass it thru to the next stage).

I think the latter case applies here.

[adrianicolae]

In my use case, the SMBIOS entry point exists inside a larger memory region already reserved for firmware data but we can split it into separate nodes.

If we do move this from chosen to a dedicated /reserved-memory node, what is the best way to ensure standardized discovery? In the chosen case we could rely on the property name, but for the /reserved-memory node we can't restrict the node naming.

One possible approach would be to define a new compatible string (compatible = "smbios3-entrypoint") for the kernel to find instead of searching for the property name:

/reserved-memory {    
    smbios3@0xffff0000 {
        compatible = "smbios3-entrypoint";
        reg = <0x0 0xffff0000 0x0 0x20>;
    };
};

[robherring]

In my use case, the SMBIOS entry point exists inside a larger memory region already reserved for firmware data but we can split it into separate nodes.

That may be a reason to keep it in /chosen. How does this work for EFI?

If we do move this from chosen to a dedicated /reserved-memory node, what is the best way to ensure standardized discovery? In the chosen case we could rely on the property name, but for the /reserved-memory node we can't restrict the node naming.

One possible approach would be to define a new compatible string (compatible = "smbios3-entrypoint") for the kernel to find instead of searching for the property name:

That would be the way to do it if we went this route.

/reserved-memory {    
    smbios3@0xffff0000 {
        compatible = "smbios3-entrypoint";
        reg = <0x0 0xffff0000 0x0 0x20>;
    };
};

Only 32 bytes? I don't think we want to make reserved regions that fine grained. Is this just pointing to another chunk of memory presumably in that same firmware data region?

[adrianicolae]

That may be a reason to keep it in /chosen. How does this work for EFI?

On EFI boot the bootloader also updates the memory map with the SMBIOS region address. This means for this case were U-boot fills that data when EFI is not used, the bootloader should also check that there is a reserved region protecting that memory, otherwise add it in the device tree.

Only 32 bytes? I don't think we want to make reserved regions that fine grained. Is this just pointing to another chunk of memory presumably in that same firmware data region?

We only need 32 bytes for SMBIOS data which is fixed in size, the node above was an example how it would look like. In my actual case I have a 4KB region reserved for firmware data which includes those 32 bytes filled in by bootloader something like this:

		uboot_data@0xffff0000 {
			no-map;
			reg = <0x0 0xffff0000 0x0 0x00001000>;
		};

[ardbiesheuvel]

In my use case, the SMBIOS entry point exists inside a larger memory region already reserved for firmware data but we can split it into separate nodes.

That may be a reason to keep it in /chosen. How does this work for EFI?

EFI has its own memory map, which typically describes the SMBIOS as EfiRuntimeServicesData, which helps the OS understand that these regions need to be left alone.

If we do move this from chosen to a dedicated /reserved-memory node, what is the best way to ensure standardized discovery? In the chosen case we could rely on the property name, but for the /reserved-memory node we can't restrict the node naming.
One possible approach would be to define a new compatible string (compatible = "smbios3-entrypoint") for the kernel to find instead of searching for the property name:

That would be the way to do it if we went this route.

/reserved-memory {    
    smbios3@0xffff0000 {
        compatible = "smbios3-entrypoint";
        reg = <0x0 0xffff0000 0x0 0x20>;
    };
};

Only 32 bytes? I don't think we want to make reserved regions that fine grained. Is this just pointing to another chunk of memory presumably in that same firmware data region?

32 bytes is the size of the so-called SMBIOS3 entry point structure, which carries the major/minor version, and the address and size of the tables. Those tables could potentially be elsewhere in memory (although only for a good reason on legacy x86).

We might decide to be normative here, and state that SMBIOS tables reported this way need to be covered by a single reservation, along with the entry point structure, which needs to stay in place, as existing tools expect to be able to access it. That would mean, though, that the entry point structure needs to be at the start of the region, and I'm not sure whether or not existing implementations manage it that way, given that no such requirement exists today.

[ardbiesheuvel]

We only need 32 bytes for SMBIOS data which is fixed in size, the node above was an example how it would look like. In my actual case I have a 4KB region reserved for firmware data which includes those 32 bytes filled in by bootloader something like this:

		uboot_data@0xffff0000 {
			no-map;
			reg = <0x0 0xffff0000 0x0 0x00001000>;
		};

Nit: the no-map property is unnecessary here: the contents of the region are for consumption by the OS only, and so it will need to map the memory in any case, so telling it to omit it from the general mapping of RAM is kind of pointless.

[robherring]

I guess the conclusion here is this is fine as-is. However, your Signed-off-by needs to be your full name. Also, please rebase on current main as the first commit can be dropped now.