| Message ID | 20091122140853.GI3193@redhat.com |
|---|---|
| State | New |
| Headers | show |
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
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
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.
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
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.
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 > > >
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
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
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.
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.
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.
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.
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
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
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.
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
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.
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.
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
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
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.
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
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.
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
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
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.
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
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;
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> -- Gleb.