On Tue, Jun 18, 2013 at 6:31 AM, Andrew Fish <af...@apple.com> wrote: > I agree that the MiscSubClass SMBIOS scheme is horrendous and > complicated for no reason.
Indeed. I could only argue for following this model in the hopes of providing a better 'EDK II' SMBIOS sample implementation. And the *only* reason I say better is because for just about all EDK II platforms, this is how I've seen SMBIOS done. So, it would seem to be more indicative of what EDK II platforms should do. Why it had to be so complicated was a point lost on me, so I assumed there must be some grand plan that this design helps support. Given your longer history with Tiano, I guess this is not the case. It might be nice to get more feedback from the community on the history of this, and what is best for a sample platform. Anyone? > This is why I contributed EmulatorPkg/PlatformSmbiosDxe/ Hmm, It seems the sample that I pointed Laszlo at (EmulatorPkg/MiscSubClassPlatformDxe) is not even used! Should we remove this? > that builds SMBIOS structures from static, easy to construct and > understand SMBIOS data structures that are very easy to patch > using the SmbiosLib. Andrew: SmbiosLib exists in EmulatorPkg and not somewhere more common, because?? I'll assume because it is difficult to add new features to MdePkg. Laszlo, what do you think of using Andrew's EmulatorPkg/PlatformSmbiosDxe as a basis? Could you review it? It would be nice if EDK II could provide consistent samples for this sort of thing. One other piece of feedback, it that I would prefer the model to be more like ACPI. In other words a set of 'OVMF' information which is patched to be QEMU specific. So, maybe less files with Qemu in the name, and hopefully reasonable OVMF tables would be produced, even if fw-cfg is not present. -Jordan > On Jun 18, 2013, at 12:39 AM, Laszlo Ersek <ler...@redhat.com> wrote: > >> 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 > ------------------------------------------------------------------------------ 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
