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

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

[Gleb Natapov]

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),
@@ -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");
     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);
 
-    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;
 }
 
 /* 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;
--
			Gleb.

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

[Sebastian Herbszt]

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?
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?

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

> /* 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"?

> --
> Gleb.
> 

- Sebastian

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

[Gleb Natapov]

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.

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

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

> 
> >/* 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.

--
			Gleb.

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

[Sebastian Herbszt]

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

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

[Gleb Natapov]

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.

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

[Sebastian Herbszt]

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.

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

- Sebastian

[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 <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?

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

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

[Sebastian Herbszt]

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.

>> [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)

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.

- Sebastian

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

[Gleb Natapov]

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.

> >>[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).

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

--
			Gleb.

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

[Sebastian Herbszt]

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

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

[Gleb Natapov]

On Mon, Nov 23, 2009 at 07:48:20PM +0100, Sebastian Herbszt wrote:
> 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.
> 
We can't change CPUID leaf 0 in QEMU. There are programs out there that
relies on this value. coreboot can do whatever is right for them in
if (CONFIG_COREBOOT).
 
--
			Gleb.

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

[Gleb Natapov]

On Mon, Nov 23, 2009 at 09:39:52PM +0200, Gleb Natapov wrote:
> On Mon, Nov 23, 2009 at 07:48:20PM +0100, Sebastian Herbszt wrote:
> > 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.
> > 
> We can't change CPUID leaf 0 in QEMU. There are programs out there that
> relies on this value. coreboot can do whatever is right for them in
> if (CONFIG_COREBOOT).
>  
BTW coreboot has to setup correct manufacturer info in other tables too.
Table 1 system info. Table 2 base board Info, Table 3 chassis info.
 
--
			Gleb.

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

[Kevin O'Connor]

On Mon, Nov 23, 2009 at 08:30:41PM +0200, Gleb Natapov wrote:
> On Mon, Nov 23, 2009 at 07:15:55PM +0100, Sebastian Herbszt wrote:
> > 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.

I don't know what bios_characteristics should be set to.

SeaBIOS does generate the smbios table even on coreboot.  This is a
hack to work around the fact that coreboot boards don't generate
smbios tables currently and Linux wont use ACPI unless an smbios table
is present.  So, the smbios table is just used to make Linux accept
the acpi table.  It is not a requirement that SeaBIOS be able to
generate fully populated and correct smbios tables for coreboot - it's
understood that any coreboot user that needs a full smbios table needs
to have that table generated by coreboot itself.

That said, I think SeaBIOS should autodetect any values where that's
feasible.  So, for example, if the cpu identification is available via
cpuid, then I think that should be used.  However, for example, if cpu
model isn't available anywhere, then I think hardcoding something is
okay.

> > >>>>>>>-    p->max_speed = 0; /* unknown */
> > >>>>>>>-    p->current_speed = 0; /* unknown */
> > >>>>>>>+    p->max_speed = 2000;
> > >>>>>>>+    p->current_speed = 2000;

SeaBIOS detects the current Mhz - see calibrate_tsc() in src/clock.c.

-Kevin

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

[Gleb Natapov]

On Tue, Nov 24, 2009 at 10:57:02AM -0500, Kevin O'Connor wrote:
> On Mon, Nov 23, 2009 at 08:30:41PM +0200, Gleb Natapov wrote:
> > On Mon, Nov 23, 2009 at 07:15:55PM +0100, Sebastian Herbszt wrote:
> > > 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.
> 
> I don't know what bios_characteristics should be set to.
> 
http://www.phoenix.com/NR/rdonlyres/51EEA1E6-20C1-4FA2-A3D8-AD8E45335C47/0/specssmbios.pdf
Page 30 have the description.

> SeaBIOS does generate the smbios table even on coreboot.  This is a
> hack to work around the fact that coreboot boards don't generate
> smbios tables currently and Linux wont use ACPI unless an smbios table
> is present.  So, the smbios table is just used to make Linux accept
> the acpi table.  It is not a requirement that SeaBIOS be able to
> generate fully populated and correct smbios tables for coreboot - it's
> understood that any coreboot user that needs a full smbios table needs
> to have that table generated by coreboot itself.
> 
> That said, I think SeaBIOS should autodetect any values where that's
> feasible.  So, for example, if the cpu identification is available via
> cpuid, then I think that should be used.  However, for example, if cpu
> model isn't available anywhere, then I think hardcoding something is
> okay.
It is used already where appropriate. To fill processor_id field in type
4 table. CPU manufacturer is different issue. CPU a guest is running on is
not manufactured by Intel or AMD, it is emulated by QEMU.
 
