Re: [PATCH trivial for-9.0] hw/i386/fw_cfg.c: fix non-legacy smbios build

[Igor Mammedov]

On Mon, 25 Mar 2024 14:40:21 +0100
Philippe Mathieu-Daudé <philmd@linaro.org> wrote:

> Hi Michael,
> 
> On 25/3/24 14:09, Michael Tokarev wrote:
> > When building qemu with smbios but not legacy mode (eg minimal microvm 
> > build),
> > link fails with:
> > 
> >    hw/i386/fw_cfg.c:74: undefined reference to `smbios_get_table_legacy'
> > 
> > This is because fw_cfg interface can call this function if CONFIG_SMBIOS
> > is defined.  Made this code block to depend on CONFIG_SMBIOS_LEGACY.
> > 
> > Fixes: b42b0e4daaa5 "smbios: build legacy mode code only for 'pc' machine"
> > Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
> > ---
> >   hw/i386/fw_cfg.c | 2 ++
> >   1 file changed, 2 insertions(+)
> > 
> > diff --git a/hw/i386/fw_cfg.c b/hw/i386/fw_cfg.c
> > index d802d2787f..d5e78a9183 100644
> > --- a/hw/i386/fw_cfg.c
> > +++ b/hw/i386/fw_cfg.c
> > @@ -70,6 +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]);
> >   
> > +#ifdef CONFIG_SMBIOS_LEGACY
> >       if (pcmc->smbios_legacy_mode) {  
> 
> But then having pcmc->smbios_legacy_mode == true without
> CONFIG_SMBIOS_LEGACY would be a bug. IMHO what we want is:
> 
> -- >8 --  
> diff --git a/hw/smbios/smbios_legacy_stub.c b/hw/smbios/smbios_legacy_stub.c
> index f29b15316c..7d593dca98 100644
> --- a/hw/smbios/smbios_legacy_stub.c
> +++ b/hw/smbios/smbios_legacy_stub.c
> @@ -13,3 +13,8 @@
>   void smbios_add_usr_blob_size(size_t size)
>   {
>   }
> +
> +uint8_t *smbios_get_table_legacy(size_t *length, Error **errp)
> +{
> +    g_assert_not_reached();
> +}

Agreed

Philippe,
shall I post a patch or will you do this?

> ---
> 
> >           smbios_tables = smbios_get_table_legacy(&smbios_tables_len,
> >                                                   &error_fatal);
> > @@ -77,6 +78,7 @@ void fw_cfg_build_smbios(PCMachineState *pcms, FWCfgState 
> > *fw_cfg,
> >                            smbios_tables, smbios_tables_len);
> >           return;
> >       }
> > +#endif
> >   
> >       /* build the array of physical mem area from e820 table */
> >       mem_array = g_malloc0(sizeof(*mem_array) * e820_get_num_entries());  
>