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

[Gleb Natapov]

On Sun, Nov 22, 2009 at 09:41:26PM +0100, Sebastian Herbszt wrote:
> Gleb Natapov wrote:
> >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 <address@hidden>
> >>>>>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?
> 
> I would suggest to remove the line if it still does pass the test.
> 
As a different patch. Also may be putting real info there instead of
just deleting the line?

> [snip]
> 
> >>>>>/* 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.
> 
> I don't think this argumentation brings us forward. After all i could argue 
> for stopping using Intels
> pci vendor id for the pci bridge since they didn't manufactured it either.
> 
pci ids are different in that they are used to find driver for a device.
If there was a field in PCI config space to store device manufacturer
name it would be reasonable to put "QEMU" there.

This SMBIOS field describe CPU manufacturer and serves only informational
purpose. Look at /proc/cpuinfo on qemu VM. The model name reported there
is "QEMU Virtual CPU version 0.9.1" not some real value.

--
                        Gleb.