From: "Leif Lindholm" <leif.lindholm@linaro.org>
To: Pete Batard <pete@akeo.ie>
Cc: devel@edk2.groups.io, ard.biesheuvel@linaro.org
Subject: Re: [edk2-platforms PATCH 2/2] Platform/RPi3: Improve the population of PlatformSmbiosDxe elements
Date: Fri, 27 Sep 2019 20:03:55 +0100 [thread overview]
Message-ID: <20190927190355.GS25504@bivouac.eciton.net> (raw)
In-Reply-To: <20190904121418.3700-3-pete@akeo.ie>
On Wed, Sep 04, 2019 at 01:14:18PM +0100, Pete Batard wrote:
> This patch cleans up the population SMBIOS entries by removing elements
> that we don't have data for, as well as properly filling the ones for
> which we do, through the newly added queries from RpiFirmwareDxe.
>
> String parsing code is also added to BIOSInfoUpdateSmbiosType0() so
> that any numeric "x.y" value being passed in PcdFirmwareVersionString
> will now be used to populate the BIOS major and minor.
>
> Additional minor improvements are also applied, such as consistent use
> of uppercase values.
Could you split out the alsos?
/
Leif
> Signed-off-by: Pete Batard <pete@akeo.ie>
> ---
> Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c | 193 ++++++++++++--------
> Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.inf | 1 +
> Platform/RaspberryPi/RPi3/RPi3.dsc | 2 +-
> 3 files changed, 117 insertions(+), 79 deletions(-)
>
> diff --git a/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c b/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
> index bc35175279f2..66ffadd0cade 100644
> --- a/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
> +++ b/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
> @@ -35,11 +35,14 @@
> #include <Library/UefiDriverEntryPoint.h>
> #include <Library/UefiLib.h>
> #include <Library/BaseLib.h>
> +#include <Library/PcdLib.h>
> #include <Library/BaseMemoryLib.h>
> #include <Library/MemoryAllocationLib.h>
> #include <Library/UefiBootServicesTableLib.h>
> #include <Library/PrintLib.h>
>
> +#define SMB_IS_DIGIT(c) (((c) >= '0') && ((c) <= '9'))
> +
> STATIC RASPBERRY_PI_FIRMWARE_PROTOCOL *mFwProtocol;
>
> /***********************************************************************
> @@ -49,7 +52,7 @@ SMBIOS_TABLE_TYPE0 mBIOSInfoType0 = {
> { EFI_SMBIOS_TYPE_BIOS_INFORMATION, sizeof (SMBIOS_TABLE_TYPE0), 0 },
> 1, // Vendor String
> 2, // BiosVersion String
> - 0x0, // BiosSegment
> + 0, // BiosSegment
> 3, // BiosReleaseDate String
> 0x1F, // BiosSize
> { // BiosCharacteristics
> @@ -97,23 +100,25 @@ SMBIOS_TABLE_TYPE0 mBIOSInfoType0 = {
> // Boot1394IsSupported :1;
> // SmartBatteryIsSupported :1;
> // BIOSCharacteristicsExtensionBytes[1]
> - 0x0e, // BiosBootSpecIsSupported :1;
> + 0x0E, // BiosBootSpecIsSupported :1;
> // FunctionKeyNetworkBootIsSupported :1;
> // TargetContentDistributionEnabled :1;
> // UefiSpecificationSupported :1;
> // VirtualMachineSupported :1;
> // ExtensionByte2Reserved :3;
> },
> - 0xFF, // SystemBiosMajorRelease
> - 0xFF, // SystemBiosMinorRelease
> - 0xFF, // EmbeddedControllerFirmwareMajorRelease
> - 0xFF, // EmbeddedControllerFirmwareMinorRelease
> + 0, // SystemBiosMajorRelease
> + 0, // SystemBiosMinorRelease
> + 0, // EmbeddedControllerFirmwareMajorRelease
> + 0, // EmbeddedControllerFirmwareMinorRelease
> };
>
> +CHAR8 mBiosVersion[128] = "EDK2-DEV";
> +
> CHAR8 *mBIOSInfoType0Strings[] = {
> - "https://github.com/andreiw/RaspberryPiPkg", // Vendor String
> - "Raspberry Pi 64-bit UEFI (" __DATE__ " " __TIME__ ")", // BiosVersion String
> - __DATE__,
> + "TianoCore", // Vendor
> + mBiosVersion, // Version
> + __DATE__ " " __TIME__, // Release Date
> NULL
> };
>
> @@ -132,42 +137,19 @@ SMBIOS_TABLE_TYPE1 mSysInfoType1 = {
> 6, // Family String
> };
>
> -#define PROD_BASE 8
> -#define PROD_KNOWN 13
> -#define PROD_UNKNOWN 11
> -CHAR8 *ProductNames[] = {
> - /* 8 */ "3",
> - /* 9 */ "Zero",
> - /* 10 */ "CM3",
> - /* 11 */ "???",
> - /* 12 */ "Zero W",
> - /* 13 */ "3B+"
> -};
> -
> -#define MANU_UNKNOWN 0
> -#define MANU_KNOWN 4
> -#define MANU_BASE 1
> -CHAR8 *ManufNames[] = {
> - "???",
> - /* 0 */ "Sony",
> - /* 1 */ "Egoman",
> - /* 2 */ "Embest",
> - /* 3 */ "Sony Japan",
> - /* 4 */ "Embest"
> -};
> -
> -CHAR8 mSysInfoManufName[sizeof ("Sony Japan")];
> -CHAR8 mSysInfoProductName[sizeof ("64-bit Raspberry Pi XXXXXX (rev. xxxxxxxx)")];
> +CHAR8 mSysInfoManufName[128];
> +CHAR8 mSysInfoProductName[128];
> +CHAR8 mSysInfoVersionName[128];
> CHAR8 mSysInfoSerial[sizeof (UINT64) * 2 + 1];
> CHAR8 mSysInfoSKU[sizeof (UINT64) * 2 + 1];
>
> CHAR8 *mSysInfoType1Strings[] = {
> mSysInfoManufName,
> mSysInfoProductName,
> - mSysInfoProductName,
> + mSysInfoVersionName,
> mSysInfoSerial,
> mSysInfoSKU,
> - "edk2",
> + "Raspberry Pi",
> NULL
> };
>
> @@ -180,7 +162,7 @@ SMBIOS_TABLE_TYPE2 mBoardInfoType2 = {
> 2, // ProductName String
> 3, // Version String
> 4, // SerialNumber String
> - 5, // AssetTag String
> + 0, // AssetTag String
> { // FeatureFlag
> 1, // Motherboard :1;
> 0, // RequiresDaughterCard :1;
> @@ -189,7 +171,7 @@ SMBIOS_TABLE_TYPE2 mBoardInfoType2 = {
> 0, // HotSwappable :1;
> 0, // Reserved :3;
> },
> - 6, // LocationInChassis String
> + 0, // LocationInChassis String
> 0, // ChassisHandle;
> BaseBoardTypeMotherBoard, // BoardType;
> 0, // NumberOfContainedObjectHandles;
> @@ -198,10 +180,8 @@ SMBIOS_TABLE_TYPE2 mBoardInfoType2 = {
> CHAR8 *mBoardInfoType2Strings[] = {
> mSysInfoManufName,
> mSysInfoProductName,
> - mSysInfoProductName,
> + mSysInfoVersionName,
> mSysInfoSerial,
> - "None",
> - mSysInfoSKU,
> NULL
> };
>
> @@ -214,7 +194,7 @@ SMBIOS_TABLE_TYPE3 mEnclosureInfoType3 = {
> MiscChassisEmbeddedPc, // Type;
> 2, // Version String
> 3, // SerialNumber String
> - 4, // AssetTag String
> + 0, // AssetTag String
> ChassisStateSafe, // BootupState;
> ChassisStateSafe, // PowerSupplyState;
> ChassisStateSafe, // ThermalState;
> @@ -230,7 +210,6 @@ CHAR8 *mEnclosureInfoType3Strings[] = {
> mSysInfoManufName,
> mSysInfoProductName,
> mSysInfoSerial,
> - "None",
> NULL
> };
>
> @@ -240,9 +219,9 @@ CHAR8 *mEnclosureInfoType3Strings[] = {
> SMBIOS_TABLE_TYPE4 mProcessorInfoType4 = {
> { EFI_SMBIOS_TYPE_PROCESSOR_INFORMATION, sizeof (SMBIOS_TABLE_TYPE4), 0},
> 1, // Socket String
> - CentralProcessor, // ProcessorType; ///< The enumeration value from PROCESSOR_TYPE_DATA.
> + CentralProcessor, // ProcessorType; ///< The enumeration value from PROCESSOR_TYPE_DATA.
> ProcessorFamilyIndicatorFamily2, // ProcessorFamily; ///< The enumeration value from PROCESSOR_FAMILY2_DATA.
> - 2, // ProcessorManufacture String;
> + 2, // ProcessorManufacturer String;
> { // ProcessorId;
> { // PROCESSOR_SIGNATURE
> 0, // ProcessorSteppingId:4;
> @@ -306,9 +285,9 @@ SMBIOS_TABLE_TYPE4 mProcessorInfoType4 = {
> 0, // L1CacheHandle;
> 0, // L2CacheHandle;
> 0, // L3CacheHandle;
> - 4, // SerialNumber;
> - 5, // AssetTag;
> - 6, // PartNumber;
> + 0, // SerialNumber;
> + 0, // AssetTag;
> + 0, // PartNumber;
> 4, // CoreCount;
> 4, // EnabledCoreCount;
> 4, // ThreadCount;
> @@ -316,13 +295,12 @@ SMBIOS_TABLE_TYPE4 mProcessorInfoType4 = {
> ProcessorFamilyARM, // ARM Processor Family;
> };
>
> +CHAR8 mCpuName[128] = "Unknown ARM CPU";
> +
> CHAR8 *mProcessorInfoType4Strings[] = {
> "Socket",
> - "ARM",
> - "BCM2837 ARMv8",
> - "1.0",
> - "1.0",
> - "1.0",
> + "Broadcom",
> + mCpuName,
> NULL
> };
>
> @@ -430,7 +408,7 @@ SMBIOS_TABLE_TYPE17 mMemDevInfoType17 = {
> 0x0400, // Size; // When bit 15 is 0: Size in MB
> // When bit 15 is 1: Size in KB, and continues in ExtendedSize
> MemoryFormFactorUnknown, // FormFactor; ///< The enumeration value from MEMORY_FORM_FACTOR.
> - 0xff, // DeviceSet;
> + 0xFF, // DeviceSet;
> 1, // DeviceLocator String
> 2, // BankLocator String
> MemoryTypeDram, // MemoryType; ///< The enumeration value from MEMORY_DEVICE_TYPE.
> @@ -618,6 +596,70 @@ BIOSInfoUpdateSmbiosType0 (
> VOID
> )
> {
> + UINT32 FirmwareRevision = 0;
> + EFI_STATUS Status = EFI_SUCCESS;
> + INTN i;
> + INTN State = 0;
> + INTN Value[2];
> +
> + // Populate the Firmware major and minor.
> + Status = mFwProtocol->GetFirmwareRevision (&FirmwareRevision);
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "Failed to get firmware revision: %r\n", Status));
> + } else {
> + // This expects Broadcom / The Raspberry Pi Foundation to switch to
> + // less nonsensical VideoCore firmware revisions in the future...
> + mBIOSInfoType0.EmbeddedControllerFirmwareMajorRelease =
> + (UINT8)((FirmwareRevision >> 16) & 0xFF);
> + mBIOSInfoType0.EmbeddedControllerFirmwareMinorRelease =
> + (UINT8)(FirmwareRevision & 0xFF);
> + }
> +
> + // mBiosVersion, which is referenced in mBIOSInfoType0Strings,
> + // is not modified if the following call fails.
> + UnicodeStrToAsciiStrS ((CHAR16*)PcdGetPtr (PcdFirmwareVersionString),
> + mBiosVersion, sizeof (mBiosVersion));
> +
> + // Look for a "x.y" numeric string anywhere in mBiosVersion and
> + // try to parse it to populate the BIOS major and minor.
> + for (i = 0; (i < AsciiStrLen (mBiosVersion)) && (State < 4); i++) {
> + switch (State) {
> + case 0:
> + if (!SMB_IS_DIGIT (mBiosVersion[i]))
> + break;
> + Value[0] = Value[1] = 0;
> + State++;
> + // Fall through
> + case 1:
> + case 3:
> + if (SMB_IS_DIGIT (mBiosVersion[i])) {
> + Value[State / 2] = (Value[State / 2] * 10) + (mBiosVersion[i] - '0');
> + if (Value[State / 2] > 255) {
> + while (SMB_IS_DIGIT (mBiosVersion[i + 1]))
> + i++;
> + // Reset our state (we may have something like "Firmware X83737.1 v1.23")
> + State = 0;
> + }
> + } else {
> + State++;
> + }
> + if (State != 2)
> + break;
> + // Fall through
> + case 2:
> + if ((mBiosVersion[i] == '.') && (SMB_IS_DIGIT (mBiosVersion[i + 1]))) {
> + State++;
> + } else {
> + State = 0;
> + }
> + break;
> + }
> + }
> + if ((State == 3) || (State == 4)) {
> + mBIOSInfoType0.SystemBiosMajorRelease = (UINT8)Value[0];
> + mBIOSInfoType0.SystemBiosMinorRelease = (UINT8)Value[1];
> + }
> +
> LogSmbiosData ((EFI_SMBIOS_TABLE_HEADER*)&mBIOSInfoType0, mBIOSInfoType0Strings, NULL);
> }
>
> @@ -631,7 +673,7 @@ I64ToHexString (
> IN UINT64 Value
> )
> {
> - static CHAR8 ItoH[] = { '0','1','2','3','4','5','6','7','8','9','A','B','C','D','E','F' };
> + STATIC CHAR8 ItoH[] = { '0','1','2','3','4','5','6','7','8','9','A','B','C','D','E','F' };
> UINT8 StringInx;
> INT8 NibbleInx;
>
> @@ -664,34 +706,25 @@ SysInfoUpdateSmbiosType1 (
> UINT32 BoardRevision = 0;
> EFI_STATUS Status = EFI_SUCCESS;
> UINT64 BoardSerial = 0;
> - UINTN Prod = PROD_UNKNOWN;
> - UINTN Manu = MANU_UNKNOWN;
> + INTN Prod = -1;
> + INTN Manu = -1;
>
> Status = mFwProtocol->GetModelRevision (&BoardRevision);
> if (EFI_ERROR (Status)) {
> DEBUG ((DEBUG_ERROR, "Failed to get board model: %r\n", Status));
> } else {
> Prod = (BoardRevision >> 4) & 0xFF;
> + Manu = (BoardRevision >> 16) & 0x0F;
> }
>
> - if (Prod > PROD_KNOWN) {
> - Prod = PROD_UNKNOWN;
> - }
> - Prod -= PROD_BASE;
> - AsciiSPrint (mSysInfoProductName, sizeof (mSysInfoProductName),
> - "64-bit Raspberry Pi %a (rev. %x)", ProductNames[Prod], BoardRevision);
> -
> - Manu = (BoardRevision >> 16) & 0xF;
> - if (Manu > MANU_KNOWN) {
> - Manu = MANU_UNKNOWN;
> - } else {
> - Manu += MANU_BASE;
> - }
> - AsciiSPrint (mSysInfoManufName, sizeof (mSysInfoManufName), "%a", ManufNames[Manu]);
> + AsciiStrCpyS (mSysInfoProductName, sizeof (mSysInfoProductName),
> + mFwProtocol->GetModelName (Prod));
> + AsciiStrCpyS (mSysInfoManufName, sizeof (mSysInfoManufName),
> + mFwProtocol->GetManufacturerName (Manu));
> + AsciiSPrint (mSysInfoVersionName, sizeof (mSysInfoVersionName),
> + "%X", BoardRevision);
>
> - I64ToHexString (mSysInfoSKU,
> - sizeof (mSysInfoSKU),
> - BoardRevision);
> + I64ToHexString (mSysInfoSKU, sizeof (mSysInfoSKU), BoardRevision);
>
> Status = mFwProtocol->GetSerial (&BoardSerial);
> if (EFI_ERROR (Status)) {
> @@ -702,9 +735,11 @@ SysInfoUpdateSmbiosType1 (
>
> DEBUG ((DEBUG_ERROR, "Board Serial Number: %a\n", mSysInfoSerial));
>
> - mSysInfoType1.Uuid.Data1 = *(UINT32*)"RPi3";
> - mSysInfoType1.Uuid.Data2 = 0x0;
> - mSysInfoType1.Uuid.Data3 = 0x0;
> + mSysInfoType1.Uuid.Data1 = BoardRevision;
> + mSysInfoType1.Uuid.Data2 = 0;
> + mSysInfoType1.Uuid.Data3 = 0;
> + // Swap endianness, so that the serial is more user-friendly as a UUID
> + BoardSerial = SwapBytes64 (BoardSerial);
> CopyMem (mSysInfoType1.Uuid.Data4, &BoardSerial, sizeof (BoardSerial));
>
> LogSmbiosData ((EFI_SMBIOS_TABLE_HEADER*)&mSysInfoType1, mSysInfoType1Strings, NULL);
> @@ -763,6 +798,8 @@ ProcessorInfoUpdateSmbiosType4 (
> DEBUG ((DEBUG_INFO, "Current CPU speed: %uHz\n", Rate));
> }
>
> + AsciiStrCpyS (mCpuName, sizeof (mCpuName), mFwProtocol->GetCpuName (-1));
> +
> LogSmbiosData ((EFI_SMBIOS_TABLE_HEADER*)&mProcessorInfoType4, mProcessorInfoType4Strings, NULL);
> }
>
> diff --git a/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.inf b/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.inf
> index f7c74f7f5456..fde194ea5d90 100644
> --- a/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.inf
> +++ b/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.inf
> @@ -48,3 +48,4 @@ [Depex]
>
> [Pcd]
> gArmTokenSpaceGuid.PcdSystemMemorySize
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString
> diff --git a/Platform/RaspberryPi/RPi3/RPi3.dsc b/Platform/RaspberryPi/RPi3/RPi3.dsc
> index 2b9e619ad55c..138d3691dc56 100644
> --- a/Platform/RaspberryPi/RPi3/RPi3.dsc
> +++ b/Platform/RaspberryPi/RPi3/RPi3.dsc
> @@ -309,7 +309,7 @@ [PcdsFixedAtBuild.common]
>
> gEmbeddedTokenSpaceGuid.PcdDmaDeviceOffset|0xc0000000
>
> - gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"edk2-1.0"
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"EDK2-DEV"
>
> !if $(SECURE_BOOT_ENABLE) == TRUE
> # override the default values from SecurityPkg to ensure images from all sources are verified in secure boot
> --
> 2.21.0.windows.1
>
prev parent reply other threads:[~2019-09-27 19:03 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-04 12:14 [edk2-platforms PATCH 0/2] Platform/RPi3: PlatformSmbios improvements Pete Batard
2019-09-04 12:14 ` [edk2-platforms PATCH 1/2] Platform/RPi3: Add more query functions in RpiFirmwareDxe Pete Batard
2019-09-27 18:34 ` Leif Lindholm
2019-09-04 12:14 ` [edk2-platforms PATCH 2/2] Platform/RPi3: Improve the population of PlatformSmbiosDxe elements Pete Batard
2019-09-27 19:03 ` Leif Lindholm [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190927190355.GS25504@bivouac.eciton.net \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox