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

[Gleb Natapov <gleb@redhat.com>]

On Sun, Nov 22, 2009 at 06:39:16PM +0100, Sebastian Herbszt wrote:
> 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?
> 
I will, but I don't expect different result. Why should I?

> >>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.
> 
Yes.

> >>
> >>>    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?
> 
Well, real system don't report cpuid value here why should we? It is
QEMU and not intel or amd manufactured this CPU after all.

> >>
> >>>/* 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.
> 
I guess Microsoft got lazy, so they used the same program they use to
certify real physical HW for windows logo program. So some of the things
they require doesn't make sense for virtualization.

--
			Gleb.