> 
> > > >>>>>>>-    p->max_speed = 0; /* unknown */
> > > >>>>>>>-    p->current_speed = 0; /* unknown */
> > > >>>>>>>+    p->max_speed = 2000;
> > > >>>>>>>+    p->current_speed = 2000;
> 
> SeaBIOS detects the current Mhz - see calibrate_tsc() in src/clock.c.
> 
How accurate is it? What if I boot 100 guests on 16 cpu host
simultaneously? Not uncommon scenario. Those field really have no
meaning in virtualization environment. I'd rather have predictable
values there from boot to boot. Who know what Windows may use them for.

--
			Gleb.

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

[Kevin O'Connor]

On Tue, Nov 24, 2009 at 06:59:01PM +0200, Gleb Natapov wrote:
> On Tue, Nov 24, 2009 at 10:57:02AM -0500, Kevin O'Connor wrote:
> > On Mon, Nov 23, 2009 at 08:30:41PM +0200, Gleb Natapov wrote:
> > > On Mon, Nov 23, 2009 at 07:15:55PM +0100, Sebastian Herbszt wrote:
> > > > 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.
> > 
> > I don't know what bios_characteristics should be set to.
> > 
> http://www.phoenix.com/NR/rdonlyres/51EEA1E6-20C1-4FA2-A3D8-AD8E45335C47/0/specssmbios.pdf
> Page 30 have the description.

   Bit 4   ISA is supported
   Bit 7   PCI is supported
   Bit 9   Plug and Play is supported
  Bit 10   APM is supported
  Bit 12   BIOS shadowing is allowed
  Bit 15   Boot from CD is supported
  Bit 16   Selectable Boot is supported
  Bit 19   EDD (Enhanced Disk Drive) Specification is supported
  Bit 22   Int 13h - 5.25" / 360 KB Floppy Services are supported
  Bit 23   Int 13h - 5.25" /1.2MB Floppy Services are supported
  Bit 24   Int 13h - 3.5" / 720 KB Floppy Services are supported
  Bit 25   Int 13h - 3.5" / 2.88 MB Floppy Services are supported
  Bit 27   Int 9h, 8042 Keyboard services are supported
  Bit 28   Int 14h, Serial Services are supported
  Bit 29   Int 17h, Printer Services are supported
  Bit 30   Int 10h, CGA/Mono Video Services are supported

Several of the above are dependent on CONFIG_xxx settings.

However, I'm not really sure how the above overlaps with your SVVP
patch..

> It is used already where appropriate. To fill processor_id field in type
> 4 table. CPU manufacturer is different issue. CPU a guest is running on is
> not manufactured by Intel or AMD, it is emulated by QEMU.

Okay.

> > > > >>>>>>>-    p->max_speed = 0; /* unknown */
> > > > >>>>>>>-    p->current_speed = 0; /* unknown */
> > > > >>>>>>>+    p->max_speed = 2000;
> > > > >>>>>>>+    p->current_speed = 2000;
> > 
> > SeaBIOS detects the current Mhz - see calibrate_tsc() in src/clock.c.
> > 
> How accurate is it? What if I boot 100 guests on 16 cpu host
> simultaneously? Not uncommon scenario.

It's not very accurate under qemu due to Linux power saving code which
will frequently change cpu speeds.  (The length of the runqueue
doesn't seem to skew it, but the power stuff does.)

>Those field really have no
> meaning in virtualization environment. I'd rather have predictable
> values there from boot to boot. Who know what Windows may use them for.

Fair enough.  There is also a risk that Windows will do the wrong
thing with the dummy values.  However, I agree with your point about
consistency.

-Kevin

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

[Gleb Natapov]

On Tue, Nov 24, 2009 at 12:53:00PM -0500, Kevin O'Connor wrote:
> On Tue, Nov 24, 2009 at 06:59:01PM +0200, Gleb Natapov wrote:
> > On Tue, Nov 24, 2009 at 10:57:02AM -0500, Kevin O'Connor wrote:
> > > On Mon, Nov 23, 2009 at 08:30:41PM +0200, Gleb Natapov wrote:
> > > > On Mon, Nov 23, 2009 at 07:15:55PM +0100, Sebastian Herbszt wrote:
> > > > > 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.
> > > 
> > > I don't know what bios_characteristics should be set to.
> > > 
> > http://www.phoenix.com/NR/rdonlyres/51EEA1E6-20C1-4FA2-A3D8-AD8E45335C47/0/specssmbios.pdf
> > Page 30 have the description.
> 
>    Bit 4   ISA is supported
>    Bit 7   PCI is supported
>    Bit 9   Plug and Play is supported
>   Bit 10   APM is supported
>   Bit 12   BIOS shadowing is allowed
>   Bit 15   Boot from CD is supported
>   Bit 16   Selectable Boot is supported
>   Bit 19   EDD (Enhanced Disk Drive) Specification is supported
>   Bit 22   Int 13h - 5.25" / 360 KB Floppy Services are supported
>   Bit 23   Int 13h - 5.25" /1.2MB Floppy Services are supported
>   Bit 24   Int 13h - 3.5" / 720 KB Floppy Services are supported
>   Bit 25   Int 13h - 3.5" / 2.88 MB Floppy Services are supported
>   Bit 27   Int 9h, 8042 Keyboard services are supported
>   Bit 28   Int 14h, Serial Services are supported
>   Bit 29   Int 17h, Printer Services are supported
>   Bit 30   Int 10h, CGA/Mono Video Services are supported
> 
> Several of the above are dependent on CONFIG_xxx settings.
> 
> However, I'm not really sure how the above overlaps with your SVVP
> patch..
> 
It is not. I am just thinking about putting real info there instead of
"not supported". SVVP pass as it is. Do you prefer to leave it as is?

--
			Gleb.

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

[Kevin O'Connor]

On Tue, Nov 24, 2009 at 08:41:47PM +0200, Gleb Natapov wrote:
> On Tue, Nov 24, 2009 at 12:53:00PM -0500, Kevin O'Connor wrote:
> > However, I'm not really sure how the above overlaps with your SVVP
> > patch..
> > 
> It is not. I am just thinking about putting real info there instead of
> "not supported". SVVP pass as it is. Do you prefer to leave it as is?

Okay.  I defer to your and Sebastian's best judgment.

-Kevin

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

[Sebastian Herbszt]

Gleb Natapov wrote:
> On Tue, Nov 24, 2009 at 10:57:02AM -0500, Kevin O'Connor wrote:
>> 
>> That said, I think SeaBIOS should autodetect any values where that's
>> feasible.  So, for example, if the cpu identification is available via
>> cpuid, then I think that should be used.  However, for example, if cpu
>> model isn't available anywhere, then I think hardcoding something is
>> okay.
> It is used already where appropriate. To fill processor_id field in type
> 4 table. CPU manufacturer is different issue. CPU a guest is running on is
> not manufactured by Intel or AMD, it is emulated by QEMU.

I am still wondering why you're against using the vendor reported by CPUID.
The cross vendor host cpu migration doesn't seem to be a sound reason, because
the cpu in the guest is emulated and has no relationship to the host cpu.
If i specify "-cpu phenom", i end up with an AMD cpu. Since noone but AMD
produces this cpu it seems only reasonable to advertise the vendor as AMD.

>> > > >>>>>>>-    p->max_speed = 0; /* unknown */
>> > > >>>>>>>-    p->current_speed = 0; /* unknown */
>> > > >>>>>>>+    p->max_speed = 2000;
>> > > >>>>>>>+    p->current_speed = 2000;
>> 
>> SeaBIOS detects the current Mhz - see calibrate_tsc() in src/clock.c.
>> 
> How accurate is it? What if I boot 100 guests on 16 cpu host
> simultaneously? Not uncommon scenario. Those field really have no
> meaning in virtualization environment. I'd rather have predictable
> values there from boot to boot. Who know what Windows may use them for.

Speaking of not knowing what an OS or application might do with values in the
SMBIOS table. Doesn't the same argument apply to the cpu vendor?

- Sebastian

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

[Gleb Natapov]

On Wed, Nov 25, 2009 at 09:09:19PM +0100, Sebastian Herbszt wrote:
> Gleb Natapov wrote:
> >On Tue, Nov 24, 2009 at 10:57:02AM -0500, Kevin O'Connor wrote:
> >>
> >>That said, I think SeaBIOS should autodetect any values where that's
> >>feasible.  So, for example, if the cpu identification is available via
> >>cpuid, then I think that should be used.  However, for example, if cpu
> >>model isn't available anywhere, then I think hardcoding something is
> >>okay.
> >It is used already where appropriate. To fill processor_id field in type
> >4 table. CPU manufacturer is different issue. CPU a guest is running on is
> >not manufactured by Intel or AMD, it is emulated by QEMU.
> 
> I am still wondering why you're against using the vendor reported by CPUID.
I am still wondering why you want this :) But let me ask you a question:
You are running some program inside QEMU and you encounter a bug. Some
instruction does not update eflags like it should and program fails. Do
you complain to
a) AMD
b) Intel
c) QEMU mailing list.

