[Qemu-devel] SMBIOS (Set of 10 patches)

[Qemu-devel] SMBIOS (Set of 10 patches)

[Gabriel L. Somlo]

Hi,

This patch set builds full smbios tables in QEMU, and sends them to
the bios via fw_cfg as SMBIOS_TABLE_ENTRY blobs.

I'm resending these without the "In-Reply-To" line to avoid having
them getting buried under a bunch of other email, per Stefan's
suggestion in an unrelated thread. Also cc-ing Alex and Markus as
the original authors of the QEMU smbios.c file.

On Thu, Mar 06, 2014 at 10:03:32AM +0100, Gerd Hoffmann wrote:
> So, if we manage to get the patches into shape in time for qemu 2.0 your
> way to do that is fine.  We are pretty close to the 2.0 freeze though,
> so maybe we should better plan for post-2.0 anyway, especially as you
> plan to add more tables.

Hopefully it's not too late, and the patches are in good enough shape :)

I'd still like to get types 19, 20, 32, and 127 build in QEMU, but
that change wouldn't be visible to guests, and therefore could probably
be put in later.

Please let me know what you think.

Thanks,
--Gabriel

Re: [Qemu-devel] SMBIOS (Set of 10 patches)

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1077 bytes --]

On 03/10/2014 10:56 AM, Gabriel L. Somlo wrote:
> Hi,
> 
> This patch set builds full smbios tables in QEMU, and sends them to
> the bios via fw_cfg as SMBIOS_TABLE_ENTRY blobs.
> 
> I'm resending these without the "In-Reply-To" line to avoid having
> them getting buried under a bunch of other email, per Stefan's
> suggestion in an unrelated thread. Also cc-ing Alex and Markus as
> the original authors of the QEMU smbios.c file.

Not quite right.  The cover letter should not have In-Reply-To, but the
remaining patches SHOULD be in-reply-to the cover letter.  That is, you
don't want to create 11 top-level threads:

0/10
1/10
2/10
...
10/10

but instead create 1 top-level thread with all patches under the cover
letter:

0/10
|- 1/10
|- 2/10
...
|- 10/10

'qemu send-email -10 --annotate --cover-letter' defaults to the proper
threading, along with letting you modify your cover letter in your
editor before actually sending.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] SMBIOS (Set of 10 patches)

[Gabriel L. Somlo]

On Mon, Mar 10, 2014 at 11:55:13AM -0600, Eric Blake wrote:
> On 03/10/2014 10:56 AM, Gabriel L. Somlo wrote:
> 
> Not quite right.  The cover letter should not have In-Reply-To, but the
> remaining patches SHOULD be in-reply-to the cover letter.  That is, you
> don't want to create 11 top-level threads:
> 
> 0/10
> 1/10
> 2/10
> ...
> 10/10
> 
> but instead create 1 top-level thread with all patches under the cover
> letter:
> 
> 0/10
> |- 1/10
> |- 2/10
> ...
> |- 10/10

OK, so I screwed up, now what ? Should I re-send the 10 patches
in-reply-to the existing cover letter, or leave it be for now to
avoid further spamming the list ?

> 'qemu send-email -10 --annotate --cover-letter' defaults to the proper
> threading, along with letting you modify your cover letter in your
> editor before actually sending.

That sounds awesome, but there's no "send-email" script under my
qemu/scripts folder (using latest git). Where would I get this from?
(The qemu-system-i386 binary itself doesn't have this as an
"easter-egg" addition, neither is it in qemu-kvm-tools.rpm :)

TIA for the clue,
--Gabriel

Re: [Qemu-devel] SMBIOS (Set of 10 patches)

[Gabriel L. Somlo]

On Mon, Mar 10, 2014 at 02:17:25PM -0400, Gabriel L. Somlo wrote:
> > 'qemu send-email -10 --annotate --cover-letter' defaults to the proper
> > threading, along with letting you modify your cover letter in your
> > editor before actually sending.
> 
> That sounds awesome, but there's no "send-email" script under my
> qemu/scripts folder (using latest git). Where would I get this from?
> (The qemu-system-i386 binary itself doesn't have this as an
> "easter-egg" addition, neither is it in qemu-kvm-tools.rpm :)

Nevermind, it's "git send-email", and the "git-email" package wasn't
installed by default :(

Thanks,
--G

Re: [Qemu-devel] SMBIOS (Set of 10 patches)

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1590 bytes --]

On 03/10/2014 12:17 PM, Gabriel L. Somlo wrote:
>> but instead create 1 top-level thread with all patches under the cover
>> letter:
>>
>> 0/10
>> |- 1/10
>> |- 2/10
>> ...
>> |- 10/10
> 
> OK, so I screwed up, now what ? Should I re-send the 10 patches
> in-reply-to the existing cover letter, or leave it be for now to
> avoid further spamming the list ?

Don't stress too hard - we were all new once :)

Personally, I'd wait a day or two for any review comments, then send a
v2 properly threaded with those comments addressed.

You might also find it helpful to read
http://wiki.qemu.org/Contribute/SubmitAPatch (and if we are missing
something there, let us know how to improvement - newcomers are the BEST
sources of information on how well a page designed to help newcomers
actually does its job).

> 
>> 'qemu send-email -10 --annotate --cover-letter' defaults to the proper
>> threading, along with letting you modify your cover letter in your
>> editor before actually sending.
> 
> That sounds awesome, but there's no "send-email" script under my
> qemu/scripts folder (using latest git).

Upstream git.git has included send-email for years; but many distros
ship it as a separate package because of the additional dependencies it
drags in.  On Fedora systems, it is part of the 'git-email' package.
Hmm - something to add to the SubmitAPatch wiki page - we recommend
send-email without saying where to get it on common distros :)

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] SMBIOS (Set of 10 patches)

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 686 bytes --]

On 03/10/2014 01:14 PM, Eric Blake wrote:
> Upstream git.git has included send-email for years; but many distros
> ship it as a separate package because of the additional dependencies it
> drags in.  On Fedora systems, it is part of the 'git-email' package.
> Hmm - something to add to the SubmitAPatch wiki page - we recommend
> send-email without saying where to get it on common distros :)

I've updated the wiki to call out Fedora's git-email package, but I
don't use Debian-based distros often enough to know if another tweak is
needed for that camp of users.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] SMBIOS (Set of 10 patches)

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 636 bytes --]

On 03/10/2014 11:55 AM, Eric Blake wrote:
> On 03/10/2014 10:56 AM, Gabriel L. Somlo wrote:
>> Hi,
>>
>> This patch set builds full smbios tables in QEMU, and sends them to
>> the bios via fw_cfg as SMBIOS_TABLE_ENTRY blobs.
>>

> 
> 'qemu send-email -10 --annotate --cover-letter' defaults to the proper

Aaargh, sorry for causing confusion.  It looks like you figured it out,
but other readers might not - I meant 'git send-email'.  (qemu may be
quite powerful, but we haven't yet taught it to replace git :)

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] SMBIOS (Set of 10 patches)

[Gerd Hoffmann]

  Hi,

> On Thu, Mar 06, 2014 at 10:03:32AM +0100, Gerd Hoffmann wrote:
> > So, if we manage to get the patches into shape in time for qemu 2.0 your
> > way to do that is fine.  We are pretty close to the 2.0 freeze though,
> > so maybe we should better plan for post-2.0 anyway, especially as you
> > plan to add more tables.
> 
> Hopefully it's not too late, and the patches are in good enough shape :)

I don't feel like rushing it, and hard freeze is tomorrow ...

Issue #1: There are checkpatch errors (scripts/checkpatch.pl).

Issue #2: There is one build warning:

/home/kraxel/projects/qemu/hw/i386/smbios.c: In function
'smbios_build_type_16_table':
/home/kraxel/projects/qemu/hw/i386/smbios.c:520:5: warning: comparison
is always true due to limited range of data type [-Wtype-limits]
     t->maximum_capacity = ram_size < 2ULL << 40 ? ram_size >> 10 :
0x80000000;
     ^

Issue #3: Running a diff on the dmidecode output with and without the
patches yields this:

--- dmidecode.master	2014-03-11 10:38:06.799233009 +0100
+++ dmidecode.smbios	2014-03-11 10:39:36.664377785 +0100
@@ -1,20 +1,20 @@
 # dmidecode 2.12
 SMBIOS 2.4 present.
-10 structures occupying 304 bytes.
-Table at 0x000F09D0.
+10 structures occupying 351 bytes.
+Table at 0x000F09A0.

That comes from upgrading some of the tables to newer versions, ok.
 
 Handle 0x0000, DMI type 0, 24 bytes
 BIOS Information
-	Vendor: Bochs
-	Version: Bochs
-	Release Date: 01/01/2011
+	Vendor: QEMU
+	Version: pc-i440fx-2.0
+	Release Date: 01/01/2014
 	Address: 0xE8000
 	Runtime Size: 96 kB
 	ROM Size: 64 kB
 	Characteristics:
 		BIOS characteristics not supported
 		Targeted content distribution is supported
-	BIOS Revision: 1.0
+	BIOS Revision: 0.0

I think we should not generate a type0 table unless -smbios type0=... is
explicitly specified on the qemu command line.  It is about the
firmware, and we should leave it to the firmware to fill it by default.
If you are running OVMF (EFI) instead of SeaBIOS you should see it in
the dmidecode output.
 
 Handle 0x0300, DMI type 3, 20 bytes
 Chassis Information
-	Manufacturer: Bochs
+	Manufacturer: QEMU
 	Type: Other
 	Lock: Not Present
-	Version: Not Specified
+	Version: pc-i440fx-2.0
 	Serial Number: Not Specified
 	Asset Tag: Not Specified
 	Boot-up State: Safe

That is ok I think.

-Handle 0x0401, DMI type 4, 32 bytes
+Handle 0x0400, DMI type 4, 35 bytes
 Processor Information
-	Socket Designation: CPU 1
+	Socket Designation: CPU 0

Hmm?

 	Type: Central Processor
 	Family: Other
-	Manufacturer: Bochs
+	Manufacturer: QEMU
 	ID: 63 06 00 00 FD FB 8B 07
-	Version: Not Specified
+	Version: pc-i440fx-2.0
 	Voltage: Unknown
 	External Clock: Unknown

Ok.

-	Max Speed: 2000 MHz
-	Current Speed: 2000 MHz
+	Max Speed: Unknown
+	Current Speed: Unknown

Where does 2000 MHz come from?  Does SeaBIOS pull something out of thin
air or does it try to measure the speed?

-Handle 0x1100, DMI type 17, 21 bytes
+Handle 0x1100, DMI type 17, 27 bytes
 Memory Device
 	Array Handle: 0x1000
-	Error Information Handle: 0x0003
+	Error Information Handle: Not Provided

Same question.

cheers,
  Gerd

Re: [Qemu-devel] SMBIOS (Set of 10 patches)

