[Qemu-devel] SMBIOS strings

[Qemu-devel] SMBIOS strings

[Jes Sorensen]

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

[Qemu-devel] Re: [SeaBIOS] SMBIOS strings

[Gleb Natapov]

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.

[Qemu-devel] Re: [SeaBIOS] SMBIOS strings

[Sebastian Herbszt]

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

[Qemu-devel] Re: [SeaBIOS] SMBIOS strings

[Kevin O'Connor]

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

[Qemu-devel] Re: [SeaBIOS] SMBIOS strings

[Jes Sorensen]

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

[Qemu-devel] Re: [SeaBIOS] SMBIOS strings

[Jes Sorensen]

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

[Qemu-devel] Re: [SeaBIOS] SMBIOS strings

[Gleb Natapov]

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.

[Qemu-devel] Re: [SeaBIOS] SMBIOS strings

[Sebastian Herbszt]

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

Re: [Qemu-devel] Re: [SeaBIOS] SMBIOS strings

[Markus Armbruster]

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

[Qemu-devel] Re: [SeaBIOS] SMBIOS strings

[Gleb Natapov]

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.

Re: [Qemu-devel] Re: [SeaBIOS] SMBIOS strings

[Jes Sorensen]

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

Re: [SeaBIOS] [Qemu-devel] Re: SMBIOS strings

[Olivier Galibert]

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.

[Qemu-devel] Re: [SeaBIOS] SMBIOS strings

[Sebastian Herbszt]

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

[Qemu-devel] Re: [SeaBIOS] SMBIOS strings

[Kevin O'Connor]

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

Re: [SeaBIOS] [Qemu-devel] Re: SMBIOS strings

[Kevin O'Connor]

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

[Qemu-devel] Re: [SeaBIOS] SMBIOS strings

[Jes Sorensen]

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

[Qemu-devel] Re: [SeaBIOS] SMBIOS strings

[Sebastian Herbszt]

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

[Qemu-devel] Re: [SeaBIOS] SMBIOS strings

[Sebastian Herbszt]

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