Re: [Qemu-devel] [PATCH 01/18] smbios: Add a function to directly add an entry

[Kevin O'Connor]

On Mon, Jul 30, 2012 at 12:25:16PM -0500, Anthony Liguori wrote:
> Corey Minyard <address@hidden> writes:
> > On 07/30/2012 10:37 AM, Anthony Liguori wrote:
> >> address@hidden writes:
> >>> There was no way to directly add a table entry to the SMBIOS table,
> >>> even though the BIOS supports this.  So add a function to do this.
> >>> This is in preparation for the IPMI handler adding it's SMBIOS table
> >>> entry.
> >> I don't expect that hardware ever adds SMBIOS entries.  Rather, the BIOS
> >> adds the entries by probing the hardware.
> >
> > Well, memory entries are added by QEMU, why not let the BIOS probe for 
> > that?
> QEMU doesn't add any entries by default.  SeaBIOS owns SMBIOS.  QEMU
> uses a backchannel to hand SeaBIOS tables that SeaBIOS can then expose.
> The reason we use a table based interface is because type 0 and type 1
> tables can have vendor extensions that are in a vendor specific format.
> 
> But SeaBIOS unquestionably owns SMBIOS generation.
[...]
> >> So I think you should solve this in SeaBIOS, instead of trying to do it
> >> in QEMU.  I think that also solves the problem you have with
> >> pre-firmware init.
> > The user can pass the I/O base and IRQ values in on the QEMU command 
> > line, and they can be arbitrary values.  The BIOS is not going to be 
> > able to probe for those.
> 
> Then pass the information that the BIOS needs through fw_cfg.  That's
> what it's there for.

This approach, unfortunately, leads to extra code and "double
handling" of infomation.

The ultimate consumer of the data wants a binary struct which looks
like:

struct smbios_type_38 {
    struct smbios_structure_header header;
    u8 interface_type;
    u8 ipmi_version;
    u8 i2c_slave_addr;
    u8 nv_storage_dev_addr;
    u64 base_addr;
    u8 base_addr_mod_and_irq_info;
    u8 interrupt_number;
};

The proposed solution is to put the info in a binary struct which
looks like:

struct ipmi_info {
    u8 str_version;
    u8 interface;
    u8 reg_space;
    u8 reg_spacing;
    u8 slave_addr;
    u8 irq;
    u8 version;
    u8 reserved1;
    u64 base_addr;
} PACKED;

and then have SeaBIOS translate the latter binary struct into the
former.  However, there's no compelling reason to introduce a new
binary struct instead of using the industry standard binary struct.

I'd prefer QEMU to just pass the SMBIOS struct and let SeaBIOS pass it
through to the OS.  I also think the existing fw_cfg mechanism for
passing SMBIOS tables should be used.  I understand that this causes
some contortions due to the current SMBIOS fw_cfg impementation in
QEMU, but it seems wrong to introduce a new fw_cfg port that
accomplishes the same goal as an existing fw_cfg port.

-Kevin