From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57217) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WGDya-0007uJ-C3 for qemu-devel@nongnu.org; Wed, 19 Feb 2014 15:42:48 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WGDyR-00068J-Fj for qemu-devel@nongnu.org; Wed, 19 Feb 2014 15:42:40 -0500 Received: from mail-qc0-x230.google.com ([2607:f8b0:400d:c01::230]:53892) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WGDyR-00068B-A3 for qemu-devel@nongnu.org; Wed, 19 Feb 2014 15:42:31 -0500 Received: by mail-qc0-f176.google.com with SMTP id e16so1554526qcx.21 for ; Wed, 19 Feb 2014 12:42:30 -0800 (PST) Date: Wed, 19 Feb 2014 15:40:31 -0500 From: "Gabriel L. Somlo" Message-ID: <20140219204030.GA25087@ERROL.INI.CMU.EDU> References: <20140217160947.GO29329@ERROL.INI.CMU.EDU> <20140217203333.GB30268@morn.localdomain> <1392718893.4724.12.camel@nilsson.home.kraxel.org> <20140218191728.GS29329@ERROL.INI.CMU.EDU> <1392803974.20987.13.camel@nilsson.home.kraxel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1392803974.20987.13.camel@nilsson.home.kraxel.org> Subject: Re: [Qemu-devel] [PATCH v2, Ping] SMBIOS: Upgrade Type17 to v2.3, add Type2 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gerd Hoffmann Cc: seabios@seabios.org, qemu-devel@nongnu.org, agraf@suse.de, Kevin O'Connor , pbonzini@redhat.com, lersek@redhat.com On Wed, Feb 19, 2014 at 10:59:34AM +0100, Gerd Hoffmann wrote: > Hi, > > > However, when I compare unmodified SMBIOS against what I get when > > supplying the patched binary table via command line, I get this: > > As Laszlo already sayed: one table per file. So I gave up on that relatively quickly, as there's no easy and convenient way to "harvest" a binary of just one table type from a host that works the way I want it... :( > > > If seabios finds a table provided by qemu it used it, otherwise it > > > (possibly) generates its own. So we can smoothly switch over to qemu, > > > table-by-table. You can have qemu provide type2+type17 tables, and > > > leave everything else as-is. And when doing it in qemu it is easy to do > > > it for new machine types only. > > > > I could try to hack at the QEMU smbios source file to try to find > > where the problem lies (at least why handover to SeaBIOS doesn't work > > as expected), but I'm not sure providing command line flags for > > inputting each record type individually is a scalable way to move > > forward. > > Agree. qemu should simply autogenerate the entries (where it can). > i.e. basically port seabios smbios_init_type_17 function to qemu, then > hook the result into the smbios_entries array. The code to do that is > in smbios_entry_add(). You probably want to factor that out ino a small > helper function which is then called by both smbios_entry_add() and the > type17 init function. So I tried the patch below (hardcoded Type 2 initialization). The guest (tried this on Fedora-20 live) still can't see a Type 2 entry. Besides, I think smbios_maybe_add_str() looks wrong: It looks like it's trying to paste a string into a given type structure at the field offset, but I think the field should actually be an *index* into a concatenated set of zero-terminated strings tacked on to the end of the type struct (see load_str_field_* macros in seabios src/fw/smbios.c). What am I missing ? Thanks, --Gabriel diff --git a/include/hw/i386/smbios.h b/include/hw/i386/smbios.h index 18fb970..3d2749b 100644 --- a/include/hw/i386/smbios.h +++ b/include/hw/i386/smbios.h @@ -60,6 +60,22 @@ struct smbios_type_1 { uint8_t family_str; } QEMU_PACKED; +/* SMBIOS type 2 - Base Board */ +struct smbios_type_2 { + struct smbios_structure_header header; + uint8_t manufacturer_str; + uint8_t product_str; + uint8_t version_str; + uint8_t serial_number_str; + uint8_t asset_tag_number_str; + uint8_t feature_flags; + uint8_t location_str; + uint16_t chassis_handle; + uint8_t board_type; + uint8_t contained_element_count; + // contained elements follow +} QEMU_PACKED; + /* SMBIOS type 3 - System Enclosure (v2.3) */ struct smbios_type_3 { struct smbios_structure_header header; @@ -111,7 +127,7 @@ struct smbios_type_16 { uint16_t memory_error_information_handle; uint16_t number_of_memory_devices; } QEMU_PACKED; -/* SMBIOS type 17 - Memory Device +/* SMBIOS type 17 - Memory Device (v2.3) * Associated with one type 19 */ struct smbios_type_17 { @@ -127,6 +143,12 @@ struct smbios_type_17 { uint8_t bank_locator_str; uint8_t memory_type; uint16_t type_detail; + // v2.3 fields: + uint16_t speed; + uint8_t manufacturer_str; + uint8_t serial_number_str; + uint8_t asset_tag_number_str; + uint8_t part_number_str; } QEMU_PACKED; /* SMBIOS type 19 - Memory Array Mapped Address */ diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c index e8f41ad..e875493 100644 --- a/hw/i386/smbios.c +++ b/hw/i386/smbios.c @@ -270,11 +270,41 @@ void smbios_set_type1_defaults(const char *manufacturer, } } +static void smbios_build_type_2_fields(void) +{ + uint8_t f_flags = 0x01, b_type = 0x0a; /* Motherboard / Hosting Board */ + uint8_t elem_cnt = 0; /* None */ + uint16_t chassis_hndl = 0x300; /* Type 3 System Enclosure */ + + smbios_maybe_add_str(2, offsetof(struct smbios_type_2, manufacturer_str), + "abcxyz"); + smbios_maybe_add_str(2, offsetof(struct smbios_type_2, product_str), + NULL); + smbios_maybe_add_str(2, offsetof(struct smbios_type_2, version_str), + NULL); + smbios_maybe_add_str(2, offsetof(struct smbios_type_2, serial_number_str), + NULL); + smbios_maybe_add_str(2, offsetof(struct smbios_type_2, + asset_tag_number_str), + NULL); + smbios_add_field(2, offsetof(struct smbios_type_2, feature_flags), + &f_flags, sizeof(f_flags)); + smbios_maybe_add_str(2, offsetof(struct smbios_type_2, location_str), + NULL); + smbios_add_field(2, offsetof(struct smbios_type_2, chassis_handle), + &chassis_hndl, sizeof(chassis_hndl)); + smbios_add_field(2, offsetof(struct smbios_type_2, board_type), + &b_type, sizeof(b_type)); + smbios_add_field(2, offsetof(struct smbios_type_2, contained_element_count), + &elem_cnt, sizeof(elem_cnt)); +} + uint8_t *smbios_get_table(size_t *length) { if (!smbios_immutable) { smbios_build_type_0_fields(); smbios_build_type_1_fields(); + smbios_build_type_2_fields(); smbios_validate_table(); smbios_immutable = true; }