Hi, We were looking at the dmidecode output from qemu-kvm pre-seabios and current qemu-kvm and noticed some of the strings have changed. The main problem with this is that certain OSes are quite sensitive to system changes and avoiding to change things unnecessarily would probably be a good thing. I wanted to check with the lists if there are any strong feelings about this, and whether some of these changes were made for specific reasons? For example: Handle 0x0000, DMI type 0, 24 bytes BIOS Information - Vendor: QEMU - Version: QEMU + Vendor: Bochs + Version: Bochs Release Date: 01/01/2007 Address: 0xE8000 Runtime Size: 96 kB and this: Handle 0x0401, DMI type 4, 32 bytes Processor Information - Socket Designation: CPU 1 + Socket Designation: CPU01 Type: Central Processor Family: Other - Manufacturer: QEMU - ID: 63 06 00 00 FD FB 8B 07 + Manufacturer: Bochs + ID: 23 06 00 00 FD FB 8B 07 Version: Not Specified Voltage: Unknown External Clock: Unknown I guess the Socket Designation in particular might have been done for a reason? Otherwise, if there are no objections, I'll look at adding some patches to make it more backwards compatible. Cheers, Jes
On Fri, May 28, 2010 at 05:24:47PM +0200, Jes Sorensen wrote:
I guess the Socket Designation in particular might have been done for a reason?
It was part of commit cf2affa6de. And was a result of moving to snprintf() instead of direct string manipulation. Before that string was created like that: memcpy((char *)start, "CPU " "\0" "" "\0" "", 7); ((char *)start)[4] = cpu_number + '0'; Which start to produce strange cpu numbers for cpus greater then 9. I doubt we want to go back to that ;) -- Gleb.
On 05/28/10 17:44, Gleb Natapov wrote:
On Fri, May 28, 2010 at 05:24:47PM +0200, Jes Sorensen wrote:
I guess the Socket Designation in particular might have been done for a reason?
It was part of commit cf2affa6de. And was a result of moving to snprintf() instead of direct string manipulation. Before that string was created like that: memcpy((char *)start, "CPU " "\0" "" "\0" "", 7); ((char *)start)[4] = cpu_number + '0'; Which start to produce strange cpu numbers for cpus greater then 9. I doubt we want to go back to that ;)
Hi Gleb, I see. Well I guess we could do something slightly more compatible by printing along the lines: printf("CPU:"); if (nr < 10) printf(" "); snprintf() Not sure if it is worth it, but it should be doable without reverting to memcpy(). Thoughts? Cheers, Jes
On Mon, May 31, 2010 at 09:32:08AM +0200, Jes Sorensen wrote:
On 05/28/10 17:44, Gleb Natapov wrote:
On Fri, May 28, 2010 at 05:24:47PM +0200, Jes Sorensen wrote:
I guess the Socket Designation in particular might have been done for a reason?
It was part of commit cf2affa6de. And was a result of moving to snprintf() instead of direct string manipulation. Before that string was created like that: memcpy((char *)start, "CPU " "\0" "" "\0" "", 7); ((char *)start)[4] = cpu_number + '0'; Which start to produce strange cpu numbers for cpus greater then 9. I doubt we want to go back to that ;)
Hi Gleb,
I see. Well I guess we could do something slightly more compatible by printing along the lines:
printf("CPU:"); if (nr < 10) printf(" "); snprintf()
You mean snprintf() not printf? AFAIR you can tell snprintf to pad with spaces not zeroes.
Not sure if it is worth it, but it should be doable without reverting to memcpy().
Thoughts?
I don't care much as long as we will not have "CPU :". It looks like something that can change after BIOS upgrade, so it is hard to believe Windows will stop working because of this change. -- Gleb.
Gleb Natapov wrote:
I don't care much as long as we will not have "CPU :". It looks like something that can change after BIOS upgrade, so it is hard to believe Windows will stop working because of this change.
Maybe it could trigger the Windows activation process? Sebastian
"Sebastian Herbszt" <herbszt@gmx.de> writes:
Gleb Natapov wrote:
I don't care much as long as we will not have "CPU :". It looks like something that can change after BIOS upgrade, so it is hard to believe Windows will stop working because of this change.
Maybe it could trigger the Windows activation process?
Isn't that testable?
On 06/01/10 07:34, Markus Armbruster wrote:
"Sebastian Herbszt" <herbszt@gmx.de> writes:
Gleb Natapov wrote:
I don't care much as long as we will not have "CPU :". It looks like something that can change after BIOS upgrade, so it is hard to believe Windows will stop working because of this change.
Maybe it could trigger the Windows activation process?
Isn't that testable?
The problem there is that the number of possible combinations to test is endless. I think older versions of windows are far more prone to such problems than newer ones. Cheers, Jes
On Tue, Jun 01, 2010 at 08:05:26AM +0200, Jes Sorensen wrote:
On 06/01/10 07:34, Markus Armbruster wrote:
"Sebastian Herbszt" <herbszt@gmx.de> writes:
Gleb Natapov wrote:
I don't care much as long as we will not have "CPU :". It looks like something that can change after BIOS upgrade, so it is hard to believe Windows will stop working because of this change. Maybe it could trigger the Windows activation process? Isn't that testable? The problem there is that the number of possible combinations to test is endless. I think older versions of windows are far more prone to such problems than newer ones.
Lets select a handful of OS versions that we think are important and then test. If the smbios changes cause a problem, then we can document that fact and be careful going forward. I'm leery about reverting improvements on the premise that some guest could possibly observe the change and be finicky about it. That way leads to stagnation. (For example, for all I know, the recent smbios memory layout bug fix could cause a Windows activation.) BTW, is there a known issue with activation on the latest qemu? -Kevin
On Tue, Jun 01, 2010 at 07:34:33AM +0200, Markus Armbruster wrote:
"Sebastian Herbszt" <herbszt@gmx.de> writes:
Gleb Natapov wrote:
I don't care much as long as we will not have "CPU :". It looks like something that can change after BIOS upgrade, so it is hard to believe Windows will stop working because of this change.
Maybe it could trigger the Windows activation process?
Isn't that testable?
Not easily, that process trips when the total number of changes goes over a fixed value. OG.
On Mon, May 31, 2010 at 10:38:03PM +0200, Sebastian Herbszt wrote:
Gleb Natapov wrote:
I don't care much as long as we will not have "CPU :". It looks like something that can change after BIOS upgrade, so it is hard to believe Windows will stop working because of this change.
Maybe it could trigger the Windows activation process?
Who knows? But as I said it looks like something that can change after BIOS upgrade. If I were Microsoft I would rely on serial numbers for that (after moving kernel to Unix like one of course). -- Gleb.
Jes Sorensen wrote:
Hi,
We were looking at the dmidecode output from qemu-kvm pre-seabios and current qemu-kvm and noticed some of the strings have changed.
The main problem with this is that certain OSes are quite sensitive to system changes and avoiding to change things unnecessarily would probably be a good thing.
Which OSes do care? Windows only?
I wanted to check with the lists if there are any strong feelings about this, and whether some of these changes were made for specific reasons?
For example:
Handle 0x0000, DMI type 0, 24 bytes BIOS Information - Vendor: QEMU - Version: QEMU + Vendor: Bochs + Version: Bochs Release Date: 01/01/2007 Address: 0xE8000 Runtime Size: 96 kB
You can configure this with CONFIG_APPNAME.
and this:
Handle 0x0401, DMI type 4, 32 bytes Processor Information - Socket Designation: CPU 1 + Socket Designation: CPU01 Type: Central Processor Family: Other - Manufacturer: QEMU - ID: 63 06 00 00 FD FB 8B 07 + Manufacturer: Bochs + ID: 23 06 00 00 FD FB 8B 07 Version: Not Specified Voltage: Unknown External Clock: Unknown
I guess the Socket Designation in particular might have been done for a reason?
Otherwise, if there are no objections, I'll look at adding some patches to make it more backwards compatible.
Cheers, Jes
Is the different ID displayed on the same VM configuration (esp. -cpu option) ? The value is gained by calling CPUID so it should not be different. Which pre-seabios qemu-kvm bios are you comparing to? Sebastian
On 05/29/10 14:49, Sebastian Herbszt wrote:
Jes Sorensen wrote:
We were looking at the dmidecode output from qemu-kvm pre-seabios and current qemu-kvm and noticed some of the strings have changed.
The main problem with this is that certain OSes are quite sensitive to system changes and avoiding to change things unnecessarily would probably be a good thing.
Which OSes do care? Windows only?
The problem with this kind of stuff is that we don't know what is sensitive and what doesn't care. Most of the Open Source OSes should be fine, but still Windows is a pretty big customer in the virtualization guest space.
Handle 0x0401, DMI type 4, 32 bytes Processor Information - Socket Designation: CPU 1 + Socket Designation: CPU01 Type: Central Processor Family: Other - Manufacturer: QEMU - ID: 63 06 00 00 FD FB 8B 07 + Manufacturer: Bochs + ID: 23 06 00 00 FD FB 8B 07 Version: Not Specified Voltage: Unknown External Clock: Unknown
I guess the Socket Designation in particular might have been done for a reason?
Otherwise, if there are no objections, I'll look at adding some patches to make it more backwards compatible.
Is the different ID displayed on the same VM configuration (esp. -cpu option) ? The value is gained by calling CPUID so it should not be different.
Which pre-seabios qemu-kvm bios are you comparing to?
Hmmm good point, I'll go back and dig some more on this. Cheers, Jes
On Fri, May 28, 2010 at 05:24:47PM +0200, Jes Sorensen wrote:
We were looking at the dmidecode output from qemu-kvm pre-seabios and current qemu-kvm and noticed some of the strings have changed. [...] I wanted to check with the lists if there are any strong feelings about this, and whether some of these changes were made for specific reasons?
For example: [...] - Vendor: QEMU - Version: QEMU + Vendor: Bochs + Version: Bochs [...] - Manufacturer: QEMU - ID: 63 06 00 00 FD FB 8B 07 + Manufacturer: Bochs + ID: 23 06 00 00 FD FB 8B 07
I made these changes in SeaBIOS when I was cleaning up the ifdefs in the code. Instead of "ifdef QEMU" spread around the code, I abstracted out the various names into CONFIG_APPNAME. I defaulted the names to the Bochs based ones because they were the default in the original Bochs bios code. So, the only reason for these string differences is that the default name hasn't been changed.
Otherwise, if there are no objections, I'll look at adding some patches to make it more backwards compatible.
I wonder if making the default names be "SeaBIOS" based instead of bochs/qemu based would make sense. That way there isn't an impulse to change the settings with every emulator seabios runs on. (Ideally, the same binary would be run on multiple emulators to simplify testing.) -Kevin
Jes Sorensen wrote:
Handle 0x0401, DMI type 4, 32 bytes Processor Information - Socket Designation: CPU 1 + Socket Designation: CPU01
smbios.c got snprintf((char*)start, 6, "CPU%2x", cpu_number); It should print "CPU 1" instead of "CPU01" because the padding should be done with spaces not zeros. Maybe bvprintf() doesn't handle it correctly? Sebastian
On Tue, Jun 01, 2010 at 10:26:12PM +0200, Sebastian Herbszt wrote:
Jes Sorensen wrote:
Handle 0x0401, DMI type 4, 32 bytes Processor Information - Socket Designation: CPU 1 + Socket Designation: CPU01
smbios.c got snprintf((char*)start, 6, "CPU%2x", cpu_number);
It should print "CPU 1" instead of "CPU01" because the padding should be done with spaces not zeros. Maybe bvprintf() doesn't handle it correctly?
Space padding hasn't been implemented - nothing needed it. The bvprintf code is called from 16bit code which is very stack sensitive - if space padding is implemented it will have to be tested carefully. -Kevin
Kevin O'Connor wrote:
On Tue, Jun 01, 2010 at 10:26:12PM +0200, Sebastian Herbszt wrote:
Jes Sorensen wrote:
Handle 0x0401, DMI type 4, 32 bytes Processor Information - Socket Designation: CPU 1 + Socket Designation: CPU01
smbios.c got snprintf((char*)start, 6, "CPU%2x", cpu_number);
It should print "CPU 1" instead of "CPU01" because the padding should be done with spaces not zeros. Maybe bvprintf() doesn't handle it correctly?
Space padding hasn't been implemented - nothing needed it.
With correct space padding this would be a non issue, because the socket designation string would not have changed. Sebastian
The bvprintf code is called from 16bit code which is very stack sensitive - if space padding is implemented it will have to be tested carefully.
-Kevin
On 06/01/10 22:26, Sebastian Herbszt wrote:
Jes Sorensen wrote:
Handle 0x0401, DMI type 4, 32 bytes Processor Information - Socket Designation: CPU 1 + Socket Designation: CPU01
smbios.c got snprintf((char*)start, 6, "CPU%2x", cpu_number);
It should print "CPU 1" instead of "CPU01" because the padding should be done with spaces not zeros. Maybe bvprintf() doesn't handle it correctly?
I looked at the man page for snprintf() and it isn't clear to me that it is required to space pad when printing hex numbers. Having looked at the other pieces, I think this is probably the only one we might want to change. It should be pretty easy to just do something like: if (cpu_number < 0x10) snprintf("CPU %x", cpu_number); else snprintf("CPU%2x", cpu_number); Esthetically I think this would be prettier, but question is whether it's something to worry about or not. Cheers, Jes
Jes Sorensen wrote:
On 06/01/10 22:26, Sebastian Herbszt wrote:
Jes Sorensen wrote:
Handle 0x0401, DMI type 4, 32 bytes Processor Information - Socket Designation: CPU 1 + Socket Designation: CPU01
smbios.c got snprintf((char*)start, 6, "CPU%2x", cpu_number);
It should print "CPU 1" instead of "CPU01" because the padding should be done with spaces not zeros. Maybe bvprintf() doesn't handle it correctly?
I looked at the man page for snprintf() and it isn't clear to me that it is required to space pad when printing hex numbers.
"The flag characters ... 0 The value should be zero padded. For d, i, o, u, x, X, a, A, e, E, f, F, g, and G conversions, the converted value is padded on the left with zeros rather than blanks. If the 0 and - flags both appear, the 0 flag is ignored. If a precision is given with a numeric conversion (d, i, o, u, x, and X), the 0 flag is ignored. For other conversions, the behavior is undefined." [1] So without "0" flag it should be space padded.
Having looked at the other pieces, I think this is probably the only one we might want to change. It should be pretty easy to just do something like:
if (cpu_number < 0x10) snprintf("CPU %x", cpu_number); else snprintf("CPU%2x", cpu_number);
Esthetically I think this would be prettier, but question is whether it's something to worry about or not.
Either change it here or fix bvprintf(). Also "CPU a" looks a bit weird. Maybe change it to "CPU A" (upper case letter) ?
Cheers, Jes
[1] http://linux.die.net/man/3/printf Sebastian
participants (6)
-
Gleb Natapov -
Jes Sorensen -
Kevin O'Connor -
Markus Armbruster -
Olivier Galibert -
Sebastian Herbszt