[PATCH 00/19] Support SMBIOS 3.0 entry points
This series implements support for SMBIOS 3.0 entry points in SeaBIOS. The main advantage of SMBIOS 3.0 entry points is the higher limit for total table size. The SMBIOS 2.1 64435 bytes limit can be easily hit in QEMU if creating virtual machines with more than 720 VCPUs. Eduardo Habkost (19): biostables: copy_fseg_table() function util.h: Delete unused get_smbios_entry_point() prototype smbios: Rename code specific for SMBIOS 2.1 entry points smbios: Generic smbios_next() function smbios: smbios_get_tables() function smbios: Use smbios_get_tables()/smbios_next() at display_uuid() smbios: smbios_major_version()/smbios_minor_version() helpers tpm: Use smbios_get_tables() csm: Don't check SMBios21Addr before calling copy_smbios_21() smbios: Make SMBios21Addr variable static smbios: Use smbios_next() at smbios_romfile_setup() smbios: Extract SMBIOS table building code to separate function smbios: Make smbios_build_tables() more generic smbios: smbios_21_setup_entry_point() function smbios: Make some smbios_build_tables() arguments optional smbios: Make smbios_build_tables() ready for 64-bit tables smbios: copy_smbios_30() function smbios: Support SMBIOS 3.0 entry point at copy_table() smbios: Support SMBIOS 3.0 entry point at smbios_romfile_setup() src/std/smbios.h | 17 ++- src/util.h | 5 +- src/fw/biostables.c | 259 +++++++++++++++++++++++++++++++++----------- src/fw/csm.c | 6 +- src/fw/smbios.c | 12 +- src/tcgbios.c | 12 +- 6 files changed, 227 insertions(+), 84 deletions(-) -- 2.28.0
Replace the common malloc_fseg() + memcpy() code pattern with a helper function. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- src/fw/biostables.c | 40 ++++++++++++++++------------------------ 1 file changed, 16 insertions(+), 24 deletions(-) diff --git a/src/fw/biostables.c b/src/fw/biostables.c index 794b5be8..a6f316fa 100644 --- a/src/fw/biostables.c +++ b/src/fw/biostables.c @@ -21,6 +21,19 @@ struct pir_header *PirAddr VARFSEG; +static void * +copy_fseg_table(const char *name, void *pos, u32 size) +{ + void *newpos = malloc_fseg(size); + if (!newpos) { + warn_noalloc(); + return NULL; + } + dprintf(1, "Copying %s from %p to %p\n", name, pos, newpos); + memcpy(newpos, pos, size); + return newpos; +} + void copy_pir(void *pos) { @@ -33,14 +46,7 @@ copy_pir(void *pos) return; if (checksum(pos, p->size) != 0) return; - void *newpos = malloc_fseg(p->size); - if (!newpos) { - warn_noalloc(); - return; - } - dprintf(1, "Copying PIR from %p to %p\n", pos, newpos); - memcpy(newpos, pos, p->size); - PirAddr = newpos; + PirAddr = copy_fseg_table("PIR", pos, p->size); } void @@ -111,14 +117,7 @@ copy_acpi_rsdp(void *pos) int length = get_acpi_rsdp_length(pos, -1); if (length < 0) return; - void *newpos = malloc_fseg(length); - if (!newpos) { - warn_noalloc(); - return; - } - dprintf(1, "Copying ACPI RSDP from %p to %p\n", pos, newpos); - memcpy(newpos, pos, length); - RsdpAddr = newpos; + RsdpAddr = copy_fseg_table("ACPI RSDP", pos, length); } void *find_acpi_rsdp(void) @@ -305,14 +304,7 @@ copy_smbios(void *pos) return; if (checksum(pos+0x10, p->length-0x10) != 0) return; - struct smbios_entry_point *newpos = malloc_fseg(p->length); - if (!newpos) { - warn_noalloc(); - return; - } - dprintf(1, "Copying SMBIOS entry point from %p to %p\n", pos, newpos); - memcpy(newpos, pos, p->length); - SMBiosAddr = newpos; + SMBiosAddr = copy_fseg_table("SMBIOS", pos, p->length); } void -- 2.28.0
The get_smbios_entry_point() function doesn't exist, delete unused prototype. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- src/util.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/util.h b/src/util.h index 0de35229..652ca6b1 100644 --- a/src/util.h +++ b/src/util.h @@ -88,7 +88,6 @@ u32 find_resume_vector(void); void acpi_reboot(void); void find_acpi_features(void); extern struct smbios_entry_point *SMBiosAddr; -struct smbios_entry_point *get_smbios_entry_point(); void copy_smbios(void *pos); void display_uuid(void); void copy_table(void *pos); -- 2.28.0
Rename copy_smbios(), smbios_next(), SMBIOS_SIGNATURE, smbios_entry_point, and SMBiosAddr, to indicate they refer to SMBIOS 2.1 entry points. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- src/std/smbios.h | 4 ++-- src/util.h | 4 ++-- src/fw/biostables.c | 36 ++++++++++++++++++------------------ src/fw/csm.c | 6 +++--- src/fw/smbios.c | 12 ++++++------ src/tcgbios.c | 4 ++-- 6 files changed, 33 insertions(+), 33 deletions(-) diff --git a/src/std/smbios.h b/src/std/smbios.h index 4ccf2ea3..17fdfed6 100644 --- a/src/std/smbios.h +++ b/src/std/smbios.h @@ -3,12 +3,12 @@ #include "types.h" // u32 -#define SMBIOS_SIGNATURE 0x5f4d535f // "_SM_" +#define SMBIOS_21_SIGNATURE 0x5f4d535f // "_SM_" /* SMBIOS entry point -- must be written to a 16-bit aligned address between 0xf0000 and 0xfffff. */ -struct smbios_entry_point { +struct smbios_21_entry_point { u32 signature; u8 checksum; u8 length; diff --git a/src/util.h b/src/util.h index 652ca6b1..f761271c 100644 --- a/src/util.h +++ b/src/util.h @@ -87,8 +87,8 @@ void *find_acpi_table(u32 signature); u32 find_resume_vector(void); void acpi_reboot(void); void find_acpi_features(void); -extern struct smbios_entry_point *SMBiosAddr; -void copy_smbios(void *pos); +extern struct smbios_21_entry_point *SMBios21Addr; +void copy_smbios_21(void *pos); void display_uuid(void); void copy_table(void *pos); void smbios_setup(void); diff --git a/src/fw/biostables.c b/src/fw/biostables.c index a6f316fa..8e53ae2e 100644 --- a/src/fw/biostables.c +++ b/src/fw/biostables.c @@ -14,7 +14,7 @@ #include "std/acpi.h" // struct rsdp_descriptor #include "std/mptable.h" // MPTABLE_SIGNATURE #include "std/pirtable.h" // struct pir_header -#include "std/smbios.h" // struct smbios_entry_point +#include "std/smbios.h" // struct smbios_21_entry_point #include "string.h" // memcpy #include "util.h" // copy_table #include "x86.h" // outb @@ -265,7 +265,7 @@ find_acpi_features(void) // Iterator for each sub-table in the smbios blob. void * -smbios_next(struct smbios_entry_point *smbios, void *prev) +smbios_21_next(struct smbios_21_entry_point *smbios, void *prev) { if (!smbios) return NULL; @@ -288,15 +288,15 @@ smbios_next(struct smbios_entry_point *smbios, void *prev) return prev; } -struct smbios_entry_point *SMBiosAddr; +struct smbios_21_entry_point *SMBios21Addr; void -copy_smbios(void *pos) +copy_smbios_21(void *pos) { - if (SMBiosAddr) + if (SMBios21Addr) return; - struct smbios_entry_point *p = pos; - if (p->signature != SMBIOS_SIGNATURE) + struct smbios_21_entry_point *p = pos; + if (p->signature != SMBIOS_21_SIGNATURE) return; if (checksum(pos, 0x10) != 0) return; @@ -304,15 +304,15 @@ copy_smbios(void *pos) return; if (checksum(pos+0x10, p->length-0x10) != 0) return; - SMBiosAddr = copy_fseg_table("SMBIOS", pos, p->length); + SMBios21Addr = copy_fseg_table("SMBIOS", pos, p->length); } void display_uuid(void) { - struct smbios_type_1 *tbl = smbios_next(SMBiosAddr, NULL); + struct smbios_type_1 *tbl = smbios_21_next(SMBios21Addr, NULL); int minlen = offsetof(struct smbios_type_1, uuid) + sizeof(tbl->uuid); - for (; tbl; tbl = smbios_next(SMBiosAddr, tbl)) + for (; tbl; tbl = smbios_21_next(SMBios21Addr, tbl)) if (tbl->header.type == 1 && tbl->header.length >= minlen) { u8 *uuid = tbl->uuid; u8 empty_uuid[sizeof(tbl->uuid)] = { 0 }; @@ -325,9 +325,9 @@ display_uuid(void) * the encoding, but we follow dmidecode and assume big-endian * encoding. */ - if (SMBiosAddr->smbios_major_version > 2 || - (SMBiosAddr->smbios_major_version == 2 && - SMBiosAddr->smbios_minor_version >= 6)) { + if (SMBios21Addr->smbios_major_version > 2 || + (SMBios21Addr->smbios_major_version == 2 && + SMBios21Addr->smbios_minor_version >= 6)) { printf("Machine UUID" " %02x%02x%02x%02x" "-%02x%02x" @@ -421,7 +421,7 @@ smbios_romfile_setup(void) { struct romfile_s *f_anchor = romfile_find("etc/smbios/smbios-anchor"); struct romfile_s *f_tables = romfile_find("etc/smbios/smbios-tables"); - struct smbios_entry_point ep; + struct smbios_21_entry_point ep; struct smbios_type_0 *t0; u16 qtables_len, need_t0 = 1; u8 *qtables, *tables; @@ -440,10 +440,10 @@ smbios_romfile_setup(void) return 0; } f_tables->copy(f_tables, qtables, f_tables->size); - ep.structure_table_address = (u32)qtables; /* for smbios_next(), below */ + ep.structure_table_address = (u32)qtables; /* for smbios_21_next(), below */ /* did we get a type 0 structure ? */ - for (t0 = smbios_next(&ep, NULL); t0; t0 = smbios_next(&ep, t0)) + for (t0 = smbios_21_next(&ep, NULL); t0; t0 = smbios_21_next(&ep, t0)) if (t0->header.type == 0) { need_t0 = 0; break; @@ -488,7 +488,7 @@ smbios_romfile_setup(void) ep.checksum -= checksum(&ep, 0x10); ep.intermediate_checksum -= checksum((void *)&ep + 0x10, ep.length - 0x10); - copy_smbios(&ep); + copy_smbios_21(&ep); return 1; } @@ -506,5 +506,5 @@ copy_table(void *pos) copy_pir(pos); copy_mptable(pos); copy_acpi_rsdp(pos); - copy_smbios(pos); + copy_smbios_21(pos); } diff --git a/src/fw/csm.c b/src/fw/csm.c index 8359bcba..74069028 100644 --- a/src/fw/csm.c +++ b/src/fw/csm.c @@ -19,7 +19,7 @@ #include "std/acpi.h" // RSDP_SIGNATURE #include "std/bda.h" // struct bios_data_area_s #include "std/optionrom.h" // struct rom_header -#include "util.h" // copy_smbios +#include "util.h" // copy_smbios_21 #define UINT8 u8 #define UINT16 u16 @@ -172,8 +172,8 @@ handle_csm_0002(struct bregs *regs) // SMBIOS table needs to be copied into the f-seg // XX: OVMF doesn't seem to set SmbiosTableLength so don't check it - if (csm_boot_table->SmbiosTable && !SMBiosAddr) - copy_smbios((void *)csm_boot_table->SmbiosTable); + if (csm_boot_table->SmbiosTable && !SMBios21Addr) + copy_smbios_21((void *)csm_boot_table->SmbiosTable); // MPTABLE is just there; we don't care where. diff --git a/src/fw/smbios.c b/src/fw/smbios.c index 62a563b2..730b6898 100644 --- a/src/fw/smbios.c +++ b/src/fw/smbios.c @@ -11,13 +11,13 @@ #include "output.h" // dprintf #include "paravirt.h" // RamSize #include "romfile.h" // romfile_findprefix -#include "std/smbios.h" // struct smbios_entry_point +#include "std/smbios.h" // struct smbios_21_entry_point #include "string.h" // memset #include "util.h" // MaxCountCPUs #include "x86.h" // cpuid static void -smbios_entry_point_setup(u16 max_structure_size, +smbios_21_entry_point_setup(u16 max_structure_size, u16 structure_table_length, void *structure_table_address, u16 number_of_structures) @@ -35,9 +35,9 @@ smbios_entry_point_setup(u16 max_structure_size, } memcpy(finaltable, structure_table_address, structure_table_length); - struct smbios_entry_point ep; + struct smbios_21_entry_point ep; memset(&ep, 0, sizeof(ep)); - ep.signature = SMBIOS_SIGNATURE; + ep.signature = SMBIOS_21_SIGNATURE; ep.length = 0x1f; ep.smbios_major_version = 2; ep.smbios_minor_version = 4; @@ -53,7 +53,7 @@ smbios_entry_point_setup(u16 max_structure_size, ep.intermediate_checksum -= checksum((void*)&ep + 0x10, ep.length - 0x10); - copy_smbios(&ep); + copy_smbios_21(&ep); } static int @@ -584,6 +584,6 @@ smbios_legacy_setup(void) #undef add_struct - smbios_entry_point_setup(max_struct_size, p - start, start, nr_structs); + smbios_21_entry_point_setup(max_struct_size, p - start, start, nr_structs); free(start); } diff --git a/src/tcgbios.c b/src/tcgbios.c index 82894f5c..148c0220 100644 --- a/src/tcgbios.c +++ b/src/tcgbios.c @@ -18,7 +18,7 @@ #include "output.h" // dprintf #include "sha1.h" // sha1 #include "std/acpi.h" // RSDP_SIGNATURE, rsdt_descriptor -#include "std/smbios.h" // struct smbios_entry_point +#include "std/smbios.h" // struct smbios_21_entry_point #include "std/tcg.h" // TCG_PC_LOGOVERFLOW #include "string.h" // checksum #include "tcgbios.h"// tpm_*, prototypes @@ -1028,7 +1028,7 @@ tpm_smbios_measure(void) .eventid = 1, .eventdatasize = SHA1_BUFSIZE, }; - struct smbios_entry_point *sep = SMBiosAddr; + struct smbios_21_entry_point *sep = SMBios21Addr; dprintf(DEBUG_tcg, "TCGBIOS: SMBIOS at %p\n", sep); -- 2.28.0
Extract generic code from smbios_21_next(), so it can be reused for SMBIOS 3.0 support. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- src/fw/biostables.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/fw/biostables.c b/src/fw/biostables.c index 8e53ae2e..3c30331e 100644 --- a/src/fw/biostables.c +++ b/src/fw/biostables.c @@ -265,12 +265,11 @@ find_acpi_features(void) // Iterator for each sub-table in the smbios blob. void * -smbios_21_next(struct smbios_21_entry_point *smbios, void *prev) +smbios_next(void *start, u32 length, void *prev) { - if (!smbios) + if (!start) return NULL; - void *start = (void*)smbios->structure_table_address; - void *end = start + smbios->structure_table_length; + void *end = start + length; if (!prev) { prev = start; @@ -288,6 +287,15 @@ smbios_21_next(struct smbios_21_entry_point *smbios, void *prev) return prev; } +void * +smbios_21_next(struct smbios_21_entry_point *smbios, void *prev) +{ + if (!smbios) + return NULL; + return smbios_next((void*)smbios->structure_table_address, + smbios->structure_table_length, prev); +} + struct smbios_21_entry_point *SMBios21Addr; void -- 2.28.0
The new function will be useful for code that just needs the address and length of SMBIOS tables. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- src/util.h | 1 + src/fw/biostables.c | 9 +++++++++ 2 files changed, 10 insertions(+) diff --git a/src/util.h b/src/util.h index f761271c..2e151752 100644 --- a/src/util.h +++ b/src/util.h @@ -88,6 +88,7 @@ u32 find_resume_vector(void); void acpi_reboot(void); void find_acpi_features(void); extern struct smbios_21_entry_point *SMBios21Addr; +void *smbios_get_tables(u32 *length); void copy_smbios_21(void *pos); void display_uuid(void); void copy_table(void *pos); diff --git a/src/fw/biostables.c b/src/fw/biostables.c index 3c30331e..1adc694c 100644 --- a/src/fw/biostables.c +++ b/src/fw/biostables.c @@ -315,6 +315,15 @@ copy_smbios_21(void *pos) SMBios21Addr = copy_fseg_table("SMBIOS", pos, p->length); } +void *smbios_get_tables(u32 *length) +{ + if (SMBios21Addr) { + *length = SMBios21Addr->structure_table_length; + return (void *)SMBios21Addr->structure_table_address; + } + return NULL; +} + void display_uuid(void) { -- 2.28.0
Make the code more generic, and not specific for SMBIOS 2.1 entry points. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- src/fw/biostables.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/fw/biostables.c b/src/fw/biostables.c index 1adc694c..d22ab643 100644 --- a/src/fw/biostables.c +++ b/src/fw/biostables.c @@ -327,9 +327,11 @@ void *smbios_get_tables(u32 *length) void display_uuid(void) { - struct smbios_type_1 *tbl = smbios_21_next(SMBios21Addr, NULL); + u32 smbios_len = 0; + void *smbios_tables = smbios_get_tables(&smbios_len); + struct smbios_type_1 *tbl = smbios_next(smbios_tables, smbios_len, NULL); int minlen = offsetof(struct smbios_type_1, uuid) + sizeof(tbl->uuid); - for (; tbl; tbl = smbios_21_next(SMBios21Addr, tbl)) + for (; tbl; tbl = smbios_next(smbios_tables, smbios_len, tbl)) if (tbl->header.type == 1 && tbl->header.length >= minlen) { u8 *uuid = tbl->uuid; u8 empty_uuid[sizeof(tbl->uuid)] = { 0 }; -- 2.28.0
They new helpers will be useful when we start supporting SMBIOS 3.0 entry points. Use the new helpers at display_uuid(). Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- src/fw/biostables.c | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/src/fw/biostables.c b/src/fw/biostables.c index d22ab643..10eaa369 100644 --- a/src/fw/biostables.c +++ b/src/fw/biostables.c @@ -324,6 +324,24 @@ void *smbios_get_tables(u32 *length) return NULL; } +static int +smbios_major_version(void) +{ + if (SMBios21Addr) + return SMBios21Addr->smbios_major_version; + else + return 0; +} + +static int +smbios_minor_version(void) +{ + if (SMBios21Addr) + return SMBios21Addr->smbios_minor_version; + else + return 0; +} + void display_uuid(void) { @@ -344,9 +362,9 @@ display_uuid(void) * the encoding, but we follow dmidecode and assume big-endian * encoding. */ - if (SMBios21Addr->smbios_major_version > 2 || - (SMBios21Addr->smbios_major_version == 2 && - SMBios21Addr->smbios_minor_version >= 6)) { + if (smbios_major_version() > 2 || + (smbios_major_version() == 2 && + smbios_minor_version() >= 6)) { printf("Machine UUID" " %02x%02x%02x%02x" "-%02x%02x" -- 2.28.0
Instead of using the SMBios21Addr global variable, use the smbios_get_tables() helper. This doesn't change any behavior yet, but it will be useful when we start supporting SMBIOS 3.0 entry points. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- src/tcgbios.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/tcgbios.c b/src/tcgbios.c index 148c0220..841da708 100644 --- a/src/tcgbios.c +++ b/src/tcgbios.c @@ -1028,15 +1028,15 @@ tpm_smbios_measure(void) .eventid = 1, .eventdatasize = SHA1_BUFSIZE, }; - struct smbios_21_entry_point *sep = SMBios21Addr; + u32 smbios_len; + void *smbios_tables = smbios_get_tables(&smbios_len); - dprintf(DEBUG_tcg, "TCGBIOS: SMBIOS at %p\n", sep); + dprintf(DEBUG_tcg, "TCGBIOS: SMBIOS tables at %p\n", smbios_tables); - if (!sep) + if (!smbios_tables) return; - sha1((const u8 *)sep->structure_table_address, - sep->structure_table_length, pcctes.digest); + sha1((const u8 *)smbios_tables, smbios_len, pcctes.digest); tpm_add_measurement_to_log(1, EV_EVENT_TAG, (const char *)&pcctes, sizeof(pcctes), -- 2.28.0
copy_smbios_21() already checks if SMBios21Addr is NULL, there's no need to check it before calling the function. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- src/fw/csm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fw/csm.c b/src/fw/csm.c index 74069028..bc14a925 100644 --- a/src/fw/csm.c +++ b/src/fw/csm.c @@ -172,7 +172,7 @@ handle_csm_0002(struct bregs *regs) // SMBIOS table needs to be copied into the f-seg // XX: OVMF doesn't seem to set SmbiosTableLength so don't check it - if (csm_boot_table->SmbiosTable && !SMBios21Addr) + if (csm_boot_table->SmbiosTable) copy_smbios_21((void *)csm_boot_table->SmbiosTable); // MPTABLE is just there; we don't care where. -- 2.28.0
The variable is now only used inside biostables.c, there's no need to make it global. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- src/util.h | 1 - src/fw/biostables.c | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/util.h b/src/util.h index 2e151752..aff8e888 100644 --- a/src/util.h +++ b/src/util.h @@ -87,7 +87,6 @@ void *find_acpi_table(u32 signature); u32 find_resume_vector(void); void acpi_reboot(void); void find_acpi_features(void); -extern struct smbios_21_entry_point *SMBios21Addr; void *smbios_get_tables(u32 *length); void copy_smbios_21(void *pos); void display_uuid(void); diff --git a/src/fw/biostables.c b/src/fw/biostables.c index 10eaa369..47463be9 100644 --- a/src/fw/biostables.c +++ b/src/fw/biostables.c @@ -296,7 +296,7 @@ smbios_21_next(struct smbios_21_entry_point *smbios, void *prev) smbios->structure_table_length, prev); } -struct smbios_21_entry_point *SMBios21Addr; +static struct smbios_21_entry_point *SMBios21Addr; void copy_smbios_21(void *pos) -- 2.28.0
Use smbios_next() instead of smbios_21_next(), to make the code more generic and reusable for SMBIOS 3.0 support. Note that `qtables_len` is initialized to `ftables->size` instead of `ep.structure_table_length` now, but both fields are guaranteed to have exactly the same value. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- src/fw/biostables.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/fw/biostables.c b/src/fw/biostables.c index 47463be9..11eaeaec 100644 --- a/src/fw/biostables.c +++ b/src/fw/biostables.c @@ -477,16 +477,17 @@ smbios_romfile_setup(void) return 0; } f_tables->copy(f_tables, qtables, f_tables->size); - ep.structure_table_address = (u32)qtables; /* for smbios_21_next(), below */ + qtables_len = f_tables->size; /* did we get a type 0 structure ? */ - for (t0 = smbios_21_next(&ep, NULL); t0; t0 = smbios_21_next(&ep, t0)) + for (t0 = smbios_next(qtables, qtables_len, NULL); t0; + t0 = smbios_next(qtables, qtables_len, t0)) { if (t0->header.type == 0) { need_t0 = 0; break; } + } - qtables_len = ep.structure_table_length; if (need_t0) { /* common case: add our own type 0, with 3 strings and 4 '\0's */ u16 t0_len = sizeof(struct smbios_type_0) + strlen(BIOS_NAME) + -- 2.28.0
Move the code that builds the SMBIOS tables to a separate smbios_build_tables() function, to keep it isolated from the code that initializes the SMBIOS entry point. Thew new function will still take a smbios_21_entry_point argument to make code review easier, but this will be changed by the next commits. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- src/fw/biostables.c | 54 ++++++++++++++++++++++++++++----------------- 1 file changed, 34 insertions(+), 20 deletions(-) diff --git a/src/fw/biostables.c b/src/fw/biostables.c index 11eaeaec..5eeceb52 100644 --- a/src/fw/biostables.c +++ b/src/fw/biostables.c @@ -453,22 +453,19 @@ smbios_new_type_0(void *start, #define BIOS_NAME "SeaBIOS" #define BIOS_DATE "04/01/2014" +/* + * Build tables using qtables as input, adding additional type 0 + * table if necessary. + */ static int -smbios_romfile_setup(void) +smbios_build_tables(struct romfile_s *f_tables, + struct smbios_21_entry_point *ep) { - struct romfile_s *f_anchor = romfile_find("etc/smbios/smbios-anchor"); - struct romfile_s *f_tables = romfile_find("etc/smbios/smbios-tables"); - struct smbios_21_entry_point ep; struct smbios_type_0 *t0; u16 qtables_len, need_t0 = 1; u8 *qtables, *tables; - if (!f_anchor || !f_tables || f_anchor->size != sizeof(ep)) - return 0; - - f_anchor->copy(f_anchor, &ep, f_anchor->size); - - if (f_tables->size != ep.structure_table_length) + if (f_tables->size != ep->structure_table_length) return 0; qtables = malloc_tmphigh(f_tables->size); @@ -492,35 +489,52 @@ smbios_romfile_setup(void) /* common case: add our own type 0, with 3 strings and 4 '\0's */ u16 t0_len = sizeof(struct smbios_type_0) + strlen(BIOS_NAME) + strlen(VERSION) + strlen(BIOS_DATE) + 4; - if (t0_len > (0xffff - ep.structure_table_length)) { + if (t0_len > (0xffff - ep->structure_table_length)) { dprintf(1, "Insufficient space (%d bytes) to add SMBIOS type 0 table (%d bytes)\n", - 0xffff - ep.structure_table_length, t0_len); + 0xffff - ep->structure_table_length, t0_len); need_t0 = 0; } else { - ep.structure_table_length += t0_len; - if (t0_len > ep.max_structure_size) - ep.max_structure_size = t0_len; - ep.number_of_structures++; + ep->structure_table_length += t0_len; + if (t0_len > ep->max_structure_size) + ep->max_structure_size = t0_len; + ep->number_of_structures++; } } /* allocate final blob and record its address in the entry point */ - if (ep.structure_table_length > BUILD_MAX_SMBIOS_FSEG) - tables = malloc_high(ep.structure_table_length); + if (ep->structure_table_length > BUILD_MAX_SMBIOS_FSEG) + tables = malloc_high(ep->structure_table_length); else - tables = malloc_fseg(ep.structure_table_length); + tables = malloc_fseg(ep->structure_table_length); if (!tables) { warn_noalloc(); free(qtables); return 0; } - ep.structure_table_address = (u32)tables; + ep->structure_table_address = (u32)tables; /* populate final blob */ if (need_t0) tables = smbios_new_type_0(tables, BIOS_NAME, VERSION, BIOS_DATE); memcpy(tables, qtables, qtables_len); free(qtables); + return 1; +} + +static int +smbios_romfile_setup(void) +{ + struct romfile_s *f_anchor = romfile_find("etc/smbios/smbios-anchor"); + struct romfile_s *f_tables = romfile_find("etc/smbios/smbios-tables"); + struct smbios_21_entry_point ep; + + if (!f_anchor || !f_tables || f_anchor->size != sizeof(ep)) + return 0; + + f_anchor->copy(f_anchor, &ep, f_anchor->size); + + if (!smbios_build_tables(f_tables, &ep)) + return 0; /* finalize entry point */ ep.checksum -= checksum(&ep, 0x10); -- 2.28.0
Instead of taking a SMBIOS 2.1 entry point as argument, make smbios_build_tables() take pointers to the fields it actually changes. This will allow us to reuse the function for SMBIOS 3.0 later. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- src/fw/biostables.c | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/src/fw/biostables.c b/src/fw/biostables.c index 5eeceb52..f0aa7ed3 100644 --- a/src/fw/biostables.c +++ b/src/fw/biostables.c @@ -459,13 +459,15 @@ smbios_new_type_0(void *start, */ static int smbios_build_tables(struct romfile_s *f_tables, - struct smbios_21_entry_point *ep) + u32 *address, u16 *length, + u16 *max_structure_size, + u16 *number_of_structures) { struct smbios_type_0 *t0; u16 qtables_len, need_t0 = 1; u8 *qtables, *tables; - if (f_tables->size != ep->structure_table_length) + if (f_tables->size != *length) return 0; qtables = malloc_tmphigh(f_tables->size); @@ -489,29 +491,29 @@ smbios_build_tables(struct romfile_s *f_tables, /* common case: add our own type 0, with 3 strings and 4 '\0's */ u16 t0_len = sizeof(struct smbios_type_0) + strlen(BIOS_NAME) + strlen(VERSION) + strlen(BIOS_DATE) + 4; - if (t0_len > (0xffff - ep->structure_table_length)) { + if (t0_len > (0xffff - *length)) { dprintf(1, "Insufficient space (%d bytes) to add SMBIOS type 0 table (%d bytes)\n", - 0xffff - ep->structure_table_length, t0_len); + 0xffff - *length, t0_len); need_t0 = 0; } else { - ep->structure_table_length += t0_len; - if (t0_len > ep->max_structure_size) - ep->max_structure_size = t0_len; - ep->number_of_structures++; + *length += t0_len; + if (t0_len > *max_structure_size) + *max_structure_size = t0_len; + (*number_of_structures)++; } } /* allocate final blob and record its address in the entry point */ - if (ep->structure_table_length > BUILD_MAX_SMBIOS_FSEG) - tables = malloc_high(ep->structure_table_length); + if (*length > BUILD_MAX_SMBIOS_FSEG) + tables = malloc_high(*length); else - tables = malloc_fseg(ep->structure_table_length); + tables = malloc_fseg(*length); if (!tables) { warn_noalloc(); free(qtables); return 0; } - ep->structure_table_address = (u32)tables; + *address = (u32)tables; /* populate final blob */ if (need_t0) @@ -533,7 +535,11 @@ smbios_romfile_setup(void) f_anchor->copy(f_anchor, &ep, f_anchor->size); - if (!smbios_build_tables(f_tables, &ep)) + if (!smbios_build_tables(f_tables, + &ep.structure_table_address, + &ep.structure_table_length, + &ep.max_structure_size, + &ep.number_of_structures)) return 0; /* finalize entry point */ -- 2.28.0
Extract the code specific for building the SMBIOS 2.1 entry point from smbios_romfile_setup() to a new smbios_21_setup_entry_point() function. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- src/fw/biostables.c | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/src/fw/biostables.c b/src/fw/biostables.c index f0aa7ed3..559a5867 100644 --- a/src/fw/biostables.c +++ b/src/fw/biostables.c @@ -523,6 +523,25 @@ smbios_build_tables(struct romfile_s *f_tables, return 1; } +static int +smbios_21_setup_entry_point(struct romfile_s *f_tables, + struct smbios_21_entry_point *ep) +{ + if (!smbios_build_tables(f_tables, + &ep->structure_table_address, + &ep->structure_table_length, + &ep->max_structure_size, + &ep->number_of_structures)) + return 0; + + /* finalize entry point */ + ep->checksum -= checksum(ep, 0x10); + ep->intermediate_checksum -= checksum((void *)ep + 0x10, ep->length - 0x10); + + copy_smbios_21(ep); + return 1; +} + static int smbios_romfile_setup(void) { @@ -535,19 +554,7 @@ smbios_romfile_setup(void) f_anchor->copy(f_anchor, &ep, f_anchor->size); - if (!smbios_build_tables(f_tables, - &ep.structure_table_address, - &ep.structure_table_length, - &ep.max_structure_size, - &ep.number_of_structures)) - return 0; - - /* finalize entry point */ - ep.checksum -= checksum(&ep, 0x10); - ep.intermediate_checksum -= checksum((void *)&ep + 0x10, ep.length - 0x10); - - copy_smbios_21(&ep); - return 1; + return smbios_21_setup_entry_point(f_tables, &ep); } void -- 2.28.0
Make max_structure_size and number_of_structures optional, as we don't have those fields in SMBIOS 3.0 entry points. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- src/fw/biostables.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/fw/biostables.c b/src/fw/biostables.c index 559a5867..d65d91ab 100644 --- a/src/fw/biostables.c +++ b/src/fw/biostables.c @@ -456,6 +456,9 @@ smbios_new_type_0(void *start, /* * Build tables using qtables as input, adding additional type 0 * table if necessary. + * + * @address and @length can't be NULL. @max_structure_size and + * @number_of_structures are optional and can be NULL. */ static int smbios_build_tables(struct romfile_s *f_tables, @@ -497,9 +500,10 @@ smbios_build_tables(struct romfile_s *f_tables, need_t0 = 0; } else { *length += t0_len; - if (t0_len > *max_structure_size) + if (max_structure_size && t0_len > *max_structure_size) *max_structure_size = t0_len; - (*number_of_structures)++; + if (number_of_structures) + (*number_of_structures)++; } } -- 2.28.0
Make smbios_build_tables() get u64 address and u32 length arguments, making it usable for SMBIOS 3.0. Adapt smbios_21_setup_entry_point() to use intermediate variables when calling smbios_build_tables(). Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- src/fw/biostables.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/fw/biostables.c b/src/fw/biostables.c index d65d91ab..91fe7470 100644 --- a/src/fw/biostables.c +++ b/src/fw/biostables.c @@ -462,7 +462,7 @@ smbios_new_type_0(void *start, */ static int smbios_build_tables(struct romfile_s *f_tables, - u32 *address, u16 *length, + u64 *address, u32 *length, u16 *max_structure_size, u16 *number_of_structures) { @@ -531,14 +531,24 @@ static int smbios_21_setup_entry_point(struct romfile_s *f_tables, struct smbios_21_entry_point *ep) { + u64 address = ep->structure_table_address; + u32 length = ep->structure_table_length; + if (!smbios_build_tables(f_tables, - &ep->structure_table_address, - &ep->structure_table_length, + &address, + &length, &ep->max_structure_size, &ep->number_of_structures)) return 0; + if ((u32)address != address || (u16)length != length) { + warn_internalerror(); + return 0; + } + /* finalize entry point */ + ep->structure_table_address = address; + ep->structure_table_length = length; ep->checksum -= checksum(ep, 0x10); ep->intermediate_checksum -= checksum((void *)ep + 0x10, ep->length - 0x10); -- 2.28.0
Add new copy_smbios_30() function, that will be used to support SMBIOS 3.0 entry points. The SMBIOS 3.0 entry point will be tracked in a separate SMBios30Addr variable, because both 2.1 and 3.0 entry points may exist at the same time. Adjust the smbios_get_tables(), smbios_major_version(), and smbios_minor_version() helpers to use the SMBIOS 3.0 entry point if available. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- src/std/smbios.h | 13 +++++++++++++ src/fw/biostables.c | 36 ++++++++++++++++++++++++++++++++++-- 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/src/std/smbios.h b/src/std/smbios.h index 17fdfed6..208440b1 100644 --- a/src/std/smbios.h +++ b/src/std/smbios.h @@ -25,6 +25,19 @@ struct smbios_21_entry_point { u8 smbios_bcd_revision; } PACKED; +struct smbios_30_entry_point { + char signature[5]; + u8 checksum; + u8 length; + u8 smbios_major_version; + u8 smbios_minor_version; + u8 smbios_docrev; + u8 entry_point_revision; + u8 reserved; + u32 structure_table_max_size; + u64 structure_table_address; +} PACKED; + /* This goes at the beginning of every SMBIOS structure. */ struct smbios_structure_header { u8 type; diff --git a/src/fw/biostables.c b/src/fw/biostables.c index 91fe7470..b2c84a12 100644 --- a/src/fw/biostables.c +++ b/src/fw/biostables.c @@ -315,8 +315,36 @@ copy_smbios_21(void *pos) SMBios21Addr = copy_fseg_table("SMBIOS", pos, p->length); } +static struct smbios_30_entry_point *SMBios30Addr; + +static int +valid_smbios_30_signature(struct smbios_30_entry_point *p) +{ + return !memcmp(p->signature, "_SM3_", 5); +} + +void +copy_smbios_30(void *pos) +{ + if (SMBios30Addr) + return; + struct smbios_30_entry_point *p = pos; + if (!valid_smbios_30_signature(p)) + return; + if (checksum(pos, p->length) != 0) + return; + SMBios30Addr = copy_fseg_table("SMBIOS 3.0", pos, p->length); +} + void *smbios_get_tables(u32 *length) { + if (SMBios30Addr) { + u32 addr32 = SMBios30Addr->structure_table_address; + if (addr32 == SMBios30Addr->structure_table_address) { + *length = SMBios30Addr->structure_table_max_size; + return (void *)addr32; + } + } if (SMBios21Addr) { *length = SMBios21Addr->structure_table_length; return (void *)SMBios21Addr->structure_table_address; @@ -327,7 +355,9 @@ void *smbios_get_tables(u32 *length) static int smbios_major_version(void) { - if (SMBios21Addr) + if (SMBios30Addr) + return SMBios30Addr->smbios_major_version; + else if (SMBios21Addr) return SMBios21Addr->smbios_major_version; else return 0; @@ -336,7 +366,9 @@ smbios_major_version(void) static int smbios_minor_version(void) { - if (SMBios21Addr) + if (SMBios30Addr) + return SMBios30Addr->smbios_minor_version; + else if (SMBios21Addr) return SMBios21Addr->smbios_minor_version; else return 0; -- 2.28.0
This will make coreboot code (scan_tables()) and xen code (xen_biostable_setup()) copy SMBIOS 3.0 entry points if found. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- src/fw/biostables.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/fw/biostables.c b/src/fw/biostables.c index b2c84a12..5ff1b0ed 100644 --- a/src/fw/biostables.c +++ b/src/fw/biostables.c @@ -618,4 +618,5 @@ copy_table(void *pos) copy_mptable(pos); copy_acpi_rsdp(pos); copy_smbios_21(pos); + copy_smbios_30(pos); } -- 2.28.0
Support SMBIOS 3.0 entry points if exposed by QEMU in fw_cfg. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- src/fw/biostables.c | 33 ++++++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/src/fw/biostables.c b/src/fw/biostables.c index 5ff1b0ed..1c09a785 100644 --- a/src/fw/biostables.c +++ b/src/fw/biostables.c @@ -588,19 +588,46 @@ smbios_21_setup_entry_point(struct romfile_s *f_tables, return 1; } +static int +smbios_30_setup_entry_point(struct romfile_s *f_tables, + struct smbios_30_entry_point *ep) +{ + if (!smbios_build_tables(f_tables, + &ep->structure_table_address, + &ep->structure_table_max_size, + NULL, NULL)) + return 0; + + ep->checksum -= checksum(ep, sizeof(*ep)); + copy_smbios_30(ep); + return 1; +} + static int smbios_romfile_setup(void) { struct romfile_s *f_anchor = romfile_find("etc/smbios/smbios-anchor"); struct romfile_s *f_tables = romfile_find("etc/smbios/smbios-tables"); - struct smbios_21_entry_point ep; + union { + struct smbios_21_entry_point ep21; + struct smbios_30_entry_point ep30; + } ep; - if (!f_anchor || !f_tables || f_anchor->size != sizeof(ep)) + if (!f_anchor || !f_tables || f_anchor->size > sizeof(ep)) return 0; f_anchor->copy(f_anchor, &ep, f_anchor->size); - return smbios_21_setup_entry_point(f_tables, &ep); + if (f_anchor->size == sizeof(ep.ep21) && + ep.ep21.signature == SMBIOS_21_SIGNATURE) { + return smbios_21_setup_entry_point(f_tables, &ep.ep21); + } else if (f_anchor->size == sizeof(ep.ep30) && + valid_smbios_30_signature(&ep.ep30)) { + return smbios_30_setup_entry_point(f_tables, &ep.ep30); + } else { + dprintf(1, "Invalid SMBIOS signature at etc/smbios/smbios-anchor\n"); + return 0; + } } void -- 2.28.0
On Thu, Dec 10, 2020 at 04:26:21PM -0500, Eduardo Habkost wrote:
This series implements support for SMBIOS 3.0 entry points in
SeaBIOS.
The main advantage of SMBIOS 3.0 entry points is the higher limit for total table size. The SMBIOS 2.1 64435 bytes limit can be easily hit in QEMU if creating virtual machines with more than 720 VCPUs.
Thanks. The series looks good to me. What is the integration plan for merging this and making the associated changes to QEMU? Are you looking to merge this first? I noticed that the new code only has a single qemu smbios "anchor" file - does that mean qemu will only ever use smbios3 or smbios2 and never need to export both simultaneously? -Kevin
On Fri, Dec 11, 2020 at 10:46:00AM -0500, Kevin O'Connor wrote:
On Thu, Dec 10, 2020 at 04:26:21PM -0500, Eduardo Habkost wrote:
This series implements support for SMBIOS 3.0 entry points in
SeaBIOS.
The main advantage of SMBIOS 3.0 entry points is the higher limit for total table size. The SMBIOS 2.1 64435 bytes limit can be easily hit in QEMU if creating virtual machines with more than 720 VCPUs.
Thanks. The series looks good to me.
What is the integration plan for merging this and making the associated changes to QEMU? Are you looking to merge this first?
The QEMU part will be based on this patch by Daniel; https://lore.kernel.org/qemu-devel/20200908165438.1008942-6-berrange@redhat.... I'm looking to merge this before the QEMU option, so non-working configurations (VM configured with SMBIOS 3.0 + lack of SeaBIOS support for SMBIOS 3.0) are less likely.
I noticed that the new code only has a single qemu smbios "anchor" file - does that mean qemu will only ever use smbios3 or smbios2 and never need to export both simultaneously?
QEMU doesn't need to export both simultaneously, as both entry points would contain redundant information. It would be possible to add code to SeaBIOS to generate both entry points for compatibility, but I'm not sure how useful that would be (as compatibility with SMBIOS 2.1 could be easily achieved by simply changing the QEMU configuration). -- Eduardo
On Fri, Dec 11, 2020 at 02:46:29PM -0500, Eduardo Habkost wrote:
On Fri, Dec 11, 2020 at 10:46:00AM -0500, Kevin O'Connor wrote:
What is the integration plan for merging this and making the associated changes to QEMU? Are you looking to merge this first?
The QEMU part will be based on this patch by Daniel; https://lore.kernel.org/qemu-devel/20200908165438.1008942-6-berrange@redhat....
I'm looking to merge this before the QEMU option, so non-working configurations (VM configured with SMBIOS 3.0 + lack of SeaBIOS support for SMBIOS 3.0) are less likely.
Thanks. We generally prefer to merge the QEMU parts first and then merge the SeaBIOS parts. Though there is some flexibility. If desired, I can merge the first 18 parts of this series, and then merge the 19th patch after the QEMU part has passed the necessary reviews. Gerd - do you have any thoughts on this? -Kevin
On Mon, Dec 21, 2020 at 10:54:39AM -0500, Kevin O'Connor wrote:
On Fri, Dec 11, 2020 at 02:46:29PM -0500, Eduardo Habkost wrote:
On Fri, Dec 11, 2020 at 10:46:00AM -0500, Kevin O'Connor wrote:
What is the integration plan for merging this and making the associated changes to QEMU? Are you looking to merge this first?
The QEMU part will be based on this patch by Daniel; https://lore.kernel.org/qemu-devel/20200908165438.1008942-6-berrange@redhat....
I'm looking to merge this before the QEMU option, so non-working configurations (VM configured with SMBIOS 3.0 + lack of SeaBIOS support for SMBIOS 3.0) are less likely.
Thanks. We generally prefer to merge the QEMU parts first and then merge the SeaBIOS parts. Though there is some flexibility.
If desired, I can merge the first 18 parts of this series, and then merge the 19th patch after the QEMU part has passed the necessary reviews.
Gerd - do you have any thoughts on this?
Sounds good to me. This also means we should plan a seabios release for qemu 6.0. We don't have a release schedule yet, but traditionally the first release of the year is in April with freeze in March. So a new seabios release by end of February would be good, although there is some wiggle room as I can update to a snapshot before qemu freeze, then update to the final seabios release in the qemu freeze period. take care, Gerd
On Thu, Dec 10, 2020 at 04:26:21PM -0500, Eduardo Habkost wrote:
This series implements support for SMBIOS 3.0 entry points in
SeaBIOS.
The main advantage of SMBIOS 3.0 entry points is the higher limit for total table size. The SMBIOS 2.1 64435 bytes limit can be easily hit in QEMU if creating virtual machines with more than 720 VCPUs.
Thanks. Somehow this slipped through the cracks. However, I merged this patch series as found at: https://gitlab.com/ehabkost/seabios on the work/smbios-3.0 branch. -Kevin
participants (3)
-
Eduardo Habkost -
Gerd Hoffmann -
Kevin O'Connor