Skip to contentSkip to conversation

qemu: SMBIOS passing support

PATCH Superseded Alex Williamson 5 participants 14 messages Mar 23, 2009 – Apr 7, 2009 KVM

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

cover qemu: SMBIOS passing support

This series adds a new -smbios option for x86 that allows individual SMBIOS entries to be passed into the guest VM. This follows the same basic path as the support for loading ACPI tables. While SMBIOS is independent of ACPI, I chose to add the smbios_entry_add() function to acpi.c because they're both somewhat PC BIOS related (and ia64 can support SMBIOS and might be able to make use of it there).

This feature allows the guest to see certain properties of the host if configured correctly. For instance, the system model and serial number in the type 1 entry. Obviously its only built at boot, so doesn't get updated for migration scenarios. User provided entries will supersede generated entries, so care should be taken when passing entries which describe physical properties, such as memory size and address ranges. Thanks,

Alex

#1 lore raw

PATCH 1/2 qemu: Allow SMBIOS entries to be loaded and provided to the VM BIOS

Create a new -smbios options that takes binary SMBIOS entries to provide to the VM BIOS. The binary can be easily generated using something like:

dmidecode -t 1 -u | grep $'^\t\t[^"]' | xargs -n1 | \
	perl -lne 'printf "%c", hex($_)' > smbios_type_1.bin

For some inventory tools, this makes the VM report the system information for the host. One entry per binary file, multiple files can be chained together as:

-smbios file1,file2,...

or specified independently:

-smbios file1 -smbios file2

--

hw/acpi.c +66 -0
index 52f50a0..0bd93bf 100644
@@ -915,3 +915,69 @@ out:
     }
     return -1;
 }
+
+char *smbios_entries;
+size_t smbios_entries_len;
+
+int smbios_entry_add(const char *t)
+{
+    struct stat s;
+    char file[1024], *p, *f, *n;
+    int fd, r;
+    size_t len, off;
+
+    f = (char *)t;
+    do {
+        n = strchr(f, ',');
+        if (n) {
+            strncpy(file, f, (n - f));
+            file[n - f] = '\0';
+            f = n + 1;
+        } else {
+            strcpy(file, f);
+            f += strlen(file);
+        }
+
+        fd = open(file, O_RDONLY);
+        if (fd < 0)
+            return -1;
+
+        if (fstat(fd, &s) < 0) {
+            close(fd);
+            return -1;
+        }
+
+        if (!smbios_entries) {
+            smbios_entries_len = sizeof(uint16_t);
+            smbios_entries = qemu_mallocz(smbios_entries_len);
+        }
+
+        len = s.st_size;
+        smbios_entries = qemu_realloc(smbios_entries, smbios_entries_len +
+                                                      len + sizeof(uint16_t));
+        p = smbios_entries + smbios_entries_len;
+
+        *(uint16_t *)p = cpu_to_le32(len);
+        p += sizeof(uint16_t);
+
+        off = 0;
+        do {
+            r = read(fd, p + off, len);
+            if (r > 0) {
+                off += r;
+                len -= r;
+            } else if ((r < 0 && errno != EINTR) || r == 0) {
+                close(fd);
+                return -1;
+            }
+        } while (len);
+
+        close(fd);
+
+        smbios_entries_len += s.st_size + sizeof(uint16_t);
+        (*(uint16_t *)smbios_entries) =
+	        cpu_to_le32(le32_to_cpu(*(uint16_t *)smbios_entries) + 1);
+    } while (*f);
+
+    return 0;
+}
hw/pc.c +3 -0
index 69f25f3..ec65e33 100644
@@ -51,6 +51,7 @@
 #define ACPI_DATA_SIZE       0x10000
 #define BIOS_CFG_IOPORT 0x510
 #define FW_CFG_ACPI_TABLES (FW_CFG_ARCH_LOCAL + 0)
+#define FW_CFG_SMBIOS_ENTRIES (FW_CFG_ARCH_LOCAL + 1)
 
 #define MAX_IDE_BUS 2
 
@@ -442,6 +443,8 @@ static void bochs_bios_init(void)
     fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
     fw_cfg_add_bytes(fw_cfg, FW_CFG_ACPI_TABLES, (uint8_t *)acpi_tables,
                      acpi_tables_len);
