| Message ID | 20240227154749.1818189-9-imammedo@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Workaround Windows failing to find 64bit SMBIOS entry point with SeaBIOS | expand |
On Tue, 27 Feb 2024, Igor Mammedov wrote: > legacy mode doesn't support structures of type 2 and more, > and CLI has a check for '-smbios type' option, however it's > still possible to sneak in type4 as a blob with '-smbios file' > option. However doing the later makes SMBIOS tables broken > since SeaBIOS doesn't expect that. > > Rather than trying to add support for type4 to legacy code > (both QEMU and SeaBIOS), simplify smbios_get_table_legacy() > by dropping not relevant check in legacy code and error out > on type4 blob. > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> Reviewed-by: Ani Sinha <anisinha@redhat.com> > --- > * The issue affects 'isapc' and pc-i440fx-2.0. the later is > in deprecated state and to be dropped in near future > * possibly the same issue applies to other SMBIOS types above type 1 > but I haven't tested that, and well tables that aren't > generated by SeaBIOS can get be added just fine > (tested type11 blob). So I went with a minimal change > to fixup type4 only that I'm touching. Leaving the rest > for other time or when someone complains about it, which is > very unlikely given it's really only remaining isapc machine. > > I'd very much prefer to deprecate 'isapc' and then drop > all legacy related code (it will benefit not only SMBIOS > but other code as well). > BTW: 'isapc' is in semi-dead, I cna't boot RHEL6 on it > with KVM enabled anymore (RHEL9 host), TCG still boots though. > One more reason to get deprecate it. > --- > include/hw/firmware/smbios.h | 2 +- > hw/i386/fw_cfg.c | 3 +-- > hw/smbios/smbios.c | 18 ++++++++++++++---- > 3 files changed, 16 insertions(+), 7 deletions(-) > > diff --git a/include/hw/firmware/smbios.h b/include/hw/firmware/smbios.h > index 0818184834..1fbff3c55f 100644 > --- a/include/hw/firmware/smbios.h > +++ b/include/hw/firmware/smbios.h > @@ -296,7 +296,7 @@ void smbios_set_defaults(const char *manufacturer, const char *product, > const char *version, > bool uuid_encoded, SmbiosEntryPointType ep_type); > void smbios_set_default_processor_family(uint16_t processor_family); > -uint8_t *smbios_get_table_legacy(uint32_t expected_t4_count, size_t *length); > +uint8_t *smbios_get_table_legacy(size_t *length); > void smbios_get_tables(MachineState *ms, > const struct smbios_phys_mem_area *mem_array, > const unsigned int mem_array_size, > diff --git a/hw/i386/fw_cfg.c b/hw/i386/fw_cfg.c > index c1e9c0fd9c..d1281066f4 100644 > --- a/hw/i386/fw_cfg.c > +++ b/hw/i386/fw_cfg.c > @@ -71,8 +71,7 @@ void fw_cfg_build_smbios(PCMachineState *pcms, FWCfgState *fw_cfg) > smbios_set_cpuid(cpu->env.cpuid_version, cpu->env.features[FEAT_1_EDX]); > > if (pcmc->smbios_legacy_mode) { > - smbios_tables = smbios_get_table_legacy(ms->smp.cpus, > - &smbios_tables_len); > + smbios_tables = smbios_get_table_legacy(&smbios_tables_len); > fw_cfg_add_bytes(fw_cfg, FW_CFG_SMBIOS_ENTRIES, > smbios_tables, smbios_tables_len); > return; > diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c > index aa2cc5bdbd..97cf762228 100644 > --- a/hw/smbios/smbios.c > +++ b/hw/smbios/smbios.c > @@ -482,14 +482,17 @@ opts_init(smbios_register_config); > */ > #define SMBIOS_21_MAX_TABLES_LEN 0xffff > > -static void smbios_validate_table(uint32_t expected_t4_count) > +static void smbios_check_type4_count(uint32_t expected_t4_count) > { > if (smbios_type4_count && smbios_type4_count != expected_t4_count) { > error_report("Expected %d SMBIOS Type 4 tables, got %d instead", > expected_t4_count, smbios_type4_count); > exit(1); > } > +} > > +static void smbios_validate_table(void) > +{ > if (smbios_ep_type == SMBIOS_ENTRY_POINT_TYPE_32 && > smbios_tables_len > SMBIOS_21_MAX_TABLES_LEN) { > error_report("SMBIOS 2.1 table length %zu exceeds %d", > @@ -574,7 +577,7 @@ static void smbios_build_type_1_fields(void) > } > } > > -uint8_t *smbios_get_table_legacy(uint32_t expected_t4_count, size_t *length) > +uint8_t *smbios_get_table_legacy(size_t *length) > { > int i; > size_t usr_offset; > @@ -587,6 +590,12 @@ uint8_t *smbios_get_table_legacy(uint32_t expected_t4_count, size_t *length) > exit(1); > } > > + if (test_bit(4, have_binfile_bitmap)) { > + error_report("can't process table for smbios " > + "type 4 on machine versions < 2.1!"); > + exit(1); > + } > + > g_free(smbios_entries); > smbios_entries_len = sizeof(uint16_t); > smbios_entries = g_malloc0(smbios_entries_len); > @@ -613,7 +622,7 @@ uint8_t *smbios_get_table_legacy(uint32_t expected_t4_count, size_t *length) > > smbios_build_type_0_fields(); > smbios_build_type_1_fields(); > - smbios_validate_table(expected_t4_count); > + smbios_validate_table(); > *length = smbios_entries_len; > return smbios_entries; > } > @@ -1180,7 +1189,8 @@ void smbios_get_tables(MachineState *ms, > smbios_build_type_41_table(errp); > smbios_build_type_127_table(); > > - smbios_validate_table(ms->smp.sockets); > + smbios_check_type4_count(ms->smp.sockets); > + smbios_validate_table(); > smbios_entry_point_setup(); > > /* return tables blob and entry point (anchor), and their sizes */ > -- > 2.39.3 > >
diff --git a/include/hw/firmware/smbios.h b/include/hw/firmware/smbios.h index 0818184834..1fbff3c55f 100644 --- a/include/hw/firmware/smbios.h +++ b/include/hw/firmware/smbios.h @@ -296,7 +296,7 @@ void smbios_set_defaults(const char *manufacturer, const char *product, const char *version, bool uuid_encoded, SmbiosEntryPointType ep_type); void smbios_set_default_processor_family(uint16_t processor_family); -uint8_t *smbios_get_table_legacy(uint32_t expected_t4_count, size_t *length); +uint8_t *smbios_get_table_legacy(size_t *length); void smbios_get_tables(MachineState *ms, const struct smbios_phys_mem_area *mem_array, const unsigned int mem_array_size, diff --git a/hw/i386/fw_cfg.c b/hw/i386/fw_cfg.c index c1e9c0fd9c..d1281066f4 100644 --- a/hw/i386/fw_cfg.c +++ b/hw/i386/fw_cfg.c @@ -71,8 +71,7 @@ void fw_cfg_build_smbios(PCMachineState *pcms, FWCfgState *fw_cfg) smbios_set_cpuid(cpu->env.cpuid_version, cpu->env.features[FEAT_1_EDX]); if (pcmc->smbios_legacy_mode) { - smbios_tables = smbios_get_table_legacy(ms->smp.cpus, - &smbios_tables_len); + smbios_tables = smbios_get_table_legacy(&smbios_tables_len); fw_cfg_add_bytes(fw_cfg, FW_CFG_SMBIOS_ENTRIES, smbios_tables, smbios_tables_len); return; diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c index aa2cc5bdbd..97cf762228 100644 --- a/hw/smbios/smbios.c +++ b/hw/smbios/smbios.c @@ -482,14 +482,17 @@ opts_init(smbios_register_config); */ #define SMBIOS_21_MAX_TABLES_LEN 0xffff -static void smbios_validate_table(uint32_t expected_t4_count) +static void smbios_check_type4_count(uint32_t expected_t4_count) { if (smbios_type4_count && smbios_type4_count != expected_t4_count) { error_report("Expected %d SMBIOS Type 4 tables, got %d instead", expected_t4_count, smbios_type4_count); exit(1); } +} +static void smbios_validate_table(void) +{ if (smbios_ep_type == SMBIOS_ENTRY_POINT_TYPE_32 && smbios_tables_len > SMBIOS_21_MAX_TABLES_LEN) { error_report("SMBIOS 2.1 table length %zu exceeds %d", @@ -574,7 +577,7 @@ static void smbios_build_type_1_fields(void) } } -uint8_t *smbios_get_table_legacy(uint32_t expected_t4_count, size_t *length) +uint8_t *smbios_get_table_legacy(size_t *length) { int i; size_t usr_offset; @@ -587,6 +590,12 @@ uint8_t *smbios_get_table_legacy(uint32_t expected_t4_count, size_t *length) exit(1); } + if (test_bit(4, have_binfile_bitmap)) { + error_report("can't process table for smbios " + "type 4 on machine versions < 2.1!"); + exit(1); + } + g_free(smbios_entries); smbios_entries_len = sizeof(uint16_t); smbios_entries = g_malloc0(smbios_entries_len); @@ -613,7 +622,7 @@ uint8_t *smbios_get_table_legacy(uint32_t expected_t4_count, size_t *length) smbios_build_type_0_fields(); smbios_build_type_1_fields(); - smbios_validate_table(expected_t4_count); + smbios_validate_table(); *length = smbios_entries_len; return smbios_entries; } @@ -1180,7 +1189,8 @@ void smbios_get_tables(MachineState *ms, smbios_build_type_41_table(errp); smbios_build_type_127_table(); - smbios_validate_table(ms->smp.sockets); + smbios_check_type4_count(ms->smp.sockets); + smbios_validate_table(); smbios_entry_point_setup(); /* return tables blob and entry point (anchor), and their sizes */
legacy mode doesn't support structures of type 2 and more, and CLI has a check for '-smbios type' option, however it's still possible to sneak in type4 as a blob with '-smbios file' option. However doing the later makes SMBIOS tables broken since SeaBIOS doesn't expect that. Rather than trying to add support for type4 to legacy code (both QEMU and SeaBIOS), simplify smbios_get_table_legacy() by dropping not relevant check in legacy code and error out on type4 blob. Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- * The issue affects 'isapc' and pc-i440fx-2.0. the later is in deprecated state and to be dropped in near future * possibly the same issue applies to other SMBIOS types above type 1 but I haven't tested that, and well tables that aren't generated by SeaBIOS can get be added just fine (tested type11 blob). So I went with a minimal change to fixup type4 only that I'm touching. Leaving the rest for other time or when someone complains about it, which is very unlikely given it's really only remaining isapc machine. I'd very much prefer to deprecate 'isapc' and then drop all legacy related code (it will benefit not only SMBIOS but other code as well). BTW: 'isapc' is in semi-dead, I cna't boot RHEL6 on it with KVM enabled anymore (RHEL9 host), TCG still boots though. One more reason to get deprecate it. --- include/hw/firmware/smbios.h | 2 +- hw/i386/fw_cfg.c | 3 +-- hw/smbios/smbios.c | 18 ++++++++++++++---- 3 files changed, 16 insertions(+), 7 deletions(-)