If your answer is (c), then I don't see how you can propose to put
something else then QEMU in manufacturer field.

> The cross vendor host cpu migration doesn't seem to be a sound reason, because
> the cpu in the guest is emulated and has no relationship to the host cpu.
> If i specify "-cpu phenom", i end up with an AMD cpu. Since noone but AMD
> produces this cpu it seems only reasonable to advertise the vendor as AMD.
> 
> >>> > >>>>>>>-    p->max_speed = 0; /* unknown */
> >>> > >>>>>>>-    p->current_speed = 0; /* unknown */
> >>> > >>>>>>>+    p->max_speed = 2000;
> >>> > >>>>>>>+    p->current_speed = 2000;
> >>
> >>SeaBIOS detects the current Mhz - see calibrate_tsc() in src/clock.c.
> >>
> >How accurate is it? What if I boot 100 guests on 16 cpu host
> >simultaneously? Not uncommon scenario. Those field really have no
> >meaning in virtualization environment. I'd rather have predictable
> >values there from boot to boot. Who know what Windows may use them for.
> 
> Speaking of not knowing what an OS or application might do with values in the
> SMBIOS table. Doesn't the same argument apply to the cpu vendor?
> 
I am concern with SMBIOS table be different on each boot, not what
information is stored in those fields. CPU manufacturer is free form
string. I have computers that have "Intel" there, others have "Intel(R)
Corporation". As long as it consistent from boot to boot it is OK IMO.

