On 06/18/13 00:24, Jordan Justen wrote:
> I tested this a little bit, and did some very minor review.
> 
> I think EDK II platforms generally do something that looks quite
> different for building SMBIOS tables. See
> EmulatorPkg/MiscSubClassPlatformDxe.

Can you please spell out for me what happens in there, for example in
MiscBiosVendor*, which seem to prepare the Type0 table?

As far as I can see,

(a) "MiscBiosVendorData.c" provides a static template (without strings,
    ie. only for the formatted area),

(b) "MiscBiosVendor.uni" contains the following set of unicode strings,
    in UTF-16 encoding, with a BOM at the beginning:

    
-------------------------------------------------------------------------------------
    // *++
    //
    // Copyright (c) 2006 - 2010, Intel Corporation. All rights reserved.<BR>
    //
    // This program and the accompanying materials
    // are licensed and made available under the terms and conditions of the 
BSD License
    // which accompanies this distribution.  The full text of the license may 
be found at
    // http://opensource.org/licenses/bsd-license.php
    //
    // THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
    // WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
    //
    // --*/

    /=#

    #langdef en-US "English"

    #string STR_MISC_BIOS_VENDOR           #language en-US  "Bios Vendor"
    #string STR_MISC_BIOS_VERSION          #language en-US  "Bios Version"
    #string STR_MISC_BIOS_RELEASE_DATE     #language en-US  "Bios Release Date"
    
-------------------------------------------------------------------------------------

(c) "MiscBiosVendorFunction.c" constructs a table from the template in
    (a), and from the (unformatted area) strings in (b), using
    EFI_HII_STRING_PROTOCOL to access (b), and some PCDs.


> Any reason why we would not want to do something more like that?

(a) We do have the templates,

(b)/(c) We have no need for HII. According to the UEFI spec, chapter 29;
        HII Protocols "are the required architectural mechanisms by
        which UEFI-compliant systems manage user input."

There's no user input occurring in OVMF's SMBIOS tables.
EmulatorPkg/MiscSubClassPlatformDxe creatively repurposes HII so that it
can store default strings in a UNI file. The multi-language aspects of
HII are not used (likely for a good reason, SMBIOS tables should not be
localized).

For OvmfPkg I can't see any benefit in exchange for the additional
complexity. We should do the absolute minimum that allows the tables and
fields to trickle down from qemu. HII would only complicate the way we
construct the defaults, and save us nothing from the runtime updates via
fw_cfg.

EmulatorPkg/MiscSubClassPlatformDxe also uses a set of custom macros
(MiscSubClassDriver.h), just like my series.

One difference is that "MiscBiosVendorFunction.c" constructs the entire
dynamically allocated table in one go, and installs it with a single
Smbios->Add() call, while my series modifies the unformatted text area
with Smbios->UpdateString() calls after installation.
"MiscBiosVendorFunction.c" is probably somewhat faster in this regard
(should be negligible), but it requires one to sum the length of all
strings in advance, for memory allocation. Touching all strings by name
twice (once for size calculation, once for actual formatting) is uglier
than what my series has currently, in my opinion.

I'd be willing to imitate retrieval of actual field values, for the
default tables, from EmulatorPkg/MiscSubClassPlatformDxe, in case of
fields where I'm currently just making up a value so that the table
contents matches the one prepared by SeaBIOS.

Two examples are the start address of the BIOS, and the size of the
BIOS. However even "EmulatorPkg/MiscSubClassPlatformDxe" hardwires
those:

  SmbiosRecord->BiosSegment = (UINT16)ForType0InputData->BiosStartingAddress;

  /* ... */

  //
  // Nt32 has no PCD value to indicate BIOS Size, just fill 0 for simply.
  //
  SmbiosRecord->BiosSize = 0;

For "ForType0InputData->BiosStartingAddress", see the template,

  0xBABE, // BiosStartingAddress

Hence I think there's nothing to cherry-pick in this aspect either (in
any case qemu can patch these fields over fw_cfg too).

Thanks,
Laszlo

------------------------------------------------------------------------------
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel

Reply via email to