From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=Je3Yck2d; spf=pass (domain: linaro.org, ip: 209.85.221.45, mailfrom: leif.lindholm@linaro.org) Received: from mail-wr1-f45.google.com (mail-wr1-f45.google.com [209.85.221.45]) by groups.io with SMTP; Fri, 27 Sep 2019 12:03:59 -0700 Received: by mail-wr1-f45.google.com with SMTP id v8so4396196wrt.2 for ; Fri, 27 Sep 2019 12:03:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=3PuWWfXdKe22ylD6x3pJBBeMq7gkmpffX26J2pr9Yfo=; b=Je3Yck2d4DkJcUx/SpvdzGA+xA498fddGDoYfPT1ElYNoIi/BADuaNJF0naLZm93Vi xXwRgvSLDdviO86Hl+gQvs76CiRijTQkv3PsR/D45GBoa1PEEbPaQgOGVcqPtMVM2SFM 0pOnGQdmB7qcGeY/l+XNxsFeozlKMBDPEcoHbHO1+03oAOOY8iGeg3DsLtyBv+HMjYe3 FyjfAS2o9xvxjNVaTtNr+66wi6W/mGZhlxX36MyleoAKMHImtWNezT87J37LWE0QULb7 iCr745JCAL/LdcIz1jFUyxkPu8ZxzJCjFklV0MLSszQbn+30oouICuHhIW+yyf1a5hzv xR7w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=3PuWWfXdKe22ylD6x3pJBBeMq7gkmpffX26J2pr9Yfo=; b=eTah2u5jGGvkcmEF1tqV9zS9RCyWaF2YuPV0Bc/YnEQdDfV3MuvpXg2qho8KoDzwmE p1Qzi+7OUlRr3/YkDm5fx5bsouSYveEZspysh9jVKf1UGBuY5sj/xXVs14jambwXfjK7 QoyuOUW7V02pDV+j0gl7wHBY8i/8QBpoNypxx87v1raICrNgB4qJ0S2CXFFx+ICfT+6Q hUMdD8BPGAkO/Mzg/pPoIPpW9DqBYBn2//QKScg1bskF+AW2y5mn6r/sFF0qP88DQOST WTa3PtsiE5lcJadNv2hzdSNKhf5f9OvPTuFUvrOUZKJlCL+62P0OTs+IrUAXEBYVd8Sg InwQ== X-Gm-Message-State: APjAAAVg9Biap3DpsMKWXLHfcNiBT+VHGmjaL1qZHiMrgiLNC8iaXDqO k9GRE92D/L9IzdNgDQM9FVF2hA== X-Google-Smtp-Source: APXvYqwV4ZFuMM8+Q050s9O8O8SPysSwJIOCijHHV4ptKel+8EgBVj0+mc7GUX6WAFg/Sxwoyv9iIA== X-Received: by 2002:a5d:430f:: with SMTP id h15mr4310164wrq.177.1569611037571; Fri, 27 Sep 2019 12:03:57 -0700 (PDT) Return-Path: Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id w22sm5772235wmc.16.2019.09.27.12.03.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 27 Sep 2019 12:03:56 -0700 (PDT) Date: Fri, 27 Sep 2019 20:03:55 +0100 From: "Leif Lindholm" To: Pete Batard Cc: devel@edk2.groups.io, ard.biesheuvel@linaro.org Subject: Re: [edk2-platforms PATCH 2/2] Platform/RPi3: Improve the population of PlatformSmbiosDxe elements Message-ID: <20190927190355.GS25504@bivouac.eciton.net> References: <20190904121418.3700-1-pete@akeo.ie> <20190904121418.3700-3-pete@akeo.ie> MIME-Version: 1.0 In-Reply-To: <20190904121418.3700-3-pete@akeo.ie> User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 > --- > 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 > #include > #include > +#include > #include > #include > #include > #include > > +#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 >