Add SMBIOS OEM Strings support (aarch64 only) #187
Conversation
germag
requested review from
crobinso,
mtjhrc,
slp and
tylerfanelli
as code owners
germag
force-pushed
the
add-smbios-support
branch
from
04424bd
to
2f04e8b
Compare
This edk2 firmware will search for the SMBIOS data in address 0x4000_F000. Signed-off-by: German Maglione <gmaglione@redhat.com>
This crate will build and write in memory all the required tables for SMBIOS. It build tables type 0, 1 and 11 if provided. It's not SMBIOS compliant, since it only set 2 of the 10 required tables. It also doesn't set the UUID in type 1 table (i.e., system information), that is not clear it's required in the spec (QEMU set a 0 UUID if not provided). The type 0 characteristics bits follow the example in QEMU and EDK2 Signed-off-by: German Maglione <gmaglione@redhat.com>
germag
force-pushed
the
add-smbios-support
branch
from
2f04e8b
to
612f706
Compare
|
The EDK2 firmware was created using slp/edk2#1 |
include/libkrun.h
Outdated
| * Returns: | ||
| * Zero on success or a negative error number on failure. | ||
| */ | ||
| int32_t krun_set_smbios_oem_strings(uint32_t ctx_id, const char *const oem_strings[], char count); |
There was a problem hiding this comment.
It's a bit weird passing count as char. And, while I agree this strategy is better than using from_raw_parts with MAX_ARGS, please use it and drop count for consistency with other functions in the API. We'll change all of them at once in v2.0.
There was a problem hiding this comment.
Ok, should I explicitly ask for a NULL ptr terminator of the array?, like:
/**
* Sets the SMBIOS OEM Strings.
*
* Arguments:
* "ctx_id" - the configuration context ID.
* "oem_strings" - an array of string pointers. Is terminated with an additional NULL pointer.
*
* Returns:
* Zero on success or a negative error number on failure.
*/
int32_t krun_set_smbios_oem_strings(uint32_t ctx_id, const char *const oem_strings[]);
I used char (btw it should be unsigned char) because the SMBIOS use a u8 to define the number of strings, and I think it's better to have these restriction in the type system than as a runtime check.
There was a problem hiding this comment.
I just tested it more, and I got a segfault, we need to explicitly ask for a NULL terminator (but the from_raw_part is still UB)
There was a problem hiding this comment.
I just realized that inttypes.h is included... instead of unsigned char I could have use uint8_t
germag
force-pushed
the
add-smbios-support
branch
from
612f706
to
a5f590d
Compare
Add a new API `krun_set_smbios_oem_strings()` to set SMBIOS OEM Strings (type 11), and all the internal machinery to provide them to the firmware. The EDK2 firmware expect the SMBIOS tables on address 0x4000_F000. Currently, this is only supported on aarch64 architectures. Signed-off-by: German Maglione <gmaglione@redhat.com>
germag
force-pushed
the
add-smbios-support
branch
from
a5f590d
to
a49bda8
Compare
This PR adds support for SMBIOS, specifically to be able to set
SMBIOS OEM Strings. The main use case is to pass credentials to
systemd[0], this is a requirement to support krunkit on podman-bootc[1].
It adds a new API
krun_set_smbios_oem_strings()to set SMBIOS OEMStrings (type 11), and all the internal machinery to provide them to
the firmware. The included EDK2 firmware expects the SMBIOS tables on
address 0x4000_F000. This is only supported on aarch64 architectures.
[0] https://systemd.io/CREDENTIALS/
[1] https://github.com/containers/podman-bootc/