+    fw_cfg_add_bytes(fw_cfg, FW_CFG_SMBIOS_ENTRIES, (uint8_t *)smbios_entries,
+                     smbios_entries_len);
 }
 
 /* Generate an initial boot sector which sets state and jump to
hw/pc.h +3 -0
index 5b378d4..6c200b3 100644
@@ -106,12 +106,15 @@ int ioport_get_a20(void);
 extern int acpi_enabled;
 extern char *acpi_tables;
 extern size_t acpi_tables_len;
+extern char *smbios_entries;
+extern size_t smbios_entries_len;
 
 i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
                        qemu_irq sci_irq);
 void piix4_smbus_register_device(SMBusDevice *dev, uint8_t addr);
 void acpi_bios_init(void);
 int acpi_table_add(const char *table_desc);
+int smbios_entry_add(const char *smbios_entry);
 
 /* hpet.c */
 extern int no_hpet;
vl.c +9 -0
index b62a2d4..372b83c 100644
@@ -4061,6 +4061,7 @@ static void help(int exitcode)
            "-no-hpet        disable HPET\n"
            "-acpitable [sig=str][,rev=n][,oem_id=str][,oem_table_id=str][,oem_rev=n][,asl_compiler_id=str][,asl_compiler_rev=n][,data=file1[:file2]...]\n"
            "                ACPI table description\n"
+           "-smbios file1[,file2]  SMBIOS entry\n"
 #endif
            "Linux boot specific:\n"
            "-kernel bzImage use 'bzImage' as kernel image\n"
@@ -4201,6 +4202,7 @@ enum {
     QEMU_OPTION_no_acpi,
     QEMU_OPTION_no_hpet,
     QEMU_OPTION_acpitable,
+    QEMU_OPTION_smbios,
 
     /* Linux boot specific: */
     QEMU_OPTION_kernel,
@@ -4322,6 +4324,7 @@ static const QEMUOption qemu_options[] = {
     { "no-acpi", 0, QEMU_OPTION_no_acpi },
     { "no-hpet", 0, QEMU_OPTION_no_hpet },
     { "acpitable", HAS_ARG, QEMU_OPTION_acpitable },
+    { "smbios", HAS_ARG, QEMU_OPTION_smbios },
 #endif
 
     /* Linux boot specific: */
@@ -5152,6 +5155,12 @@ int main(int argc, char **argv, char **envp)
                     exit(1);
                 }
                 break;
+            case QEMU_OPTION_smbios:
+                if(smbios_entry_add(optarg) < 0) {
+                    fprintf(stderr, "Wrong smbios provided\n");
+                    exit(1);
+                }
+                break;
 #endif
 #ifdef USE_KQEMU
             case QEMU_OPTION_no_kqemu:
#2 lore raw

Re: [PATCH 1/2] qemu: Allow SMBIOS entries to be loaded and provided to the VM BIOS

Quoted 11 lines · Alex Williamson

Create a new -smbios options that takes binary SMBIOS entries to provide to the VM BIOS. The binary can be easily generated using something like:

dmidecode -t 1 -u | grep $'^\t\t[^"]' | xargs -n1 | \
	perl -lne 'printf "%c", hex($_)' > smbios_type_1.bin

For some inventory tools, this makes the VM report the system information for the host. One entry per binary file, multiple files can be chained together as:

Quoted 8 lines

-smbios file1,file2,...

or specified independently:

-smbios file1 -smbios file2

Hi Alex,

I know we have to support blobs because of OEM specific smbios entries, but there are a number of common ones that it would probably be good to specify in a less user-unfriendly way. What do you think?

Anyway, comments below.

hw/acpi.c out: · line 920

Quoted 12 lines
hw/acpi.c +3 -0
index 52f50a0..0bd93bf 100644
@@ -915,3 +915,69 @@ out:
     }
     return -1;
 }
+
+char *smbios_entries;
+size_t smbios_entries_len;

I think an accessor would be better than making these variables global.

hw/acpi.c out: · line 923

Quoted 3 lines
hw/acpi.c +2 -0
+int smbios_entry_add(const char *t)
+{

acpi.c is hardware emulation, I'd rather see the command line parsing done somewhere else (like vl.c).

Quoted 7 lines
hw/acpi.c +6 -0
+    struct stat s;
+    char file[1024], *p, *f, *n;
+    int fd, r;
+    size_t len, off;
+
+    f = (char *)t;

hw/acpi.c out: · line 939

Quoted 10 lines
hw/acpi.c +10 -0
+    do {
+        n = strchr(f, ',');
+        if (n) {
+            strncpy(file, f, (n - f));
+            file[n - f] = '\0';
+            f = n + 1;
+        } else {
+            strcpy(file, f);
+            f += strlen(file);
+        }

I'm happy to just require multiple -smbios options. I dislike overloading with ','s even though we do it a lot in QEMU.

hw/acpi.c out: · line 948

Quoted 9 lines
hw/acpi.c +8 -0
+        fd = open(file, O_RDONLY);
+        if (fd < 0)
+            return -1;
+
+        if (fstat(fd, &s) < 0) {
+            close(fd);
+            return -1;
+        }

May want to look at load_image/get_image_size.

Signature · 3 lines
Regards,

Anthony Liguori
#3 lore raw

Re: [PATCH 1/2] qemu: Allow SMBIOS entries to be loaded and provided to the VM BIOS

Hi Anthony,

Quoted 5 lines · Anthony Liguori

Alex Williamson wrote:

I know we have to support blobs because of OEM specific smbios entries, but there are a number of common ones that it would probably be good to specify in a less user-unfriendly way. What do you think?

Yeah, I'll admit this is a pretty unfriendly interface. I get from your comment on the other part of the patch that you'd prefer not to get into the mess of having both binary blobs and command line switches augmenting the blobs. This seems reasonable, but also means that we need a way to fully define the tables we generate from the command line. For a type 0 entry, that might mean the following set of switches:

-bios-version, -bios-date, -bios-characteristics, -bios-release

And for a type 1:

-system-manufacturer, -system-name, -system-version, -system-serial, -system-sku, -system-family

type 3:

-chassis-manufacturer, -chassis-type, -chassis-version, -chassis-serial, -chassis-asset, -chassis-oem

I'm sure I'm missing some, plus we might want to allow the memory and processor entries to have some fields changed. Do we want to add that many switches and means to access them from the rombios?

Quoted 16 lines

Anyway, comments below.

hw/acpi.c out: · line 920

Quoted 12 lines
hw/acpi.c +3 -0
index 52f50a0..0bd93bf 100644
@@ -915,3 +915,69 @@ out:
     }
     return -1;
 }
+
+char *smbios_entries;
+size_t smbios_entries_len;

I think an accessor would be better than making these variables global.

Ok

Quoted 6 lines

hw/acpi.c out: · line 923

Quoted 3 lines
hw/acpi.c +2 -0
+int smbios_entry_add(const char *t)
+{

acpi.c is hardware emulation, I'd rather see the command line parsing done somewhere else (like vl.c).

Ok. acpi.c was just a convenient place to not bother architectures that don't care about smbios.

Quoted 18 lines
Quoted 7 lines
hw/acpi.c +6 -0
+    struct stat s;
+    char file[1024], *p, *f, *n;
+    int fd, r;
+    size_t len, off;
+
+    f = (char *)t;

hw/acpi.c out: · line 939

Quoted 10 lines
hw/acpi.c +10 -0
+    do {
+        n = strchr(f, ',');
+        if (n) {
+            strncpy(file, f, (n - f));
+            file[n - f] = '\0';
+            f = n + 1;
+        } else {
+            strcpy(file, f);
+            f += strlen(file);
+        }
Quoted 2 lines

I'm happy to just require multiple -smbios options. I dislike overloading with ','s even though we do it a lot in QEMU.

Yup, I didn't have it initially, but added it because I thought someone might complain other qemu options allow it.

Quoted 11 lines

hw/acpi.c out: · line 948

Quoted 9 lines
hw/acpi.c +8 -0
+        fd = open(file, O_RDONLY);
+        if (fd < 0)
+            return -1;
+
+        if (fstat(fd, &s) < 0) {
+            close(fd);
+            return -1;
+        }

May want to look at load_image/get_image_size.

Will do. Thanks,

Alex

#4 lore raw

Re: [PATCH 1/2] qemu: Allow SMBIOS entries to be loaded and provided to the VM BIOS

Quoted 9 lines · Alex Williamson

Hi Anthony,

Quoted 4 lines · Anthony Liguori

Alex Williamson wrote:

I know we have to support blobs because of OEM specific smbios entries, but there are a number of common ones that it would probably be good to specify in a less user-unfriendly way. What do you think?

Quoted 9 lines

Yeah, I'll admit this is a pretty unfriendly interface. I get from your comment on the other part of the patch that you'd prefer not to get into the mess of having both binary blobs and command line switches augmenting the blobs. This seems reasonable, but also means that we need a way to fully define the tables we generate from the command line. For a type 0 entry, that might mean the following set of switches:

-bios-version, -bios-date, -bios-characteristics, -bios-release

You could go one level higher:

-smbios type=0,bios-version='1.0',bios-date='2009/10/20' etc.

Quoted 4 lines

I'm sure I'm missing some, plus we might want to allow the memory and processor entries to have some fields changed. Do we want to add that many switches and means to access them from the rombios?

I think it's okay to start with some of the more common tables and provide the parsing in QEMU. We could then introduce humanize more tables down the road as people saw fit.

At the end of the day, I'm most interested in the tables that are going to be frequently used by management applications. That is, the tables that are required for things like SVVP certification should be specified in a human readable format that QEMU can build reasonable defaults for and management tools can override.

I'm torn between exposing the tables directly to the firmware or providing a higher level interface. I really don't like -uuid overriding a binary blob though so I'd prefer to avoid that. -uuid should only be respected if using the QEMU generated version of the SMBIOS table. I'll defer to whatever you think is better what is exposed in the firmware interface as I can see arguments for both.

Signature · 3 lines
Regards,

Anthony Liguori
#5 lore raw

Re: Re: [PATCH 1/2] qemu: Allow SMBIOS entries to be loaded and provided to the VM BIOS

Quoted 19 lines · Anthony Liguori
Quoted 18 lines · Alex Williamson

Hi Anthony,

Quoted 6 lines · Anthony Liguori

Alex Williamson wrote:

I know we have to support blobs because of OEM specific smbios entries, but there are a number of common ones that it would probably be good to specify in a less user-unfriendly way. What do you think?

Yeah, I'll admit this is a pretty unfriendly interface. I get from your comment on the other part of the patch that you'd prefer not to get into the mess of having both binary blobs and command line switches augmenting the blobs. This seems reasonable, but also means that we need a way to fully define the tables we generate from the command line. For a type 0 entry, that might mean the following set of switches:

Quoted 6 lines
Quoted 2 lines

-bios-version, -bios-date, -bios-characteristics, -bios-release

You could go one level higher:

-smbios type=0,bios-version='1.0',bios-date='2009/10/20' etc.

That helps, but we have the same complexity in getting the data into the the bios. Adding a new QEMU_CFG_* for each field in every table we want to specify seems excessive. I'm half tempted to generate all the smbios entries in qemu and push them through a port to the bios. That would leave a lot of duplicate code in bochs though. Maybe the bios could read a stream of these through the port:

uint16_t length;
uint8_t type; /* 0: field, 1: table */

union {
    struct {
        uint8_t type; /* smbios entry type */
        uint8_t offset;
        uint8_t data[];
    } field;
    struct {
        uint8_t data[]; /* binary blob */
    } table;
} entry;

We could convert uuid to use this too. The bios doesn't have a very effective means to seek through these, but maybe its not an issue as long as these entries are sparsely used. We could also use the table generation to enforce mutual exclusion between specifying fields and tables to avoid the uuid issue in the previous set. Other ideas? Thanks,

Alex

#6 lore raw

Re: Re: [PATCH 1/2] qemu: Allow SMBIOS entries to be loaded and provided to the VM BIOS

Quoted 5 lines · Alex Williamson

On Mon, 2009-04-06 at 17:42 -0500, Anthony Liguori wrote:

That helps, but we have the same complexity in getting the data into the the bios. Adding a new QEMU_CFG_* for each field in every table we want to specify seems excessive.

Right.

Quoted 18 lines

I'm half tempted to generate all the smbios
entries in qemu and push them through a port to the bios. That would leave a lot of duplicate code in bochs though. Maybe the bios could read a stream of these through the port:

uint16_t length;
uint8_t type; /* 0: field, 1: table */

union {
    struct {
        uint8_t type; /* smbios entry type */
        uint8_t offset;
        uint8_t data[];
    } field;
    struct {
        uint8_t data[]; /* binary blob */
    } table;
} entry;
Quoted 1 line

We could convert uuid to use this too.

Yes, this is what I was leaning toward too.

Quoted 6 lines

The bios doesn't have a very
effective means to seek through these, but maybe its not an issue as long as these entries are sparsely used. We could also use the table generation to enforce mutual exclusion between specifying fields and tables to avoid the uuid issue in the previous set. Other ideas?

I'm pretty happy with this. The argument against it is that if we pass higher level information down via the FW interface, other types of FW (like OpenBIOS) could also use that information in a meaningful way. The problem is, beyond things like UUID, you start to get into pretty specific stuff and I'm not convinced it would all generalize very well. OEM tables are also impossible to represent this way.

So yeah, plumbing the tables directly through to the BIOS seems to make sense to me.

Quoted 6 lines

Thanks,

Alex

Signature · 3 lines
Regards,

Anthony Liguori
#7 lore raw

PATCH 2/2 qemu:bios: Read external SMBIOS entries from the VM

In reply to Alex Williamson

SMBIOS entries can be read from the VM using the same mechanism as additional ACPI tables. External entries will supercede generated entries.

--

bios/rombios32.c +127 -26
index 7be4216..f0e0f8c 100644
@@ -471,6 +471,7 @@ void wrmsr_smp(uint32_t index, uint64_t val)
 #define QEMU_CFG_UUID       0x02
 #define QEMU_CFG_ARCH_LOCAL     0x8000
 #define QEMU_CFG_ACPI_TABLES  (QEMU_CFG_ARCH_LOCAL + 0)
+#define QEMU_CFG_SMBIOS_ENTRIES  (QEMU_CFG_ARCH_LOCAL + 1)
 
 int qemu_cfg_port;
 
@@ -519,6 +520,16 @@ static int acpi_load_table(int i, uint32_t addr, uint16_t *len)
     qemu_cfg_read((uint8_t*)addr, *len);
     return 0;
 }
+
+static uint16_t smbios_entries(void)
+{
+    uint16_t cnt;
+
+    qemu_cfg_select(QEMU_CFG_SMBIOS_ENTRIES);
+    qemu_cfg_read((uint8_t*)&cnt, sizeof(cnt));
+
+    return cnt;
+}
 #endif
 
 void uuid_probe(void)
@@ -1966,7 +1977,7 @@ smbios_entry_point_init(void *start,
 /* Type 0 -- BIOS Information */
 #define RELEASE_DATE_STR "01/01/2007"
 static void *
-smbios_type_0_init(void *start)
+smbios_init_type_0(void *start)
 {
     struct smbios_type_0 *p = (struct smbios_type_0 *)start;
 
@@ -2002,7 +2013,7 @@ smbios_type_0_init(void *start)
 
 /* Type 1 -- System Information */
 static void *
-smbios_type_1_init(void *start)
+smbios_init_type_1(void *start)
 {
     struct smbios_type_1 *p = (struct smbios_type_1 *)start;
     p->header.type = 1;
@@ -2028,7 +2039,7 @@ smbios_type_1_init(void *start)
 
 /* Type 3 -- System Enclosure */
 static void *
-smbios_type_3_init(void *start)
+smbios_init_type_3(void *start)
 {
     struct smbios_type_3 *p = (struct smbios_type_3 *)start;
 
@@ -2058,7 +2069,7 @@ smbios_type_3_init(void *start)
 
 /* Type 4 -- Processor Information */
 static void *
-smbios_type_4_init(void *start, unsigned int cpu_number)
+smbios_init_type_4(void *start, unsigned int cpu_number)
 {
     struct smbios_type_4 *p = (struct smbios_type_4 *)start;
 
@@ -2098,7 +2109,7 @@ smbios_type_4_init(void *start, unsigned int cpu_number)
 
 /* Type 16 -- Physical Memory Array */
 static void *
-smbios_type_16_init(void *start, uint32_t memsize, int nr_mem_devs)
+smbios_init_type_16(void *start, uint32_t memsize, int nr_mem_devs)
 {
     struct smbios_type_16 *p = (struct smbios_type_16*)start;
 
@@ -2121,7 +2132,7 @@ smbios_type_16_init(void *start, uint32_t memsize, int nr_mem_devs)
 
 /* Type 17 -- Memory Device */
 static void *
-smbios_type_17_init(void *start, uint32_t memory_size_mb, int instance)
+smbios_init_type_17(void *start, uint32_t memory_size_mb, int instance)
 {
     struct smbios_type_17 *p = (struct smbios_type_17 *)start;
 
@@ -2151,7 +2162,7 @@ smbios_type_17_init(void *start, uint32_t memory_size_mb, int instance)
 
 /* Type 19 -- Memory Array Mapped Address */
 static void *
-smbios_type_19_init(void *start, uint32_t memory_size_mb, int instance)
+smbios_init_type_19(void *start, uint32_t memory_size_mb, int instance)
 {
     struct smbios_type_19 *p = (struct smbios_type_19 *)start;
 
@@ -2172,7 +2183,7 @@ smbios_type_19_init(void *start, uint32_t memory_size_mb, int instance)
 
 /* Type 20 -- Memory Device Mapped Address */
 static void *
-smbios_type_20_init(void *start, uint32_t memory_size_mb, int instance)
+smbios_init_type_20(void *start, uint32_t memory_size_mb, int instance)
 {
     struct smbios_type_20 *p = (struct smbios_type_20 *)start;
 
@@ -2196,7 +2207,7 @@ smbios_type_20_init(void *start, uint32_t memory_size_mb, int instance)
 
 /* Type 32 -- System Boot Information */
 static void *
-smbios_type_32_init(void *start)
+smbios_init_type_32(void *start)
 {
     struct smbios_type_32 *p = (struct smbios_type_32 *)start;
 
@@ -2214,7 +2225,7 @@ smbios_type_32_init(void *start)
 
 /* Type 127 -- End of Table */
 static void *
-smbios_type_127_init(void *start)
+smbios_init_type_127(void *start)
 {
     struct smbios_type_127 *p = (struct smbios_type_127 *)start;
 
@@ -2228,6 +2239,91 @@ smbios_type_127_init(void *start)
     return start + 2;
 }
 
+static int
+smbios_load_external(int type, char **p, char **q, unsigned *nr_structs,
+                     unsigned *max_struct_size)
+{
+#ifdef BX_QEMU
+    static uint64_t used_bitmap[4] = { 0 };
+    static uint16_t used_cnt = 0;
+    char *start = *p;
+    uint16_t len;
+    int i;
+
+    /* Keep track of the entry types we've already processed */
+    if (used_bitmap[(type >> 6) & 0x3] & (1ULL << (type & 0x3f)))
+        return 1;
+
+    /* Skip end markers, they could lead to bogus tables */
+    if (type == 127)
+        return 0;
+
+    /* Check if there are any tables left to report, also reset read index */
+    i = smbios_entries();
+    if (used_cnt == i)
+        return 0;
+
+    for (; i > 0; *q = *p, i--) {
+        int string_data;
+        qemu_cfg_read((uint8_t*)&len, sizeof(len));
+        if (!len)
+            continue;
+        if (len < sizeof(struct smbios_structure_header)) {
+            while (len--)
+                inb(QEMU_CFG_DATA_PORT); /* Invalid, skip to the next one */
+            continue;
+        }
+
+        qemu_cfg_read((uint8_t*)*p, sizeof(struct smbios_structure_header));
+        if (((struct smbios_structure_header *)*p)->type != type) {
+            len -= sizeof(struct smbios_structure_header);
+            while (len--)
+                inb(QEMU_CFG_DATA_PORT); /* skip to the next one */
+            continue;
+        }
+
+        /* Entries end with a double NULL char, if there's a string at
+         * the end (length is greater than formatted length), the string
+         * terminator provides the first NULL. */
+        string_data = (len > ((struct smbios_structure_header *)*p)->length);
+
+        /* Read the rest and terminate the entry */
+        len -= sizeof(struct smbios_structure_header);
+        *p += sizeof(struct smbios_structure_header);
+        qemu_cfg_read((uint8_t*)*p, len);
+        *p += len;
+        *((uint8_t*)*p) = 0;
+        (*p)++;
+        if (!string_data) {
+            *((uint8_t*)*p) = 0;
+            (*p)++;
+        }
+
+        (*nr_structs)++;
+        used_cnt++;
+        if (*p - *q > *max_struct_size)
+            *max_struct_size = *p - *q;
+
+        /* Mark that we've reported on this type */
+        used_bitmap[(type >> 6) & 0x3] |= (1ULL << (type & 0x3f));
+
+        /* A type 1 entry provides a UUID, but so does the QEMU_CFG_UUID
+         * port.  If the QEMU_CFG_UUID value is not zero, use it, otherwise
+         * use whatever was in the provided table. */
+        if (type == 1) {
+            const static uint8_t null_uuid[16] = { 0 };
+            if (memcmp(bios_uuid, null_uuid, 16)) {
+                struct smbios_type_1 *t = (struct smbios_type_1 *)*q;
+                memcpy(t->uuid, bios_uuid, 16);
+            }
+        }
+    }
+    return (start != *p);
+#else /* !BX_QEMU */
+    return 0;
+#endif
+}
+
 void smbios_init(void)
 {
     unsigned cpu_num, nr_structs = 0, max_struct_size = 0;
@@ -2246,34 +2342,39 @@ void smbios_init(void)
 
 	p = (char *)start + sizeof(struct smbios_entry_point);
 
-#define add_struct(fn) do{ \
-    q = (fn); \
-    nr_structs++; \
-    if ((q - p) > max_struct_size) \
-        max_struct_size = q - p; \
-    p = q; \
+#define add_struct(type, args...) do{ \
+    if (!smbios_load_external(type, &p, &q, &nr_structs, &max_struct_size)) { \
+        q = smbios_init_type_##type(args); \
+        nr_structs++; \
+        if ((q - p) > max_struct_size) \
+            max_struct_size = q - p; \
+        p = q; \
+    } \
 }while (0)
 
-    add_struct(smbios_type_0_init(p));
-    add_struct(smbios_type_1_init(p));
-    add_struct(smbios_type_3_init(p));
+    add_struct(0, p);
+    add_struct(1, p);
+    add_struct(3, p);
     for (cpu_num = 1; cpu_num <= smp_cpus; cpu_num++)
-        add_struct(smbios_type_4_init(p, cpu_num));
+        add_struct(4, p, cpu_num);
 
     /* Each 'memory device' covers up to 16GB of address space. */
     nr_mem_devs = (memsize + 0x3fff) >> 14;
-    add_struct(smbios_type_16_init(p, memsize, nr_mem_devs));
+    add_struct(16, p, memsize, nr_mem_devs);
     for ( i = 0; i < nr_mem_devs; i++ )
     {
         uint32_t dev_memsize = ((i == (nr_mem_devs - 1))
                                 ? (((memsize-1) & 0x3fff)+1) : 0x4000);
-        add_struct(smbios_type_17_init(p, dev_memsize, i));
-        add_struct(smbios_type_19_init(p, dev_memsize, i));
-        add_struct(smbios_type_20_init(p, dev_memsize, i));
+        add_struct(17, p, dev_memsize, i);
+        add_struct(19, p, dev_memsize, i);
+        add_struct(20, p, dev_memsize, i);
     }
 
-    add_struct(smbios_type_32_init(p));
-    add_struct(smbios_type_127_init(p));
+    add_struct(32, p);
+    /* Add any remaining provided entries before the end marker */
+    for (i = 0; i < 256; i++)
+        smbios_load_external(i, &p, &q, &nr_structs, &max_struct_size);
+    add_struct(127, p);
 
 #undef add_struct
 
#8 lore raw

Re: [PATCH 2/2] qemu:bios: Read external SMBIOS entries from the VM

Quoted 7 lines · Alex Williamson

SMBIOS entries can be read from the VM using the same mechanism as additional ACPI tables. External entries will supercede generated entries.

bios/rombios32.c +1 -0
+

bios/rombios32.c smbios_type_127_init(void *start) · line 2319

Quoted 10 lines
bios/rombios32.c +10 -0
+        /* A type 1 entry provides a UUID, but so does the QEMU_CFG_UUID
+         * port.  If the QEMU_CFG_UUID value is not zero, use it, otherwise
+         * use whatever was in the provided table. */
+        if (type == 1) {
+            const static uint8_t null_uuid[16] = { 0 };
+            if (memcmp(bios_uuid, null_uuid, 16)) {
+                struct smbios_type_1 *t = (struct smbios_type_1 *)*q;
+                memcpy(t->uuid, bios_uuid, 16);
+            }
+        }

This is what I was getting at in my other post. I'd sort of rather that a certain set of SMBIOS tables be specified at a higher level (like uuid, manufacture, vendor, etc.) and the blobs just be the OEM tables. I think that matches the work going on with the device configuration files more appropriately.

Regards,

Anthony Liguori

#9 lore raw
In reply to Alex Williamson

Is there any interest in this series? Aside from copying host SMBIOS entries, it also seems useful for providing information to the guest about their virtual machine pool (perhaps via a type 3 entry), or whatever other bits of data someone might find useful (type 11, OEM string for instance). Thanks,

Alex

Quoted 17 lines · Alex Williamson

This series adds a new -smbios option for x86 that allows individual SMBIOS entries to be passed into the guest VM. This follows the same basic path as the support for loading ACPI tables. While SMBIOS is independent of ACPI, I chose to add the smbios_entry_add() function to acpi.c because they're both somewhat PC BIOS related (and ia64 can support SMBIOS and might be able to make use of it there).

This feature allows the guest to see certain properties of the host if configured correctly. For instance, the system model and serial number in the type 1 entry. Obviously its only built at boot, so doesn't get updated for migration scenarios. User provided entries will supersede generated entries, so care should be taken when passing entries which describe physical properties, such as memory size and address ranges. Thanks,

Alex

#10 lore raw
Quoted 7 lines · Alex Williamson

Is there any interest in this series? Aside from copying host SMBIOS entries, it also seems useful for providing information to the guest about their virtual machine pool (perhaps via a type 3 entry), or whatever other bits of data someone might find useful (type 11, OEM string for instance). Thanks,

I think the patch is useful. Haven't looked at implementation though.

Quoted 18 lines

Alex

Quoted 15 lines · Alex Williamson

This series adds a new -smbios option for x86 that allows individual SMBIOS entries to be passed into the guest VM. This follows the same basic path as the support for loading ACPI tables. While SMBIOS is independent of ACPI, I chose to add the smbios_entry_add() function to acpi.c because they're both somewhat PC BIOS related (and ia64 can support SMBIOS and might be able to make use of it there).

This feature allows the guest to see certain properties of the host if configured correctly. For instance, the system model and serial number in the type 1 entry. Obviously its only built at boot, so doesn't get updated for migration scenarios. User provided entries will supersede generated entries, so care should be taken when passing entries which describe physical properties, such as memory size and address ranges. Thanks,

Quoted 8 lines
Quoted 2 lines

Alex

--
To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Gleb.

#11 lore raw
In reply to Alex Williamson
Quoted 7 lines · Alex Williamson

Is there any interest in this series? Aside from copying host SMBIOS entries, it also seems useful for providing information to the guest about their virtual machine pool (perhaps via a type 3 entry), or whatever other bits of data someone might find useful (type 11, OEM string for instance). Thanks,

Quoted 17 lines

Alex

Quoted 14 lines · Alex Williamson

This series adds a new -smbios option for x86 that allows individual SMBIOS entries to be passed into the guest VM. This follows the same basic path as the support for loading ACPI tables. While SMBIOS is independent of ACPI, I chose to add the smbios_entry_add() function to acpi.c because they're both somewhat PC BIOS related (and ia64 can support SMBIOS and might be able to make use of it there).

This feature allows the guest to see certain properties of the host if configured correctly. For instance, the system model and serial number in the type 1 entry. Obviously its only built at boot, so doesn't get updated for migration scenarios. User provided entries will supersede generated entries, so care should be taken when passing entries which describe physical properties, such as memory size and address ranges. Thanks,

I can't help thinking that if we wish to provide metadata to guest OS like system model, serial number, etc, then we'd be better off using explicit named flags (or QEMU config file settings once that exists)

-system-serial 2141241521 -system-model "Some Virtual Machine"

and have QEMU generate the neccessary SMBIOS data, or other equivalent data tables to suit the non-PC based machine types for which SMBIOS is not relevant.

Regards,
Daniel

Signature · 4 lines
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
#12 lore raw
Quoted 9 lines · Daniel P. Berrange

I can't help thinking that if we wish to provide metadata to guest OS like system model, serial number, etc, then we'd be better off using explicit named flags (or QEMU config file settings once that exists)

-system-serial 2141241521 -system-model "Some Virtual Machine"

and have QEMU generate the neccessary SMBIOS data, or other equivalent data tables to suit the non-PC based machine types for which SMBIOS is not relevant.

-smbios serial=blah,model=bleach ?

Signature · 1 line
error compiling committee.c: too many arguments to function
#13 lore raw
Quoted 5 lines · Avi Kivity
Quoted 4 lines · Daniel P. Berrange

I can't help thinking that if we wish to provide metadata to guest OS like system model, serial number, etc, then we'd be better off using explicit named flags (or QEMU config file settings once that exists)

Quoted 9 lines
Quoted 6 lines

-system-serial 2141241521 -system-model "Some Virtual Machine"

and have QEMU generate the neccessary SMBIOS data, or other equivalent data tables to suit the non-PC based machine types for which SMBIOS is not relevant.

-smbios serial=blah,model=bleach ?

Unfortunately that does make them smbios specific, while I think Daniel is pointing out that several options may be useful on other platforms.

This is basically the same issue we have with -uuid already. -uuid is a non-smbios specific option, but rombios will incorporate the data when it builds the type 1 entry. I've retained this functionality, so that a -uuid option will override the uuid in a passed in type 1 entry. This could be further extended with separate patches to provide serial or model numbers generically, but allow them to override smbios values. This seems complimentary to the patches in this series, but I don't think it replaces all the functionality we get from a raw smbios entry interface. Thanks,

Alex