Patchwork [01/16] smbios: Add a function to directly add an entry

login
register
mail settings
Submitter Corey Minyard
Date July 15, 2012, 8:24 p.m.
Message ID <1342383911-6094-1-git-send-email-minyard@acm.org>
Download mbox | patch
Permalink /patch/171104/
State New
Headers show

Comments

Corey Minyard - July 15, 2012, 8:24 p.m.
From: Corey Minyard <cminyard@mvista.com>

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.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 hw/smbios.c |   27 +++++++++++++++++++++++++++
 hw/smbios.h |   15 ++++++++-------
 2 files changed, 35 insertions(+), 7 deletions(-)
Corey Minyard - July 15, 2012, 9:05 p.m.
I messed up and didn't get an introduction for these patches.  So here goes.

These patches are for adding an IPMI interface to QEMU.  It adds a KCS 
and BT interface; it should be easy to add a SMIC interface if anyone 
wants that, but it's not often used.  For simulation of an IPMI 
management controller, it is a basic one that is built in to KVM, and it 
can make a connection to an external simulator.

I think I got all the previous comments handled.  Once I realized how 
the class stuff worked, things made a lot more sense to me.

The OpenIPMI library (openipmi.sf.net) has a simulator that will work as 
an external simulator in the latest release candidate.

This patch fixes up a few things that the IPMI driver needs:

Allow a driver to add entries to the SMBIOS table.

Add the ability for a chardev that makes an external connection to 
reconnect if the initial connection fails or if it fails while in use.

In order to move the chardev to the external interface simulator, it had 
to be released or the add to the external interface failed.

Patches 7 and 8 in the series are not really related, they are things 
that I saw that seemed to be issues.  I meant to pull them out but I 
forgot.  These patches really got accidentally sent, but since they are 
out, well, they are out.

There are two things that I know of that are still missing:

Documentation.  I'm planning to write docs for this, but I don't really 
see a place to do this.  I don't see many other docs for devices.  Where 
should I put them?  If it's in qemu-options.hx, where in that file 
should they be?

Tests.  I looked at the test infrastructure, and I don't really see many 
tests, so not much to go on for examples.  I think I have it figured 
out, but I'll basically have to write a driver to be able to really test 
this.  It would be nice if I could use the Linux driver, but I guess 
that's not really feasible.

-corey
Eric Blake - July 16, 2012, 3:46 p.m.
On 07/15/2012 02:24 PM, minyard@acm.org wrote:
> From: Corey Minyard <cminyard@mvista.com>
> 
> 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.
> 
> Signed-off-by: Corey Minyard <cminyard@mvista.com>
> ---
>  hw/smbios.c |   27 +++++++++++++++++++++++++++
>  hw/smbios.h |   15 ++++++++-------
>  2 files changed, 35 insertions(+), 7 deletions(-)

Are you planning to expose this via the command line?  Libvirt would
like the ability to set arbitrary name/value pairs in SMBIOS (such as
asset tag in the type 3 page), rather than the current limitation of
only being able to set pre-defined names in just type 0 and 1 sections.
Corey Minyard - July 17, 2012, 12:06 a.m.
On 07/16/2012 10:46 AM, Eric Blake wrote:
> On 07/15/2012 02:24 PM, minyard@acm.org wrote:
>> From: Corey Minyard <cminyard@mvista.com>
>>
>> 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.
>>
>> Signed-off-by: Corey Minyard <cminyard@mvista.com>
>> ---
>>   hw/smbios.c |   27 +++++++++++++++++++++++++++
>>   hw/smbios.h |   15 ++++++++-------
>>   2 files changed, 35 insertions(+), 7 deletions(-)
> Are you planning to expose this via the command line?  Libvirt would
> like the ability to set arbitrary name/value pairs in SMBIOS (such as
> asset tag in the type 3 page), rather than the current limitation of
> only being able to set pre-defined names in just type 0 and 1 sections.
>
Well, I wan't planning on it, I only did what I needed for IPMI. Adding 
a command line interface would be more learning that I can probably 
afford right now.

-corey

Patch

diff --git a/hw/smbios.c b/hw/smbios.c
index c57237d..98c7f99 100644
--- a/hw/smbios.c
+++ b/hw/smbios.c
@@ -178,6 +178,33 @@  static void smbios_build_type_1_fields(const char *t)
                          strlen(buf) + 1, buf);
 }
 
+int smbios_table_entry_add(struct smbios_structure_header *entry)
+{
+    struct smbios_table *table;
+    struct smbios_structure_header *header;
+    unsigned int size = entry->length;
+
+    if (!smbios_entries) {
+        smbios_entries_len = sizeof(uint16_t);
+        smbios_entries = g_malloc0(smbios_entries_len);
+    }
+    smbios_entries = g_realloc(smbios_entries, smbios_entries_len +
+			       sizeof(*table) + size);
+    table = (struct smbios_table *)(smbios_entries + smbios_entries_len);
+    table->header.type = SMBIOS_TABLE_ENTRY;
+    table->header.length = cpu_to_le16(sizeof(*table) + size);
+
+    header = (struct smbios_structure_header *)(table->data);
+    memcpy(header, entry, size);
+
+    smbios_check_collision(header->type, SMBIOS_TABLE_ENTRY);
+
+    smbios_entries_len += sizeof(*table) + size;
+    (*(uint16_t *)smbios_entries) =
+	cpu_to_le16(le16_to_cpu(*(uint16_t *)smbios_entries) + 1);
+    return 0;
+}
+
 int smbios_entry_add(const char *t)
 {
     char buf[1024];
diff --git a/hw/smbios.h b/hw/smbios.h
index 94e3641..6431a15 100644
--- a/hw/smbios.h
+++ b/hw/smbios.h
@@ -13,21 +13,22 @@ 
  *
  */
 
+/* This goes at the beginning of every SMBIOS structure. */
+struct smbios_structure_header {
+    uint8_t type;
+    uint8_t length;
+    uint16_t handle;
+} QEMU_PACKED;
+
 int smbios_entry_add(const char *t);
 void smbios_add_field(int type, int offset, int len, void *data);
 uint8_t *smbios_get_table(size_t *length);
+int smbios_table_entry_add(struct smbios_structure_header *entry);
 
 /*
  * SMBIOS spec defined tables
  */
 
-/* This goes at the beginning of every SMBIOS structure. */
-struct smbios_structure_header {
-    uint8_t type;
-    uint8_t length;
-    uint16_t handle;
-} QEMU_PACKED;
-
 /* SMBIOS type 0 - BIOS Information */
 struct smbios_type_0 {
     struct smbios_structure_header header;