Re: [PATCH 14/19] smbios: in case of entry point is 'auto' try to build v2 tables 1st

[Ani Sinha]

> On 27-Feb-2024, at 21:17, Igor Mammedov <imammedo@redhat.com> wrote:
> 
> QEMU for some time now uses SMBIOS 3.0 for PC/Q35 machines by
> default, however Windows has a bug in locating SMBIOS 3.0
> entrypoint and fails to find tables when booted on SeaBIOS
> (on UEFI SMBIOS 3.0 tables work fine since firmware hands
> over tables in another way)
> 
> Missing SMBIOS tables may lead to some issues for guest
> though (worst are: possible reactiveation, inability to
> get virtio drivers from 'Windows Update')
> 
> It's unclear  at this point if MS will fix the issue on their
> side. So instead of it (or rather in addition) this patch
> will try to workaround the issue.
> 
> aka, use smbios-entry-point-type=auto to make QEMU try
> generating conservative SMBIOS 2.0 tables and if that
> fails (due to limits/requested configuration) fallback
> to SMBIOS 3.0 tables.
> 
> With this in place majority of users will use SMBIOS 2.0
> tables which work fine with (Windows + legacy BIOS).
> The configurations that is not to possible to describe
> with SMBIOS 2.0 will switch automatically to SMBIOS 3.0
> (which will trigger Windows bug but there is nothing
> QEMU can do here, so go and aks Microsoft to real fix).
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

Reviewed-by: Ani Sinha <anisinha@redhat.com>

> ---
> hw/smbios/smbios.c | 52 +++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 49 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> index 5a791fd9eb..e54a9f21e6 100644
> --- a/hw/smbios/smbios.c
> +++ b/hw/smbios/smbios.c
> @@ -959,7 +959,7 @@ static void smbios_entry_point_setup(SmbiosEntryPointType 
> ep_type)
>     }
> }
> 
> -void smbios_get_tables(MachineState *ms,
> +static bool smbios_get_tables_ep(MachineState *ms,
>                        SmbiosEntryPointType ep_type,
>                        const struct smbios_phys_mem_area *mem_array,
>                        const unsigned int mem_array_size,
> @@ -968,6 +968,7 @@ void smbios_get_tables(MachineState *ms,
>                        Error **errp)
> {
>     unsigned i, dimm_cnt, offset;
> +    ERRP_GUARD();
> 
>     assert(ep_type == SMBIOS_ENTRY_POINT_TYPE_32 ||
>            ep_type == SMBIOS_ENTRY_POINT_TYPE_64);
> @@ -1052,11 +1053,56 @@ void smbios_get_tables(MachineState *ms,
>         abort();
>     }
> 
> -    return;
> +    return true;
> err_exit:
>     g_free(smbios_tables);
>     smbios_tables = NULL;
> -    return;
> +    return false;
> +}
> +
> +void smbios_get_tables(MachineState *ms,
> +                       SmbiosEntryPointType ep_type,
> +                       const struct smbios_phys_mem_area *mem_array,
> +                       const unsigned int mem_array_size,
> +                       uint8_t **tables, size_t *tables_len,
> +                       uint8_t **anchor, size_t *anchor_len,
> +                       Error **errp)
> +{
> +    Error *local_err = NULL;
> +    bool is_valid;
> +    ERRP_GUARD();
> +
> +    switch (ep_type) {
> +    case SMBIOS_ENTRY_POINT_TYPE_AUTO:
> +    case SMBIOS_ENTRY_POINT_TYPE_32:
> +        is_valid = smbios_get_tables_ep(ms, SMBIOS_ENTRY_POINT_TYPE_32,
> +                                        mem_array, mem_array_size,
> +                                        tables, tables_len,
> +                                        anchor, anchor_len,
> +                                        &local_err);
> +        if (is_valid || ep_type != SMBIOS_ENTRY_POINT_TYPE_AUTO) {
> +            break;
> +        }
> +        /*
> +         * fall through in case AUTO endpoint is selected and
> +         * SMBIOS 2.x tables can't be generated, to try if SMBIOS 3.x
> +         * tables would work
> +         */
> +    case SMBIOS_ENTRY_POINT_TYPE_64:
> +        error_free(local_err);
> +        local_err = NULL;
> +        is_valid = smbios_get_tables_ep(ms, SMBIOS_ENTRY_POINT_TYPE_64,
> +                                        mem_array, mem_array_size,
> +                                        tables, tables_len,
> +                                        anchor, anchor_len,
> +                                        &local_err);
> +        break;
> +    default:
> +        abort();
> +    }
> +    if (!is_valid) {
> +        error_propagate(errp, local_err);
> +    }
> }
> 
> static void save_opt(const char **dest, QemuOpts *opts, const char *name)
> -- 
> 2.39.3
>