--
			Gleb.

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

[Sebastian Herbszt]

Gleb Natapov wrote:
> On Wed, Nov 25, 2009 at 09:09:19PM +0100, Sebastian Herbszt wrote:
>> Gleb Natapov wrote:
>> >On Tue, Nov 24, 2009 at 10:57:02AM -0500, Kevin O'Connor wrote:
>> >>
>> >>That said, I think SeaBIOS should autodetect any values where that's
>> >>feasible.  So, for example, if the cpu identification is available via
>> >>cpuid, then I think that should be used.  However, for example, if cpu
>> >>model isn't available anywhere, then I think hardcoding something is
>> >>okay.
>> >It is used already where appropriate. To fill processor_id field in type
>> >4 table. CPU manufacturer is different issue. CPU a guest is running on is
>> >not manufactured by Intel or AMD, it is emulated by QEMU.
>> 
>> I am still wondering why you're against using the vendor reported by CPUID.
> I am still wondering why you want this :) But let me ask you a question:
> You are running some program inside QEMU and you encounter a bug. Some
> instruction does not update eflags like it should and program fails. Do
> you complain to
> a) AMD
> b) Intel
> c) QEMU mailing list.
> 
> If your answer is (c), then I don't see how you can propose to put
> something else then QEMU in manufacturer field.

Since i know i run the program inside QEMU my answer has to be (c). On the other
hand the competition doesn't put "VMware" there.

>> The cross vendor host cpu migration doesn't seem to be a sound reason, because
>> the cpu in the guest is emulated and has no relationship to the host cpu.
>> If i specify "-cpu phenom", i end up with an AMD cpu. Since noone but AMD
>> produces this cpu it seems only reasonable to advertise the vendor as AMD.
>> 
>> >>> > >>>>>>>-    p->max_speed = 0; /* unknown */
>> >>> > >>>>>>>-    p->current_speed = 0; /* unknown */
>> >>> > >>>>>>>+    p->max_speed = 2000;
>> >>> > >>>>>>>+    p->current_speed = 2000;
>> >>
>> >>SeaBIOS detects the current Mhz - see calibrate_tsc() in src/clock.c.
>> >>
>> >How accurate is it? What if I boot 100 guests on 16 cpu host
>> >simultaneously? Not uncommon scenario. Those field really have no
>> >meaning in virtualization environment. I'd rather have predictable
>> >values there from boot to boot. Who know what Windows may use them for.
>> 
>> Speaking of not knowing what an OS or application might do with values in the
>> SMBIOS table. Doesn't the same argument apply to the cpu vendor?
>> 
> I am concern with SMBIOS table be different on each boot, not what
> information is stored in those fields. CPU manufacturer is free form
> string. I have computers that have "Intel" there, others have "Intel(R)
> Corporation". As long as it consistent from boot to boot it is OK IMO.

Then i must admit i understood your  "Who know what Windows may use them for"
statement different.

- Sebastian

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

[Yaniv Kaul]

On 11/22/2009 7:39 PM, Sebastian Herbszt wrote:


<SNIP>
>>>
>>> >/* 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.


The test probably does not realize it's a VM and SVVP runs the 2008R2 
system tests - which requires ECC memory (and PCI express bus for 
network and block devices, but that's another exception).
Y.

>
>> -- 
>> Gleb.
>
> - Sebastian
>
>
>

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

[Carl-Daniel Hailfinger]

On 22.11.2009 18:39, Sebastian Herbszt wrote:
> Gleb Natapov wrote:
>> On Sun, Nov 22, 2009 at 05:51:41PM +0100, Sebastian Herbszt wrote:
>>> 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.

coreboot already uses SeaBIOS on various x86 hardware platforms.

Regards,
Carl-Daniel

-- 
Developer quote of the month: 
"We are juggling too many chainsaws and flaming arrows and tigers."

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

[Gleb Natapov]

On Mon, Nov 23, 2009 at 12:57:14AM +0100, Carl-Daniel Hailfinger wrote:
> On 22.11.2009 18:39, Sebastian Herbszt wrote:
> > Gleb Natapov wrote:
> >> On Sun, Nov 22, 2009 at 05:51:41PM +0100, Sebastian Herbszt wrote:
> >>> 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.
> 
> coreboot already uses SeaBIOS on various x86 hardware platforms.
> 
But does coreboot use SMBIOS tables generated by SeaBIOS?

--
			Gleb.

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

[Sebastian Herbszt]

Gleb Natapov wrote:
> On Mon, Nov 23, 2009 at 12:57:14AM +0100, Carl-Daniel Hailfinger wrote:
>> On 22.11.2009 18:39, Sebastian Herbszt wrote:
>> > Gleb Natapov wrote:
>> >> On Sun, Nov 22, 2009 at 05:51:41PM +0100, Sebastian Herbszt wrote:
>> >>> 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.
>> 
>> coreboot already uses SeaBIOS on various x86 hardware platforms.
>> 
> But does coreboot use SMBIOS tables generated by SeaBIOS?

seabios seems to generate SMBIOS tables when used with coreboot.
See coreboot_copy_biostable() in coreboot.c:

    // XXX - just create dummy smbios table for now - should detect if
    // smbios/dmi table is found from coreboot and use that instead.
    smbios_init();

- Sebastian

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

[Gleb Natapov]

On Sun, Nov 22, 2009 at 06:39:16PM +0100, Sebastian Herbszt wrote:
> >>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?
> 
Done. No difference.

--
			Gleb.

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

[Kevin O'Connor]

On Sun, Nov 22, 2009 at 04:08:53PM +0200, 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.
[...]
> -    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");
>      load_str_field_or_skip(1, version_str);
>      load_str_field_or_skip(1, serial_number_str);

Can the CONFIG_APPNAMExx defines in config.h be used instead of hard
coding QEMU?  (If desired, the defaults can be changed from bochs to
qemu.)

> -    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';

BTW, snprintf can now be used here.

-Kevin

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

[Gleb Natapov]

On Sun, Nov 22, 2009 at 12:07:50PM -0500, Kevin O'Connor wrote:
> On Sun, Nov 22, 2009 at 04:08:53PM +0200, 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.
> [...]
> > -    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");
> >      load_str_field_or_skip(1, version_str);
> >      load_str_field_or_skip(1, serial_number_str);
> 
> Can the CONFIG_APPNAMExx defines in config.h be used instead of hard
> coding QEMU?  (If desired, the defaults can be changed from bochs to
> qemu.)
Qemu can do it in a local branch. But since Qemu uses SeaBIOS right now and
BOCHS doesn't lets change it.

> 
> > -    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';
> 
> BTW, snprintf can now be used here.
> 
snprintf in SeaBIOS doesn't have return value. In some situation it is
impossible to use it properly without it.


--
			Gleb.

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

[Kevin O'Connor]

On Mon, Nov 23, 2009 at 01:08:30PM +0200, Gleb Natapov wrote:
> > > -    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';
> > 
> > BTW, snprintf can now be used here.
> > 
> snprintf in SeaBIOS doesn't have return value. In some situation it is
> impossible to use it properly without it.

It's straight forward to add - commit 2be312c1.

-Kevin