Re: [Qemu-devel] [PATCH] smbios: stop ignoring command line options for TARGET_ARM

[Igor Mammedov <imammedo@redhat.com>]

On Fri, 16 Dec 2016 15:23:19 +0000
Leif Lindholm <leif.lindholm@linaro.org> wrote:

> Commit c30e1565 ("smbios: implement smbios support for mach-virt")
> enabled automatic generation of SMBIOS tables for TARGET_ARM, and
> actually provides data for the "virt" machine.
> 
> However, do_smbios_option() still had an #ifdef TARGET_I386, preventing
> any -smbios command line options from being parsed for any non-x86
> targets.
> Change this to use a status variable instead of compile-time filtering.
> 
> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
> ---
> 
> Verified on ARM mach-virt with UEFI shell "smbiosview" command and QEMU
> command line parameter -smbios type=0,version=foobar.
> 
>  arch_init.c                | 6 +++---
>  include/hw/smbios/smbios.h | 2 ++
>  vl.c                       | 2 ++
>  3 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/arch_init.c b/arch_init.c
> index 5cc58b2..d4e28c0 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -250,9 +250,9 @@ void do_acpitable_option(const QemuOpts *opts)
>  
>  void do_smbios_option(QemuOpts *opts)
>  {
> -#ifdef TARGET_I386
extending above condition to make it compiled for ARM
would do what you need

> -    smbios_entry_add(opts);
> -#endif
> +    if (smbios_override) {
> +        smbios_entry_add(opts);
> +    }
>  }
>  
>  int kvm_available(void)
> diff --git a/include/hw/smbios/smbios.h b/include/hw/smbios/smbios.h
> index 1cd53cc..2a3dca2 100644
> --- a/include/hw/smbios/smbios.h
> +++ b/include/hw/smbios/smbios.h
> @@ -267,4 +267,6 @@ void smbios_get_tables(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);
> +
> +extern int smbios_override;
Adding global variables generally is not welcomed and one
should try to avoid it if it could be helped.

>  #endif /* QEMU_SMBIOS_H */
> diff --git a/vl.c b/vl.c
> index d77dd86..8e71b06 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -159,6 +159,7 @@ int smp_cpus = 1;
>  int max_cpus = 1;
>  int smp_cores = 1;
>  int smp_threads = 1;
> +int smbios_override = 0;
>  int acpi_enabled = 1;
>  int no_hpet = 0;
>  int fd_bootchk = 1;
> @@ -3711,6 +3712,7 @@ int main(int argc, char **argv, char **envp)
>                  if (!opts) {
>                      exit(1);
>                  }
> +                smbios_override = 1;
                   ^^^ practically turns follow up call into unconditional
smbios_entry_add(opts) call,
so what's the point for adding 'smbios_override' at all?

Also this patch would break build for targets that don't link smbios.c
(i.e. which don't have CONFIG_SMBIOS=y)

>                  do_smbios_option(opts);
>                  break;
>              case QEMU_OPTION_fwcfg: