[Qemu-devel] [PATCH] Correct SMBIOS handling of multiple tables

[Qemu-devel] [PATCH] Correct SMBIOS handling of multiple tables

[Beth Kon]

The current code prevents multiple entries of the same type table, as required,
for example, by table type 4.


Signed-off-by:  Beth Kon <eak@us.ibm.com>


diff --git a/bios/rombios32.c b/bios/rombios32.c
index f861f81..43aa065 100644
--- a/bios/rombios32.c
+++ b/bios/rombios32.c
@@ -2146,6 +2146,10 @@ struct smbios_table {
 #define SMBIOS_FIELD_ENTRY 0
 #define SMBIOS_TABLE_ENTRY 1
 
+#ifdef BX_QEMU
+    static uint64_t smbios_used_bitmap[4] = { 0 };
+#endif
+
 static size_t
 smbios_load_field(int type, size_t offset, void *addr)
 {
@@ -2496,14 +2500,9 @@ smbios_load_external(int type, char **p, unsigned *nr_structs,
                      unsigned *max_struct_size)
 {
 #ifdef BX_QEMU
-    static uint64_t used_bitmap[4] = { 0 };
     char *start = *p;
     int i;
 
-    /* Check if we've already reported these tables */
-    if (used_bitmap[(type >> 6) & 0x3] & (1ULL << (type & 0x3f)))
-        return 1;
-
     /* Don't introduce spurious end markers */
     if (type == 127)
         return 0;
@@ -2555,7 +2554,7 @@ smbios_load_external(int type, char **p, unsigned *nr_structs,
     }
 
     /* Mark that we've reported on this type */
-    used_bitmap[(type >> 6) & 0x3] |= (1ULL << (type & 0x3f));
+    smbios_used_bitmap[(type >> 6) & 0x3] |= (1ULL << (type & 0x3f));
 
     return (start != *p);
 #else /* !BX_QEMU */
@@ -2612,6 +2611,9 @@ void smbios_init(void)
     add_struct(32, p);
     /* Add any remaining provided entries before the end marker */
     for (i = 0; i < 256; i++)
+        /* Check if we've already reported these tables */
+        if (smbios_used_bitmap[(i >> 6) & 0x3] & (1ULL << (i & 0x3f)))
+            continue;
         smbios_load_external(i, &p, &nr_structs, &max_struct_size);
     add_struct(127, p);
 

[Qemu-devel] Re: [PATCH] Correct SMBIOS handling of multiple tables

[Alex Williamson]

On Wed, 2009-05-27 at 17:42 -0400, Beth Kon wrote:
> The current code prevents multiple entries of the same type table, as required,
> for example, by table type 4.

Hi Beth,

Are you trying to add multiple type 4 entries from a single type 4
binary passed in via -smbios file=<type4>?  ex:

qemu -smp 2 -smbios file=type4.bin

My intention was that once the user overrides an entry with a binary
image, they're responsible for providing all of the entries of that
type.  That means for a 2-way guest, if you want to override the type 4
entry with a binary, you need to specify it twice on the command line.
The code below would allow you to load a type 4 entry for each vCPU
while only specifying one on the command line, but what if you want 2
slightly different entries?  We have no way of associating a provided
entry to the one needed at table creating time, which is why we load
them all on the first pass and ignore requests to add the same type
again later.  Thanks,

Alex


> diff --git a/bios/rombios32.c b/bios/rombios32.c
> index f861f81..43aa065 100644
> --- a/bios/rombios32.c
> +++ b/bios/rombios32.c
> @@ -2146,6 +2146,10 @@ struct smbios_table {
>  #define SMBIOS_FIELD_ENTRY 0
>  #define SMBIOS_TABLE_ENTRY 1
>  
> +#ifdef BX_QEMU
> +    static uint64_t smbios_used_bitmap[4] = { 0 };
> +#endif
> +
>  static size_t
>  smbios_load_field(int type, size_t offset, void *addr)
>  {
> @@ -2496,14 +2500,9 @@ smbios_load_external(int type, char **p, unsigned *nr_structs,
>                       unsigned *max_struct_size)
>  {
>  #ifdef BX_QEMU
> -    static uint64_t used_bitmap[4] = { 0 };
>      char *start = *p;
>      int i;
>  
> -    /* Check if we've already reported these tables */
> -    if (used_bitmap[(type >> 6) & 0x3] & (1ULL << (type & 0x3f)))
> -        return 1;
> -
>      /* Don't introduce spurious end markers */
>      if (type == 127)
>          return 0;
> @@ -2555,7 +2554,7 @@ smbios_load_external(int type, char **p, unsigned *nr_structs,
>      }
>  
>      /* Mark that we've reported on this type */
> -    used_bitmap[(type >> 6) & 0x3] |= (1ULL << (type & 0x3f));
> +    smbios_used_bitmap[(type >> 6) & 0x3] |= (1ULL << (type & 0x3f));
>  
>      return (start != *p);
>  #else /* !BX_QEMU */
> @@ -2612,6 +2611,9 @@ void smbios_init(void)
>      add_struct(32, p);
>      /* Add any remaining provided entries before the end marker */
>      for (i = 0; i < 256; i++)
> +        /* Check if we've already reported these tables */
> +        if (smbios_used_bitmap[(i >> 6) & 0x3] & (1ULL << (i & 0x3f)))
> +            continue;
>          smbios_load_external(i, &p, &nr_structs, &max_struct_size);
>      add_struct(127, p);
>  
> 

Re: [Qemu-devel] Re: [PATCH] Correct SMBIOS handling of multiple tables

[Beth Kon]

Alex Williamson wrote:
> On Wed, 2009-05-27 at 17:42 -0400, Beth Kon wrote:
>   
>> The current code prevents multiple entries of the same type table, as required,
>> for example, by table type 4.
>>     
>
> Hi Beth,
>
> Are you trying to add multiple type 4 entries from a single type 4
> binary passed in via -smbios file=<type4>?  ex:
>
> qemu -smp 2 -smbios file=type4.bin
>
> My intention was that once the user overrides an entry with a binary
> image, they're responsible for providing all of the entries of that
> type.  That means for a 2-way guest, if you want to override the type 4
> entry with a binary, you need to specify it twice on the command line.
> The code below would allow you to load a type 4 entry for each vCPU
> while only specifying one on the command line, but what if you want 2
> slightly different entries?  We have no way of associating a provided
> entry to the one needed at table creating time, which is why we load
> them all on the first pass and ignore requests to add the same type
> again later.  Thanks,
>
> Alex
>   
Hi Alex.

No, I wasn't trying to allow only one table to be specified on the 
command line for multiple same-type tables. The user is still 
responsible for providing all the entries. 

Without this patch, the current code has a problem even in the case of 
default tables (nothing specified on command line). Following the code 
path for add_struct(4, p, cpu_num) (for example) add_struct is called 
once for each cpu. add_struct calls smbios_load_external, and the first 
thing that does is check used_bitmap. The first pass through for table 4 
works. But if the vm is smp, the second pass through doesn't work 
because used_bitmap reports that a table 4 entry was already created, so 
smbios_load_external returns a 1 and add_struct becomes a noop in 
effect. I think this patch does what you intended, to override default 
creation with external creation if specified on the command line, and to 
get only one complete set of tables for each type.

>
>   
>> diff --git a/bios/rombios32.c b/bios/rombios32.c
>> index f861f81..43aa065 100644
>> --- a/bios/rombios32.c
>> +++ b/bios/rombios32.c
>> @@ -2146,6 +2146,10 @@ struct smbios_table {
>>  #define SMBIOS_FIELD_ENTRY 0
>>  #define SMBIOS_TABLE_ENTRY 1
>>  
>> +#ifdef BX_QEMU
>> +    static uint64_t smbios_used_bitmap[4] = { 0 };
>> +#endif
>> +
>>  static size_t
>>  smbios_load_field(int type, size_t offset, void *addr)
>>  {
>> @@ -2496,14 +2500,9 @@ smbios_load_external(int type, char **p, unsigned *nr_structs,
>>                       unsigned *max_struct_size)
>>  {
>>  #ifdef BX_QEMU
>> -    static uint64_t used_bitmap[4] = { 0 };
>>      char *start = *p;
>>      int i;
>>  
>> -    /* Check if we've already reported these tables */
>> -    if (used_bitmap[(type >> 6) & 0x3] & (1ULL << (type & 0x3f)))
>> -        return 1;
>> -
>>      /* Don't introduce spurious end markers */
>>      if (type == 127)
>>          return 0;
>> @@ -2555,7 +2554,7 @@ smbios_load_external(int type, char **p, unsigned *nr_structs,
>>      }
>>  
>>      /* Mark that we've reported on this type */
>> -    used_bitmap[(type >> 6) & 0x3] |= (1ULL << (type & 0x3f));
>> +    smbios_used_bitmap[(type >> 6) & 0x3] |= (1ULL << (type & 0x3f));
>>  
>>      return (start != *p);
>>  #else /* !BX_QEMU */
>> @@ -2612,6 +2611,9 @@ void smbios_init(void)
>>      add_struct(32, p);
>>      /* Add any remaining provided entries before the end marker */
>>      for (i = 0; i < 256; i++)
>> +        /* Check if we've already reported these tables */
>> +        if (smbios_used_bitmap[(i >> 6) & 0x3] & (1ULL << (i & 0x3f)))
>> +            continue;
>>          smbios_load_external(i, &p, &nr_structs, &max_struct_size);
>>      add_struct(127, p);
>>  
>>
>>     
>
>
>
>
>   

Re: [Qemu-devel] Re: [PATCH] Correct SMBIOS handling of multiple tables

[Alex Williamson]

On Wed, 2009-05-27 at 20:10 -0400, Beth Kon wrote:
> Without this patch, the current code has a problem even in the case of 
> default tables (nothing specified on command line). Following the code 
> path for add_struct(4, p, cpu_num) (for example) add_struct is called 
> once for each cpu. add_struct calls smbios_load_external, and the first 
> thing that does is check used_bitmap. The first pass through for table 4 
> works. But if the vm is smp, the second pass through doesn't work 
> because used_bitmap reports that a table 4 entry was already created, so 
> smbios_load_external returns a 1 and add_struct becomes a noop in 
> effect. I think this patch does what you intended, to override default 
> creation with external creation if specified on the command line, and to 
> get only one complete set of tables for each type.

Oh, I see the problem now, thanks for the explanation.  I don't think
that's the right fix though.  If we remove the duplicate check at the
start of smbios_load_external() and the user passes a binary, we might
add it more than once.  For instance the case where the user specifies
'-smp 2' and passes 2 type 4 entries, they would end up with 4 type 4
entries.

Don't we just need to only mark the bitmap if we load something
externally?  Something like this:

Signed-off-by: Alex Williamson <alex.williamson@hp.com>
--

diff --git a/bios/rombios32.c b/bios/rombios32.c
index f861f81..c869798 100644
--- a/bios/rombios32.c
+++ b/bios/rombios32.c
@@ -2554,13 +2554,14 @@ smbios_load_external(int type, char **p, unsigned *nr_structs,
             *max_struct_size = *p - (char *)header;
     }
 
-    /* Mark that we've reported on this type */
-    used_bitmap[(type >> 6) & 0x3] |= (1ULL << (type & 0x3f));
+    if (start != *p) {
+        /* Mark that we've reported on this type */
+        used_bitmap[(type >> 6) & 0x3] |= (1ULL << (type & 0x3f));
+        return 1;
+    }
 
-    return (start != *p);
-#else /* !BX_QEMU */
+#endif /* !BX_QEMU */
     return 0;
-#endif
 }
 
 void smbios_init(void)

Re: [Qemu-devel] Re: [PATCH] Correct SMBIOS handling of multiple tables

[Beth Kon]

[-- Attachment #1: Type: text/plain, Size: 2359 bytes --]

Alex Williamson wrote:
> On Wed, 2009-05-27 at 20:10 -0400, Beth Kon wrote:
>   
>> Without this patch, the current code has a problem even in the case of 
>> default tables (nothing specified on command line). Following the code 
>> path for add_struct(4, p, cpu_num) (for example) add_struct is called 
>> once for each cpu. add_struct calls smbios_load_external, and the first 
>> thing that does is check used_bitmap. The first pass through for table 4 
>> works. But if the vm is smp, the second pass through doesn't work 
>> because used_bitmap reports that a table 4 entry was already created, so 
>> smbios_load_external returns a 1 and add_struct becomes a noop in 
>> effect. I think this patch does what you intended, to override default 
>> creation with external creation if specified on the command line, and to 
>> get only one complete set of tables for each type.
>>     
>
> Oh, I see the problem now, thanks for the explanation.  I don't think
> that's the right fix though.  If we remove the duplicate check at the
> start of smbios_load_external() and the user passes a binary, we might
> add it more than once.  For instance the case where the user specifies
> '-smp 2' and passes 2 type 4 entries, they would end up with 4 type 4
> entries.
>   
Binaries don't get added more than once with my patch, as long as the 
"proper" number of binaries is entered (e.g., number of table 4 entries 
matches number of cpus specified). But I think your approach has an 
advantage if an improper number of tables is entered. If one table 4 
binary is entered and -smp 2 is specified, my patch would create 2 
identical tables and yours would only create one. Wrong in both cases, 
but yours matches what the user entered.

This leads to the question of whether qemu should verify that 
command-line smbios tables make "sense" (e.g. number of type 4 tables == 
smp_cpus).  The SMBIOS spec has many cases where multiples of a table 
are required. Covering all of them would be very burdensome with little 
or no payback. But I think it is worth putting in a check for table 4, 
since it is relatively simple and is pretty likely to be specified on 
the command line.

One other nit I fixed was detection of a bad filename.

So if you agree, I'd like to submit your rombios32.c patch along with my 
table 4 check patch (combined here for simplicity).

[-- Attachment #2: smbios_fix.patch --]
[-- Type: text/x-patch, Size: 2812 bytes --]

diff --git a/hw/smbios.c b/hw/smbios.c
index ced90ce..a8b52c7 100644
--- a/hw/smbios.c
+++ b/hw/smbios.c
@@ -173,8 +173,8 @@ int smbios_entry_add(const char *t)
         struct smbios_table *table;
         int size = get_image_size(buf);
 
-        if (size < sizeof(struct smbios_structure_header)) {
-            fprintf(stderr, "Cannot read smbios file %s", buf);
+        if (size == -1 || size < sizeof(struct smbios_structure_header)) {
+            fprintf(stderr, "Cannot read smbios file %s\n", buf);
             exit(1);
         }
 
@@ -196,6 +196,9 @@ int smbios_entry_add(const char *t)
 
         header = (struct smbios_structure_header *)(table->data);
         smbios_check_collision(header->type, SMBIOS_TABLE_ENTRY);
+        if (header->type == 4) {
+            smbios_type4_count++;
+        }
 
         smbios_entries_len += sizeof(*table) + size;
         (*(uint16_t *)smbios_entries) =
diff --git a/kvm/bios/rombios32.c b/kvm/bios/rombios32.c
index cbd5f15..f3e75f8 100755
--- a/kvm/bios/rombios32.c
+++ b/kvm/bios/rombios32.c
@@ -2531,13 +2531,14 @@ smbios_load_external(int type, char **p, unsigned *nr_structs,
             *max_struct_size = *p - (char *)header;
     }
 
-    /* Mark that we've reported on this type */
-    used_bitmap[(type >> 6) & 0x3] |= (1ULL << (type & 0x3f));
+    if (start != *p) {
+        /* Mark that we've reported on this type */
+        used_bitmap[(type >> 6) & 0x3] |= (1ULL << (type & 0x3f));
+        return 1;
+    }
 
-    return (start != *p);
-#else /* !BX_QEMU */
+#endif /* !BX_QEMU */
     return 0;
-#endif
 }
 
 void smbios_init(void)
diff --git a/pc-bios/bios.bin b/pc-bios/bios.bin
index 70bd7ad..50d5365 100644
Binary files a/pc-bios/bios.bin and b/pc-bios/bios.bin differ
diff --git a/sysemu.h b/sysemu.h
index 47d001e..0b982ed 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -115,6 +115,7 @@ extern int rtc_td_hack;
 extern int alt_grab;
 extern int usb_enabled;
 extern int smp_cpus;
+extern int smbios_type4_count;
 extern int cursor_hide;
 extern int graphic_rotate;
 extern int no_quit;
diff --git a/vl.c b/vl.c
index db8265b..c9cc5b7 100644
--- a/vl.c
+++ b/vl.c
@@ -246,6 +246,7 @@ int singlestep = 0;
 const char *assigned_devices[MAX_DEV_ASSIGN_CMDLINE];
 int assigned_devices_index;
 int smp_cpus = 1;
+int smbios_type4_count = 0;
 const char *vnc_display;
 int acpi_enabled = 1;
 int no_hpet = 0;
@@ -5775,6 +5776,14 @@ int main(int argc, char **argv, char **envp)
         }
     }
 
+#ifdef TARGET_I386
+    if (smbios_type4_count && smbios_type4_count != smp_cpus) {
+         fprintf(stderr,
+                 "count of SMBIOS type 4 tables != SMP CPUs specified.\n");
+         exit(1);
+    }
+#endif
+
 #if defined(CONFIG_KVM) && defined(CONFIG_KQEMU)
     if (kvm_allowed && kqemu_allowed) {
         fprintf(stderr,

Re: [Qemu-devel] Re: [PATCH] Correct SMBIOS handling of multiple tables

[Alex Williamson]

On Thu, 2009-05-28 at 11:05 -0400, Beth Kon wrote:
> This leads to the question of whether qemu should verify that 
> command-line smbios tables make "sense" (e.g. number of type 4 tables == 
> smp_cpus).  The SMBIOS spec has many cases where multiples of a table 
> are required. Covering all of them would be very burdensome with little 
> or no payback. But I think it is worth putting in a check for table 4, 
> since it is relatively simple and is pretty likely to be specified on 
> the command line.

I think that makes sense, we can add other checks as needed.

> One other nit I fixed was detection of a bad filename.
> 
> So if you agree, I'd like to submit your rombios32.c patch along with my 
> table 4 check patch (combined here for simplicity).

Patch comment below....

> diff --git a/hw/smbios.c b/hw/smbios.c
> index ced90ce..a8b52c7 100644
> --- a/hw/smbios.c
> +++ b/hw/smbios.c
> @@ -173,8 +173,8 @@ int smbios_entry_add(const char *t)
>          struct smbios_table *table;
>          int size = get_image_size(buf);
>  
> -        if (size < sizeof(struct smbios_structure_header)) {
> -            fprintf(stderr, "Cannot read smbios file %s", buf);
> +        if (size == -1 || size < sizeof(struct
> smbios_structure_header)) {
> +            fprintf(stderr, "Cannot read smbios file %s\n", buf);
>              exit(1);
>          }
>  
> @@ -196,6 +196,9 @@ int smbios_entry_add(const char *t)
>  
>          header = (struct smbios_structure_header *)(table->data);
>          smbios_check_collision(header->type, SMBIOS_TABLE_ENTRY);
> +        if (header->type == 4) {
> +            smbios_type4_count++;
> +        }
>  
>          smbios_entries_len += sizeof(*table) + size;
>          (*(uint16_t *)smbios_entries) =
> diff --git a/kvm/bios/rombios32.c b/kvm/bios/rombios32.c
> index cbd5f15..f3e75f8 100755
> --- a/kvm/bios/rombios32.c
> +++ b/kvm/bios/rombios32.c
> @@ -2531,13 +2531,14 @@ smbios_load_external(int type, char **p,
> unsigned *nr_structs,
>              *max_struct_size = *p - (char *)header;
>      }
>  
> -    /* Mark that we've reported on this type */
> -    used_bitmap[(type >> 6) & 0x3] |= (1ULL << (type & 0x3f));
> +    if (start != *p) {
> +        /* Mark that we've reported on this type */
> +        used_bitmap[(type >> 6) & 0x3] |= (1ULL << (type & 0x3f));
> +        return 1;
> +    }
>  
> -    return (start != *p);
> -#else /* !BX_QEMU */
> +#endif /* !BX_QEMU */
>      return 0;
> -#endif
>  }
>  
>  void smbios_init(void)
> diff --git a/pc-bios/bios.bin b/pc-bios/bios.bin
> index 70bd7ad..50d5365 100644
> Binary files a/pc-bios/bios.bin and b/pc-bios/bios.bin differ
> diff --git a/sysemu.h b/sysemu.h
> index 47d001e..0b982ed 100644
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -115,6 +115,7 @@ extern int rtc_td_hack;
>  extern int alt_grab;
>  extern int usb_enabled;
>  extern int smp_cpus;
> +extern int smbios_type4_count;
>  extern int cursor_hide;
>  extern int graphic_rotate;
>  extern int no_quit;
> diff --git a/vl.c b/vl.c
> index db8265b..c9cc5b7 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -246,6 +246,7 @@ int singlestep = 0;
>  const char *assigned_devices[MAX_DEV_ASSIGN_CMDLINE];
>  int assigned_devices_index;
>  int smp_cpus = 1;
> +int smbios_type4_count = 0;
>  const char *vnc_display;
>  int acpi_enabled = 1;
>  int no_hpet = 0;
> @@ -5775,6 +5776,14 @@ int main(int argc, char **argv, char **envp)
>          }
>      }
>  
> +#ifdef TARGET_I386
> +    if (smbios_type4_count && smbios_type4_count != smp_cpus) {
> +         fprintf(stderr,
> +                 "count of SMBIOS type 4 tables != SMP CPUs
> specified.\n");
> +         exit(1);
> +    }
> +#endif
> +

I'm not thrilled with adding globals and externs in unrelated parts of
the code.  I think we can keep this all internal to smbios.c if we make
a new smbios_validate_table() function that gets called from
smbios_get_table().  Thanks,

Alex

Re: [Qemu-devel] Re: [PATCH] Correct SMBIOS handling of multiple tables

[Beth Kon]

Alex Williamson wrote:
> On Thu, 2009-05-28 at 11:05 -0400, Beth Kon wrote:
>   
>> This leads to the question of whether qemu should verify that 
>> command-line smbios tables make "sense" (e.g. number of type 4 tables == 
>> smp_cpus).  The SMBIOS spec has many cases where multiples of a table 
>> are required. Covering all of them would be very burdensome with little 
>> or no payback. But I think it is worth putting in a check for table 4, 
>> since it is relatively simple and is pretty likely to be specified on 
>> the command line.
>>     
>
> I think that makes sense, we can add other checks as needed.
>
>   
>> One other nit I fixed was detection of a bad filename.
>>
>> So if you agree, I'd like to submit your rombios32.c patch along with my 
>> table 4 check patch (combined here for simplicity).
>>     
>
> Patch comment below....
>
>   
>> diff --git a/hw/smbios.c b/hw/smbios.c
>> index ced90ce..a8b52c7 100644
>> --- a/hw/smbios.c
>> +++ b/hw/smbios.c
>> @@ -173,8 +173,8 @@ int smbios_entry_add(const char *t)
>>          struct smbios_table *table;
>>          int size = get_image_size(buf);
>>  
>> -        if (size < sizeof(struct smbios_structure_header)) {
>> -            fprintf(stderr, "Cannot read smbios file %s", buf);
>> +        if (size == -1 || size < sizeof(struct
>> smbios_structure_header)) {
>> +            fprintf(stderr, "Cannot read smbios file %s\n", buf);
>>              exit(1);
>>          }
>>  
>> @@ -196,6 +196,9 @@ int smbios_entry_add(const char *t)
>>  
>>          header = (struct smbios_structure_header *)(table->data);
>>          smbios_check_collision(header->type, SMBIOS_TABLE_ENTRY);
>> +        if (header->type == 4) {
>> +            smbios_type4_count++;
>> +        }
>>  
>>          smbios_entries_len += sizeof(*table) + size;
>>          (*(uint16_t *)smbios_entries) =
>> diff --git a/kvm/bios/rombios32.c b/kvm/bios/rombios32.c
>> index cbd5f15..f3e75f8 100755
>> --- a/kvm/bios/rombios32.c
>> +++ b/kvm/bios/rombios32.c
>> @@ -2531,13 +2531,14 @@ smbios_load_external(int type, char **p,
>> unsigned *nr_structs,
>>              *max_struct_size = *p - (char *)header;
>>      }
>>  
>> -    /* Mark that we've reported on this type */
>> -    used_bitmap[(type >> 6) & 0x3] |= (1ULL << (type & 0x3f));
>> +    if (start != *p) {
>> +        /* Mark that we've reported on this type */
>> +        used_bitmap[(type >> 6) & 0x3] |= (1ULL << (type & 0x3f));
>> +        return 1;
>> +    }
>>  
>> -    return (start != *p);
>> -#else /* !BX_QEMU */
>> +#endif /* !BX_QEMU */
>>      return 0;
>> -#endif
>>  }
>>  
>>  void smbios_init(void)
>> diff --git a/pc-bios/bios.bin b/pc-bios/bios.bin
>> index 70bd7ad..50d5365 100644
>> Binary files a/pc-bios/bios.bin and b/pc-bios/bios.bin differ
>> diff --git a/sysemu.h b/sysemu.h
>> index 47d001e..0b982ed 100644
>> --- a/sysemu.h
>> +++ b/sysemu.h
>> @@ -115,6 +115,7 @@ extern int rtc_td_hack;
>>  extern int alt_grab;
>>  extern int usb_enabled;
>>  extern int smp_cpus;
>> +extern int smbios_type4_count;
>>  extern int cursor_hide;
>>  extern int graphic_rotate;
>>  extern int no_quit;
>> diff --git a/vl.c b/vl.c
>> index db8265b..c9cc5b7 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -246,6 +246,7 @@ int singlestep = 0;
>>  const char *assigned_devices[MAX_DEV_ASSIGN_CMDLINE];
>>  int assigned_devices_index;
>>  int smp_cpus = 1;
>> +int smbios_type4_count = 0;
>>  const char *vnc_display;
>>  int acpi_enabled = 1;
>>  int no_hpet = 0;
>> @@ -5775,6 +5776,14 @@ int main(int argc, char **argv, char **envp)
>>          }
>>      }
>>  
>> +#ifdef TARGET_I386
>> +    if (smbios_type4_count && smbios_type4_count != smp_cpus) {
>> +         fprintf(stderr,
>> +                 "count of SMBIOS type 4 tables != SMP CPUs
>> specified.\n");
>> +         exit(1);
>> +    }
>> +#endif
>> +
>>     
>
> I'm not thrilled with adding globals and externs in unrelated parts of
> the code.  I think we can keep this all internal to smbios.c if we make
> a new smbios_validate_table() function that gets called from
> smbios_get_table().  Thanks,
>
> Alex
>
>
>   
Yep, I agree. I didn't like that either. Your solution is better. I'll 
change that and submit the userspace part and, as discussed, you can 
submit the rombios32.c change.