> On 27-Feb-2024, at 21:17, Igor Mammedov <imammedo@redhat.com> wrote:
>
> it makes smbios_validate_table() independent from
> smbios_smp_sockets global, which in turn lets
> smbios_get_tables() avoid using not related legacy code.
>
Good cleanup!
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Ani Sinha <anisinha@redhat.com>
> ---
> goal here is to isolate legacy handling from generic smbios_get_tables()
> ---
> include/hw/firmware/smbios.h | 2 +-
> hw/i386/fw_cfg.c | 2 +-
> hw/smbios/smbios.c | 22 +++++++++-------------
> 3 files changed, 11 insertions(+), 15 deletions(-)
>
> diff --git a/include/hw/firmware/smbios.h b/include/hw/firmware/smbios.h
> index 6e514982d4..a187fbbd3d 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 legacy_mode,
> bool uuid_encoded, SmbiosEntryPointType ep_type);
> void smbios_set_default_processor_family(uint16_t processor_family);
> -uint8_t *smbios_get_table_legacy(MachineState *ms, size_t *length);
> +uint8_t *smbios_get_table_legacy(uint32_t expected_t4_count, 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 a635234e68..fcb4fb0769 100644
> --- a/hw/i386/fw_cfg.c
> +++ b/hw/i386/fw_cfg.c
> @@ -70,7 +70,7 @@ void fw_cfg_build_smbios(PCMachineState *pcms, FWCfgState *fw_cfg)
> /* tell smbios about cpuid version and features */
> smbios_set_cpuid(cpu->env.cpuid_version, cpu->env.features[FEAT_1_EDX]);
>
> - smbios_tables = smbios_get_table_legacy(ms, &smbios_tables_len);
> + smbios_tables = smbios_get_table_legacy(ms->smp.cpus, &smbios_tables_len);
> if (smbios_tables) {
> fw_cfg_add_bytes(fw_cfg, FW_CFG_SMBIOS_ENTRIES,
> smbios_tables, smbios_tables_len);
> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> index 8e86c62184..15339d8dbe 100644
> --- a/hw/smbios/smbios.c
> +++ b/hw/smbios/smbios.c
> @@ -70,7 +70,7 @@ static SmbiosEntryPoint ep;
> static int smbios_type4_count = 0;
> static bool smbios_immutable;
> static bool smbios_have_defaults;
> -static uint32_t smbios_cpuid_version, smbios_cpuid_features, smbios_smp_sockets;
> +static uint32_t smbios_cpuid_version, smbios_cpuid_features;
>
> static DECLARE_BITMAP(have_binfile_bitmap, SMBIOS_MAX_TYPE+1);
> static DECLARE_BITMAP(have_fields_bitmap, SMBIOS_MAX_TYPE+1);
> @@ -476,14 +476,11 @@ opts_init(smbios_register_config);
> */
> #define SMBIOS_21_MAX_TABLES_LEN 0xffff
>
> -static void smbios_validate_table(MachineState *ms)
> +static void smbios_validate_table(uint32_t expected_t4_count)
> {
> - uint32_t expect_t4_count = smbios_legacy ?
> - ms->smp.cpus : smbios_smp_sockets;
> -
> - if (smbios_type4_count && smbios_type4_count != expect_t4_count) {
> + if (smbios_type4_count && smbios_type4_count != expected_t4_count) {
> error_report("Expected %d SMBIOS Type 4 tables, got %d instead",
> - expect_t4_count, smbios_type4_count);
> + expected_t4_count, smbios_type4_count);
> exit(1);
> }
>
> @@ -571,7 +568,7 @@ static void smbios_build_type_1_fields(void)
> }
> }
>
> -uint8_t *smbios_get_table_legacy(MachineState *ms, size_t *length)
> +uint8_t *smbios_get_table_legacy(uint32_t expected_t4_count, size_t *length)
> {
> if (!smbios_legacy) {
> *length = 0;
> @@ -581,7 +578,7 @@ uint8_t *smbios_get_table_legacy(MachineState *ms, size_t *length)
> if (!smbios_immutable) {
> smbios_build_type_0_fields();
> smbios_build_type_1_fields();
> - smbios_validate_table(ms);
> + smbios_validate_table(expected_t4_count);
> smbios_immutable = true;
> }
> *length = smbios_entries_len;
> @@ -1112,10 +1109,9 @@ void smbios_get_tables(MachineState *ms,
> smbios_build_type_2_table();
> smbios_build_type_3_table();
>
> - smbios_smp_sockets = ms->smp.sockets;
> - assert(smbios_smp_sockets >= 1);
> + assert(ms->smp.sockets >= 1);
>
> - for (i = 0; i < smbios_smp_sockets; i++) {
> + for (i = 0; i < ms->smp.sockets; i++) {
> smbios_build_type_4_table(ms, i);
> }
>
> @@ -1160,7 +1156,7 @@ void smbios_get_tables(MachineState *ms,
> smbios_build_type_41_table(errp);
> smbios_build_type_127_table();
>
> - smbios_validate_table(ms);
> + smbios_validate_table(ms->smp.sockets);
> smbios_entry_point_setup();
> smbios_immutable = true;
> }
> --
> 2.39.3
>