Re: [Qemu-devel] [PATCH v2, Ping] SMBIOS: Upgrade Type17 to v2.3, add Type2

[Gabriel L. Somlo]

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;
     }