[Kevin O'Connor]

On Tue, Mar 11, 2014 at 11:03:06AM +0100, Gerd Hoffmann wrote:
> Issue #3: Running a diff on the dmidecode output with and without the
> patches yields this:

I think it would be best to get the patch series to the point that
there is no diff between old and new.  That will make review easier,
and subsequent patches can then add new features.

> --- dmidecode.master	2014-03-11 10:38:06.799233009 +0100
> +++ dmidecode.smbios	2014-03-11 10:39:36.664377785 +0100
> @@ -1,20 +1,20 @@
>  # dmidecode 2.12
>  SMBIOS 2.4 present.
> -10 structures occupying 304 bytes.
> -Table at 0x000F09D0.
> +10 structures occupying 351 bytes.
> +Table at 0x000F09A0.
> 
> That comes from upgrading some of the tables to newer versions, ok.
>  
>  Handle 0x0000, DMI type 0, 24 bytes
>  BIOS Information
> -	Vendor: Bochs
> -	Version: Bochs
> -	Release Date: 01/01/2011
> +	Vendor: QEMU
> +	Version: pc-i440fx-2.0
> +	Release Date: 01/01/2014
>  	Address: 0xE8000
>  	Runtime Size: 96 kB
>  	ROM Size: 64 kB
>  	Characteristics:
>  		BIOS characteristics not supported
>  		Targeted content distribution is supported
> -	BIOS Revision: 1.0
> +	BIOS Revision: 0.0
> 
> I think we should not generate a type0 table unless -smbios type0=... is
> explicitly specified on the qemu command line.  It is about the
> firmware, and we should leave it to the firmware to fill it by default.
> If you are running OVMF (EFI) instead of SeaBIOS you should see it in
> the dmidecode output.

Everything that SeaBIOS puts into table 0 is hard coded.  I'd prefer
it if QEMU created the table (with the same hardcoded fields) because
having split ownership of the smbios is painful.

[...]
> -Handle 0x0401, DMI type 4, 32 bytes
> +Handle 0x0400, DMI type 4, 35 bytes
>  Processor Information
> -	Socket Designation: CPU 1
> +	Socket Designation: CPU 0
> 
> Hmm?
> 
>  	Type: Central Processor
>  	Family: Other
> -	Manufacturer: Bochs
> +	Manufacturer: QEMU
>  	ID: 63 06 00 00 FD FB 8B 07
> -	Version: Not Specified
> +	Version: pc-i440fx-2.0
>  	Voltage: Unknown
>  	External Clock: Unknown
> 
> Ok.
> 
> -	Max Speed: 2000 MHz
> -	Current Speed: 2000 MHz
> +	Max Speed: Unknown
> +	Current Speed: Unknown
> 
> Where does 2000 MHz come from?  Does SeaBIOS pull something out of thin
> air or does it try to measure the speed?

The 2000 is hardcoded in SeaBIOS (see smbios_init_type_4() ).

> -Handle 0x1100, DMI type 17, 21 bytes
> +Handle 0x1100, DMI type 17, 27 bytes
>  Memory Device
>  	Array Handle: 0x1000
> -	Error Information Handle: 0x0003
> +	Error Information Handle: Not Provided
> 
> Same question.

I don't see the 0x0003 in smbios_init_type_17() but I think it must
come from some hardcode in seabios.

-Kevin

Re: [Qemu-devel] SMBIOS (Set of 10 patches)

[Gerd Hoffmann]

  Hi,

> > I think we should not generate a type0 table unless -smbios type0=... is
> > explicitly specified on the qemu command line.  It is about the
> > firmware, and we should leave it to the firmware to fill it by default.
> > If you are running OVMF (EFI) instead of SeaBIOS you should see it in
> > the dmidecode output.
> 
> Everything that SeaBIOS puts into table 0 is hard coded.  I'd prefer
> it if QEMU created the table (with the same hardcoded fields) because
> having split ownership of the smbios is painful.

The information seabios puts in there isn't correct for OVMF though.
type0 on ovmf looks like this:

Handle 0x0000, DMI type 0, 24 bytes
BIOS Information
        Vendor: EFI Development Kit II / OVMF
        Version: 0.1
        Release Date: 06/03/2013
        Address: 0xE8000
        Runtime Size: 96 kB
        ROM Size: 64 kB
        Characteristics:
                BIOS characteristics not supported
                UEFI is supported
                System is a virtual machine
        BIOS Revision: 0.1

At very least the UEFI support bit would have to be different depending
on whenever seabios or ovmf is used as firmware ...

cheers,
  Gerd

[Qemu-devel] [v2 PATCH 00/13] SMBIOS: build full tables in QEMU

[Gabriel L. Somlo]

From: "Gabriel L. Somlo" <somlo@cmu.edu>

On Tue, Mar 11, 2014 at 11:03:06AM +0100, Gerd Hoffmann wrote:
> > On Thu, Mar 06, 2014 at 10:03:32AM +0100, Gerd Hoffmann wrote:
> > > So, if we manage to get the patches into shape in time for qemu 2.0 your
> > > way to do that is fine.  We are pretty close to the 2.0 freeze though,
> > > so maybe we should better plan for post-2.0 anyway, especially as you
> > > plan to add more tables.
> > 
> > Hopefully it's not too late, and the patches are in good enough shape :)
> 
> I don't feel like rushing it, and hard freeze is tomorrow ...

No big deal for me, I was just trying to help prevent the need to
deal with wn extra compatibility mode, per your earlier explanation...

> Issue #1: There are checkpatch errors (scripts/checkpatch.pl).

I fixed most of those, with one exception (patch #5):

ERROR: do not use assignment in if condition
#139: FILE: hw/i386/smbios.c:253:
+        if (value != NULL && (len = strlen(value) + 1) > 1) { \

'value' can be NULL, "", or "foo". The whole block looks like this:

  int len;
  if (value != NULL && (len = strlen(value)) > 0) {
      smbios_entries = g_realloc(smbios_entries, smbios_entries_len + len);
      ...
  } else {
      ...
  }

So, I can't call strlen before I compare 'value' against NULL, and
nesting a bunch of if statements makes things more complex, and using
goto would also suck worse. I can call strlen() twice (once in the
condition, and again to set 'len' inside the true branch, but that
would also feel a bit cheesy :)

I'd say checkpatch.pl is missing the big picture in this one instance,
what do you think ?  :)

> Issue #2: There is one build warning:
> 
> /home/kraxel/projects/qemu/hw/i386/smbios.c: In function
> 'smbios_build_type_16_table':
> /home/kraxel/projects/qemu/hw/i386/smbios.c:520:5: warning: comparison
> is always true due to limited range of data type [-Wtype-limits]
>      t->maximum_capacity = ram_size < 2ULL << 40 ? ram_size >> 10 :
> 0x80000000;
>      ^

Weird, I wasn't getting that. Might have been a 64-bit arch issue.
Anyhow, I reworked the code to avoid it altogether, and it looks
cleaner and easier to comprehend as a bonus...

> I think we should not generate a type0 table unless -smbios type0=... is
> explicitly specified on the qemu command line.  It is about the
> firmware, and we should leave it to the firmware to fill it by default.
> If you are running OVMF (EFI) instead of SeaBIOS you should see it in
> the dmidecode output.

Agreed. Type 0 is now optional, just like type 2. It won't get added
unless requested explicitly.

> -Handle 0x0401, DMI type 4, 32 bytes
> +Handle 0x0400, DMI type 4, 35 bytes
>  Processor Information
> -	Socket Designation: CPU 1
> +	Socket Designation: CPU 0
> 
> Hmm?

I think SeaBIOS starts counting from 1. Do we want to stick with that,
or count starting from 0 ? I think it's purely cosmetic, but in the
interest of minimizing initial visual noise, I've changed it to count
from 1 in QEMU as well. :)
> 
> -	Max Speed: 2000 MHz
> -	Current Speed: 2000 MHz
> +	Max Speed: Unknown
> +	Current Speed: Unknown
> 
> Where does 2000 MHz come from?  Does SeaBIOS pull something out of thin
> air or does it try to measure the speed?

Thin air, AFAICT. I hardcoded it in QEMU also, to minimize unexpected
changes.

> 
> -Handle 0x1100, DMI type 17, 21 bytes
> +Handle 0x1100, DMI type 17, 27 bytes
>  Memory Device
>  	Array Handle: 0x1000
> -	Error Information Handle: 0x0003
> +	Error Information Handle: Not Provided

AFAICT, SeaBIOS doesn't set the error info handle at all, and you simply
get whatever garbage happens to be contained in that memory location at
the time. Typically it's 0x0000, but you happened to get 0x0003 :) I
figured I'd set it to a known value (0xFFFE or "n/a").

On Tue, Mar 11, 2014 at 09:27:31AM -0400, Kevin O'Connor wrote:
> I think it would be best to get the patch series to the point that
> there is no diff between old and new.  That will make review easier,
> and subsequent patches can then add new features.

Modulo the error info handle on type 17, and the fact that QEMU's version
of type 17 has v2.3 fields and SeaBIOS's does not (kinda the whole reason
I'm doing this in the first place :), that should be possible by just
tweaking the defaults in the new smbios_set_defaults() function. I just
feel weird setting "Bochs" as the default manufacturer... 

> Everything that SeaBIOS puts into table 0 is hard coded.  I'd prefer
> it if QEMU created the table (with the same hardcoded fields) because
> having split ownership of the smbios is painful.

Instinctively, I feel Gerd's right, and type 0 is really the BIOS's
jurisdiction, so I made it optional. You'd have to simply do "-smbios
type=0" on the qemu command line, and it would be inserted into fw_cfg.

New patch set follows, please let me know what you think.

Thanks,
--Gabriel


Gabriel L. Somlo (13):
  SMBIOS: Update all table definitions to smbios spec v2.3
  SMBIOS: Rename smbios_set_type1_defaults() for more general use
  SMBIOS: Use macro to set smbios defaults
  SMBIOS: Use bitmaps to check for smbios table collisions
  SMBIOS: Add code to build full smbios tables; build type 2 table
  SMBIOS: Build full tables for types 0 and 1
  SMBIOS: Remove unused code for passing individual fields to bios
  SMBIOS: Build full type 3 table
  SMBIOS: Build full type 4 tables
  SMBIOS: Build full smbios v2.3 compliant type 16 and 17 tables
  SMBIOS: Build full type 19 tables
  SMBIOS: Build full type 20 tables
  SMBIOS: Build full tables for type 32 and 127

 hw/i386/pc.c             |   3 +
 hw/i386/pc_piix.c        |  15 +-
 hw/i386/pc_q35.c         |  11 +-
 hw/i386/smbios.c         | 669 ++++++++++++++++++++++++++++++++++++++++-------
 include/hw/i386/smbios.h |  40 ++-
 5 files changed, 622 insertions(+), 116 deletions(-)

-- 
1.8.1.4

[Qemu-devel] [v2 PATCH 01/13] SMBIOS: Update all table definitions to smbios spec v2.3

[Gabriel L. Somlo]

From: "Gabriel L. Somlo" <somlo@cmu.edu>

Table definitions for types 4 and 17 are only up to v2.0,
so add fields specified in smbios v2.3, as expected (and
advertised) by the SeaBIOS smbios entry point structure.

Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
---
 include/hw/i386/smbios.h | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/include/hw/i386/smbios.h b/include/hw/i386/smbios.h
index 18fb970..de1da87 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -79,7 +79,7 @@ struct smbios_type_3 {
     // contained elements follow
 } QEMU_PACKED;
 
-/* SMBIOS type 4 - Processor Information (v2.0) */
+/* SMBIOS type 4 - Processor Information (v2.3) */
 struct smbios_type_4 {
     struct smbios_structure_header header;
     uint8_t socket_designation_str;
@@ -97,6 +97,10 @@ struct smbios_type_4 {
     uint16_t l1_cache_handle;
     uint16_t l2_cache_handle;
     uint16_t l3_cache_handle;
+    uint8_t serial_number_str;
+    uint8_t asset_tag_number_str;
+    uint8_t part_number_str;
+
 } QEMU_PACKED;
 
 /* SMBIOS type 16 - Physical Memory Array
@@ -111,7 +115,7 @@ struct smbios_type_16 {
     uint16_t memory_error_information_handle;
     uint16_t number_of_memory_devices;
 } QEMU_PACKED;
-/* SMBIOS type 17 - Memory Device
+/* SMBIOS type 17 - Memory Device (v2.3)
  *   Associated with one type 19
  */
 struct smbios_type_17 {
@@ -127,6 +131,11 @@ struct smbios_type_17 {
     uint8_t bank_locator_str;
     uint8_t memory_type;
     uint16_t type_detail;
+    uint16_t speed;
+    uint8_t manufacturer_str;
+    uint8_t serial_number_str;
+    uint8_t asset_tag_number_str;
+    uint8_t part_number_str;
 } QEMU_PACKED;
 
 /* SMBIOS type 19 - Memory Array Mapped Address */
-- 
1.8.1.4

[Qemu-devel] [v2 PATCH 02/13] SMBIOS: Rename smbios_set_type1_defaults() for more general use

[Gabriel L. Somlo]

From: "Gabriel L. Somlo" <somlo@cmu.edu>

Subsequent patches will utilize this function to set defaults for
more smbios types than just type 1, so the function name should
reflect this.

Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
---
 hw/i386/pc_piix.c        | 12 ++++++------
 hw/i386/pc_q35.c         |  8 ++++----
 hw/i386/smbios.c         |  4 ++--
 include/hw/i386/smbios.h |  4 ++--
 4 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index ae1699d..ef2d062 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -60,7 +60,7 @@ static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
 
 static bool has_pci_info;
 static bool has_acpi_build = true;
-static bool smbios_type1_defaults = true;
+static bool smbios_defaults = true;
 /* Make sure that guest addresses aligned at 1Gbyte boundaries get mapped to
  * host addresses aligned at 1Gbyte boundaries.  This way we can use 1GByte
  * pages in the host.
@@ -143,9 +143,9 @@ static void pc_init1(QEMUMachineInitArgs *args,
     guest_info->has_pci_info = has_pci_info;
     guest_info->isapc_ram_fw = !pci_enabled;
 
-    if (smbios_type1_defaults) {
+    if (smbios_defaults) {
         /* These values are guest ABI, do not change */
-        smbios_set_type1_defaults("QEMU", "Standard PC (i440FX + PIIX, 1996)",
+        smbios_set_defaults("QEMU", "Standard PC (i440FX + PIIX, 1996)",
                                   args->machine->name);
     }
 
@@ -264,7 +264,7 @@ static void pc_init_pci(QEMUMachineInitArgs *args)
 
 static void pc_compat_1_7(QEMUMachineInitArgs *args)
 {
-    smbios_type1_defaults = false;
+    smbios_defaults = false;
     gigabyte_align = false;
 }
 
@@ -343,7 +343,7 @@ static void pc_init_pci_no_kvmclock(QEMUMachineInitArgs *args)
 {
     has_pci_info = false;
     has_acpi_build = false;
-    smbios_type1_defaults = false;
+    smbios_defaults = false;
     disable_kvm_pv_eoi();
     enable_compat_apic_id_mode();
     pc_init1(args, 1, 0);
@@ -353,7 +353,7 @@ static void pc_init_isa(QEMUMachineInitArgs *args)
 {
     has_pci_info = false;
     has_acpi_build = false;
-    smbios_type1_defaults = false;
+    smbios_defaults = false;
     if (!args->cpu_model) {
         args->cpu_model = "486";
     }
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index a7f6260..dfcc252 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -50,7 +50,7 @@
 
 static bool has_pci_info;
 static bool has_acpi_build = true;
-static bool smbios_type1_defaults = true;
+static bool smbios_defaults = true;
 /* Make sure that guest addresses aligned at 1Gbyte boundaries get mapped to
  * host addresses aligned at 1Gbyte boundaries.  This way we can use 1GByte
  * pages in the host.
@@ -130,9 +130,9 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
     guest_info->isapc_ram_fw = false;
     guest_info->has_acpi_build = has_acpi_build;
 
-    if (smbios_type1_defaults) {
+    if (smbios_defaults) {
         /* These values are guest ABI, do not change */
-        smbios_set_type1_defaults("QEMU", "Standard PC (Q35 + ICH9, 2009)",
+        smbios_set_defaults("QEMU", "Standard PC (Q35 + ICH9, 2009)",
                                   args->machine->name);
     }
 
@@ -242,7 +242,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
 
 static void pc_compat_1_7(QEMUMachineInitArgs *args)
 {
-    smbios_type1_defaults = false;
+    smbios_defaults = false;
     gigabyte_align = false;
 }
 
diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index e8f41ad..89dc070 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -256,8 +256,8 @@ static void smbios_build_type_1_fields(void)
     }
 }
 
-void smbios_set_type1_defaults(const char *manufacturer,
-                               const char *product, const char *version)
+void smbios_set_defaults(const char *manufacturer,
+                         const char *product, const char *version)
 {
     if (!type1.manufacturer) {
         type1.manufacturer = manufacturer;
diff --git a/include/hw/i386/smbios.h b/include/hw/i386/smbios.h
index de1da87..a7ec973 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -16,8 +16,8 @@
 #include "qemu/option.h"
 
 void smbios_entry_add(QemuOpts *opts);
-void smbios_set_type1_defaults(const char *manufacturer,
-                               const char *product, const char *version);
+void smbios_set_defaults(const char *manufacturer,
+                         const char *product, const char *version);
 uint8_t *smbios_get_table(size_t *length);
 
 /*
-- 
1.8.1.4

[Qemu-devel] [v2 PATCH 03/13] SMBIOS: Use macro to set smbios defaults

[Gabriel L. Somlo]

From: "Gabriel L. Somlo" <somlo@cmu.edu>

The function smbios_set_defaults() uses a repeating code pattern
for each field. This patch replaces that pattern with a macro.

Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
---
 hw/i386/smbios.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 89dc070..f4ee7b4 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -256,18 +256,17 @@ static void smbios_build_type_1_fields(void)
     }
 }
 
+#define SMBIOS_SET_DEFAULT(field, value)                                  \
+    if (!field) {                                                         \
+        field = value;                                                    \
+    }
+
 void smbios_set_defaults(const char *manufacturer,
                          const char *product, const char *version)
 {
-    if (!type1.manufacturer) {
-        type1.manufacturer = manufacturer;
-    }
-    if (!type1.product) {
-        type1.product = product;
-    }
-    if (!type1.version) {
-        type1.version = version;
-    }
+    SMBIOS_SET_DEFAULT(type1.manufacturer, manufacturer);
+    SMBIOS_SET_DEFAULT(type1.product, product);
+    SMBIOS_SET_DEFAULT(type1.version, version);
 }
 
 uint8_t *smbios_get_table(size_t *length)
-- 
1.8.1.4

[Qemu-devel] [v2 PATCH 04/13] SMBIOS: Use bitmaps to check for smbios table collisions

[Gabriel L. Somlo]

From: "Gabriel L. Somlo" <somlo@cmu.edu>

Replace existing smbios_check_collision() functionality with
a pair of bitmaps: have_binfile_bitmap and have_fields_bitmap.
Bits corresponding to each smbios type are set by smbios_entry_add(),
which also uses the bitmaps to ensure that binary blobs and field
values are never accepted for the same type.

These bitmaps will also be used in the future to decide whether
or not to build a full table for a given smbios type.

Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
---
 hw/i386/smbios.c         | 51 ++++++++++++++++++++----------------------------
 include/hw/i386/smbios.h |  2 ++
 2 files changed, 23 insertions(+), 30 deletions(-)

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index f4ee7b4..6889332 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -49,11 +49,8 @@ static size_t smbios_entries_len;
 static int smbios_type4_count = 0;
 static bool smbios_immutable;
 
-static struct {
-    bool seen;
-    int headertype;
-    Location loc;
-} first_opt[2];
+static DECLARE_BITMAP(have_binfile_bitmap, SMBIOS_MAX_TYPE+1);
+static DECLARE_BITMAP(have_fields_bitmap, SMBIOS_MAX_TYPE+1);
 
 static struct {
     const char *vendor, *version, *date;
@@ -164,29 +161,6 @@ static void smbios_validate_table(void)
     }
 }
 
-/*
- * To avoid unresolvable overlaps in data, don't allow both
- * tables and fields for the same smbios type.
- */
-static void smbios_check_collision(int type, int entry)
-{
-    if (type < ARRAY_SIZE(first_opt)) {
-        if (first_opt[type].seen) {
-            if (first_opt[type].headertype != entry) {
-                error_report("Can't mix file= and type= for same type");
-                loc_push_restore(&first_opt[type].loc);
-                error_report("This is the conflicting setting");
-                loc_pop(&first_opt[type].loc);
-                exit(1);
-            }
-        } else {
-            first_opt[type].seen = true;
-            first_opt[type].headertype = entry;
-            loc_save(&first_opt[type].loc);
-        }
-    }
-}
-
 static void smbios_add_field(int type, int offset, const void *data, size_t len)
 {
     struct smbios_field *field;
@@ -331,7 +305,14 @@ void smbios_entry_add(QemuOpts *opts)
         }
 
         header = (struct smbios_structure_header *)(table->data);
-        smbios_check_collision(header->type, SMBIOS_TABLE_ENTRY);
+
+        if (test_bit(header->type, have_fields_bitmap)) {
+            error_report("Can't add binary type %d table! "
+                         "(fields already specified)", header->type);
+            exit(1);
+        }
+        set_bit(header->type, have_binfile_bitmap);
+
         if (header->type == 4) {
             smbios_type4_count++;
         }
@@ -346,7 +327,17 @@ void smbios_entry_add(QemuOpts *opts)
     if (val) {
         unsigned long type = strtoul(val, NULL, 0);
 
-        smbios_check_collision(type, SMBIOS_FIELD_ENTRY);
+        if (type > SMBIOS_MAX_TYPE) {
+            error_report("smbios type (%ld) out of range!", type);
+            exit(1);
+        }
+
+        if (test_bit(type, have_binfile_bitmap)) {
+            error_report("Can't add fields for type %ld table! "
+                         "(binary file already loaded)", type);
+            exit(1);
+        }
+        set_bit(type, have_fields_bitmap);
 
         switch (type) {
         case 0:
diff --git a/include/hw/i386/smbios.h b/include/hw/i386/smbios.h
index a7ec973..8b63441 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -15,6 +15,8 @@
 
 #include "qemu/option.h"
 
+#define SMBIOS_MAX_TYPE 127
+
 void smbios_entry_add(QemuOpts *opts);
 void smbios_set_defaults(const char *manufacturer,
                          const char *product, const char *version);
-- 
1.8.1.4

[Qemu-devel] [v2 PATCH 05/13] SMBIOS: Add code to build full smbios tables; build type 2 table

[Gabriel L. Somlo]

From: "Gabriel L. Somlo" <somlo@cmu.edu>

This patch adds a set of macros which build full smbios tables
of a given type, including the logic to decide whether a given
table type should be built or not.

To illustrate this new functionality, we introduce and optionally
build a table of type 2 (base board), which is required by some
versions of OS X (10.7 and 10.8).

Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
---
 hw/i386/smbios.c         | 158 +++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/i386/smbios.h |  18 +++++-
 2 files changed, 175 insertions(+), 1 deletion(-)

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 6889332..9679e06 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -48,6 +48,7 @@ static uint8_t *smbios_entries;
 static size_t smbios_entries_len;
 static int smbios_type4_count = 0;
 static bool smbios_immutable;
+static bool smbios_have_defaults;
 
 static DECLARE_BITMAP(have_binfile_bitmap, SMBIOS_MAX_TYPE+1);
 static DECLARE_BITMAP(have_fields_bitmap, SMBIOS_MAX_TYPE+1);
@@ -63,6 +64,10 @@ static struct {
     /* uuid is in qemu_uuid[] */
 } type1;
 
+static struct {
+    const char *manufacturer, *product, *version, *serial, *asset, *location;
+} type2;
+
 static QemuOptsList qemu_smbios_opts = {
     .name = "smbios",
     .head = QTAILQ_HEAD_INITIALIZER(qemu_smbios_opts.head),
@@ -146,6 +151,39 @@ static const QemuOptDesc qemu_smbios_type1_opts[] = {
     { /* end of list */ }
 };
 
+static const QemuOptDesc qemu_smbios_type2_opts[] = {
+    {
+        .name = "type",
+        .type = QEMU_OPT_NUMBER,
+        .help = "SMBIOS element type",
+    },{
+        .name = "manufacturer",
+        .type = QEMU_OPT_STRING,
+        .help = "manufacturer name",
+    },{
+        .name = "product",
+        .type = QEMU_OPT_STRING,
+        .help = "product name",
+    },{
+        .name = "version",
+        .type = QEMU_OPT_STRING,
+        .help = "version number",
+    },{
+        .name = "serial",
+        .type = QEMU_OPT_STRING,
+        .help = "serial number",
+    },{
+        .name = "asset",
+        .type = QEMU_OPT_STRING,
+        .help = "asset tag number",
+    },{
+        .name = "location",
+        .type = QEMU_OPT_STRING,
+        .help = "location in chassis",
+    },
+    { /* end of list */ }
+};
+
 static void smbios_register_config(void)
 {
     qemu_add_opts(&qemu_smbios_opts);
@@ -161,6 +199,90 @@ static void smbios_validate_table(void)
     }
 }
 
+static bool smbios_skip_table(uint8_t type, bool required_table)
+{
+    if (test_bit(type, have_binfile_bitmap)) {
+        return true; /* user provided their own binary blob(s) */
+    }
+    if (test_bit(type, have_fields_bitmap)) {
+        return false; /* user provided fields via command line */
+    }
+    if (smbios_have_defaults && required_table) {
+        return false; /* we're building tables, and this one's required */
+    }
+    return true;
+}
+
+#define SMBIOS_BUILD_TABLE_PRE(tbl_type, tbl_handle, tbl_required)        \
+    struct smbios_table *w;                                               \
+    struct smbios_type_##tbl_type *t;                                     \
+    size_t w_off, t_off; /* wrapper, table offsets into smbios_entries */ \
+    int str_index = 0;                                                    \
+    do {                                                                  \
+        /* should we skip building this table ? */                        \
+        if (smbios_skip_table(tbl_type, tbl_required)) {                  \
+            return;                                                       \
+        }                                                                 \
+                                                                          \
+        /* initialize fw_cfg smbios element count */                      \
+        if (!smbios_entries) {                                            \
+            smbios_entries_len = sizeof(uint16_t);                        \
+            smbios_entries = g_malloc0(smbios_entries_len);               \
+        }                                                                 \
+                                                                          \
+        /* use offsets of wrapper w and table t within smbios_entries  */ \
+        /* (pointers must be updated after each realloc)               */ \
+        w_off = smbios_entries_len;                                       \
+        t_off = w_off + sizeof(*w);                                       \
+        smbios_entries_len = t_off + sizeof(*t);                          \
+        smbios_entries = g_realloc(smbios_entries, smbios_entries_len);   \
+        w = (struct smbios_table *)(smbios_entries + w_off);              \
+        t = (struct smbios_type_##tbl_type *)(smbios_entries + t_off);    \
+                                                                          \
+        w->header.type = SMBIOS_TABLE_ENTRY;                              \
+        w->header.length = sizeof(*w) + sizeof(*t);                       \
+                                                                          \
+        t->header.type = tbl_type;                                        \
+        t->header.length = sizeof(*t);                                    \
+        t->header.handle = tbl_handle;                                    \
+    } while (0)
+
+#define SMBIOS_TABLE_SET_STR(tbl_type, field, value)                      \
+    do {                                                                  \
+        int len;                                                          \
+        if (value != NULL && (len = strlen(value) + 1) > 1) {             \
+            smbios_entries = g_realloc(smbios_entries,                    \
+                                       smbios_entries_len + len);         \
+            memcpy(smbios_entries + smbios_entries_len, value, len);      \
+            smbios_entries_len += len;                                    \
+            /* update pointer(s) post-realloc */                          \
+            w = (struct smbios_table *)(smbios_entries + w_off);          \
+            t = (struct smbios_type_##tbl_type *)(smbios_entries + t_off);\
+            t->field = ++str_index;                                       \
+            w->header.length += len;                                      \
+        } else {                                                          \
+            t->field = 0;                                                 \
+        }                                                                 \
+    } while (0)
+
+#define SMBIOS_BUILD_TABLE_POST                                           \
+    do {                                                                  \
+        /* add empty string if no strings defined in table */             \
+        /* (NOTE: terminating \0 currently handled by fw_cfg/seabios) */  \
+        if (str_index == 0) {                                             \
+            smbios_entries = g_realloc(smbios_entries,                    \
+                                       smbios_entries_len + 1);           \
+            *(smbios_entries + smbios_entries_len) = 0;                   \
+            smbios_entries_len += 1;                                      \
+            /* update pointer(s) post-realloc */                          \
+            w = (struct smbios_table *)(smbios_entries + w_off);          \
+            w->header.length += 1;                                        \
+        }                                                                 \
+                                                                          \
+        /* update fw_cfg smbios element count */                          \
+        *(uint16_t *)smbios_entries += 1;                                 \
+    } while (0)
+
 static void smbios_add_field(int type, int offset, const void *data, size_t len)
 {
     struct smbios_field *field;
@@ -230,6 +352,24 @@ static void smbios_build_type_1_fields(void)
     }
 }
 
+static void smbios_build_type_2_table(void)
+{
+    SMBIOS_BUILD_TABLE_PRE(2, 0x200, false); /* optional */
+
+    SMBIOS_TABLE_SET_STR(2, manufacturer_str, type2.manufacturer);
+    SMBIOS_TABLE_SET_STR(2, product_str, type2.product);
+    SMBIOS_TABLE_SET_STR(2, version_str, type2.version);
+    SMBIOS_TABLE_SET_STR(2, serial_number_str, type2.serial);
+    SMBIOS_TABLE_SET_STR(2, asset_tag_number_str, type2.asset);
+    t->feature_flags = 0x01; /* Motherboard */
+    SMBIOS_TABLE_SET_STR(2, location_str, type2.location);
+    t->chassis_handle = 0x300; /* Type 3 (System enclosure) */
+    t->board_type = 0x0A; /* Motherboard */
+    t->contained_element_count = 0;
+
+    SMBIOS_BUILD_TABLE_POST;
+}
+
 #define SMBIOS_SET_DEFAULT(field, value)                                  \
     if (!field) {                                                         \
         field = value;                                                    \
@@ -238,14 +378,19 @@ static void smbios_build_type_1_fields(void)
 void smbios_set_defaults(const char *manufacturer,
                          const char *product, const char *version)
 {
+    smbios_have_defaults = true;
     SMBIOS_SET_DEFAULT(type1.manufacturer, manufacturer);
     SMBIOS_SET_DEFAULT(type1.product, product);
     SMBIOS_SET_DEFAULT(type1.version, version);
+    SMBIOS_SET_DEFAULT(type2.manufacturer, manufacturer);
+    SMBIOS_SET_DEFAULT(type2.product, product);
+    SMBIOS_SET_DEFAULT(type2.version, version);
 }
 
 uint8_t *smbios_get_table(size_t *length)
 {
     if (!smbios_immutable) {
+        smbios_build_type_2_table();
         smbios_build_type_0_fields();
         smbios_build_type_1_fields();
         smbios_validate_table();
@@ -381,6 +526,19 @@ void smbios_entry_add(QemuOpts *opts)
                 qemu_uuid_set = true;
             }
             return;
+        case 2:
+            qemu_opts_validate(opts, qemu_smbios_type2_opts, &local_err);
+            if (local_err) {
+                error_report("%s", error_get_pretty(local_err));
+                exit(1);
+            }
+            save_opt(&type2.manufacturer, opts, "manufacturer");
+            save_opt(&type2.product, opts, "product");
+            save_opt(&type2.version, opts, "version");
+            save_opt(&type2.serial, opts, "serial");
+            save_opt(&type2.asset, opts, "asset");
+            save_opt(&type2.location, opts, "location");
+            return;
         default:
             error_report("Don't know how to build fields for SMBIOS type %ld",
                          type);
diff --git a/include/hw/i386/smbios.h b/include/hw/i386/smbios.h
index 8b63441..d0fb908 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -62,6 +62,22 @@ struct smbios_type_1 {
     uint8_t family_str;
 } QEMU_PACKED;
 
+/* SMBIOS type 2 - Base Board */
+struct smbios_type_2 {
+    struct smbios_structure_header header;
+    uint8_t manufacturer_str;
+    uint8_t product_str;
+    uint8_t version_str;
+    uint8_t serial_number_str;
+    uint8_t asset_tag_number_str;
+    uint8_t feature_flags;
+    uint8_t location_str;
+    uint16_t chassis_handle;
+    uint8_t board_type;
+    uint8_t contained_element_count;
+    /* contained elements follow */
+} QEMU_PACKED;
+
 /* SMBIOS type 3 - System Enclosure (v2.3) */
 struct smbios_type_3 {
     struct smbios_structure_header header;
@@ -78,7 +94,7 @@ struct smbios_type_3 {
     uint8_t height;
     uint8_t number_of_power_cords;
     uint8_t contained_element_count;
-    // contained elements follow
+    /* contained elements follow */
 } QEMU_PACKED;
 
 /* SMBIOS type 4 - Processor Information (v2.3) */
-- 
1.8.1.4

[Qemu-devel] [v3 PATCH 05/13] SMBIOS: Add code to build full smbios tables; build type 2 table

[Gabriel L. Somlo]

This patch adds a set of macros which build full smbios tables
of a given type, including the logic to decide whether a given
table type should be built or not.

To illustrate this new functionality, we introduce and optionally
build a table of type 2 (base board), which is required by some
versions of OS X (10.7 and 10.8).

Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
---

On Tue, Mar 11, 2014 at 11:46:17AM -0400, Kevin O'Connor wrote:
> I wont comment on the value of checkpatch, but you could write it this
> way if you wanted:
> 
> int len = value ? strlen(value) : 0;
> if (len > 0) {

Yeah, that's a neat way around it, thanks !

Submitting v3 of that patch only (5/15), every other patch in
the series is unchanged. Now checkpatch.pl is completely happy :)

Thanks,
   Gabriel

 hw/i386/smbios.c         | 158 +++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/i386/smbios.h |  18 +++++-
 2 files changed, 175 insertions(+), 1 deletion(-)

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 6889332..06f572d 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -48,6 +48,7 @@ static uint8_t *smbios_entries;
 static size_t smbios_entries_len;
 static int smbios_type4_count = 0;
 static bool smbios_immutable;
+static bool smbios_have_defaults;
 
 static DECLARE_BITMAP(have_binfile_bitmap, SMBIOS_MAX_TYPE+1);
 static DECLARE_BITMAP(have_fields_bitmap, SMBIOS_MAX_TYPE+1);
@@ -63,6 +64,10 @@ static struct {
     /* uuid is in qemu_uuid[] */
 } type1;
 
+static struct {
+    const char *manufacturer, *product, *version, *serial, *asset, *location;
+} type2;
+
 static QemuOptsList qemu_smbios_opts = {
     .name = "smbios",
     .head = QTAILQ_HEAD_INITIALIZER(qemu_smbios_opts.head),
@@ -146,6 +151,39 @@ static const QemuOptDesc qemu_smbios_type1_opts[] = {
     { /* end of list */ }
 };
 
+static const QemuOptDesc qemu_smbios_type2_opts[] = {
+    {
+        .name = "type",
+        .type = QEMU_OPT_NUMBER,
+        .help = "SMBIOS element type",
+    },{
+        .name = "manufacturer",
+        .type = QEMU_OPT_STRING,
+        .help = "manufacturer name",
+    },{
+        .name = "product",
+        .type = QEMU_OPT_STRING,
+        .help = "product name",
+    },{
+        .name = "version",
+        .type = QEMU_OPT_STRING,
+        .help = "version number",
+    },{
+        .name = "serial",
+        .type = QEMU_OPT_STRING,
+        .help = "serial number",
+    },{
+        .name = "asset",
+        .type = QEMU_OPT_STRING,
+        .help = "asset tag number",
+    },{
+        .name = "location",
+        .type = QEMU_OPT_STRING,
+        .help = "location in chassis",
+    },
+    { /* end of list */ }
+};
+
 static void smbios_register_config(void)
 {
     qemu_add_opts(&qemu_smbios_opts);
@@ -161,6 +199,90 @@ static void smbios_validate_table(void)
     }
 }
 
+static bool smbios_skip_table(uint8_t type, bool required_table)
+{
+    if (test_bit(type, have_binfile_bitmap)) {
+        return true; /* user provided their own binary blob(s) */
+    }
+    if (test_bit(type, have_fields_bitmap)) {
+        return false; /* user provided fields via command line */
+    }
+    if (smbios_have_defaults && required_table) {
+        return false; /* we're building tables, and this one's required */
+    }
+    return true;
+}
+
+#define SMBIOS_BUILD_TABLE_PRE(tbl_type, tbl_handle, tbl_required)        \
+    struct smbios_table *w;                                               \
+    struct smbios_type_##tbl_type *t;                                     \
+    size_t w_off, t_off; /* wrapper, table offsets into smbios_entries */ \
+    int str_index = 0;                                                    \
+    do {                                                                  \
+        /* should we skip building this table ? */                        \
+        if (smbios_skip_table(tbl_type, tbl_required)) {                  \
+            return;                                                       \
+        }                                                                 \
+                                                                          \
+        /* initialize fw_cfg smbios element count */                      \
+        if (!smbios_entries) {                                            \
+            smbios_entries_len = sizeof(uint16_t);                        \
+            smbios_entries = g_malloc0(smbios_entries_len);               \
+        }                                                                 \
+                                                                          \
+        /* use offsets of wrapper w and table t within smbios_entries  */ \
+        /* (pointers must be updated after each realloc)               */ \
+        w_off = smbios_entries_len;                                       \
+        t_off = w_off + sizeof(*w);                                       \
+        smbios_entries_len = t_off + sizeof(*t);                          \
+        smbios_entries = g_realloc(smbios_entries, smbios_entries_len);   \
+        w = (struct smbios_table *)(smbios_entries + w_off);              \
+        t = (struct smbios_type_##tbl_type *)(smbios_entries + t_off);    \
+                                                                          \
+        w->header.type = SMBIOS_TABLE_ENTRY;                              \
+        w->header.length = sizeof(*w) + sizeof(*t);                       \
+                                                                          \
+        t->header.type = tbl_type;                                        \
+        t->header.length = sizeof(*t);                                    \
+        t->header.handle = tbl_handle;                                    \
+    } while (0)
+
+#define SMBIOS_TABLE_SET_STR(tbl_type, field, value)                      \
+    do {                                                                  \
+        int len = (value != NULL) ? strlen(value) + 1 : 0;                \
+        if (len > 1) {                                                    \
+            smbios_entries = g_realloc(smbios_entries,                    \
+                                       smbios_entries_len + len);         \
+            memcpy(smbios_entries + smbios_entries_len, value, len);      \
+            smbios_entries_len += len;                                    \
+            /* update pointer(s) post-realloc */                          \
+            w = (struct smbios_table *)(smbios_entries + w_off);          \
+            t = (struct smbios_type_##tbl_type *)(smbios_entries + t_off);\
+            t->field = ++str_index;                                       \
+            w->header.length += len;                                      \
+        } else {                                                          \
+            t->field = 0;                                                 \
+        }                                                                 \
+    } while (0)
+
+#define SMBIOS_BUILD_TABLE_POST                                           \
+    do {                                                                  \
+        /* add empty string if no strings defined in table */             \
+        /* (NOTE: terminating \0 currently handled by fw_cfg/seabios) */  \
+        if (str_index == 0) {                                             \
+            smbios_entries = g_realloc(smbios_entries,                    \
+                                       smbios_entries_len + 1);           \
+            *(smbios_entries + smbios_entries_len) = 0;                   \
+            smbios_entries_len += 1;                                      \
+            /* update pointer(s) post-realloc */                          \
+            w = (struct smbios_table *)(smbios_entries + w_off);          \
+            w->header.length += 1;                                        \
+        }                                                                 \
+                                                                          \
+        /* update fw_cfg smbios element count */                          \
+        *(uint16_t *)smbios_entries += 1;                                 \
+    } while (0)
+
 static void smbios_add_field(int type, int offset, const void *data, size_t len)
 {
     struct smbios_field *field;
@@ -230,6 +352,24 @@ static void smbios_build_type_1_fields(void)
     }
 }
 
+static void smbios_build_type_2_table(void)
+{
+    SMBIOS_BUILD_TABLE_PRE(2, 0x200, false); /* optional */
+
+    SMBIOS_TABLE_SET_STR(2, manufacturer_str, type2.manufacturer);
+    SMBIOS_TABLE_SET_STR(2, product_str, type2.product);
+    SMBIOS_TABLE_SET_STR(2, version_str, type2.version);
+    SMBIOS_TABLE_SET_STR(2, serial_number_str, type2.serial);
+    SMBIOS_TABLE_SET_STR(2, asset_tag_number_str, type2.asset);
+    t->feature_flags = 0x01; /* Motherboard */
+    SMBIOS_TABLE_SET_STR(2, location_str, type2.location);
+    t->chassis_handle = 0x300; /* Type 3 (System enclosure) */
+    t->board_type = 0x0A; /* Motherboard */
+    t->contained_element_count = 0;
+
+    SMBIOS_BUILD_TABLE_POST;
+}
+
 #define SMBIOS_SET_DEFAULT(field, value)                                  \
     if (!field) {                                                         \
         field = value;                                                    \
@@ -238,14 +378,19 @@ static void smbios_build_type_1_fields(void)
 void smbios_set_defaults(const char *manufacturer,
                          const char *product, const char *version)
 {
+    smbios_have_defaults = true;
     SMBIOS_SET_DEFAULT(type1.manufacturer, manufacturer);
     SMBIOS_SET_DEFAULT(type1.product, product);
     SMBIOS_SET_DEFAULT(type1.version, version);
+    SMBIOS_SET_DEFAULT(type2.manufacturer, manufacturer);
+    SMBIOS_SET_DEFAULT(type2.product, product);
+    SMBIOS_SET_DEFAULT(type2.version, version);
 }
 
 uint8_t *smbios_get_table(size_t *length)
 {
     if (!smbios_immutable) {
+        smbios_build_type_2_table();
         smbios_build_type_0_fields();
         smbios_build_type_1_fields();
         smbios_validate_table();
@@ -381,6 +526,19 @@ void smbios_entry_add(QemuOpts *opts)
                 qemu_uuid_set = true;
             }
             return;
+        case 2:
+            qemu_opts_validate(opts, qemu_smbios_type2_opts, &local_err);
+            if (local_err) {
+                error_report("%s", error_get_pretty(local_err));
+                exit(1);
+            }
+            save_opt(&type2.manufacturer, opts, "manufacturer");
+            save_opt(&type2.product, opts, "product");
+            save_opt(&type2.version, opts, "version");
+            save_opt(&type2.serial, opts, "serial");
+            save_opt(&type2.asset, opts, "asset");
+            save_opt(&type2.location, opts, "location");
+            return;
         default:
             error_report("Don't know how to build fields for SMBIOS type %ld",
                          type);
diff --git a/include/hw/i386/smbios.h b/include/hw/i386/smbios.h
index 8b63441..d0fb908 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -62,6 +62,22 @@ struct smbios_type_1 {
     uint8_t family_str;
 } QEMU_PACKED;
 
+/* SMBIOS type 2 - Base Board */
+struct smbios_type_2 {
+    struct smbios_structure_header header;
+    uint8_t manufacturer_str;
+    uint8_t product_str;
+    uint8_t version_str;
+    uint8_t serial_number_str;
+    uint8_t asset_tag_number_str;
+    uint8_t feature_flags;
+    uint8_t location_str;
+    uint16_t chassis_handle;
+    uint8_t board_type;
+    uint8_t contained_element_count;
+    /* contained elements follow */
+} QEMU_PACKED;
+
 /* SMBIOS type 3 - System Enclosure (v2.3) */
 struct smbios_type_3 {
     struct smbios_structure_header header;
@@ -78,7 +94,7 @@ struct smbios_type_3 {
     uint8_t height;
     uint8_t number_of_power_cords;
     uint8_t contained_element_count;
-    // contained elements follow
+    /* contained elements follow */
 } QEMU_PACKED;
 
 /* SMBIOS type 4 - Processor Information (v2.3) */
-- 
1.8.1.4

[Qemu-devel] [v2 PATCH 06/13] SMBIOS: Build full tables for types 0 and 1

[Gabriel L. Somlo]

From: "Gabriel L. Somlo" <somlo@cmu.edu>

Build full tables for types 0 (bios information) and 1 (system
information). Type 0 is optional, and a table will only be built
if requested via the command line; the default is to leave type 0
tables up to the bios itself.

Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
---
 hw/i386/smbios.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 9679e06..1a7e2b5 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -352,6 +352,62 @@ static void smbios_build_type_1_fields(void)
     }
 }
 
+static void smbios_build_type_0_table(void)
+{
+    SMBIOS_BUILD_TABLE_PRE(0, 0x000, false); /* optional, leave up to BIOS */
+
+    SMBIOS_TABLE_SET_STR(0, vendor_str, type0.vendor);
+    SMBIOS_TABLE_SET_STR(0, bios_version_str, type0.version);
+
+    t->bios_starting_address_segment = 0xE800; /* hardcoded in SeaBIOS */
+
+    SMBIOS_TABLE_SET_STR(0, bios_release_date_str, type0.date);
+
+    t->bios_rom_size = 0; /* hardcoded in SeaBIOS with FIXME comment */
+
+    /* BIOS characteristics not supported */
+    memset(t->bios_characteristics, 0, 8);
+    t->bios_characteristics[0] = 0x08;
+
+    /* Enable targeted content distribution (needed for SVVP, per SeaBIOS) */
+    t->bios_characteristics_extension_bytes[0] = 0;
+    t->bios_characteristics_extension_bytes[1] = 4;
+
+    if (type0.have_major_minor) {
+        t->system_bios_major_release = type0.major;
+        t->system_bios_minor_release = type0.minor;
+    } else {
+        t->system_bios_major_release = 0;
+        t->system_bios_minor_release = 0;
+    }
+
+    /* hardcoded in SeaBIOS */
+    t->embedded_controller_major_release = 0xFF;
+    t->embedded_controller_minor_release = 0xFF;
+
+    SMBIOS_BUILD_TABLE_POST;
+}
+
+static void smbios_build_type_1_table(void)
+{
+    SMBIOS_BUILD_TABLE_PRE(1, 0x100, true); /* required */
+
+    SMBIOS_TABLE_SET_STR(1, manufacturer_str, type1.manufacturer);
+    SMBIOS_TABLE_SET_STR(1, product_name_str, type1.product);
+    SMBIOS_TABLE_SET_STR(1, version_str, type1.version);
+    SMBIOS_TABLE_SET_STR(1, serial_number_str, type1.serial);
+    if (qemu_uuid_set) {
+        memcpy(t->uuid, qemu_uuid, 16);
+    } else {
+        memset(t->uuid, 0, 16);
+    }
+    t->wake_up_type = 0x06; /* power switch */
+    SMBIOS_TABLE_SET_STR(1, sku_number_str, type1.sku);
+    SMBIOS_TABLE_SET_STR(1, family_str, type1.family);
+
+    SMBIOS_BUILD_TABLE_POST;
+}
+
 static void smbios_build_type_2_table(void)
 {
     SMBIOS_BUILD_TABLE_PRE(2, 0x200, false); /* optional */
@@ -379,6 +435,9 @@ void smbios_set_defaults(const char *manufacturer,
                          const char *product, const char *version)
 {
     smbios_have_defaults = true;
+    SMBIOS_SET_DEFAULT(type0.vendor, manufacturer);
+    SMBIOS_SET_DEFAULT(type0.version, version);
+    SMBIOS_SET_DEFAULT(type0.date, "01/01/2014");
     SMBIOS_SET_DEFAULT(type1.manufacturer, manufacturer);
     SMBIOS_SET_DEFAULT(type1.product, product);
     SMBIOS_SET_DEFAULT(type1.version, version);
@@ -390,9 +449,13 @@ void smbios_set_defaults(const char *manufacturer,
 uint8_t *smbios_get_table(size_t *length)
 {
     if (!smbios_immutable) {
+        smbios_build_type_0_table();
+        smbios_build_type_1_table();
         smbios_build_type_2_table();
+if (false) { /* shut up gcc until we remove deprecated code */
         smbios_build_type_0_fields();
         smbios_build_type_1_fields();
+}
         smbios_validate_table();
         smbios_immutable = true;
     }
-- 
1.8.1.4

[Qemu-devel] [v2 PATCH 07/13] SMBIOS: Remove unused code for passing individual fields to bios

[Gabriel L. Somlo]

From: "Gabriel L. Somlo" <somlo@cmu.edu>

This patch removes smbios_add_field() and the old code to insert
individual fields for types 0 and 1 into fw_cfg.

Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
---
 hw/i386/smbios.c | 80 --------------------------------------------------------
 1 file changed, 80 deletions(-)

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 1a7e2b5..16d400a 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -29,13 +29,6 @@ struct smbios_header {
     uint8_t type;
 } QEMU_PACKED;
 
-struct smbios_field {
-    struct smbios_header header;
-    uint8_t type;
-    uint16_t offset;
-    uint8_t data[];
-} QEMU_PACKED;
-
 struct smbios_table {
     struct smbios_header header;
     uint8_t data[];
@@ -283,75 +276,6 @@ static bool smbios_skip_table(uint8_t type, bool required_table)
         *(uint16_t *)smbios_entries += 1;                                 \
     } while (0)
 
-static void smbios_add_field(int type, int offset, const void *data, size_t len)
-{
-    struct smbios_field *field;
-
-    if (!smbios_entries) {
-        smbios_entries_len = sizeof(uint16_t);
-        smbios_entries = g_malloc0(smbios_entries_len);
-    }
-    smbios_entries = g_realloc(smbios_entries, smbios_entries_len +
-                                                  sizeof(*field) + len);
-    field = (struct smbios_field *)(smbios_entries + smbios_entries_len);
-    field->header.type = SMBIOS_FIELD_ENTRY;
-    field->header.length = cpu_to_le16(sizeof(*field) + len);
-
-    field->type = type;
-    field->offset = cpu_to_le16(offset);
-    memcpy(field->data, data, len);
-
-    smbios_entries_len += sizeof(*field) + len;
-    (*(uint16_t *)smbios_entries) =
-            cpu_to_le16(le16_to_cpu(*(uint16_t *)smbios_entries) + 1);
-}
-
-static void smbios_maybe_add_str(int type, int offset, const char *data)
-{
-    if (data) {
-        smbios_add_field(type, offset, data, strlen(data) + 1);
-    }
-}
-
-static void smbios_build_type_0_fields(void)
-{
-    smbios_maybe_add_str(0, offsetof(struct smbios_type_0, vendor_str),
-                         type0.vendor);
-    smbios_maybe_add_str(0, offsetof(struct smbios_type_0, bios_version_str),
-                         type0.version);
-    smbios_maybe_add_str(0, offsetof(struct smbios_type_0,
-                                     bios_release_date_str),
-                         type0.date);
-    if (type0.have_major_minor) {
-        smbios_add_field(0, offsetof(struct smbios_type_0,
-                                     system_bios_major_release),
-                         &type0.major, 1);
-        smbios_add_field(0, offsetof(struct smbios_type_0,
-                                     system_bios_minor_release),
-                         &type0.minor, 1);
-    }
-}
-
-static void smbios_build_type_1_fields(void)
-{
-    smbios_maybe_add_str(1, offsetof(struct smbios_type_1, manufacturer_str),
-                         type1.manufacturer);
-    smbios_maybe_add_str(1, offsetof(struct smbios_type_1, product_name_str),
-                         type1.product);
-    smbios_maybe_add_str(1, offsetof(struct smbios_type_1, version_str),
-                         type1.version);
-    smbios_maybe_add_str(1, offsetof(struct smbios_type_1, serial_number_str),
-                         type1.serial);
-    smbios_maybe_add_str(1, offsetof(struct smbios_type_1, sku_number_str),
-                         type1.sku);
-    smbios_maybe_add_str(1, offsetof(struct smbios_type_1, family_str),
-                         type1.family);
-    if (qemu_uuid_set) {
-        smbios_add_field(1, offsetof(struct smbios_type_1, uuid),
-                         qemu_uuid, 16);
-    }
-}
-
 static void smbios_build_type_0_table(void)
 {
     SMBIOS_BUILD_TABLE_PRE(0, 0x000, false); /* optional, leave up to BIOS */
@@ -452,10 +376,6 @@ uint8_t *smbios_get_table(size_t *length)
         smbios_build_type_0_table();
         smbios_build_type_1_table();
         smbios_build_type_2_table();
-if (false) { /* shut up gcc until we remove deprecated code */
-        smbios_build_type_0_fields();
-        smbios_build_type_1_fields();
-}
         smbios_validate_table();
         smbios_immutable = true;
     }
-- 
1.8.1.4

[Qemu-devel] [v2 PATCH 08/13] SMBIOS: Build full type 3 table

[Gabriel L. Somlo]

From: "Gabriel L. Somlo" <somlo@cmu.edu>

Build smbios type 3 (system enclosure) table, and make it available
to the bios via fw_cfg.

Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
---
 hw/i386/smbios.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 16d400a..c1c0513 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -61,6 +61,10 @@ static struct {
     const char *manufacturer, *product, *version, *serial, *asset, *location;
 } type2;
 
+static struct {
+    const char *manufacturer, *version, *serial, *asset;
+} type3;
+
 static QemuOptsList qemu_smbios_opts = {
     .name = "smbios",
     .head = QTAILQ_HEAD_INITIALIZER(qemu_smbios_opts.head),
@@ -177,6 +181,31 @@ static const QemuOptDesc qemu_smbios_type2_opts[] = {
     { /* end of list */ }
 };
 
+static const QemuOptDesc qemu_smbios_type3_opts[] = {
+    {
+        .name = "type",
+        .type = QEMU_OPT_NUMBER,
+        .help = "SMBIOS element type",
+    },{
+        .name = "manufacturer",
+        .type = QEMU_OPT_STRING,
+        .help = "manufacturer name",
+    },{
+        .name = "version",
+        .type = QEMU_OPT_STRING,
+        .help = "version number",
+    },{
+        .name = "serial",
+        .type = QEMU_OPT_STRING,
+        .help = "serial number",
+    },{
+        .name = "asset",
+        .type = QEMU_OPT_STRING,
+        .help = "asset tag number",
+    },
+    { /* end of list */ }
+};
+
 static void smbios_register_config(void)
 {
     qemu_add_opts(&qemu_smbios_opts);
@@ -350,6 +379,27 @@ static void smbios_build_type_2_table(void)
     SMBIOS_BUILD_TABLE_POST;
 }
 
+static void smbios_build_type_3_table(void)
+{
+    SMBIOS_BUILD_TABLE_PRE(3, 0x300, true); /* required */
+
+    SMBIOS_TABLE_SET_STR(3, manufacturer_str, type3.manufacturer);
+    t->type = 0x01; /* Other */
+    SMBIOS_TABLE_SET_STR(3, version_str, type3.version);
+    SMBIOS_TABLE_SET_STR(3, serial_number_str, type3.serial);
+    SMBIOS_TABLE_SET_STR(3, asset_tag_number_str, type3.asset);
+    t->boot_up_state = 0x03; /* Safe */
+    t->power_supply_state = 0x03; /* Safe */
+    t->thermal_state = 0x03; /* Safe */
+    t->security_status = 0x02; /* Unknown */
+    t->oem_defined = 0;
+    t->height = 0;
+    t->number_of_power_cords = 0;
+    t->contained_element_count = 0;
+
+    SMBIOS_BUILD_TABLE_POST;
+}
+
 #define SMBIOS_SET_DEFAULT(field, value)                                  \
     if (!field) {                                                         \
         field = value;                                                    \
@@ -368,6 +418,8 @@ void smbios_set_defaults(const char *manufacturer,
     SMBIOS_SET_DEFAULT(type2.manufacturer, manufacturer);
     SMBIOS_SET_DEFAULT(type2.product, product);
     SMBIOS_SET_DEFAULT(type2.version, version);
+    SMBIOS_SET_DEFAULT(type3.manufacturer, manufacturer);
+    SMBIOS_SET_DEFAULT(type3.version, version);
 }
 
 uint8_t *smbios_get_table(size_t *length)
@@ -376,6 +428,7 @@ uint8_t *smbios_get_table(size_t *length)
         smbios_build_type_0_table();
         smbios_build_type_1_table();
         smbios_build_type_2_table();
+        smbios_build_type_3_table();
         smbios_validate_table();
         smbios_immutable = true;
     }
@@ -522,6 +575,17 @@ void smbios_entry_add(QemuOpts *opts)
             save_opt(&type2.asset, opts, "asset");
             save_opt(&type2.location, opts, "location");
             return;
+        case 3:
+            qemu_opts_validate(opts, qemu_smbios_type3_opts, &local_err);
+            if (local_err) {
+                error_report("%s", error_get_pretty(local_err));
+                exit(1);
+            }
+            save_opt(&type3.manufacturer, opts, "manufacturer");
+            save_opt(&type3.version, opts, "version");
+            save_opt(&type3.serial, opts, "serial");
+            save_opt(&type3.asset, opts, "asset");
+            return;
         default:
             error_report("Don't know how to build fields for SMBIOS type %ld",
                          type);
-- 
1.8.1.4

[Qemu-devel] [v2 PATCH 09/13] SMBIOS: Build full type 4 tables

[Gabriel L. Somlo]

From: "Gabriel L. Somlo" <somlo@cmu.edu>

Build full smbios type 4 (processor information) tables, and make
them available to the bios via fw_cfg.

Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
---
 hw/i386/pc.c             |  3 ++
 hw/i386/smbios.c         | 97 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/i386/smbios.h |  1 +
 3 files changed, 101 insertions(+)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index e715a33..9c4623e 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1011,6 +1011,9 @@ void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
         sysbus_mmio_map_overlap(SYS_BUS_DEVICE(icc_bridge), 0,
                                 APIC_DEFAULT_ADDRESS, 0x1000);
     }
+
+    /* tell smbios about cpuid version and features */
+    smbios_set_cpuid(cpu->env.cpuid_version, cpu->env.features[FEAT_1_EDX]);
 }
 
 /* pci-info ROM file. Little endian format */
diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index c1c0513..e36d48c 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -42,6 +42,7 @@ static size_t smbios_entries_len;
 static int smbios_type4_count = 0;
 static bool smbios_immutable;
 static bool smbios_have_defaults;
+static uint32_t smbios_cpuid_version, smbios_cpuid_features; /* for type 4 */
 
 static DECLARE_BITMAP(have_binfile_bitmap, SMBIOS_MAX_TYPE+1);
 static DECLARE_BITMAP(have_fields_bitmap, SMBIOS_MAX_TYPE+1);
@@ -65,6 +66,10 @@ static struct {
     const char *manufacturer, *version, *serial, *asset;
 } type3;
 
+static struct {
+    const char *sock_pfx, *manufacturer, *version, *serial, *asset, *part;
+} type4;
+
 static QemuOptsList qemu_smbios_opts = {
     .name = "smbios",
     .head = QTAILQ_HEAD_INITIALIZER(qemu_smbios_opts.head),
@@ -206,6 +211,39 @@ static const QemuOptDesc qemu_smbios_type3_opts[] = {
     { /* end of list */ }
 };
 
+static const QemuOptDesc qemu_smbios_type4_opts[] = {
+    {
+        .name = "type",
+        .type = QEMU_OPT_NUMBER,
+        .help = "SMBIOS element type",
+    },{
+        .name = "sock_pfx",
+        .type = QEMU_OPT_STRING,
+        .help = "socket designation string prefix",
+    },{
+        .name = "manufacturer",
+        .type = QEMU_OPT_STRING,
+        .help = "manufacturer name",
+    },{
+        .name = "version",
+        .type = QEMU_OPT_STRING,
+        .help = "version number",
+    },{
+        .name = "serial",
+        .type = QEMU_OPT_STRING,
+        .help = "serial number",
+    },{
+        .name = "asset",
+        .type = QEMU_OPT_STRING,
+        .help = "asset tag number",
+    },{
+        .name = "part",
+        .type = QEMU_OPT_STRING,
+        .help = "part number",
+    },
+    { /* end of list */ }
+};
+
 static void smbios_register_config(void)
 {
     qemu_add_opts(&qemu_smbios_opts);
@@ -400,11 +438,48 @@ static void smbios_build_type_3_table(void)
     SMBIOS_BUILD_TABLE_POST;
 }
 
+static void smbios_build_type_4_table(unsigned instance)
+{
+    char sock_str[128];
+
+    SMBIOS_BUILD_TABLE_PRE(4, 0x400 + instance, true); /* required */
+
+    snprintf(sock_str, sizeof(sock_str), "%s%2x", type4.sock_pfx, instance);
+    SMBIOS_TABLE_SET_STR(4, socket_designation_str, sock_str);
+    t->processor_type = 0x03; /* CPU */
+    t->processor_family = 0x01; /* Other */
+    SMBIOS_TABLE_SET_STR(4, processor_manufacturer_str, type4.manufacturer);
+    t->processor_id[0] = smbios_cpuid_version;
+    t->processor_id[1] = smbios_cpuid_features;
+    SMBIOS_TABLE_SET_STR(4, processor_version_str, type4.version);
+    t->voltage = 0;
+    t->external_clock = 0;
+    t->max_speed = 2000; /* hardcoded in SeaBIOS (use 0/Unknown instead ?) */
+    t->current_speed = 2000; /* hardcoded in SeaBIOS (use 0/Unknown ?) */
+    t->status = 0x41; /* Socket populated, CPU enabled */
+    t->processor_upgrade = 0x01; /* Other */
+    t->l1_cache_handle = 0xFFFF; /* N/A */
+    t->l2_cache_handle = 0xFFFF; /* N/A */
+    t->l3_cache_handle = 0xFFFF; /* N/A */
+    SMBIOS_TABLE_SET_STR(4, serial_number_str, type4.serial);
+    SMBIOS_TABLE_SET_STR(4, asset_tag_number_str, type4.asset);
+    SMBIOS_TABLE_SET_STR(4, part_number_str, type4.part);
+
+    SMBIOS_BUILD_TABLE_POST;
+    smbios_type4_count++;
+}
+
 #define SMBIOS_SET_DEFAULT(field, value)                                  \
     if (!field) {                                                         \
         field = value;                                                    \
     }
 
+void smbios_set_cpuid(uint32_t version, uint32_t features)
+{
+    smbios_cpuid_version = version;
+    smbios_cpuid_features = features;
+}
+
 void smbios_set_defaults(const char *manufacturer,
                          const char *product, const char *version)
 {
@@ -420,15 +495,24 @@ void smbios_set_defaults(const char *manufacturer,
     SMBIOS_SET_DEFAULT(type2.version, version);
     SMBIOS_SET_DEFAULT(type3.manufacturer, manufacturer);
     SMBIOS_SET_DEFAULT(type3.version, version);
+    SMBIOS_SET_DEFAULT(type4.sock_pfx, "CPU");
+    SMBIOS_SET_DEFAULT(type4.manufacturer, manufacturer);
+    SMBIOS_SET_DEFAULT(type4.version, version);
 }
 
 uint8_t *smbios_get_table(size_t *length)
 {
+    unsigned i;
+
     if (!smbios_immutable) {
         smbios_build_type_0_table();
         smbios_build_type_1_table();
         smbios_build_type_2_table();
         smbios_build_type_3_table();
+        for (i = 0; i < smp_cpus; i++) {
+            /* count CPUs starting with 1, to minimize diff vs. SeaBIOS */
+            smbios_build_type_4_table(i + 1);
+        }
         smbios_validate_table();
         smbios_immutable = true;
     }
@@ -586,6 +670,19 @@ void smbios_entry_add(QemuOpts *opts)
             save_opt(&type3.serial, opts, "serial");
             save_opt(&type3.asset, opts, "asset");
             return;
+        case 4:
+            qemu_opts_validate(opts, qemu_smbios_type4_opts, &local_err);
+            if (local_err) {
+                error_report("%s", error_get_pretty(local_err));
+                exit(1);
+            }
+            save_opt(&type4.sock_pfx, opts, "sock_pfx");
+            save_opt(&type4.manufacturer, opts, "manufacturer");
+            save_opt(&type4.version, opts, "version");
+            save_opt(&type4.serial, opts, "serial");
+            save_opt(&type4.asset, opts, "asset");
+            save_opt(&type4.part, opts, "part");
+            return;
         default:
             error_report("Don't know how to build fields for SMBIOS type %ld",
                          type);
diff --git a/include/hw/i386/smbios.h b/include/hw/i386/smbios.h
index d0fb908..bc05a5f 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -18,6 +18,7 @@
 #define SMBIOS_MAX_TYPE 127
 
 void smbios_entry_add(QemuOpts *opts);
+void smbios_set_cpuid(uint32_t version, uint32_t features);
 void smbios_set_defaults(const char *manufacturer,
                          const char *product, const char *version);
 uint8_t *smbios_get_table(size_t *length);
-- 
1.8.1.4

[Qemu-devel] [v2 PATCH 10/13] SMBIOS: Build full smbios v2.3 compliant type 16 and 17 tables

[Gabriel L. Somlo]

From: "Gabriel L. Somlo" <somlo@cmu.edu>

Build full smbios type 16 (physical memory array) and
type 17 (memory device) tables, and make them available
to the bios via fw_cfg. Type 17 tables will comply with
smbios v2.3, which helps prevent the OS X GUI from crashing
when "about this mac" is selected.

Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
---
 hw/i386/smbios.c | 109 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 108 insertions(+), 1 deletion(-)

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index e36d48c..3c2a985 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -70,6 +70,10 @@ static struct {
     const char *sock_pfx, *manufacturer, *version, *serial, *asset, *part;
 } type4;
 
+static struct {
+    const char *loc_pfx, *bank, *manufacturer, *serial, *asset, *part;
+} type17;
+
 static QemuOptsList qemu_smbios_opts = {
     .name = "smbios",
     .head = QTAILQ_HEAD_INITIALIZER(qemu_smbios_opts.head),
@@ -244,6 +248,39 @@ static const QemuOptDesc qemu_smbios_type4_opts[] = {
     { /* end of list */ }
 };
 
+static const QemuOptDesc qemu_smbios_type17_opts[] = {
+    {
+        .name = "type",
+        .type = QEMU_OPT_NUMBER,
+        .help = "SMBIOS element type",
+    },{
+        .name = "loc_pfx",
+        .type = QEMU_OPT_STRING,
+        .help = "device locator string prefix",
+    },{
+        .name = "bank",
+        .type = QEMU_OPT_STRING,
+        .help = "bank locator string",
+    },{
+        .name = "manufacturer",
+        .type = QEMU_OPT_STRING,
+        .help = "manufacturer name",
+    },{
+        .name = "serial",
+        .type = QEMU_OPT_STRING,
+        .help = "serial number",
+    },{
+        .name = "asset",
+        .type = QEMU_OPT_STRING,
+        .help = "asset tag number",
+    },{
+        .name = "part",
+        .type = QEMU_OPT_STRING,
+        .help = "part number",
+    },
+    { /* end of list */ }
+};
+
 static void smbios_register_config(void)
 {
     qemu_add_opts(&qemu_smbios_opts);
@@ -469,6 +506,52 @@ static void smbios_build_type_4_table(unsigned instance)
     smbios_type4_count++;
 }
 
+static void smbios_build_type_16_table(unsigned memdev_count)
+{
+    unsigned ram_size_kb = ram_size >> 10;
+
+    SMBIOS_BUILD_TABLE_PRE(16, 0x1000, true); /* required */
+
+    t->location = 0x01; /* Other */
+    t->use = 0x03; /* System memory */
+    t->error_correction = 0x06; /* Multi-bit ECC (for Microsoft, per SeaBIOS) */
+    /* if ram_size < 2T, use value in Kilobytes; 0x80000000 == 2T and over;
+     * TODO: support smbios v2.7 extended capacity, or multiple arrays. */
+    t->maximum_capacity = (ram_size_kb < 0x80000000) ? ram_size_kb : 0x80000000;
+    t->memory_error_information_handle = 0xFFFE; /* Not provided */
+    t->number_of_memory_devices = memdev_count;
+
+    SMBIOS_BUILD_TABLE_POST;
+}
+
+static void smbios_build_type_17_table(unsigned instance, unsigned size_mb)
+{
+    char loc_str[128];
+
+    SMBIOS_BUILD_TABLE_PRE(17, 0x1100 + instance, true); /* required */
+
+    t->physical_memory_array_handle = 0x1000; /* Type 16 (Phys. Mem. Array) */
+    t->memory_error_information_handle = 0xFFFE; /* Not provided */
+    t->total_width = 64; /* hardcoded in SeaBIOS */
+    t->data_width = 64; /* hardcoded in SeaBIOS */
+    assert(size_mb <= 0x7FFF);
+    t->size = size_mb;
+    t->form_factor = 0x09; /* DIMM */
+    t->device_set = 0; /* Not in a set */
+    snprintf(loc_str, sizeof(loc_str), "%s %d", type17.loc_pfx, instance);
+    SMBIOS_TABLE_SET_STR(17, device_locator_str, loc_str);
+    SMBIOS_TABLE_SET_STR(17, bank_locator_str, type17.bank);
+    t->memory_type = 0x07; /* RAM */
+    t->type_detail = 0; /* hardcoded in SeaBIOS */
+    t->speed = 0; /* Unknown */
+    SMBIOS_TABLE_SET_STR(17, manufacturer_str, type17.manufacturer);
+    SMBIOS_TABLE_SET_STR(17, serial_number_str, type17.serial);
+    SMBIOS_TABLE_SET_STR(17, asset_tag_number_str, type17.asset);
+    SMBIOS_TABLE_SET_STR(17, part_number_str, type17.part);
+
+    SMBIOS_BUILD_TABLE_POST;
+}
+
 #define SMBIOS_SET_DEFAULT(field, value)                                  \
     if (!field) {                                                         \
         field = value;                                                    \
@@ -498,11 +581,13 @@ void smbios_set_defaults(const char *manufacturer,
     SMBIOS_SET_DEFAULT(type4.sock_pfx, "CPU");
     SMBIOS_SET_DEFAULT(type4.manufacturer, manufacturer);
     SMBIOS_SET_DEFAULT(type4.version, version);
+    SMBIOS_SET_DEFAULT(type17.loc_pfx, "DIMM");
+    SMBIOS_SET_DEFAULT(type17.manufacturer, manufacturer);
 }
 
 uint8_t *smbios_get_table(size_t *length)
 {
-    unsigned i;
+    unsigned i, ram_size_mb, memdev_count;
 
     if (!smbios_immutable) {
         smbios_build_type_0_table();
@@ -513,6 +598,15 @@ uint8_t *smbios_get_table(size_t *length)
             /* count CPUs starting with 1, to minimize diff vs. SeaBIOS */
             smbios_build_type_4_table(i + 1);
         }
+        ram_size_mb = ram_size >> 20;
+        /* up to 16GB (0x4000MB) per memory device: */
+        memdev_count = (ram_size_mb + 0x3FFF) >> 14;
+        smbios_build_type_16_table(memdev_count);
+        for (i = 0; i < memdev_count; i++) {
+            uint32_t size_mb = (i == memdev_count - 1) ?
+                               ((ram_size_mb - 1) & 0x3FFF) + 1 : 0x4000;
+            smbios_build_type_17_table(i, size_mb);
+        }
         smbios_validate_table();
         smbios_immutable = true;
     }
@@ -683,6 +777,19 @@ void smbios_entry_add(QemuOpts *opts)
             save_opt(&type4.asset, opts, "asset");
             save_opt(&type4.part, opts, "part");
             return;
+        case 17:
+            qemu_opts_validate(opts, qemu_smbios_type17_opts, &local_err);
+            if (local_err) {
+                error_report("%s", error_get_pretty(local_err));
+                exit(1);
+            }
+            save_opt(&type17.loc_pfx, opts, "loc_pfx");
+            save_opt(&type17.bank, opts, "bank");
+            save_opt(&type17.manufacturer, opts, "manufacturer");
+            save_opt(&type17.serial, opts, "serial");
+            save_opt(&type17.asset, opts, "asset");
+            save_opt(&type17.part, opts, "part");
+            return;
         default:
             error_report("Don't know how to build fields for SMBIOS type %ld",
                          type);
-- 
1.8.1.4

[Qemu-devel] [v2 PATCH 11/13] SMBIOS: Build full type 19 tables

[Gabriel L. Somlo]

From: "Gabriel L. Somlo" <somlo@cmu.edu>

Build full smbios type 19 (memory array mapped address) tables,
and make them available via fw_cfg

Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
---
 hw/i386/pc_piix.c        |  3 ++-
 hw/i386/pc_q35.c         |  3 ++-
 hw/i386/smbios.c         | 27 ++++++++++++++++++++++++++-
 include/hw/i386/smbios.h |  4 +++-
 4 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index ef2d062..5950d7f 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -146,7 +146,8 @@ static void pc_init1(QEMUMachineInitArgs *args,
     if (smbios_defaults) {
         /* These values are guest ABI, do not change */
         smbios_set_defaults("QEMU", "Standard PC (i440FX + PIIX, 1996)",
-                                  args->machine->name);
+                             args->machine->name,
+                             below_4g_mem_size, above_4g_mem_size);
     }
 
     /* allocate ram and load rom/bios */
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index dfcc252..482f466 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -133,7 +133,8 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
     if (smbios_defaults) {
         /* These values are guest ABI, do not change */
         smbios_set_defaults("QEMU", "Standard PC (Q35 + ICH9, 2009)",
-                                  args->machine->name);
+                             args->machine->name,
+                             below_4g_mem_size, above_4g_mem_size);
     }
 
     /* allocate ram and load rom/bios */
diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 3c2a985..c9cd295 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -43,6 +43,7 @@ static int smbios_type4_count = 0;
 static bool smbios_immutable;
 static bool smbios_have_defaults;
 static uint32_t smbios_cpuid_version, smbios_cpuid_features; /* for type 4 */
+static ram_addr_t smbios_below_4g_ram, smbios_above_4g_ram; /* for type 19 */
 
 static DECLARE_BITMAP(have_binfile_bitmap, SMBIOS_MAX_TYPE+1);
 static DECLARE_BITMAP(have_fields_bitmap, SMBIOS_MAX_TYPE+1);
@@ -552,6 +553,19 @@ static void smbios_build_type_17_table(unsigned instance, unsigned size_mb)
     SMBIOS_BUILD_TABLE_POST;
 }
 
+static void smbios_build_type_19_table(unsigned instance,
+                                       unsigned start_kb, unsigned size_kb)
+{
+    SMBIOS_BUILD_TABLE_PRE(19, 0x1300 + instance, true); /* required */
+
+    t->starting_address = start_kb;
+    t->ending_address = start_kb + size_kb - 1;
+    t->memory_array_handle = 0x1000; /* Type 16 (Phys. Mem. Array) */
+    t->partition_width = 1; /* hardcoded in SeaBIOS */
+
+    SMBIOS_BUILD_TABLE_POST;
+}
+
 #define SMBIOS_SET_DEFAULT(field, value)                                  \
     if (!field) {                                                         \
         field = value;                                                    \
@@ -564,9 +578,16 @@ void smbios_set_cpuid(uint32_t version, uint32_t features)
 }
 
 void smbios_set_defaults(const char *manufacturer,
-                         const char *product, const char *version)
+                         const char *product, const char *version,
+                         ram_addr_t below_4g_mem_size,
+                         ram_addr_t above_4g_mem_size)
 {
     smbios_have_defaults = true;
+
+    assert(ram_size == below_4g_mem_size + above_4g_mem_size);
+    smbios_below_4g_ram = below_4g_mem_size;
+    smbios_above_4g_ram = above_4g_mem_size;
+
     SMBIOS_SET_DEFAULT(type0.vendor, manufacturer);
     SMBIOS_SET_DEFAULT(type0.version, version);
     SMBIOS_SET_DEFAULT(type0.date, "01/01/2014");
@@ -607,6 +628,10 @@ uint8_t *smbios_get_table(size_t *length)
                                ((ram_size_mb - 1) & 0x3FFF) + 1 : 0x4000;
             smbios_build_type_17_table(i, size_mb);
         }
+        smbios_build_type_19_table(0, 0, smbios_below_4g_ram >> 10);
+        if (smbios_above_4g_ram) {
+            smbios_build_type_19_table(1, 4 << 20, smbios_above_4g_ram >> 10);
+        }
         smbios_validate_table();
         smbios_immutable = true;
     }
diff --git a/include/hw/i386/smbios.h b/include/hw/i386/smbios.h
index bc05a5f..c20852a 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -20,7 +20,9 @@
 void smbios_entry_add(QemuOpts *opts);
 void smbios_set_cpuid(uint32_t version, uint32_t features);
 void smbios_set_defaults(const char *manufacturer,
-                         const char *product, const char *version);
+                         const char *product, const char *version,
+                         ram_addr_t below_4g_mem_size,
+                         ram_addr_t above_4g_mem_size);
 uint8_t *smbios_get_table(size_t *length);
 
 /*
-- 
1.8.1.4

Re: [Qemu-devel] [v2 PATCH 11/13] SMBIOS: Build full type 19 tables

[Gerd Hoffmann]

On Di, 2014-03-11 at 11:16 -0400, Gabriel L. Somlo wrote:
> From: "Gabriel L. Somlo" <somlo@cmu.edu>
> 
> Build full smbios type 19 (memory array mapped address) tables,
> and make them available via fw_cfg


> +        smbios_build_type_19_table(0, 0, smbios_below_4g_ram >> 10);
> +        if (smbios_above_4g_ram) {
> +            smbios_build_type_19_table(1, 4 << 20, smbios_above_4g_ram >> 10);
> +        }

I think we should just use e820_table (see pc.c) here.  Loop over it and
add a type 19 table for each ram region in there.

cheers,
  Gerd

Re: [Qemu-devel] [v2 PATCH 11/13] SMBIOS: Build full type 19 tables

[Gabriel L. Somlo]

On Wed, Mar 12, 2014 at 09:27:18AM +0100, Gerd Hoffmann wrote:
> I think we should just use e820_table (see pc.c) here.  Loop over it and
> add a type 19 table for each ram region in there.

I'm assuming this should be another post-Seabios-compatibility patch,
at the end of the series, and I should still do the (start,size)
arithmetic cut'n'pasted from SeaBIOS first, right ?

Re: [Qemu-devel] [v2 PATCH 11/13] SMBIOS: Build full type 19 tables

[Gerd Hoffmann]

On Mi, 2014-03-12 at 09:05 -0400, Gabriel L. Somlo wrote:
> On Wed, Mar 12, 2014 at 09:27:18AM +0100, Gerd Hoffmann wrote:
> > I think we should just use e820_table (see pc.c) here.  Loop over it and
> > add a type 19 table for each ram region in there.
> 
> I'm assuming this should be another post-Seabios-compatibility patch,
> at the end of the series, and I should still do the (start,size)
> arithmetic cut'n'pasted from SeaBIOS first, right ?

You should get identical results with both methods.  It's just that the
e820 method is more future proof, i.e. if the numa people add support
for non-contignous memory some day we don't have to adapt the smbios
code to handle it.

cheers,
  Gerd

Re: [Qemu-devel] [v2 PATCH 11/13] SMBIOS: Build full type 19 tables

[Gabriel L. Somlo]

On Wed, Mar 12, 2014 at 02:24:54PM +0100, Gerd Hoffmann wrote:
> On Mi, 2014-03-12 at 09:05 -0400, Gabriel L. Somlo wrote:
> > I'm assuming this should be another post-Seabios-compatibility patch,
> > at the end of the series, and I should still do the (start,size)
> > arithmetic cut'n'pasted from SeaBIOS first, right ?
> 
> You should get identical results with both methods.  It's just that the
> e820 method is more future proof, i.e. if the numa people add support
> for non-contignous memory some day we don't have to adapt the smbios
> code to handle it.

OK, I'm looking for the least inelegant way to expose e820 to smbios.c :)
Meanwhile:

> > > I think we should just use e820_table (see pc.c) here.  Loop over it and
> > > add a type 19 table for each ram region in there.

You mean only for entries of type E820_RAM, correct ?

Thx,
--G

Re: [Qemu-devel] [v2 PATCH 11/13] SMBIOS: Build full type 19 tables

[Gerd Hoffmann]

On Mi, 2014-03-12 at 10:44 -0400, Gabriel L. Somlo wrote:
> > > > I think we should just use e820_table (see pc.c) here.  Loop
> over it and
> > > > add a type 19 table for each ram region in there.
> 
> You mean only for entries of type E820_RAM, correct ?

Yes.

cheers,
  Gerd

Re: [Qemu-devel] [v2 PATCH 11/13] SMBIOS: Build full type 19 tables

[Gabriel L. Somlo]

On Wed, Mar 12, 2014 at 02:24:54PM +0100, Gerd Hoffmann wrote:
> On Mi, 2014-03-12 at 09:05 -0400, Gabriel L. Somlo wrote:
> > On Wed, Mar 12, 2014 at 09:27:18AM +0100, Gerd Hoffmann wrote:
> > > I think we should just use e820_table (see pc.c) here.  Loop over it and
> > > add a type 19 table for each ram region in there.
> > 
> > I'm assuming this should be another post-Seabios-compatibility patch,
> > at the end of the series, and I should still do the (start,size)
> > arithmetic cut'n'pasted from SeaBIOS first, right ?
> 
> You should get identical results with both methods.  It's just that the
> e820 method is more future proof, i.e. if the numa people add support
> for non-contignous memory some day we don't have to adapt the smbios
> code to handle it.

So, for now, I have the patch at the end of this email. 

The problem is, e820_get_entry matches on indices 1 and 2, so my two
type-19 tables get handles (0x1301, 0x1302), instead of (0x1300, 0x1301).

Then, the logic setting up type 20 tables has to figure out which
indices matched for below-4g and above-4g, so that we can tie them
back to the type 19 tables.

The numbering conventions won't match SeaBIOS anymore, and I need to
clean up the cut'n'paste type 20 arithmetic from SeaBIOS to match the
e820 change for type 19.

Is this something we can decouple from the other smbios patches to buy
me some extra time ? :)

Thanks,
--Gabriel


diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 9c4623e..7c58e88 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -611,6 +611,21 @@ int e820_add_entry(uint64_t address, uint64_t length, uint32_t type)
     return e820_entries;
 }
 
+int e820_num_entries(void)
+{
+    return e820_entries;
+}
+
+bool e820_get_entry(int idx, uint32_t type, uint64_t *address, uint64_t *length)
+{
+    if (idx >= e820_entries || e820_table[idx].type != cpu_to_le32(type)) {
+      return false;
+    }
+    *address = le64_to_cpu(e820_table[idx].address);
+    *length = le64_to_cpu(e820_table[idx].length);
+    return true;
+}
+
 /* Calculates the limit to CPU APIC ID values
  *
  * This function returns the limit for the APIC ID value, so that all
diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 06e3f61..15c022e 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -18,6 +18,7 @@
 #include "qemu/config-file.h"
 #include "qemu/error-report.h"
 #include "sysemu/sysemu.h"
+#include "hw/i386/pc.h"
 #include "hw/i386/smbios.h"
 #include "hw/loader.h"
 
@@ -666,11 +667,15 @@ uint8_t *smbios_get_table(size_t *length)
                                ((ram_size_mb - 1) & 0x3FFF) + 1 : 0x4000;
             smbios_build_type_17_table(i, size_mb);
         }
-        smbios_build_type_19_table(0, 0, smbios_below_4g_ram >> 10);
+        for (i = 0; i < e820_num_entries(); i++) {
+            uint64_t address, length;
+            if (e820_get_entry(i, E820_RAM, &address, &length)) {
+                smbios_build_type_19_table(i, address >> 10, length >> 10);
+            }
+        }
         smbios_build_type_20_table(0, 0, 0, 0, smbios_below_4g_ram >> 10);
         if (smbios_above_4g_ram) {
             uint32_t start_mb = 4096, size_mb, j;
-            smbios_build_type_19_table(1, 4 << 20, smbios_above_4g_ram >> 10);
             for (j = 1, i = 0; i < memdev_count; i++, j++) {
                 size_mb = (i == memdev_count - 1) ?
                           ((ram_size_mb - 1) & 0x3FFF) + 1 : 0x4000;
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 9010246..44932b8 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -239,6 +239,8 @@ uint16_t pvpanic_port(void);
 #define E820_UNUSABLE   5
 
 int e820_add_entry(uint64_t, uint64_t, uint32_t);
+int e820_num_entries(void);
+bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
 
 #define PC_Q35_COMPAT_1_7 \
         PC_COMPAT_1_7, \

Re: [Qemu-devel] [v2 PATCH 11/13] SMBIOS: Build full type 19 tables

[Gabriel L. Somlo]

On Wed, Mar 12, 2014 at 02:24:54PM +0100, Gerd Hoffmann wrote:
> You should get identical results with both methods.  It's just that the
> e820 method is more future proof, i.e. if the numa people add support
> for non-contignous memory some day we don't have to adapt the smbios
> code to handle it.

So, assuming we're staying under 2T, a single T16 array is created
with handle 0x1000.

T17 devices are created in increments of 16Gigs, and they all point
back at the T16 array via their Array Handle field.

One T19 "array mapped address" is created for below-4g memory, and a
second one for above-4g.

T20 is where the fun begins. Apparently (assuming over-4g ram),
two T20s are created to split the first t17 device's memory into
the below-4g portion and the above-4g portion.

>From then on, one T20 corresponds to one T17. So we get X+1 T20s for
each X T17s (with the first two T20s pointing at the first T17,
and T20(x+1) pointing at T17(x) for every x after the first).

Any idea how I can best anticipate accomodating numa on top of this ?

Presumably there will be additional e820 entries resulting in extra
T19s, but what will that do to the T17s and the T20s ? :)

Thanks,
--Gabriel

Re: [Qemu-devel] [v2 PATCH 11/13] SMBIOS: Build full type 19 tables

[Gabriel L. Somlo]

On Wed, Mar 12, 2014 at 02:04:50PM -0400, Gabriel L. Somlo wrote:
> On Wed, Mar 12, 2014 at 02:24:54PM +0100, Gerd Hoffmann wrote:
> > You should get identical results with both methods.  It's just that the
> > e820 method is more future proof, i.e. if the numa people add support
> > for non-contignous memory some day we don't have to adapt the smbios
> > code to handle it.
> 
> So, assuming we're staying under 2T, a single T16 array is created
> with handle 0x1000.
> 
> T17 devices are created in increments of 16Gigs, and they all point
> back at the T16 array via their Array Handle field.
> 
> One T19 "array mapped address" is created for below-4g memory, and a
> second one for above-4g.
> 
> T20 is where the fun begins. Apparently (assuming over-4g ram),
> two T20s are created to split the first t17 device's memory into
> the below-4g portion and the above-4g portion.
> 
> From then on, one T20 corresponds to one T17. So we get X+1 T20s for
> each X T17s (with the first two T20s pointing at the first T17,
> and T20(x+1) pointing at T17(x) for every x after the first).
> 
> Any idea how I can best anticipate accomodating numa on top of this ?
> 
> Presumably there will be additional e820 entries resulting in extra
> T19s, but what will that do to the T17s and the T20s ? :)

Thinking in small increments here, sorry for the self-reply:

Do you anticipate a new field in e820_entry, perhaps hinting at which
CPU the memory belongs to ?

Even now, assuming type==E820_RAM, I know you're putting in exactly
one or two entries (below-4g and above-4g). But if I use e820 in
smbios, am I OK assuming there will always be precisely one entry
for which (start+lengh <= 4G) ?

At least with the current arithmetic I'm working directly with
[below|above]_4g, so there's no way to mistakenly assume the wrong
thing, until we explicitly add in NUMA support :)

Thanks,
--G

[Qemu-devel] [v2 PATCH 12/13] SMBIOS: Build full type 20 tables

[Gabriel L. Somlo]

From: "Gabriel L. Somlo" <somlo@cmu.edu>

Build full smbios type 20 (memory device mapped address) tables,
and make them available via fw_cfg.

Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
---
 hw/i386/smbios.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index c9cd295..9cf29d3 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -566,6 +566,23 @@ static void smbios_build_type_19_table(unsigned instance,
     SMBIOS_BUILD_TABLE_POST;
 }
 
+static void smbios_build_type_20_table(unsigned instance,
+                                       unsigned dev_hndl, unsigned array_hndl,
+                                       unsigned start_kb, unsigned size_kb)
+{
+    SMBIOS_BUILD_TABLE_PRE(20, 0x1400 + instance, true); /* required */
+
+    t->starting_address = start_kb;
+    t->ending_address = start_kb + size_kb - 1;
+    t->memory_device_handle = 0x1100 + dev_hndl; /* Type 17 (Memory Device) */
+    t->memory_array_mapped_address_handle = 0x1300 + array_hndl; /* Type 19 */
+    t->partition_row_position = 1; /* hardcoded in SeaBIOS */
+    t->interleave_position = 0; /* Not interleaved */
+    t->interleaved_data_depth = 0; /* Not interleaved */
+
+    SMBIOS_BUILD_TABLE_POST;
+}
+
 #define SMBIOS_SET_DEFAULT(field, value)                                  \
     if (!field) {                                                         \
         field = value;                                                    \
@@ -629,8 +646,20 @@ uint8_t *smbios_get_table(size_t *length)
             smbios_build_type_17_table(i, size_mb);
         }
         smbios_build_type_19_table(0, 0, smbios_below_4g_ram >> 10);
+        smbios_build_type_20_table(0, 0, 0, 0, smbios_below_4g_ram >> 10);
         if (smbios_above_4g_ram) {
+            uint32_t start_mb = 4096, size_mb, j;
             smbios_build_type_19_table(1, 4 << 20, smbios_above_4g_ram >> 10);
+            for (j = 1, i = 0; i < memdev_count; i++, j++) {
+                size_mb = (i == memdev_count - 1) ?
+                          ((ram_size_mb - 1) & 0x3FFF) + 1 : 0x4000;
+                if (i == 0) {
+                    size_mb -= smbios_below_4g_ram >> 20;
+                }
+                smbios_build_type_20_table(j, i, 1,
+                                           start_mb << 10, size_mb << 10);
+                start_mb += size_mb;
+            }
         }
         smbios_validate_table();
         smbios_immutable = true;
-- 
1.8.1.4

[Qemu-devel] [v2 PATCH 13/13] SMBIOS: Build full tables for type 32 and 127

[Gabriel L. Somlo]

From: "Gabriel L. Somlo" <somlo@cmu.edu>

Build full smbios type 32 (system boot info) and 127 (end-of-table)
tables, and make them available via fw_cfg.

Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
---
 hw/i386/smbios.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 9cf29d3..2bde2a5 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -583,6 +583,22 @@ static void smbios_build_type_20_table(unsigned instance,
     SMBIOS_BUILD_TABLE_POST;
 }
 
+static void smbios_build_type_32_table(void)
+{
+    SMBIOS_BUILD_TABLE_PRE(32, 0x2000, true); /* required */
+
+    memset(t->reserved, 0, 6);
+    t->boot_status = 0; /* No errors detected */
+
+    SMBIOS_BUILD_TABLE_POST;
+}
+
+static void smbios_build_type_127_table(void)
+{
+    SMBIOS_BUILD_TABLE_PRE(32, 0x7F00, true); /* required */
+    SMBIOS_BUILD_TABLE_POST;
+}
+
 #define SMBIOS_SET_DEFAULT(field, value)                                  \
     if (!field) {                                                         \
         field = value;                                                    \
@@ -661,6 +677,8 @@ uint8_t *smbios_get_table(size_t *length)
                 start_mb += size_mb;
             }
         }
+        smbios_build_type_32_table();
+        smbios_build_type_127_table();
         smbios_validate_table();
         smbios_immutable = true;
     }
-- 
1.8.1.4

Re: [Qemu-devel] [v2 PATCH 00/13] SMBIOS: build full tables in QEMU

[Kevin O'Connor]

On Tue, Mar 11, 2014 at 11:16:16AM -0400, Gabriel L. Somlo wrote:
> On Tue, Mar 11, 2014 at 11:03:06AM +0100, Gerd Hoffmann wrote:
> > Issue #1: There are checkpatch errors (scripts/checkpatch.pl).
> 
> I fixed most of those, with one exception (patch #5):
> 
> ERROR: do not use assignment in if condition
> #139: FILE: hw/i386/smbios.c:253:
> +        if (value != NULL && (len = strlen(value) + 1) > 1) { \
> 
> 'value' can be NULL, "", or "foo". The whole block looks like this:
> 
>   int len;
>   if (value != NULL && (len = strlen(value)) > 0) {

I wont comment on the value of checkpatch, but you could write it this
way if you wanted:

int len = value ? strlen(value) : 0;
if (len > 0) {

[...]
> > -Handle 0x1100, DMI type 17, 21 bytes
> > +Handle 0x1100, DMI type 17, 27 bytes
> >  Memory Device
> >  	Array Handle: 0x1000
> > -	Error Information Handle: 0x0003
> > +	Error Information Handle: Not Provided
> 
> AFAICT, SeaBIOS doesn't set the error info handle at all, and you simply
> get whatever garbage happens to be contained in that memory location at
> the time. Typically it's 0x0000, but you happened to get 0x0003 :) I
> figured I'd set it to a known value (0xFFFE or "n/a").

That's a seabios bug - I'll create a seabios patch.

> On Tue, Mar 11, 2014 at 09:27:31AM -0400, Kevin O'Connor wrote:
> > I think it would be best to get the patch series to the point that
> > there is no diff between old and new.  That will make review easier,
> > and subsequent patches can then add new features.
> 
> Modulo the error info handle on type 17, and the fact that QEMU's version
> of type 17 has v2.3 fields and SeaBIOS's does not (kinda the whole reason
> I'm doing this in the first place :), that should be possible by just
> tweaking the defaults in the new smbios_set_defaults() function. I just
> feel weird setting "Bochs" as the default manufacturer... 

I would suggest being "bug for bug" compatible in the first set of
patches, and then add patches on top to add the additional
functionality.  Just my 2 cents.

-Kevin

Re: [Qemu-devel] [v2 PATCH 00/13] SMBIOS: build full tables in QEMU

[Gabriel L. Somlo]

On Tue, Mar 11, 2014 at 11:46:17AM -0400, Kevin O'Connor wrote:
> I would suggest being "bug for bug" compatible in the first set of
> patches, and then add patches on top to add the additional
> functionality.  Just my 2 cents.

With the patch enclosed at the end of this email, I can get it
down to this:

-Handle 0x0401, DMI type 4, 32 bytes
+Handle 0x0401, DMI type 4, 35 bytes
 Processor Information
        Socket Designation: CPU 1
        Type: Central Processor
        Family: Other
        Manufacturer: Bochs
        ID: FB 06 00 00 FF FB 8B 0F
        Version: Not Specified
        Voltage: Unknown
        External Clock: Unknown
        Max Speed: 2000 MHz
        Current Speed: 2000 MHz
        Status: Populated, Enabled
        Upgrade: Other
        L1 Cache Handle: Not Provided
        L2 Cache Handle: Not Provided
        L3 Cache Handle: Not Provided
+       Serial Number: Not Specified
+       Asset Tag: Not Specified
+       Part Number: Not Specified

and

-Handle 0x1100, DMI type 17, 21 bytes
+Handle 0x1100, DMI type 17, 27 bytes
 Memory Device
        Array Handle: 0x1000
-       Error Information Handle: 0x0000
+       Error Information Handle: Not Provided
        Total Width: 64 bits
        Data Width: 64 bits
        Size: 16384 MB
        Form Factor: DIMM
        Set: None
        Locator: DIMM 0
        Bank Locator: Not Specified
        Type: RAM
        Type Detail: None
+       Speed: Unknown
+       Manufacturer: QEMU
+       Serial Number: Not Specified
+       Asset Tag: Not Specified
+       Part Number: Not Specified


which is basically a predictable value for type 17 error info handle,
and v2.3 fields for types 4 and 17. When fixing the error info handle
in SeaBIOS, do you plan on using anything other than "Not Provided" ?

Re. v2.3 fields, in Seabios src/fw/smbios.c, lines 44 and 45, the
smbios major/minor version is set to 2.4, so I think having all types
be compliant with v2.4 is also important. That basically means bringing
4 and 17 in compliance with v2.3 :)

The QEMU smbios_set_defaults patch I used is below. I can add it as
#14 in the series, once we all get past the review for the other
items.

Thanks again,
--Gabriel


diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 64f570f..1112c11 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -615,6 +615,7 @@ void smbios_set_defaults(const char *manufacturer,
                          ram_addr_t below_4g_mem_size,
                          ram_addr_t above_4g_mem_size)
 {
+    const char *compat = "Bochs";
     smbios_have_defaults = true;
 
     assert(ram_size == below_4g_mem_size + above_4g_mem_size);
@@ -630,11 +631,11 @@ void smbios_set_defaults(const char *manufacturer,
     SMBIOS_SET_DEFAULT(type2.manufacturer, manufacturer);
     SMBIOS_SET_DEFAULT(type2.product, product);
     SMBIOS_SET_DEFAULT(type2.version, version);
-    SMBIOS_SET_DEFAULT(type3.manufacturer, manufacturer);
-    SMBIOS_SET_DEFAULT(type3.version, version);
+    SMBIOS_SET_DEFAULT(type3.manufacturer, compat);
+    /* SMBIOS_SET_DEFAULT(type3.version, version); */
     SMBIOS_SET_DEFAULT(type4.sock_pfx, "CPU");
-    SMBIOS_SET_DEFAULT(type4.manufacturer, manufacturer);
-    SMBIOS_SET_DEFAULT(type4.version, version);
+    SMBIOS_SET_DEFAULT(type4.manufacturer, compat);
+    /* SMBIOS_SET_DEFAULT(type4.version, version); */
     SMBIOS_SET_DEFAULT(type17.loc_pfx, "DIMM");
     SMBIOS_SET_DEFAULT(type17.manufacturer, manufacturer);
 }

Re: [Qemu-devel] [v2 PATCH 00/13] SMBIOS: build full tables in QEMU

[Gerd Hoffmann]

On Di, 2014-03-11 at 12:58 -0400, Gabriel L. Somlo wrote:
> On Tue, Mar 11, 2014 at 11:46:17AM -0400, Kevin O'Connor wrote:
> > I would suggest being "bug for bug" compatible in the first set of
> > patches, and then add patches on top to add the additional
> > functionality.  Just my 2 cents.

Makes sense indeed.

> With the patch enclosed at the end of this email, I can get it
> down to this:

Just move patch #1 to the end of the series.  So we first switch over
table by table, generating output identical to current seabios, then go
fix/improve things on top of that.  That way it'll be easier to bisect
problems in case any show up.

cheers,
  Gerd

[Qemu-devel] [v3 PATCH 00/13] SMBIOS: build full tables in QEMU

[Gabriel L. Somlo]

From: "Gabriel L. Somlo" <somlo@cmu.edu>

On Wed, Mar 12, 2014 at 09:20:54AM +0100, Gerd Hoffmann wrote:
> On Tue, Mar 11, 2014 at 11:46:17AM -0400, Kevin O'Connor wrote:
> > I would suggest being "bug for bug" compatible in the first set of
> > patches, and then add patches on top to add the additional
> > functionality.  Just my 2 cents.
> 
> Makes sense indeed.
> ...
> Just move patch #1 to the end of the series.  So we first switch over
> table by table, generating output identical to current seabios, then go
> fix/improve things on top of that.  That way it'll be easier to bisect
> problems in case any show up.

OK, after patch 12/13 we're fully 100% still compatible with SeaBIOS.
Then, patch 13/13 adds v2.3 compliance for types 4 and 17.

Should we add another patch (14) to remove the manufacturer string
compatibility hack ?

I'm replying separately to the e820 suggestion, as maintaining SeaBIOS
compatibility there is slightly trickier, and maybe we can decouple that
from this set of patches ?

Thanks,
--Gabriel

Gabriel L. Somlo (13):
  SMBIOS: Rename smbios_set_type1_defaults() for more general use
  SMBIOS: Use macro to set smbios defaults
  SMBIOS: Use bitmaps to check for smbios table collisions
  SMBIOS: Add code to build full smbios tables; build type 2 table
  SMBIOS: Build full tables for types 0 and 1
  SMBIOS: Remove unused code for passing individual fields to bios
  SMBIOS: Build full type 3 table
  SMBIOS: Build full type 4 tables
  SMBIOS: Build full smbios type 16 and 17 tables
  SMBIOS: Build full type 19 tables
  SMBIOS: Build full type 20 tables
  SMBIOS: Build full tables for type 32 and 127
  SMBIOS: Update all table definitions to smbios spec v2.3

 hw/i386/pc.c             |   3 +
 hw/i386/pc_piix.c        |  15 +-
 hw/i386/pc_q35.c         |  11 +-
 hw/i386/smbios.c         | 674 ++++++++++++++++++++++++++++++++++++++++-------
 include/hw/i386/smbios.h |  40 ++-
 5 files changed, 627 insertions(+), 116 deletions(-)

-- 
1.8.1.4

[Qemu-devel] [v3 PATCH 01/13] SMBIOS: Rename smbios_set_type1_defaults() for more general use

[Gabriel L. Somlo]

From: "Gabriel L. Somlo" <somlo@cmu.edu>

Subsequent patches will utilize this function to set defaults for
more smbios types than just type 1, so the function name should
reflect this.

Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
---
 hw/i386/pc_piix.c        | 12 ++++++------
 hw/i386/pc_q35.c         |  8 ++++----
 hw/i386/smbios.c         |  4 ++--
 include/hw/i386/smbios.h |  4 ++--
 4 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index ae1699d..ef2d062 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -60,7 +60,7 @@ static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
 
 static bool has_pci_info;
 static bool has_acpi_build = true;
-static bool smbios_type1_defaults = true;
+static bool smbios_defaults = true;
 /* Make sure that guest addresses aligned at 1Gbyte boundaries get mapped to
  * host addresses aligned at 1Gbyte boundaries.  This way we can use 1GByte
  * pages in the host.
@@ -143,9 +143,9 @@ static void pc_init1(QEMUMachineInitArgs *args,
     guest_info->has_pci_info = has_pci_info;
     guest_info->isapc_ram_fw = !pci_enabled;
 
-    if (smbios_type1_defaults) {
+    if (smbios_defaults) {
         /* These values are guest ABI, do not change */
-        smbios_set_type1_defaults("QEMU", "Standard PC (i440FX + PIIX, 1996)",
+        smbios_set_defaults("QEMU", "Standard PC (i440FX + PIIX, 1996)",
                                   args->machine->name);
     }
 
@@ -264,7 +264,7 @@ static void pc_init_pci(QEMUMachineInitArgs *args)
 
 static void pc_compat_1_7(QEMUMachineInitArgs *args)
 {
-    smbios_type1_defaults = false;
+    smbios_defaults = false;
     gigabyte_align = false;
 }
 
@@ -343,7 +343,7 @@ static void pc_init_pci_no_kvmclock(QEMUMachineInitArgs *args)
 {
     has_pci_info = false;
     has_acpi_build = false;
-    smbios_type1_defaults = false;
+    smbios_defaults = false;
     disable_kvm_pv_eoi();
     enable_compat_apic_id_mode();
     pc_init1(args, 1, 0);
@@ -353,7 +353,7 @@ static void pc_init_isa(QEMUMachineInitArgs *args)
 {
     has_pci_info = false;
     has_acpi_build = false;
-    smbios_type1_defaults = false;
+    smbios_defaults = false;
     if (!args->cpu_model) {
         args->cpu_model = "486";
     }
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index a7f6260..dfcc252 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -50,7 +50,7 @@
 
 static bool has_pci_info;
 static bool has_acpi_build = true;
-static bool smbios_type1_defaults = true;
+static bool smbios_defaults = true;
 /* Make sure that guest addresses aligned at 1Gbyte boundaries get mapped to
  * host addresses aligned at 1Gbyte boundaries.  This way we can use 1GByte
  * pages in the host.
@@ -130,9 +130,9 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
     guest_info->isapc_ram_fw = false;
     guest_info->has_acpi_build = has_acpi_build;
 
-    if (smbios_type1_defaults) {
+    if (smbios_defaults) {
         /* These values are guest ABI, do not change */
-        smbios_set_type1_defaults("QEMU", "Standard PC (Q35 + ICH9, 2009)",
+        smbios_set_defaults("QEMU", "Standard PC (Q35 + ICH9, 2009)",
                                   args->machine->name);
     }
 
@@ -242,7 +242,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
 
 static void pc_compat_1_7(QEMUMachineInitArgs *args)
 {
-    smbios_type1_defaults = false;
+    smbios_defaults = false;
     gigabyte_align = false;
 }
 
diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index e8f41ad..89dc070 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -256,8 +256,8 @@ static void smbios_build_type_1_fields(void)
     }
 }
 
-void smbios_set_type1_defaults(const char *manufacturer,
-                               const char *product, const char *version)
+void smbios_set_defaults(const char *manufacturer,
+                         const char *product, const char *version)
 {
     if (!type1.manufacturer) {
         type1.manufacturer = manufacturer;
diff --git a/include/hw/i386/smbios.h b/include/hw/i386/smbios.h
index 18fb970..e088aae 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -16,8 +16,8 @@
 #include "qemu/option.h"
 
 void smbios_entry_add(QemuOpts *opts);
-void smbios_set_type1_defaults(const char *manufacturer,
-                               const char *product, const char *version);
+void smbios_set_defaults(const char *manufacturer,
+                         const char *product, const char *version);
 uint8_t *smbios_get_table(size_t *length);
 
 /*
-- 
1.8.1.4

[Qemu-devel] [v3 PATCH 02/13] SMBIOS: Use macro to set smbios defaults

[Gabriel L. Somlo]

From: "Gabriel L. Somlo" <somlo@cmu.edu>

The function smbios_set_defaults() uses a repeating code pattern
for each field. This patch replaces that pattern with a macro.

Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
---
 hw/i386/smbios.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 89dc070..f4ee7b4 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -256,18 +256,17 @@ static void smbios_build_type_1_fields(void)
     }
 }
 
+#define SMBIOS_SET_DEFAULT(field, value)                                  \
+    if (!field) {                                                         \
+        field = value;                                                    \
+    }
+
 void smbios_set_defaults(const char *manufacturer,
                          const char *product, const char *version)
 {
-    if (!type1.manufacturer) {
-        type1.manufacturer = manufacturer;
-    }
-    if (!type1.product) {
-        type1.product = product;
-    }
-    if (!type1.version) {
-        type1.version = version;
-    }
+    SMBIOS_SET_DEFAULT(type1.manufacturer, manufacturer);
+    SMBIOS_SET_DEFAULT(type1.product, product);
+    SMBIOS_SET_DEFAULT(type1.version, version);
 }
 
 uint8_t *smbios_get_table(size_t *length)
-- 
1.8.1.4

[Qemu-devel] [v3 PATCH 03/13] SMBIOS: Use bitmaps to check for smbios table collisions

[Gabriel L. Somlo]

From: "Gabriel L. Somlo" <somlo@cmu.edu>

Replace existing smbios_check_collision() functionality with
a pair of bitmaps: have_binfile_bitmap and have_fields_bitmap.
Bits corresponding to each smbios type are set by smbios_entry_add(),
which also uses the bitmaps to ensure that binary blobs and field
values are never accepted for the same type.

These bitmaps will also be used in the future to decide whether
or not to build a full table for a given smbios type.

Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
---
 hw/i386/smbios.c         | 51 ++++++++++++++++++++----------------------------
 include/hw/i386/smbios.h |  2 ++
 2 files changed, 23 insertions(+), 30 deletions(-)

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index f4ee7b4..6889332 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -49,11 +49,8 @@ static size_t smbios_entries_len;
 static int smbios_type4_count = 0;
 static bool smbios_immutable;
 
-static struct {
-    bool seen;
-    int headertype;
-    Location loc;
-} first_opt[2];
+static DECLARE_BITMAP(have_binfile_bitmap, SMBIOS_MAX_TYPE+1);
+static DECLARE_BITMAP(have_fields_bitmap, SMBIOS_MAX_TYPE+1);
 
 static struct {
     const char *vendor, *version, *date;
@@ -164,29 +161,6 @@ static void smbios_validate_table(void)
     }
 }
 
-/*
- * To avoid unresolvable overlaps in data, don't allow both
- * tables and fields for the same smbios type.
- */
-static void smbios_check_collision(int type, int entry)
-{
-    if (type < ARRAY_SIZE(first_opt)) {
-        if (first_opt[type].seen) {
-            if (first_opt[type].headertype != entry) {
-                error_report("Can't mix file= and type= for same type");
-                loc_push_restore(&first_opt[type].loc);
-                error_report("This is the conflicting setting");
-                loc_pop(&first_opt[type].loc);
-                exit(1);
-            }
-        } else {
-            first_opt[type].seen = true;
-            first_opt[type].headertype = entry;
-            loc_save(&first_opt[type].loc);
-        }
-    }
-}
-
 static void smbios_add_field(int type, int offset, const void *data, size_t len)
 {
     struct smbios_field *field;
@@ -331,7 +305,14 @@ void smbios_entry_add(QemuOpts *opts)
         }
 
         header = (struct smbios_structure_header *)(table->data);
-        smbios_check_collision(header->type, SMBIOS_TABLE_ENTRY);
+
+        if (test_bit(header->type, have_fields_bitmap)) {
+            error_report("Can't add binary type %d table! "
+                         "(fields already specified)", header->type);
+            exit(1);
+        }
+        set_bit(header->type, have_binfile_bitmap);
+
         if (header->type == 4) {
             smbios_type4_count++;
         }
@@ -346,7 +327,17 @@ void smbios_entry_add(QemuOpts *opts)
     if (val) {
         unsigned long type = strtoul(val, NULL, 0);
 
-        smbios_check_collision(type, SMBIOS_FIELD_ENTRY);
+        if (type > SMBIOS_MAX_TYPE) {
+            error_report("smbios type (%ld) out of range!", type);
+            exit(1);
+        }
+
+        if (test_bit(type, have_binfile_bitmap)) {
+            error_report("Can't add fields for type %ld table! "
+                         "(binary file already loaded)", type);
+            exit(1);
+        }
+        set_bit(type, have_fields_bitmap);
 
         switch (type) {
         case 0:
diff --git a/include/hw/i386/smbios.h b/include/hw/i386/smbios.h
index e088aae..3425d40 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -15,6 +15,8 @@
 
 #include "qemu/option.h"
 
+#define SMBIOS_MAX_TYPE 127
+
 void smbios_entry_add(QemuOpts *opts);
 void smbios_set_defaults(const char *manufacturer,
                          const char *product, const char *version);
-- 
1.8.1.4

[Qemu-devel] [v3 PATCH 04/13] SMBIOS: Add code to build full smbios tables; build type 2 table

[Gabriel L. Somlo]

From: "Gabriel L. Somlo" <somlo@cmu.edu>

This patch adds a set of macros which build full smbios tables
of a given type, including the logic to decide whether a given
table type should be built or not.

To illustrate this new functionality, we introduce and optionally
build a table of type 2 (base board), which is required by some
versions of OS X (10.7 and 10.8).

Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
---
 hw/i386/smbios.c         | 158 +++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/i386/smbios.h |  18 +++++-
 2 files changed, 175 insertions(+), 1 deletion(-)

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 6889332..06f572d 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -48,6 +48,7 @@ static uint8_t *smbios_entries;
 static size_t smbios_entries_len;
 static int smbios_type4_count = 0;
 static bool smbios_immutable;
+static bool smbios_have_defaults;
 
 static DECLARE_BITMAP(have_binfile_bitmap, SMBIOS_MAX_TYPE+1);
 static DECLARE_BITMAP(have_fields_bitmap, SMBIOS_MAX_TYPE+1);
@@ -63,6 +64,10 @@ static struct {
     /* uuid is in qemu_uuid[] */
 } type1;
 
+static struct {
+    const char *manufacturer, *product, *version, *serial, *asset, *location;
+} type2;
+
 static QemuOptsList qemu_smbios_opts = {
     .name = "smbios",
     .head = QTAILQ_HEAD_INITIALIZER(qemu_smbios_opts.head),
@@ -146,6 +151,39 @@ static const QemuOptDesc qemu_smbios_type1_opts[] = {
     { /* end of list */ }
 };
 
+static const QemuOptDesc qemu_smbios_type2_opts[] = {
+    {
+        .name = "type",
+        .type = QEMU_OPT_NUMBER,
+        .help = "SMBIOS element type",
+    },{
+        .name = "manufacturer",
+        .type = QEMU_OPT_STRING,
+        .help = "manufacturer name",
+    },{
+        .name = "product",
+        .type = QEMU_OPT_STRING,
+        .help = "product name",
+    },{
+        .name = "version",
+        .type = QEMU_OPT_STRING,
+        .help = "version number",
+    },{
+        .name = "serial",
+        .type = QEMU_OPT_STRING,
+        .help = "serial number",
+    },{
+        .name = "asset",
+        .type = QEMU_OPT_STRING,
+        .help = "asset tag number",
+    },{
+        .name = "location",
+        .type = QEMU_OPT_STRING,
+        .help = "location in chassis",
+    },
+    { /* end of list */ }
+};
+
 static void smbios_register_config(void)
 {
     qemu_add_opts(&qemu_smbios_opts);
@@ -161,6 +199,90 @@ static void smbios_validate_table(void)
     }
 }
 
+static bool smbios_skip_table(uint8_t type, bool required_table)
+{
+    if (test_bit(type, have_binfile_bitmap)) {
+        return true; /* user provided their own binary blob(s) */
+    }
+    if (test_bit(type, have_fields_bitmap)) {
+        return false; /* user provided fields via command line */
+    }
+    if (smbios_have_defaults && required_table) {
+        return false; /* we're building tables, and this one's required */
+    }
+    return true;
+}
+
+#define SMBIOS_BUILD_TABLE_PRE(tbl_type, tbl_handle, tbl_required)        \
+    struct smbios_table *w;                                               \
+    struct smbios_type_##tbl_type *t;                                     \
+    size_t w_off, t_off; /* wrapper, table offsets into smbios_entries */ \
+    int str_index = 0;                                                    \
+    do {                                                                  \
+        /* should we skip building this table ? */                        \
+        if (smbios_skip_table(tbl_type, tbl_required)) {                  \
+            return;                                                       \
+        }                                                                 \
+                                                                          \
+        /* initialize fw_cfg smbios element count */                      \
+        if (!smbios_entries) {                                            \
+            smbios_entries_len = sizeof(uint16_t);                        \
+            smbios_entries = g_malloc0(smbios_entries_len);               \
+        }                                                                 \
+                                                                          \
+        /* use offsets of wrapper w and table t within smbios_entries  */ \
+        /* (pointers must be updated after each realloc)               */ \
+        w_off = smbios_entries_len;                                       \
+        t_off = w_off + sizeof(*w);                                       \
+        smbios_entries_len = t_off + sizeof(*t);                          \
+        smbios_entries = g_realloc(smbios_entries, smbios_entries_len);   \
+        w = (struct smbios_table *)(smbios_entries + w_off);              \
+        t = (struct smbios_type_##tbl_type *)(smbios_entries + t_off);    \
+                                                                          \
+        w->header.type = SMBIOS_TABLE_ENTRY;                              \
+        w->header.length = sizeof(*w) + sizeof(*t);                       \
+                                                                          \
+        t->header.type = tbl_type;                                        \
+        t->header.length = sizeof(*t);                                    \
+        t->header.handle = tbl_handle;                                    \
+    } while (0)
+
+#define SMBIOS_TABLE_SET_STR(tbl_type, field, value)                      \
+    do {                                                                  \
+        int len = (value != NULL) ? strlen(value) + 1 : 0;                \
+        if (len > 1) {                                                    \
+            smbios_entries = g_realloc(smbios_entries,                    \
+                                       smbios_entries_len + len);         \
+            memcpy(smbios_entries + smbios_entries_len, value, len);      \
+            smbios_entries_len += len;                                    \
+            /* update pointer(s) post-realloc */                          \
+            w = (struct smbios_table *)(smbios_entries + w_off);          \
+            t = (struct smbios_type_##tbl_type *)(smbios_entries + t_off);\
+            t->field = ++str_index;                                       \
+            w->header.length += len;                                      \
+        } else {                                                          \
+            t->field = 0;                                                 \
+        }                                                                 \
+    } while (0)
+
+#define SMBIOS_BUILD_TABLE_POST                                           \
+    do {                                                                  \
+        /* add empty string if no strings defined in table */             \
+        /* (NOTE: terminating \0 currently handled by fw_cfg/seabios) */  \
+        if (str_index == 0) {                                             \
+            smbios_entries = g_realloc(smbios_entries,                    \
+                                       smbios_entries_len + 1);           \
+            *(smbios_entries + smbios_entries_len) = 0;                   \
+            smbios_entries_len += 1;                                      \
+            /* update pointer(s) post-realloc */                          \
+            w = (struct smbios_table *)(smbios_entries + w_off);          \
+            w->header.length += 1;                                        \
+        }                                                                 \
+                                                                          \
+        /* update fw_cfg smbios element count */                          \
+        *(uint16_t *)smbios_entries += 1;                                 \
+    } while (0)
+
 static void smbios_add_field(int type, int offset, const void *data, size_t len)
 {
     struct smbios_field *field;
@@ -230,6 +352,24 @@ static void smbios_build_type_1_fields(void)
     }
 }
 
+static void smbios_build_type_2_table(void)
+{
+    SMBIOS_BUILD_TABLE_PRE(2, 0x200, false); /* optional */
+
+    SMBIOS_TABLE_SET_STR(2, manufacturer_str, type2.manufacturer);
+    SMBIOS_TABLE_SET_STR(2, product_str, type2.product);
+    SMBIOS_TABLE_SET_STR(2, version_str, type2.version);
+    SMBIOS_TABLE_SET_STR(2, serial_number_str, type2.serial);
+    SMBIOS_TABLE_SET_STR(2, asset_tag_number_str, type2.asset);
+    t->feature_flags = 0x01; /* Motherboard */
+    SMBIOS_TABLE_SET_STR(2, location_str, type2.location);
+    t->chassis_handle = 0x300; /* Type 3 (System enclosure) */
+    t->board_type = 0x0A; /* Motherboard */
+    t->contained_element_count = 0;
+
+    SMBIOS_BUILD_TABLE_POST;
+}
+
 #define SMBIOS_SET_DEFAULT(field, value)                                  \
     if (!field) {                                                         \
         field = value;                                                    \
@@ -238,14 +378,19 @@ static void smbios_build_type_1_fields(void)
 void smbios_set_defaults(const char *manufacturer,
                          const char *product, const char *version)
 {
+    smbios_have_defaults = true;
     SMBIOS_SET_DEFAULT(type1.manufacturer, manufacturer);
     SMBIOS_SET_DEFAULT(type1.product, product);
     SMBIOS_SET_DEFAULT(type1.version, version);
+    SMBIOS_SET_DEFAULT(type2.manufacturer, manufacturer);
+    SMBIOS_SET_DEFAULT(type2.product, product);
+    SMBIOS_SET_DEFAULT(type2.version, version);
 }
 
 uint8_t *smbios_get_table(size_t *length)
 {
     if (!smbios_immutable) {
+        smbios_build_type_2_table();
         smbios_build_type_0_fields();
         smbios_build_type_1_fields();
         smbios_validate_table();
@@ -381,6 +526,19 @@ void smbios_entry_add(QemuOpts *opts)
                 qemu_uuid_set = true;
             }
             return;
+        case 2:
+            qemu_opts_validate(opts, qemu_smbios_type2_opts, &local_err);
+            if (local_err) {
+                error_report("%s", error_get_pretty(local_err));
+                exit(1);
+            }
+            save_opt(&type2.manufacturer, opts, "manufacturer");
+            save_opt(&type2.product, opts, "product");
+            save_opt(&type2.version, opts, "version");
+            save_opt(&type2.serial, opts, "serial");
+            save_opt(&type2.asset, opts, "asset");
+            save_opt(&type2.location, opts, "location");
+            return;
         default:
             error_report("Don't know how to build fields for SMBIOS type %ld",
                          type);
diff --git a/include/hw/i386/smbios.h b/include/hw/i386/smbios.h
index 3425d40..2642e1a 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -62,6 +62,22 @@ struct smbios_type_1 {
     uint8_t family_str;
 } QEMU_PACKED;
 
+/* SMBIOS type 2 - Base Board */
+struct smbios_type_2 {
+    struct smbios_structure_header header;
+    uint8_t manufacturer_str;
+    uint8_t product_str;
+    uint8_t version_str;
+    uint8_t serial_number_str;
+    uint8_t asset_tag_number_str;
+    uint8_t feature_flags;
+    uint8_t location_str;
+    uint16_t chassis_handle;
+    uint8_t board_type;
+    uint8_t contained_element_count;
+    /* contained elements follow */
+} QEMU_PACKED;
+
 /* SMBIOS type 3 - System Enclosure (v2.3) */
 struct smbios_type_3 {
     struct smbios_structure_header header;
@@ -78,7 +94,7 @@ struct smbios_type_3 {
     uint8_t height;
     uint8_t number_of_power_cords;
     uint8_t contained_element_count;
-    // contained elements follow
+    /* contained elements follow */
 } QEMU_PACKED;
 
 /* SMBIOS type 4 - Processor Information (v2.0) */
-- 
1.8.1.4

[Qemu-devel] [v3 PATCH 05/13] SMBIOS: Build full tables for types 0 and 1

[Gabriel L. Somlo]

From: "Gabriel L. Somlo" <somlo@cmu.edu>

Build full tables for types 0 (bios information) and 1 (system
information). Type 0 is optional, and a table will only be built
if requested via the command line; the default is to leave type 0
tables up to the bios itself.

Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
---
 hw/i386/smbios.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 06f572d..b00a367 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -352,6 +352,62 @@ static void smbios_build_type_1_fields(void)
     }
 }
 
+static void smbios_build_type_0_table(void)
+{
+    SMBIOS_BUILD_TABLE_PRE(0, 0x000, false); /* optional, leave up to BIOS */
+
+    SMBIOS_TABLE_SET_STR(0, vendor_str, type0.vendor);
+    SMBIOS_TABLE_SET_STR(0, bios_version_str, type0.version);
+
+    t->bios_starting_address_segment = 0xE800; /* hardcoded in SeaBIOS */
+
+    SMBIOS_TABLE_SET_STR(0, bios_release_date_str, type0.date);
+
+    t->bios_rom_size = 0; /* hardcoded in SeaBIOS with FIXME comment */
+
+    /* BIOS characteristics not supported */
+    memset(t->bios_characteristics, 0, 8);
+    t->bios_characteristics[0] = 0x08;
+
+    /* Enable targeted content distribution (needed for SVVP, per SeaBIOS) */
+    t->bios_characteristics_extension_bytes[0] = 0;
+    t->bios_characteristics_extension_bytes[1] = 4;
+
+    if (type0.have_major_minor) {
+        t->system_bios_major_release = type0.major;
+        t->system_bios_minor_release = type0.minor;
+    } else {
+        t->system_bios_major_release = 0;
+        t->system_bios_minor_release = 0;
+    }
+
+    /* hardcoded in SeaBIOS */
+    t->embedded_controller_major_release = 0xFF;
+    t->embedded_controller_minor_release = 0xFF;
+
+    SMBIOS_BUILD_TABLE_POST;
+}
+
+static void smbios_build_type_1_table(void)
+{
+    SMBIOS_BUILD_TABLE_PRE(1, 0x100, true); /* required */
+
+    SMBIOS_TABLE_SET_STR(1, manufacturer_str, type1.manufacturer);
+    SMBIOS_TABLE_SET_STR(1, product_name_str, type1.product);
+    SMBIOS_TABLE_SET_STR(1, version_str, type1.version);
+    SMBIOS_TABLE_SET_STR(1, serial_number_str, type1.serial);
+    if (qemu_uuid_set) {
+        memcpy(t->uuid, qemu_uuid, 16);
+    } else {
+        memset(t->uuid, 0, 16);
+    }
+    t->wake_up_type = 0x06; /* power switch */
+    SMBIOS_TABLE_SET_STR(1, sku_number_str, type1.sku);
+    SMBIOS_TABLE_SET_STR(1, family_str, type1.family);
+
+    SMBIOS_BUILD_TABLE_POST;
+}
+
 static void smbios_build_type_2_table(void)
 {
     SMBIOS_BUILD_TABLE_PRE(2, 0x200, false); /* optional */
@@ -379,6 +435,9 @@ void smbios_set_defaults(const char *manufacturer,
                          const char *product, const char *version)
 {
     smbios_have_defaults = true;
+    SMBIOS_SET_DEFAULT(type0.vendor, manufacturer);
+    SMBIOS_SET_DEFAULT(type0.version, version);
+    SMBIOS_SET_DEFAULT(type0.date, "01/01/2014");
     SMBIOS_SET_DEFAULT(type1.manufacturer, manufacturer);
     SMBIOS_SET_DEFAULT(type1.product, product);
     SMBIOS_SET_DEFAULT(type1.version, version);
@@ -390,9 +449,13 @@ void smbios_set_defaults(const char *manufacturer,
 uint8_t *smbios_get_table(size_t *length)
 {
     if (!smbios_immutable) {
+        smbios_build_type_0_table();
+        smbios_build_type_1_table();
         smbios_build_type_2_table();
+if (false) { /* shut up gcc until we remove deprecated code */
         smbios_build_type_0_fields();
         smbios_build_type_1_fields();
+}
         smbios_validate_table();
         smbios_immutable = true;
     }
-- 
1.8.1.4

[Qemu-devel] [v3 PATCH 06/13] SMBIOS: Remove unused code for passing individual fields to bios

[Gabriel L. Somlo]

From: "Gabriel L. Somlo" <somlo@cmu.edu>

This patch removes smbios_add_field() and the old code to insert
individual fields for types 0 and 1 into fw_cfg.

Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
---
 hw/i386/smbios.c | 80 --------------------------------------------------------
 1 file changed, 80 deletions(-)

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index b00a367..4584774 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -29,13 +29,6 @@ struct smbios_header {
     uint8_t type;
 } QEMU_PACKED;
 
-struct smbios_field {
-    struct smbios_header header;
-    uint8_t type;
-    uint16_t offset;
-    uint8_t data[];
-} QEMU_PACKED;
-
 struct smbios_table {
     struct smbios_header header;
     uint8_t data[];
@@ -283,75 +276,6 @@ static bool smbios_skip_table(uint8_t type, bool required_table)
         *(uint16_t *)smbios_entries += 1;                                 \
     } while (0)
 
-static void smbios_add_field(int type, int offset, const void *data, size_t len)
-{
-    struct smbios_field *field;
-
-    if (!smbios_entries) {
-        smbios_entries_len = sizeof(uint16_t);
-        smbios_entries = g_malloc0(smbios_entries_len);
-    }
-    smbios_entries = g_realloc(smbios_entries, smbios_entries_len +
-                                                  sizeof(*field) + len);
-    field = (struct smbios_field *)(smbios_entries + smbios_entries_len);
-    field->header.type = SMBIOS_FIELD_ENTRY;
-    field->header.length = cpu_to_le16(sizeof(*field) + len);
-
-    field->type = type;
-    field->offset = cpu_to_le16(offset);
-    memcpy(field->data, data, len);
-
-    smbios_entries_len += sizeof(*field) + len;
-    (*(uint16_t *)smbios_entries) =
-            cpu_to_le16(le16_to_cpu(*(uint16_t *)smbios_entries) + 1);
-}
-
-static void smbios_maybe_add_str(int type, int offset, const char *data)
-{
-    if (data) {
-        smbios_add_field(type, offset, data, strlen(data) + 1);
-    }
-}
-
-static void smbios_build_type_0_fields(void)
-{
-    smbios_maybe_add_str(0, offsetof(struct smbios_type_0, vendor_str),
-                         type0.vendor);
-    smbios_maybe_add_str(0, offsetof(struct smbios_type_0, bios_version_str),
-                         type0.version);
-    smbios_maybe_add_str(0, offsetof(struct smbios_type_0,
-                                     bios_release_date_str),
-                         type0.date);
-    if (type0.have_major_minor) {
-        smbios_add_field(0, offsetof(struct smbios_type_0,
-                                     system_bios_major_release),
-                         &type0.major, 1);
-        smbios_add_field(0, offsetof(struct smbios_type_0,
-                                     system_bios_minor_release),
-                         &type0.minor, 1);
-    }
-}
-
-static void smbios_build_type_1_fields(void)
-{
-    smbios_maybe_add_str(1, offsetof(struct smbios_type_1, manufacturer_str),
-                         type1.manufacturer);
-    smbios_maybe_add_str(1, offsetof(struct smbios_type_1, product_name_str),
-                         type1.product);
-    smbios_maybe_add_str(1, offsetof(struct smbios_type_1, version_str),
-                         type1.version);
-    smbios_maybe_add_str(1, offsetof(struct smbios_type_1, serial_number_str),
-                         type1.serial);
-    smbios_maybe_add_str(1, offsetof(struct smbios_type_1, sku_number_str),
-                         type1.sku);
-    smbios_maybe_add_str(1, offsetof(struct smbios_type_1, family_str),
-                         type1.family);
-    if (qemu_uuid_set) {
-        smbios_add_field(1, offsetof(struct smbios_type_1, uuid),
-                         qemu_uuid, 16);
-    }
-}
-
 static void smbios_build_type_0_table(void)
 {
     SMBIOS_BUILD_TABLE_PRE(0, 0x000, false); /* optional, leave up to BIOS */
@@ -452,10 +376,6 @@ uint8_t *smbios_get_table(size_t *length)
         smbios_build_type_0_table();
         smbios_build_type_1_table();
         smbios_build_type_2_table();
-if (false) { /* shut up gcc until we remove deprecated code */
-        smbios_build_type_0_fields();
-        smbios_build_type_1_fields();
-}
         smbios_validate_table();
         smbios_immutable = true;
     }
-- 
1.8.1.4

[Qemu-devel] [v3 PATCH 07/13] SMBIOS: Build full type 3 table

[Gabriel L. Somlo]

From: "Gabriel L. Somlo" <somlo@cmu.edu>

Build smbios type 3 (system enclosure) table, and make it available
to the bios via fw_cfg. For initial compatibility with SeaBIOS, use
"Bochs" as the default manufacturer string, and leave version unset.

Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
---
 hw/i386/smbios.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 67 insertions(+)

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 4584774..47f7b0d 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -61,6 +61,10 @@ static struct {
     const char *manufacturer, *product, *version, *serial, *asset, *location;
 } type2;
 
+static struct {
+    const char *manufacturer, *version, *serial, *asset;
+} type3;
+
 static QemuOptsList qemu_smbios_opts = {
     .name = "smbios",
     .head = QTAILQ_HEAD_INITIALIZER(qemu_smbios_opts.head),
@@ -177,6 +181,31 @@ static const QemuOptDesc qemu_smbios_type2_opts[] = {
     { /* end of list */ }
 };
 
+static const QemuOptDesc qemu_smbios_type3_opts[] = {
+    {
+        .name = "type",
+        .type = QEMU_OPT_NUMBER,
+        .help = "SMBIOS element type",
+    },{
+        .name = "manufacturer",
+        .type = QEMU_OPT_STRING,
+        .help = "manufacturer name",
+    },{
+        .name = "version",
+        .type = QEMU_OPT_STRING,
+        .help = "version number",
+    },{
+        .name = "serial",
+        .type = QEMU_OPT_STRING,
+        .help = "serial number",
+    },{
+        .name = "asset",
+        .type = QEMU_OPT_STRING,
+        .help = "asset tag number",
+    },
+    { /* end of list */ }
+};
+
 static void smbios_register_config(void)
 {
     qemu_add_opts(&qemu_smbios_opts);
@@ -350,6 +379,27 @@ static void smbios_build_type_2_table(void)
     SMBIOS_BUILD_TABLE_POST;
 }
 
+static void smbios_build_type_3_table(void)
+{
+    SMBIOS_BUILD_TABLE_PRE(3, 0x300, true); /* required */
+
+    SMBIOS_TABLE_SET_STR(3, manufacturer_str, type3.manufacturer);
+    t->type = 0x01; /* Other */
+    SMBIOS_TABLE_SET_STR(3, version_str, type3.version);
+    SMBIOS_TABLE_SET_STR(3, serial_number_str, type3.serial);
+    SMBIOS_TABLE_SET_STR(3, asset_tag_number_str, type3.asset);
+    t->boot_up_state = 0x03; /* Safe */
+    t->power_supply_state = 0x03; /* Safe */
+    t->thermal_state = 0x03; /* Safe */
+    t->security_status = 0x02; /* Unknown */
+    t->oem_defined = 0;
+    t->height = 0;
+    t->number_of_power_cords = 0;
+    t->contained_element_count = 0;
+
+    SMBIOS_BUILD_TABLE_POST;
+}
+
 #define SMBIOS_SET_DEFAULT(field, value)                                  \
     if (!field) {                                                         \
         field = value;                                                    \
@@ -358,6 +408,7 @@ static void smbios_build_type_2_table(void)
 void smbios_set_defaults(const char *manufacturer,
                          const char *product, const char *version)
 {
+    const char *manufacturer_compat = "Bochs"; /* SeaBIOS compatibility */
     smbios_have_defaults = true;
     SMBIOS_SET_DEFAULT(type0.vendor, manufacturer);
     SMBIOS_SET_DEFAULT(type0.version, version);
@@ -368,6 +419,10 @@ void smbios_set_defaults(const char *manufacturer,
     SMBIOS_SET_DEFAULT(type2.manufacturer, manufacturer);
     SMBIOS_SET_DEFAULT(type2.product, product);
     SMBIOS_SET_DEFAULT(type2.version, version);
+    SMBIOS_SET_DEFAULT(type3.manufacturer, manufacturer_compat);
+    /* not set in SeaBIOS
+    SMBIOS_SET_DEFAULT(type3.version, version);
+    */
 }
 
 uint8_t *smbios_get_table(size_t *length)
@@ -376,6 +431,7 @@ uint8_t *smbios_get_table(size_t *length)
         smbios_build_type_0_table();
         smbios_build_type_1_table();
         smbios_build_type_2_table();
+        smbios_build_type_3_table();
         smbios_validate_table();
         smbios_immutable = true;
     }
@@ -522,6 +578,17 @@ void smbios_entry_add(QemuOpts *opts)
             save_opt(&type2.asset, opts, "asset");
             save_opt(&type2.location, opts, "location");
             return;
+        case 3:
+            qemu_opts_validate(opts, qemu_smbios_type3_opts, &local_err);
+            if (local_err) {
+                error_report("%s", error_get_pretty(local_err));
+                exit(1);
+            }
+            save_opt(&type3.manufacturer, opts, "manufacturer");
+            save_opt(&type3.version, opts, "version");
+            save_opt(&type3.serial, opts, "serial");
+            save_opt(&type3.asset, opts, "asset");
+            return;
         default:
             error_report("Don't know how to build fields for SMBIOS type %ld",
                          type);
-- 
1.8.1.4

[Qemu-devel] [v3 PATCH 08/13] SMBIOS: Build full type 4 tables

[Gabriel L. Somlo]

From: "Gabriel L. Somlo" <somlo@cmu.edu>

Build full smbios type 4 (processor information) tables, and make
them available to the bios via fw_cfg. For initial compatibility
with SeaBIOS, use "Bochs" as the default manufacturer string, and
leave version unset.

Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
---
 hw/i386/pc.c             |  3 ++
 hw/i386/smbios.c         | 96 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/i386/smbios.h |  1 +
 3 files changed, 100 insertions(+)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index e715a33..9c4623e 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1011,6 +1011,9 @@ void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
         sysbus_mmio_map_overlap(SYS_BUS_DEVICE(icc_bridge), 0,
                                 APIC_DEFAULT_ADDRESS, 0x1000);
     }
+
+    /* tell smbios about cpuid version and features */
+    smbios_set_cpuid(cpu->env.cpuid_version, cpu->env.features[FEAT_1_EDX]);
 }
 
 /* pci-info ROM file. Little endian format */
diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 47f7b0d..fc8b3ef 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -42,6 +42,7 @@ static size_t smbios_entries_len;
 static int smbios_type4_count = 0;
 static bool smbios_immutable;
 static bool smbios_have_defaults;
+static uint32_t smbios_cpuid_version, smbios_cpuid_features; /* for type 4 */
 
 static DECLARE_BITMAP(have_binfile_bitmap, SMBIOS_MAX_TYPE+1);
 static DECLARE_BITMAP(have_fields_bitmap, SMBIOS_MAX_TYPE+1);
@@ -65,6 +66,10 @@ static struct {
     const char *manufacturer, *version, *serial, *asset;
 } type3;
 
+static struct {
+    const char *sock_pfx, *manufacturer, *version, *serial, *asset, *part;
+} type4;
+
 static QemuOptsList qemu_smbios_opts = {
     .name = "smbios",
     .head = QTAILQ_HEAD_INITIALIZER(qemu_smbios_opts.head),
@@ -206,6 +211,39 @@ static const QemuOptDesc qemu_smbios_type3_opts[] = {
     { /* end of list */ }
 };
 
+static const QemuOptDesc qemu_smbios_type4_opts[] = {
+    {
+        .name = "type",
+        .type = QEMU_OPT_NUMBER,
+        .help = "SMBIOS element type",
+    },{
+        .name = "sock_pfx",
+        .type = QEMU_OPT_STRING,
+        .help = "socket designation string prefix",
+    },{
+        .name = "manufacturer",
+        .type = QEMU_OPT_STRING,
+        .help = "manufacturer name",
+    },{
+        .name = "version",
+        .type = QEMU_OPT_STRING,
+        .help = "version number",
+    },{
+        .name = "serial",
+        .type = QEMU_OPT_STRING,
+        .help = "serial number",
+    },{
+        .name = "asset",
+        .type = QEMU_OPT_STRING,
+        .help = "asset tag number",
+    },{
+        .name = "part",
+        .type = QEMU_OPT_STRING,
+        .help = "part number",
+    },
+    { /* end of list */ }
+};
+
 static void smbios_register_config(void)
 {
     qemu_add_opts(&qemu_smbios_opts);
@@ -400,11 +438,45 @@ static void smbios_build_type_3_table(void)
     SMBIOS_BUILD_TABLE_POST;
 }
 
+static void smbios_build_type_4_table(unsigned instance)
+{
+    char sock_str[128];
+
+    SMBIOS_BUILD_TABLE_PRE(4, 0x400 + instance, true); /* required */
+
+    snprintf(sock_str, sizeof(sock_str), "%s%2x", type4.sock_pfx, instance);
+    SMBIOS_TABLE_SET_STR(4, socket_designation_str, sock_str);
+    t->processor_type = 0x03; /* CPU */
+    t->processor_family = 0x01; /* Other */
+    SMBIOS_TABLE_SET_STR(4, processor_manufacturer_str, type4.manufacturer);
+    t->processor_id[0] = smbios_cpuid_version;
+    t->processor_id[1] = smbios_cpuid_features;
+    SMBIOS_TABLE_SET_STR(4, processor_version_str, type4.version);
+    t->voltage = 0;
+    t->external_clock = 0;
+    t->max_speed = 2000; /* hardcoded in SeaBIOS (use 0/Unknown instead ?) */
+    t->current_speed = 2000; /* hardcoded in SeaBIOS (use 0/Unknown ?) */
+    t->status = 0x41; /* Socket populated, CPU enabled */
+    t->processor_upgrade = 0x01; /* Other */
+    t->l1_cache_handle = 0xFFFF; /* N/A */
+    t->l2_cache_handle = 0xFFFF; /* N/A */
+    t->l3_cache_handle = 0xFFFF; /* N/A */
+
+    SMBIOS_BUILD_TABLE_POST;
+    smbios_type4_count++;
+}
+
 #define SMBIOS_SET_DEFAULT(field, value)                                  \
     if (!field) {                                                         \
         field = value;                                                    \
     }
 
+void smbios_set_cpuid(uint32_t version, uint32_t features)
+{
+    smbios_cpuid_version = version;
+    smbios_cpuid_features = features;
+}
+
 void smbios_set_defaults(const char *manufacturer,
                          const char *product, const char *version)
 {
@@ -423,15 +495,26 @@ void smbios_set_defaults(const char *manufacturer,
     /* not set in SeaBIOS
     SMBIOS_SET_DEFAULT(type3.version, version);
     */
+    SMBIOS_SET_DEFAULT(type4.sock_pfx, "CPU");
+    SMBIOS_SET_DEFAULT(type4.manufacturer, manufacturer_compat);
+    /* not set in SeaBIOS
+    SMBIOS_SET_DEFAULT(type4.version, version);
+    */
 }
 
 uint8_t *smbios_get_table(size_t *length)
 {
+    unsigned i;
+
     if (!smbios_immutable) {
         smbios_build_type_0_table();
         smbios_build_type_1_table();
         smbios_build_type_2_table();
         smbios_build_type_3_table();
+        for (i = 0; i < smp_cpus; i++) {
+            /* count CPUs starting with 1, to minimize diff vs. SeaBIOS */
+            smbios_build_type_4_table(i + 1);
+        }
         smbios_validate_table();
         smbios_immutable = true;
     }
@@ -589,6 +672,19 @@ void smbios_entry_add(QemuOpts *opts)
             save_opt(&type3.serial, opts, "serial");
             save_opt(&type3.asset, opts, "asset");
             return;
+        case 4:
+            qemu_opts_validate(opts, qemu_smbios_type4_opts, &local_err);
+            if (local_err) {
+                error_report("%s", error_get_pretty(local_err));
+                exit(1);
+            }
+            save_opt(&type4.sock_pfx, opts, "sock_pfx");
+            save_opt(&type4.manufacturer, opts, "manufacturer");
+            save_opt(&type4.version, opts, "version");
+            save_opt(&type4.serial, opts, "serial");
+            save_opt(&type4.asset, opts, "asset");
+            save_opt(&type4.part, opts, "part");
+            return;
         default:
             error_report("Don't know how to build fields for SMBIOS type %ld",
                          type);
diff --git a/include/hw/i386/smbios.h b/include/hw/i386/smbios.h
index 2642e1a..af5ee01 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -18,6 +18,7 @@
 #define SMBIOS_MAX_TYPE 127
 
 void smbios_entry_add(QemuOpts *opts);
+void smbios_set_cpuid(uint32_t version, uint32_t features);
 void smbios_set_defaults(const char *manufacturer,
                          const char *product, const char *version);
 uint8_t *smbios_get_table(size_t *length);
-- 
1.8.1.4

[Qemu-devel] [v3 PATCH 09/13] SMBIOS: Build full smbios type 16 and 17 tables

[Gabriel L. Somlo]

From: "Gabriel L. Somlo" <somlo@cmu.edu>

Build full smbios type 16 (physical memory array) and
type 17 (memory device) tables, and make them available
to the bios via fw_cfg.

Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
---
 hw/i386/smbios.c | 103 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 102 insertions(+), 1 deletion(-)

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index fc8b3ef..58bef22 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -70,6 +70,10 @@ static struct {
     const char *sock_pfx, *manufacturer, *version, *serial, *asset, *part;
 } type4;
 
+static struct {
+    const char *loc_pfx, *bank, *manufacturer, *serial, *asset, *part;
+} type17;
+
 static QemuOptsList qemu_smbios_opts = {
     .name = "smbios",
     .head = QTAILQ_HEAD_INITIALIZER(qemu_smbios_opts.head),
@@ -244,6 +248,39 @@ static const QemuOptDesc qemu_smbios_type4_opts[] = {
     { /* end of list */ }
 };
 
+static const QemuOptDesc qemu_smbios_type17_opts[] = {
+    {
+        .name = "type",
+        .type = QEMU_OPT_NUMBER,
+        .help = "SMBIOS element type",
+    },{
+        .name = "loc_pfx",
+        .type = QEMU_OPT_STRING,
+        .help = "device locator string prefix",
+    },{
+        .name = "bank",
+        .type = QEMU_OPT_STRING,
+        .help = "bank locator string",
+    },{
+        .name = "manufacturer",
+        .type = QEMU_OPT_STRING,
+        .help = "manufacturer name",
+    },{
+        .name = "serial",
+        .type = QEMU_OPT_STRING,
+        .help = "serial number",
+    },{
+        .name = "asset",
+        .type = QEMU_OPT_STRING,
+        .help = "asset tag number",
+    },{
+        .name = "part",
+        .type = QEMU_OPT_STRING,
+        .help = "part number",
+    },
+    { /* end of list */ }
+};
+
 static void smbios_register_config(void)
 {
     qemu_add_opts(&qemu_smbios_opts);
@@ -466,6 +503,47 @@ static void smbios_build_type_4_table(unsigned instance)
     smbios_type4_count++;
 }
 
+static void smbios_build_type_16_table(unsigned memdev_count)
+{
+    unsigned ram_size_kb = ram_size >> 10;
+
+    SMBIOS_BUILD_TABLE_PRE(16, 0x1000, true); /* required */
+
+    t->location = 0x01; /* Other */
+    t->use = 0x03; /* System memory */
+    t->error_correction = 0x06; /* Multi-bit ECC (for Microsoft, per SeaBIOS) */
+    /* if ram_size < 2T, use value in Kilobytes; 0x80000000 == 2T and over;
+     * TODO: support smbios v2.7 extended capacity, or multiple arrays. */
+    t->maximum_capacity = (ram_size_kb < 0x80000000) ? ram_size_kb : 0x80000000;
+    t->memory_error_information_handle = 0xFFFE; /* Not provided */
+    t->number_of_memory_devices = memdev_count;
+
+    SMBIOS_BUILD_TABLE_POST;
+}
+
+static void smbios_build_type_17_table(unsigned instance, unsigned size_mb)
+{
+    char loc_str[128];
+
+    SMBIOS_BUILD_TABLE_PRE(17, 0x1100 + instance, true); /* required */
+
+    t->physical_memory_array_handle = 0x1000; /* Type 16 (Phys. Mem. Array) */
+    t->memory_error_information_handle = 0; /* SeaBIOS, should be 0xFFFE(N/A) */
+    t->total_width = 64; /* hardcoded in SeaBIOS */
+    t->data_width = 64; /* hardcoded in SeaBIOS */
+    assert(size_mb <= 0x7FFF);
+    t->size = size_mb;
+    t->form_factor = 0x09; /* DIMM */
+    t->device_set = 0; /* Not in a set */
+    snprintf(loc_str, sizeof(loc_str), "%s %d", type17.loc_pfx, instance);
+    SMBIOS_TABLE_SET_STR(17, device_locator_str, loc_str);
+    SMBIOS_TABLE_SET_STR(17, bank_locator_str, type17.bank);
+    t->memory_type = 0x07; /* RAM */
+    t->type_detail = 0; /* hardcoded in SeaBIOS */
+
+    SMBIOS_BUILD_TABLE_POST;
+}
+
 #define SMBIOS_SET_DEFAULT(field, value)                                  \
     if (!field) {                                                         \
         field = value;                                                    \
@@ -500,11 +578,12 @@ void smbios_set_defaults(const char *manufacturer,
     /* not set in SeaBIOS
     SMBIOS_SET_DEFAULT(type4.version, version);
     */
+    SMBIOS_SET_DEFAULT(type17.loc_pfx, "DIMM");
 }
 
 uint8_t *smbios_get_table(size_t *length)
 {
-    unsigned i;
+    unsigned i, ram_size_mb, memdev_count;
 
     if (!smbios_immutable) {
         smbios_build_type_0_table();
@@ -515,6 +594,15 @@ uint8_t *smbios_get_table(size_t *length)
             /* count CPUs starting with 1, to minimize diff vs. SeaBIOS */
             smbios_build_type_4_table(i + 1);
         }
+        ram_size_mb = ram_size >> 20;
+        /* up to 16GB (0x4000MB) per memory device: */
+        memdev_count = (ram_size_mb + 0x3FFF) >> 14;
+        smbios_build_type_16_table(memdev_count);
+        for (i = 0; i < memdev_count; i++) {
+            uint32_t size_mb = (i == memdev_count - 1) ?
+                               ((ram_size_mb - 1) & 0x3FFF) + 1 : 0x4000;
+            smbios_build_type_17_table(i, size_mb);
+        }
         smbios_validate_table();
         smbios_immutable = true;
     }
@@ -685,6 +773,19 @@ void smbios_entry_add(QemuOpts *opts)
             save_opt(&type4.asset, opts, "asset");
             save_opt(&type4.part, opts, "part");
             return;
+        case 17:
+            qemu_opts_validate(opts, qemu_smbios_type17_opts, &local_err);
+            if (local_err) {
+                error_report("%s", error_get_pretty(local_err));
+                exit(1);
+            }
+            save_opt(&type17.loc_pfx, opts, "loc_pfx");
+            save_opt(&type17.bank, opts, "bank");
+            save_opt(&type17.manufacturer, opts, "manufacturer");
+            save_opt(&type17.serial, opts, "serial");
+            save_opt(&type17.asset, opts, "asset");
+            save_opt(&type17.part, opts, "part");
+            return;
         default:
             error_report("Don't know how to build fields for SMBIOS type %ld",
                          type);
-- 
1.8.1.4

[Qemu-devel] [v3 PATCH 10/13] SMBIOS: Build full type 19 tables

[Gabriel L. Somlo]

From: "Gabriel L. Somlo" <somlo@cmu.edu>

Build full smbios type 19 (memory array mapped address) tables,
and make them available via fw_cfg

Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
---
 hw/i386/pc_piix.c        |  3 ++-
 hw/i386/pc_q35.c         |  3 ++-
 hw/i386/smbios.c         | 27 ++++++++++++++++++++++++++-
 include/hw/i386/smbios.h |  4 +++-
 4 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index ef2d062..5950d7f 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -146,7 +146,8 @@ static void pc_init1(QEMUMachineInitArgs *args,
     if (smbios_defaults) {
         /* These values are guest ABI, do not change */
         smbios_set_defaults("QEMU", "Standard PC (i440FX + PIIX, 1996)",
-                                  args->machine->name);
+                             args->machine->name,
+                             below_4g_mem_size, above_4g_mem_size);
     }
 
     /* allocate ram and load rom/bios */
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index dfcc252..482f466 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -133,7 +133,8 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
     if (smbios_defaults) {
         /* These values are guest ABI, do not change */
         smbios_set_defaults("QEMU", "Standard PC (Q35 + ICH9, 2009)",
-                                  args->machine->name);
+                             args->machine->name,
+                             below_4g_mem_size, above_4g_mem_size);
     }
 
     /* allocate ram and load rom/bios */
diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 58bef22..2560e17 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -43,6 +43,7 @@ static int smbios_type4_count = 0;
 static bool smbios_immutable;
 static bool smbios_have_defaults;
 static uint32_t smbios_cpuid_version, smbios_cpuid_features; /* for type 4 */
+static ram_addr_t smbios_below_4g_ram, smbios_above_4g_ram; /* for type 19 */
 
 static DECLARE_BITMAP(have_binfile_bitmap, SMBIOS_MAX_TYPE+1);
 static DECLARE_BITMAP(have_fields_bitmap, SMBIOS_MAX_TYPE+1);
@@ -544,6 +545,19 @@ static void smbios_build_type_17_table(unsigned instance, unsigned size_mb)
     SMBIOS_BUILD_TABLE_POST;
 }
 
+static void smbios_build_type_19_table(unsigned instance,
+                                       unsigned start_kb, unsigned size_kb)
+{
+    SMBIOS_BUILD_TABLE_PRE(19, 0x1300 + instance, true); /* required */
+
+    t->starting_address = start_kb;
+    t->ending_address = start_kb + size_kb - 1;
+    t->memory_array_handle = 0x1000; /* Type 16 (Phys. Mem. Array) */
+    t->partition_width = 1; /* hardcoded in SeaBIOS */
+
+    SMBIOS_BUILD_TABLE_POST;
+}
+
 #define SMBIOS_SET_DEFAULT(field, value)                                  \
     if (!field) {                                                         \
         field = value;                                                    \
@@ -556,10 +570,17 @@ void smbios_set_cpuid(uint32_t version, uint32_t features)
 }
 
 void smbios_set_defaults(const char *manufacturer,
-                         const char *product, const char *version)
+                         const char *product, const char *version,
+                         ram_addr_t below_4g_mem_size,
+                         ram_addr_t above_4g_mem_size)
 {
     const char *manufacturer_compat = "Bochs"; /* SeaBIOS compatibility */
     smbios_have_defaults = true;
+
+    assert(ram_size == below_4g_mem_size + above_4g_mem_size);
+    smbios_below_4g_ram = below_4g_mem_size;
+    smbios_above_4g_ram = above_4g_mem_size;
+
     SMBIOS_SET_DEFAULT(type0.vendor, manufacturer);
     SMBIOS_SET_DEFAULT(type0.version, version);
     SMBIOS_SET_DEFAULT(type0.date, "01/01/2014");
@@ -603,6 +624,10 @@ uint8_t *smbios_get_table(size_t *length)
                                ((ram_size_mb - 1) & 0x3FFF) + 1 : 0x4000;
             smbios_build_type_17_table(i, size_mb);
         }
+        smbios_build_type_19_table(0, 0, smbios_below_4g_ram >> 10);
+        if (smbios_above_4g_ram) {
+            smbios_build_type_19_table(1, 4 << 20, smbios_above_4g_ram >> 10);
+        }
         smbios_validate_table();
         smbios_immutable = true;
     }
diff --git a/include/hw/i386/smbios.h b/include/hw/i386/smbios.h
index af5ee01..a9e8411 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -20,7 +20,9 @@
 void smbios_entry_add(QemuOpts *opts);
 void smbios_set_cpuid(uint32_t version, uint32_t features);
 void smbios_set_defaults(const char *manufacturer,
-                         const char *product, const char *version);
+                         const char *product, const char *version,
+                         ram_addr_t below_4g_mem_size,
+                         ram_addr_t above_4g_mem_size);
 uint8_t *smbios_get_table(size_t *length);
 
 /*
-- 
1.8.1.4

[Qemu-devel] [v3 PATCH 11/13] SMBIOS: Build full type 20 tables

[Gabriel L. Somlo]

From: "Gabriel L. Somlo" <somlo@cmu.edu>

Build full smbios type 20 (memory device mapped address) tables,
and make them available via fw_cfg.

Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
---
 hw/i386/smbios.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 2560e17..0f436b7 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -558,6 +558,23 @@ static void smbios_build_type_19_table(unsigned instance,
     SMBIOS_BUILD_TABLE_POST;
 }
 
+static void smbios_build_type_20_table(unsigned instance,
+                                       unsigned dev_hndl, unsigned array_hndl,
+                                       unsigned start_kb, unsigned size_kb)
+{
+    SMBIOS_BUILD_TABLE_PRE(20, 0x1400 + instance, true); /* required */
+
+    t->starting_address = start_kb;
+    t->ending_address = start_kb + size_kb - 1;
+    t->memory_device_handle = 0x1100 + dev_hndl; /* Type 17 (Memory Device) */
+    t->memory_array_mapped_address_handle = 0x1300 + array_hndl; /* Type 19 */
+    t->partition_row_position = 1; /* hardcoded in SeaBIOS */
+    t->interleave_position = 0; /* Not interleaved */
+    t->interleaved_data_depth = 0; /* Not interleaved */
+
+    SMBIOS_BUILD_TABLE_POST;
+}
+
 #define SMBIOS_SET_DEFAULT(field, value)                                  \
     if (!field) {                                                         \
         field = value;                                                    \
@@ -625,8 +642,20 @@ uint8_t *smbios_get_table(size_t *length)
             smbios_build_type_17_table(i, size_mb);
         }
         smbios_build_type_19_table(0, 0, smbios_below_4g_ram >> 10);
+        smbios_build_type_20_table(0, 0, 0, 0, smbios_below_4g_ram >> 10);
         if (smbios_above_4g_ram) {
+            uint32_t start_mb = 4096, size_mb, j;
             smbios_build_type_19_table(1, 4 << 20, smbios_above_4g_ram >> 10);
+            for (j = 1, i = 0; i < memdev_count; i++, j++) {
+                size_mb = (i == memdev_count - 1) ?
+                          ((ram_size_mb - 1) & 0x3FFF) + 1 : 0x4000;
+                if (i == 0) {
+                    size_mb -= smbios_below_4g_ram >> 20;
+                }
+                smbios_build_type_20_table(j, i, 1,
+                                           start_mb << 10, size_mb << 10);
+                start_mb += size_mb;
+            }
         }
         smbios_validate_table();
         smbios_immutable = true;
-- 
1.8.1.4

[Qemu-devel] [v3 PATCH 12/13] SMBIOS: Build full tables for type 32 and 127

[Gabriel L. Somlo]

From: "Gabriel L. Somlo" <somlo@cmu.edu>

Build full smbios type 32 (system boot info) and 127 (end-of-table)
tables, and make them available via fw_cfg.

Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
---
 hw/i386/smbios.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 0f436b7..36b646f 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -575,6 +575,22 @@ static void smbios_build_type_20_table(unsigned instance,
     SMBIOS_BUILD_TABLE_POST;
 }
 
+static void smbios_build_type_32_table(void)
+{
+    SMBIOS_BUILD_TABLE_PRE(32, 0x2000, true); /* required */
+
+    memset(t->reserved, 0, 6);
+    t->boot_status = 0; /* No errors detected */
+
+    SMBIOS_BUILD_TABLE_POST;
+}
+
+static void smbios_build_type_127_table(void)
+{
+    SMBIOS_BUILD_TABLE_PRE(127, 0x7F00, true); /* required */
+    SMBIOS_BUILD_TABLE_POST;
+}
+
 #define SMBIOS_SET_DEFAULT(field, value)                                  \
     if (!field) {                                                         \
         field = value;                                                    \
@@ -657,6 +673,8 @@ uint8_t *smbios_get_table(size_t *length)
                 start_mb += size_mb;
             }
         }
+        smbios_build_type_32_table();
+        smbios_build_type_127_table();
         smbios_validate_table();
         smbios_immutable = true;
     }
-- 
1.8.1.4

[Qemu-devel] [v3 PATCH 13/13] SMBIOS: Update all table definitions to smbios spec v2.3

[Gabriel L. Somlo]

From: "Gabriel L. Somlo" <somlo@cmu.edu>

Table definitions for types 4 and 17 are only up to v2.0,
so add fields specified in smbios v2.3, as expected (and
advertised) by the SeaBIOS smbios entry point structure.

In particular, OS X guests insist on type 17 being v2.3
compliant, to avoid GUI crashes when "about this mac" is
chosen in the Finder menu.

Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
---
 hw/i386/smbios.c         |  9 +++++++++
 include/hw/i386/smbios.h | 13 +++++++++++--
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 36b646f..06e3f61 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -499,6 +499,9 @@ static void smbios_build_type_4_table(unsigned instance)
     t->l1_cache_handle = 0xFFFF; /* N/A */
     t->l2_cache_handle = 0xFFFF; /* N/A */
     t->l3_cache_handle = 0xFFFF; /* N/A */
+    SMBIOS_TABLE_SET_STR(4, serial_number_str, type4.serial);
+    SMBIOS_TABLE_SET_STR(4, asset_tag_number_str, type4.asset);
+    SMBIOS_TABLE_SET_STR(4, part_number_str, type4.part);
 
     SMBIOS_BUILD_TABLE_POST;
     smbios_type4_count++;
@@ -541,6 +544,11 @@ static void smbios_build_type_17_table(unsigned instance, unsigned size_mb)
     SMBIOS_TABLE_SET_STR(17, bank_locator_str, type17.bank);
     t->memory_type = 0x07; /* RAM */
     t->type_detail = 0; /* hardcoded in SeaBIOS */
+    t->speed = 0; /* Unknown */
+    SMBIOS_TABLE_SET_STR(17, manufacturer_str, type17.manufacturer);
+    SMBIOS_TABLE_SET_STR(17, serial_number_str, type17.serial);
+    SMBIOS_TABLE_SET_STR(17, asset_tag_number_str, type17.asset);
+    SMBIOS_TABLE_SET_STR(17, part_number_str, type17.part);
 
     SMBIOS_BUILD_TABLE_POST;
 }
@@ -633,6 +641,7 @@ void smbios_set_defaults(const char *manufacturer,
     SMBIOS_SET_DEFAULT(type4.version, version);
     */
     SMBIOS_SET_DEFAULT(type17.loc_pfx, "DIMM");
+    SMBIOS_SET_DEFAULT(type17.manufacturer, manufacturer);
 }
 
 uint8_t *smbios_get_table(size_t *length)
diff --git a/include/hw/i386/smbios.h b/include/hw/i386/smbios.h
index a9e8411..c20852a 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -100,7 +100,7 @@ struct smbios_type_3 {
     /* contained elements follow */
 } QEMU_PACKED;
 
-/* SMBIOS type 4 - Processor Information (v2.0) */
+/* SMBIOS type 4 - Processor Information (v2.3) */
 struct smbios_type_4 {
     struct smbios_structure_header header;
     uint8_t socket_designation_str;
@@ -118,6 +118,10 @@ struct smbios_type_4 {
     uint16_t l1_cache_handle;
     uint16_t l2_cache_handle;
     uint16_t l3_cache_handle;
+    uint8_t serial_number_str;
+    uint8_t asset_tag_number_str;
+    uint8_t part_number_str;
+
 } QEMU_PACKED;
 
 /* SMBIOS type 16 - Physical Memory Array
@@ -132,7 +136,7 @@ struct smbios_type_16 {
     uint16_t memory_error_information_handle;
     uint16_t number_of_memory_devices;
 } QEMU_PACKED;
-/* SMBIOS type 17 - Memory Device
+/* SMBIOS type 17 - Memory Device (v2.3)
  *   Associated with one type 19
  */
 struct smbios_type_17 {
@@ -148,6 +152,11 @@ struct smbios_type_17 {
     uint8_t bank_locator_str;
     uint8_t memory_type;
     uint16_t type_detail;
+    uint16_t speed;
+    uint8_t manufacturer_str;
+    uint8_t serial_number_str;
+    uint8_t asset_tag_number_str;
+    uint8_t part_number_str;
 } QEMU_PACKED;
 
 /* SMBIOS type 19 - Memory Array Mapped Address */
-- 
1.8.1.4

Re: [Qemu-devel] [v3 PATCH 00/13] SMBIOS: build full tables in QEMU

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 565 bytes --]

On 03/12/2014 10:39 AM, Gabriel L. Somlo wrote:

> 
> I'm replying separately to the e820 suggestion, as maintaining SeaBIOS
> compatibility there is slightly trickier, and maybe we can decouple that
> from this set of patches ?

Please send subsequent versions of a series as a new top-level thread,
rather than burying it in an existing thread (that is, your 0/13 message
should NOT have an In-Reply-To: header).  It makes it easier to spot.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]