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

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

Gleb Natapov wrote:
> On Mon, Nov 23, 2009 at 07:15:55PM +0100, Sebastian Herbszt wrote:
>> Gleb Natapov wrote:
>> >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 <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?
>> >>
>> >>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?
>> 
>> Ok - sounds good if bios_characteristics gets proper system based values.
>> 
> Kevin can you help here. I can send a patch, but I am not sure I know
> everything SeaBIOS supports.

seabios needs to check the system it runs on and then build the value, e.g. ISA is always (?)
supported, but pci only if a pci bridge is found. Kevin should be able to provide a common
base value tho (APM, Boot from CD is supported, EDD is supported, etc.).

>> >>[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.
>> 
>> Actually mine has
>> 
>> vendor_id: GenuineIntel
>> model_name: Pentium II (Klamath)
>> 
> How you run it? With -cpu pentium? I use default one (qemu64 I think).

I also use the default one (i386-softmmu target). Looking at target-i386/helper.c the following
cpu types provide real values: phenom, core2duo, coreduo, 486, pentium, pentium2, pentium3, n270.
If you use qemu64 you get vendor AMD and the model_id you mentioned above.

>> Might be different on KVM tho (or if you specify -cpu). Beside if seabios is used with coreboot on a real
>> system the cpu vendor is not QEMU; nor is it on Bochs.
>> 
> Yes, coreboot should specify real CPU manufacturer.

What about using the vendor provided by CPUID, so it displays the correct value on coreboot and others, and
change qemu cpus to a different vendor string like padded QEMU or something. Currently qemu64 uses AMD,
kvm64 and qemu32 Intel.

- Sebastian