Re: [Qemu-devel] [PATCH 2/2 (RFC)] QEMU: SMBIOS: Build full smbios tables

[Gerd Hoffmann <kraxel@redhat.com>]

On Di, 2014-03-04 at 14:20 -0500, Gabriel L. Somlo wrote:
>    - deprecate passing individual smbios fields (SMBIOS_FIELD_ENTRY)
>    - added functionality to build full tables instead, and pass them
>      to the bios via fw_cfg as SMBIOS_TABLE_ENTRY blobs
>    - added code to optionally build type 2 smbios table expected by OS X

Great.

> Kevin and Gerd: I'd like to get your acked-by for this patch (or any
> future revision): you're the two people who've been editing src/fw/smbios.c
> in SeaBIOS, and while I didn't directly cut'n'paste anything from there,
> I did in fact stare at it very intently, and I'd like to get your OK with
> the lgplv3 -> gplv2+ difference in licensing between SeaBIOS and,
> respectively, QEMU.

Fine with me.

> -static bool smbios_type1_defaults = true;
> +static bool smbios_defaults = true;
>  /* Make sure that guest addresses aligned at 1Gbyte boundaries get mapped to
>   * host addresses aligned at 1Gbyte boundaries.  This way we can use 1GByte
>   * pages in the host.
> @@ -143,9 +143,9 @@ static void pc_init1(QEMUMachineInitArgs *args,
>      guest_info->has_pci_info = has_pci_info;
>      guest_info->isapc_ram_fw = !pci_enabled;
>  
> -    if (smbios_type1_defaults) {
> +    if (smbios_defaults) {
>          /* These values are guest ABI, do not change */
> -        smbios_set_type1_defaults("QEMU", "Standard PC (i440FX + PIIX, 1996)",
> +        smbios_set_defaults("QEMU", "Standard PC (i440FX + PIIX, 1996)",
>                                    args->machine->name);
>      }

Need to be careful here.  smbios_type1_defaults handles machine type
compatibility.  qemu version 1.7 (and older) don't set any smbios fields
by default.  Current qemu behaves simliar with -M pc-i440fx-1.7 (+older)
for compatibility reasons, and smbios_type1_defaults controls that.

Given that we don't have a stable release which sets smbios defaults I
think it is ok to do it this way (assuming it is merged before the 2.0
release).

Splitting the patches would be nice for review (add table generator /
switch type0 / switch type1 / remove field support / add type2).

Maybe we should continue to use the individual fields interface for 1.7
machine types, for best backward compatibility.

Overall it looks good to me, don't see any major issues.

cheers,
  Gerd