[Qemu-devel] Re: [PATCH][SEABIOS] Make SMBIOS table pass MS SVVP test

["Sebastian Herbszt" <herbszt@gmx.de>]

Gleb Natapov wrote:
> On Sun, Nov 22, 2009 at 05:51:41PM +0100, Sebastian Herbszt wrote:
>> Gleb Natapov wrote:
>> >Microsoft SVVP (Server Virtualization Validation Program) expects
>> >arbitrary SMBIOS field to have certain values otherwise it fails.
>> >We all want to make Microsoft happy don't we? So lets put values MS
>> >expects in there.
>> >
>> >Values modified by the patch:
>> >Type 0:
>> > Bit 2 of byte 2 must be 1
>> >Type 1:
>> > Manufacturer/product string should not be empty
>> >Type 3:
>> > Manufacturer string should not be empty
>> >Type 4:
>> > Processor manufacturer should no be empty
>> > Max/current CPU speed shouldn't be unknown
>> >Type 16:
>> > Memory should have error correction.
>> >
>> >Signed-off-by: Gleb Natapov <gleb@redhat.com>
>> >diff --git a/src/smbios.c b/src/smbios.c
>> >index f1b43f2..332bb4e 100644
>> >--- a/src/smbios.c
>> >+++ b/src/smbios.c
>> >@@ -96,7 +96,8 @@ smbios_init_type_0(void *start)
>> >    memset(p->bios_characteristics, 0, 8);
>> >    p->bios_characteristics[0] = 0x08; /* BIOS characteristics not supported */
>> >    p->bios_characteristics_extension_bytes[0] = 0;
>> >-    p->bios_characteristics_extension_bytes[1] = 0;
>> >+    /* Enable targeted content distribution. Needed for SVVP */
>> >+    p->bios_characteristics_extension_bytes[1] = 4;
>> >
>> >    if (!qemu_cfg_smbios_load_field(0, offsetof(struct smbios_type_0,
>> >                                                system_bios_major_release),
>> 
>> Are the BIOS characteristics extension bytes valid if BIOS characteristics is not supported?
> I have no idea. SVVP test complains though.

p->bios_characteristics[0] = 0x08; /* BIOS characteristics not supported */

Can you retest with this line removed?

>> Is the requirement for "Targeted Content Delivery" specified somewhere with something more
>> clear than "SMBIOS data is useful in identifying the computer for targeted delivery of
>> model-specific software and firmware content" (e.g. changing BIOS version, release date, etc.)?
>> 
>> >@@ -130,8 +131,8 @@ smbios_init_type_1(void *start)
>> >    p->header.length = sizeof(struct smbios_type_1);
>> >    p->header.handle = 0x100;
>> >
>> >-    load_str_field_or_skip(1, manufacturer_str);
>> >-    load_str_field_or_skip(1, product_name_str);
>> >+    load_str_field_with_default(1, manufacturer_str, "QEMU");
>> >+    load_str_field_with_default(1, product_name_str, "QEMU");
>> 
>> Use "QEMU Virtual Platform" product name derivated from "VMware Virtual Platform" ?
>> Use "QEMU Project" or "QEMU Community" throughout for manufacturer name?
> I'll change this to use defines as per Kevin's request, so to keep number of defines to minimum
> lets make it "QEMU Project" everywhere.

Will you introduce something like CONFIG_SYSVENDOR? Would be useful in case some other project
(Bochs, Xen?) starts to use seabios.

>> 
>> >    load_str_field_or_skip(1, version_str);
>> >    load_str_field_or_skip(1, serial_number_str);
>> >
>> >@@ -165,7 +166,7 @@ smbios_init_type_3(void *start)
>> >    p->header.length = sizeof(struct smbios_type_3);
>> >    p->header.handle = 0x300;
>> >
>> >-    p->manufacturer_str = 0;
>> >+    p->manufacturer_str = 1;
>> >    p->type = 0x01; /* other */
>> >    p->version_str = 0;
>> >    p->serial_number_str = 0;
>> >@@ -180,9 +181,9 @@ smbios_init_type_3(void *start)
>> >    p->contained_element_count = 0;
>> >
>> >    start += sizeof(struct smbios_type_3);
>> >-    *((u16 *)start) = 0;
>> >+    memcpy((char *)start, "QEMU\0\0", 6);
>> 
>> s.a.
>> 
>> >-    return start+2;
>> >+    return start+6;
>> >}
>> >
>> >/* Type 4 -- Processor Information */
>> >@@ -198,7 +199,7 @@ smbios_init_type_4(void *start, unsigned int cpu_number)
>> >    p->socket_designation_str = 1;
>> >    p->processor_type = 0x03; /* CPU */
>> >    p->processor_family = 0x01; /* other */
>> >-    p->processor_manufacturer_str = 0;
>> >+    p->processor_manufacturer_str = 2;
>> >
>> >    u32 cpuid_signature, ebx, ecx, cpuid_features;
>> >    cpuid(1, &cpuid_signature, &ebx, &ecx, &cpuid_features);
>> >@@ -209,8 +210,8 @@ smbios_init_type_4(void *start, unsigned int cpu_number)
>> >    p->voltage = 0;
>> >    p->external_clock = 0;
>> >
>> >-    p->max_speed = 0; /* unknown */
>> >-    p->current_speed = 0; /* unknown */
>> >+    p->max_speed = 2000;
>> >+    p->current_speed = 2000;
>> >
>> >    p->status = 0x41; /* socket populated, CPU enabled */
>> >    p->processor_upgrade = 0x01; /* other */
>> >@@ -221,10 +222,10 @@ smbios_init_type_4(void *start, unsigned int cpu_number)
>> >
>> >    start += sizeof(struct smbios_type_4);
>> >
>> >-    memcpy((char *)start, "CPU  " "\0" "" "\0" "", 7);
>> >- ((char *)start)[4] = cpu_number + '0';
>> >+    memcpy((char *)start, "CPU  \0QEMU\0\0", 12);
>> >+    ((char *)start)[4] = cpu_number + '0';
>> >
>> >-    return start+7;
>> >+    return start+12;
>> >}
>> 
>> Should the manufacturer not depend on the emulated cpu? At least VMware uses the output from
>> CPUID (GenuineIntel) ; tho my BIOS does just report "Intel".
> I what it to be something fictional. We support migration from Intel to
> AMD and back so this info is meaningless in virtualization environment.

Does the system still report "GenuineIntel" if migrated from Intel to AMD host?
I don't see a problem reporting the emulated cpu vendor, since it's not supposed to change during
the lifetime of a VM, right?

>> 
>> >/* Type 16 -- Physical Memory Array */
>> >@@ -239,7 +240,7 @@ smbios_init_type_16(void *start, u32 memory_size_mb, int nr_mem_devs)
>> >
>> >    p->location = 0x01; /* other */
>> >    p->use = 0x03; /* system memory */
>> >-    p->error_correction = 0x01; /* other */
>> >+    p->error_correction = 0x06; /* Multi-bit ECC to make Microsoft happy */
>> >    p->maximum_capacity = memory_size_mb * 1024;
>> >    p->memory_error_information_handle = 0xfffe; /* none provided */
>> >    p->number_of_memory_devices = nr_mem_devs;
>> 
>> Does it happen to work with "Unknown" or "None"?
>> 
> No. They explicitly request single or multi-bit ECC. Though I was told
> that Microsoft gives exception for this requirement.

Odd. Why do they care whether the VM reports (non existent) error correction support.

> --
> Gleb.

- Sebastian