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=GF66/Isj; spf=pass (domain: linaro.org, ip: 209.85.128.68, mailfrom: leif.lindholm@linaro.org) Received: from mail-wm1-f68.google.com (mail-wm1-f68.google.com [209.85.128.68]) by groups.io with SMTP; Fri, 17 May 2019 10:03:10 -0700 Received: by mail-wm1-f68.google.com with SMTP id c77so6121991wmd.1 for ; Fri, 17 May 2019 10:03:10 -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=TtYfyogBtln0nxVuoIUkgLbSg8mnLDr5AZWg4zW9PMk=; b=GF66/Isj3uuXjSrlOdIXleUaYJ5Ee9LV2lru9nRwky2U6GkbftNimPr3J8+Aa2Wb4R 9htm5wNjfC5+t9MjHFB/Qqw1pFoQ0K7DrPhgnIFcWlMVBvzJmPW2ju8P3gQhQjjmU7Pm IQzctTW7RdrHXkMk3CHCqcA8wsHSQi/S9qI3W0lMdrHAqm0XWi8zZ+GAe1pOnfX+B6ms 2gdnen5lFmCirtATNB25d3L7e8RasUh9sENO83n8+5Hrale8JIL1p/ngAN1dthXz2vBY tq2Sj4tvbOE+9jv97qa371qr33A4arkGbatPtUSXnifaT1tdQK/zQYIUSVUNgdxFKC+J R7lQ== 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=TtYfyogBtln0nxVuoIUkgLbSg8mnLDr5AZWg4zW9PMk=; b=PznIv34jzTG8UXHBGxsoeHZb7UYrfIO5Ksa4XGy4mQ0lsuRdCAupAUq2lrGuOrVcb+ goS8Yhvk655ONiZanoTwfXO40FSHW+mmuWtTg27Nzse9shJCTnGwny5wz1uTPujrcgdd GgH9FwRba/IavPZ0ADmlN/kuA0PVZhGCLiHl0E7GY/G3W2v+L0CbXubgRR+d3u+kNhGm KZhsLp0PqBy5AluG9D+0gg5i5/KZoPsn8xH03Li5wzz9gYDeMRRp34PBC4Tv679tV2fv hyyKjCBdkmW2lzTRp4UQinsMMY9ORiJPRTlivi+MNykHFTsagMpS63nsENWuPzpNTjDj h1wA== X-Gm-Message-State: APjAAAX/fV65qH80KcpFJ2qbH1S75IEOkW8SceioDVPArE+sKDFlnaPp ZqCWeuj4HxcWWS+0oksY2DGbNQ== X-Google-Smtp-Source: APXvYqx/I7oLC4OvcZ5/LEaRwd07vtkgACgYm5RitC+Ptr2PHUwbGvlgGu61aYcAssd5YytBooFkhg== X-Received: by 2002:a1c:e144:: with SMTP id y65mr31769680wmg.147.1558112588572; Fri, 17 May 2019 10:03:08 -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 i15sm11200819wre.30.2019.05.17.10.03.07 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 17 May 2019 10:03:07 -0700 (PDT) Date: Fri, 17 May 2019 18:03:06 +0100 From: "Leif Lindholm" To: Marcin Wojtas Cc: devel@edk2.groups.io, ard.biesheuvel@linaro.org, jsd@semihalf.com, jaz@semihalf.com, kostap@marvell.com, Jici.Gao@arm.com Subject: Re: [edk2-platforms: PATCH] Marvell/Armada7k8k: Introduce SMBIOS/DMI support Message-ID: <20190517170306.yhh7a7az4qnam4ig@bivouac.eciton.net> References: <1558079907-5168-1-git-send-email-mw@semihalf.com> MIME-Version: 1.0 In-Reply-To: <1558079907-5168-1-git-send-email-mw@semihalf.com> User-Agent: NeoMutt/20170113 (1.7.2) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Fri, May 17, 2019 at 09:58:27AM +0200, Marcin Wojtas wrote: > Fill in the basic requirements of the SMBIOS specification by specifying > the minimum required structures. The basic fixup is performed. > CPU/DRAM frequency is obtained via SampleAtResetlib and the DRAM size > is calculated from the information stored in the HOB list. > > Add new Armada SmbiosPlatformDxe and MdeModulePkg/SmbiosDxe to the > build and firmware image. With these changes, the EFI BDS, EFI shell, > and Linux dmidecode command return useful information. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Marcin Wojtas > --- > Hi, > > I push the support as a single patch. I plan to add more detailed > fixup, using BoardDescProtocol, but I'll add it after PCIE is merged. Yes, I think this is a good approach. > In order to ease review, the patch is available in the github: > https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/smbios-upstream-r20190517 > > Looking forward to your comments. > > Best regards, > Marcin > > Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 9 + > Silicon/Marvell/Armada7k8k/Armada7k8k.fdf | 4 + > Silicon/Marvell/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf | 60 ++ > Silicon/Marvell/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c | 787 ++++++++++++++++++++ > 4 files changed, 860 insertions(+) > create mode 100644 Silicon/Marvell/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf > create mode 100644 Silicon/Marvell/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c > > diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc > index 3c685b3..0c13045 100644 > --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc > +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc > @@ -395,6 +395,11 @@ > gMarvellTokenSpaceGuid.PcdOpTeeRegionBase|0x4400000 > gMarvellTokenSpaceGuid.PcdOpTeeRegionSize|0x1000000 > > + # SMBIOS/DMI > + gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosDocRev|0x0 > + gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosEntryPointProvideMethod|0x2 > + gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosVersion|0x0300 Nothing wrong with this, but current SMBIOS revision is 3.2, and definitions for it are already in EDK2, so it could make sense for a new implementation to use that version. > + > # TRNG > gMarvellTokenSpaceGuid.PcdEip76TrngBaseAddress|0xF2760000 > > @@ -619,6 +624,10 @@ > ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf > !endif #$(INCLUDE_TFTP_COMMAND) > > + # SMBIOS/DMI > + MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf > + Silicon/Marvell/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf > + > [Components.AARCH64] > # > # Generic ACPI modules > diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.fdf b/Silicon/Marvell/Armada7k8k/Armada7k8k.fdf > index 2f4b810..1fbc744 100644 > --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.fdf > +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.fdf > @@ -221,6 +221,10 @@ FvNameGuid = 5eda4200-2c5f-43cb-9da3-0baf74b1b30c > # DTB > INF EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf > > + # SMBIOS/DMI > + INF MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf > + INF Silicon/Marvell/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf > + > !if $(ARCH) == AARCH64 > # ACPI support > INF MdeModulePkg/Universal/Acpi/AcpiPlatformDxe/AcpiPlatformDxe.inf > diff --git a/Silicon/Marvell/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf b/Silicon/Marvell/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf > new file mode 100644 > index 0000000..b81cac9 > --- /dev/null > +++ b/Silicon/Marvell/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf > @@ -0,0 +1,60 @@ > +#/** @file > +# SMBIOS Table for ARM platform > +# > +# Copyright (c) 2013, Linaro Ltd. All rights reserved.
> +# Copyright (c) 2018, Marvell International Ltd. and its affiliates -2019? > +# > +# 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. > +# > +#**/ > + > +[Defines] > + INF_VERSION = 0x0001001B > + BASE_NAME = SmbiosPlatformDxe > + FILE_GUID = 1c5028a4-3bd0-43cf-9e56-b450584dc22b > + MODULE_TYPE = DXE_DRIVER > + VERSION_STRING = 1.0 > + ENTRY_POINT = SmbiosPlatformDriverEntryPoint > + > +[Sources] > + SmbiosPlatformDxe.c > + > +[Packages] > + ArmPkg/ArmPkg.dec > + ArmPlatformPkg/ArmPlatformPkg.dec > + MdeModulePkg/MdeModulePkg.dec > + MdePkg/MdePkg.dec > + Silicon/Marvell/Marvell.dec > + > +[LibraryClasses] > + ArmLib I don't *think* ArmLib is used by the driver. > + BaseLib > + BaseMemoryLib > + DebugLib > + HobLib > + IoLib IoLib is not used by the driver. > + MemoryAllocationLib > + PcdLib > + SampleAtResetLib > + UefiBootServicesTableLib > + UefiDriverEntryPoint > + > +[Guids] > + gEfiGlobalVariableGuid I don't notice where this is referenced in the driver. > + > +[FixedPcd] > + gArmTokenSpaceGuid.PcdSystemMemoryBase > + gArmTokenSpaceGuid.PcdSystemMemorySize These don't actually appear to be used. (Which is good, because I would probably be complaining if they were.) > + gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision > + > +[Protocols] > + gEfiSmbiosProtocolGuid # PROTOCOL ALWAYS_CONSUMED > + > +[Depex] > + gEfiSmbiosProtocolGuid > diff --git a/Silicon/Marvell/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c b/Silicon/Marvell/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c > new file mode 100644 > index 0000000..5d45c92 > --- /dev/null > +++ b/Silicon/Marvell/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c > @@ -0,0 +1,787 @@ > +/** @file > + This driver installs SMBIOS information for Marvell Armada platforms > + > + Copyright (c) 2015, ARM Limited. All rights reserved. > + Copyright (c) 2018, Marvell International Ltd. and its affiliates -2019? > + > + 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. > + > +**/ > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > + > +#include > +#include The following headers appear unused by this file: #include #include #include #include If I am wrong, and some of these *are* used but pulled in by other headers, so be it. But any unused ones should be deleted until such a point code is added that actually make use of them. > + > +// > +// SMBIOS tables often reference each other using > +// fixed constants, define a list of these constants > +// for our hardcoded tables > +// > +enum SMBIOS_REFRENCE_HANDLES { > + SMBIOS_HANDLE_A53_L1I = 0x1000, > + SMBIOS_HANDLE_A53_L1D, > + SMBIOS_HANDLE_A53_L2, I would say the above three could be deleted. If we ever move this enum into a shared header somewhere, it makes sense to fill it up with generic info - but here it only makes sense to list handles actually used. > + SMBIOS_HANDLE_A72_L1I, However, no _L1D? > + SMBIOS_HANDLE_A72_L2, > + SMBIOS_HANDLE_A72_L3, Cortex-A73 does not have an L3 cache. The interconnect does. (Just drop the A72_ bit.) > + SMBIOS_HANDLE_MOTHERBOARD, > + SMBIOS_HANDLE_CHASSIS, > + SMBIOS_HANDLE_A72_CLUSTER, > + SMBIOS_HANDLE_MEMORY, > + SMBIOS_HANDLE_DIMM > +}; > + > +// > +// Type definition and contents of the default SMBIOS table. > +// This table covers only the minimum structures required by > +// the SMBIOS specification (section 6.2, version 3.0) > +// > + > +// BIOS information (section 7.1) > +STATIC SMBIOS_TABLE_TYPE0 mArmadaDefaultType0 = { > + { // SMBIOS_STRUCTURE Hdr > + EFI_SMBIOS_TYPE_BIOS_INFORMATION, // UINT8 Type > + sizeof (SMBIOS_TABLE_TYPE0), // UINT8 Length > + SMBIOS_HANDLE_PI_RESERVED, > + }, > + 1, // SMBIOS_TABLE_STRING Vendor > + 2, // SMBIOS_TABLE_STRING BiosVersion > + 0xE800,// UINT16 BiosSegment > + 3, // SMBIOS_TABLE_STRING BiosReleaseDate > + 0, // UINT8 BiosSize > + { > + 0,0,0,0,0,0, > + 1, //PCI supported > + 0, > + 1, //PNP supported > + 0, > + 1, //BIOS upgradable > + 0, 0, 0, > + 0, //Boot from CD > + 1, //selectable boot > + }, // MISC_BIOS_CHARACTERISTICS BiosCharacteristics > + { // BIOSCharacteristicsExtensionBytes[2] > + 0x3, > + 0xC, > + }, > + 0, // UINT8 SystemBiosMajorRelease > + 0, // UINT8 SystemBiosMinorRelease > + 0xFF, // UINT8 EmbeddedControllerFirmwareMajorRelease > + 0xFF, // UINT8 EmbeddedControllerFirmwareMinorRelease > +}; > + > +STATIC CHAR8 CONST *mArmadaDefaultType0Strings[] = { > + "EFI Development Kit II / Marvell\0", /* Vendor */ > + "EDK II\0", /* BiosVersion */ > + __DATE__"\0", /* BiosReleaseDate */ > + NULL > +}; > + > +// System information (section 7.2) > +STATIC SMBIOS_TABLE_TYPE1 mArmadaDefaultType1 = { > + { // SMBIOS_STRUCTURE Hdr > + EFI_SMBIOS_TYPE_SYSTEM_INFORMATION, > + sizeof (SMBIOS_TABLE_TYPE1), > + SMBIOS_HANDLE_PI_RESERVED, > + }, > + 1, //Manufacturer > + 2, //Product Name > + 3, //Version > + 4, //Serial > + { 0x97c93925, 0x1273, 0x4f03, { 0x9f,0x75,0x2f,0x2b,0x7e,0xd1,0x94,0x80 }}, //UUID > + 6, //Wakeup type > + 0, //SKU > + 0, //Family > +}; > + > +STATIC CHAR8 CONST *mArmadaDefaultType1Strings[] = { > + "Marvell\0", /* Manufacturer */ > + "Armada 7k/8k Family Board\0", /* Product Name placeholder*/ > + "Revision unknown\0", /* Version placeholder */ > + " \0", /* 20 character buffer */ > + NULL > +}; > + > +// Baseboard (section 7.3) > +STATIC SMBIOS_TABLE_TYPE2 mArmadaDefaultType2 = { > + { // SMBIOS_STRUCTURE Hdr > + EFI_SMBIOS_TYPE_BASEBOARD_INFORMATION, // UINT8 Type > + sizeof (SMBIOS_TABLE_TYPE2), // UINT8 Length > + SMBIOS_HANDLE_MOTHERBOARD, > + }, > + 1, //Manufacturer > + 2, //Product Name > + 3, //Version > + 4, //Serial > + 0, //Asset tag > + {1}, //motherboard, not replaceable > + 5, //location of board > + SMBIOS_HANDLE_CHASSIS, > + BaseBoardTypeMotherBoard, > + 1, > + {SMBIOS_HANDLE_A72_CLUSTER}, > +}; > + > +STATIC CHAR8 CONST *mArmadaDefaultType2Strings[] = { > + "Marvell\0", /* Manufacturer */ > + "Armada 7k/8k Family Board\0", /* Product Name placeholder*/ > + "Revision unknown\0", /* Version placeholder */ > + "Serial Not Set\0", /* Serial */ > + "Base of Chassis\0", /* Board location */ > + NULL > +}; > + > +// Enclosure > +STATIC SMBIOS_TABLE_TYPE3 mArmadaDefaultType3 = { > + { // SMBIOS_STRUCTURE Hdr > + EFI_SMBIOS_TYPE_SYSTEM_ENCLOSURE, // UINT8 Type > + sizeof (SMBIOS_TABLE_TYPE3), // UINT8 Length > + SMBIOS_HANDLE_CHASSIS, > + }, > + 1, //Manufacturer > + 2, //enclosure type > + 2, //version > + 3, //serial > + 0, //asset tag > + ChassisStateUnknown, //boot chassis state > + ChassisStateSafe, //power supply state > + ChassisStateSafe, //thermal state > + ChassisSecurityStatusNone, //security state > + {0,0,0,0,}, //OEM defined > + 1, //1U height > + 1, //number of power cords > + 0, //no contained elements > +}; > + > +STATIC CHAR8 CONST *mArmadaDefaultType3Strings[] = { > + "Marvell\0", /* Manufacturer */ > + "Revision unknown\0", /* Version placeholder */ > + "Serial Not Set\0", /* Serial */ > + NULL > +}; > + > +// Processor > +STATIC SMBIOS_TABLE_TYPE4 mArmadaDefaultType4 = { > + { // SMBIOS_STRUCTURE Hdr > + EFI_SMBIOS_TYPE_PROCESSOR_INFORMATION, // UINT8 Type > + sizeof (SMBIOS_TABLE_TYPE4), // UINT8 Length > + SMBIOS_HANDLE_A72_CLUSTER, > + }, > + 1, //socket type > + 3, //processor type CPU > + ProcessorFamilyIndicatorFamily2, //processor family, acquire from field2 > + 2, //manufactuer > + {{0,},{0.}}, //processor id > + 3, //version > + {0,0,0,0,0,1}, //voltage > + 0, //external clock > + 2000, //max speed > + 0, //current speed - requires update > + 0x41, //status > + ProcessorUpgradeOther, > + SMBIOS_HANDLE_A72_L1I, //l1 cache handle > + SMBIOS_HANDLE_A72_L2, //l2 cache handle > + SMBIOS_HANDLE_A72_L3, //l3 cache handle > + 0, //serial not set > + 0, //asset not set > + 4, //part number > + 4, //core count in socket > + 4, //enabled core count in socket > + 0, //threads per socket > + 0xEC, //processor characteristics > + ProcessorFamilyARM, //ARMADA core > +}; > + > +STATIC CHAR8 CONST *mArmadaDefaultType4Strings[] = { > + "Socket type unknown \0", /* Socket type placeholder */ > + "Marvell\0", /* manufactuer */ > + "Cortex-A72\0", /* processor description */ > + "0xd08\0", /* A72 part number */ > + NULL > +}; > + > +// Cache > +STATIC SMBIOS_TABLE_TYPE7 mArmadaDefaultType7_a72_l1i = { > + { // SMBIOS_STRUCTURE Hdr > + EFI_SMBIOS_TYPE_CACHE_INFORMATION, // UINT8 Type > + sizeof (SMBIOS_TABLE_TYPE7), // UINT8 Length > + SMBIOS_HANDLE_A72_L1I, > + }, > + 1, > + 0x380, //L1 enabled, unknown WB > + 48, //48k i cache max > + 48, //48k installed > + {0,1}, //SRAM type > + {0,1}, //SRAM type > + 0, //speed unknown > + CacheErrorParity, //parity checking > + CacheTypeInstruction, //instruction cache > + CacheAssociativityOther, //three way > +}; > + > +STATIC SMBIOS_TABLE_TYPE7 mArmadaDefaultType7_a72_l2 = { > + { // SMBIOS_STRUCTURE Hdr > + EFI_SMBIOS_TYPE_CACHE_INFORMATION, // UINT8 Type > + sizeof (SMBIOS_TABLE_TYPE7), // UINT8 Length > + SMBIOS_HANDLE_A72_L2, > + }, > + 3, > + 0x181, //L2 enabled, WB > + 512, //512k d cache max > + 512, //512k installed > + {0,1}, //SRAM type > + {0,1}, //SRAM type > + 0, //speed unknown > + CacheErrorSingleBit, //ECC checking > + CacheTypeUnified, //instruction cache > + CacheAssociativity16Way, //16 way associative > +}; > + > +STATIC SMBIOS_TABLE_TYPE7 mArmadaDefaultType7_a72_l3 = { > + { // SMBIOS_STRUCTURE Hdr > + EFI_SMBIOS_TYPE_CACHE_INFORMATION, // UINT8 Type > + sizeof (SMBIOS_TABLE_TYPE7), // UINT8 Length > + SMBIOS_HANDLE_A72_L3, > + }, > + 4, > + 0x182, //L3 enabled, WB > + 1024, //1M cache max > + 1024, //1M installed > + {0,1}, //SRAM type > + {0,1}, //SRAM type > + 0, //speed unknown > + CacheErrorSingleBit, //ECC checking > + CacheTypeUnified, //instruction cache > + CacheAssociativity8Way, //8 way associative > +}; > + > +STATIC CONST CHAR8 *mArmadaDefaultType7Strings[] = { > + "L1 Instruction\0", /* L1I */ > + "L1 Data\0", /* L1D */ > + "L2\0", /* L2 */ > + "L3\0", /* L3 */ > + NULL > +}; > + > +// Slots > +STATIC SMBIOS_TABLE_TYPE9 mArmadaDefaultType9_0 = { > + { // SMBIOS_STRUCTURE Hdr > + EFI_SMBIOS_TYPE_INACTIVE, // UINT8 Type > + sizeof (SMBIOS_TABLE_TYPE9), // UINT8 Length > + SMBIOS_HANDLE_PI_RESERVED, > + }, > + 1, // CP0 PCIE0 > + SlotTypePciExpressGen2X1, > + SlotDataBusWidth1X, > + SlotUsageUnknown, > + SlotLengthShort, > + 0, > + {1}, //Unknown > + {1,0,1}, //PME and SMBUS > + 0, > + 0, > + 1, > +}; > + > +STATIC SMBIOS_TABLE_TYPE9 mArmadaDefaultType9_1 = { > + { // SMBIOS_STRUCTURE Hdr > + EFI_SMBIOS_TYPE_INACTIVE, // UINT8 Type > + sizeof (SMBIOS_TABLE_TYPE9), // UINT8 Length > + SMBIOS_HANDLE_PI_RESERVED, > + }, > + 2, // CP0 PCIE1 > + SlotTypePciExpressGen2X1, > + SlotDataBusWidth1X, > + SlotUsageUnknown, > + SlotLengthShort, > + 0, > + {1}, > + {1,0,1}, //PME and SMBUS > + 0, > + 0, > + 2, > +}; > + > +STATIC SMBIOS_TABLE_TYPE9 mArmadaDefaultType9_2 = { > + { // SMBIOS_STRUCTURE Hdr > + EFI_SMBIOS_TYPE_INACTIVE, // UINT8 Type > + sizeof (SMBIOS_TABLE_TYPE9), // UINT8 Length > + SMBIOS_HANDLE_PI_RESERVED, > + }, > + 3, // CP0 PCIE2 > + SlotTypePciExpressGen2X1, > + SlotDataBusWidth1X, > + SlotUsageUnknown, > + SlotLengthShort, > + 0, > + {1}, > + {1,0,1}, //PME and SMBUS > + 0, > + 0, > + 3, > +}; > + > +STATIC SMBIOS_TABLE_TYPE9 mArmadaDefaultType9_3 = { > + { // SMBIOS_STRUCTURE Hdr > + EFI_SMBIOS_TYPE_INACTIVE, // UINT8 Type > + sizeof (SMBIOS_TABLE_TYPE9), // UINT8 Length > + SMBIOS_HANDLE_PI_RESERVED, > + }, > + 4, // CP1 PCIE0 > + SlotTypePciExpressGen2X1, > + SlotDataBusWidth1X, > + SlotUsageUnknown, > + SlotLengthShort, > + 0, > + {1}, > + {1,0,1}, //PME and SMBUS > + 0, > + 0, > + 0xc, > +}; > + > +STATIC SMBIOS_TABLE_TYPE9 mArmadaDefaultType9_4 = { > + { // SMBIOS_STRUCTURE Hdr > + EFI_SMBIOS_TYPE_INACTIVE, // UINT8 Type > + sizeof (SMBIOS_TABLE_TYPE9), // UINT8 Length > + SMBIOS_HANDLE_PI_RESERVED, > + }, > + 5, // CP1 PCIE1 > + SlotTypePciExpressGen2X1, > + SlotDataBusWidth1X, > + SlotUsageUnknown, > + SlotLengthShort, > + 0, > + {1}, > + {1,0,1}, //PME and SMBUS > + 0, > + 0, > + 0xc, > +}; > + > +STATIC SMBIOS_TABLE_TYPE9 mArmadaDefaultType9_5 = { > + { // SMBIOS_STRUCTURE Hdr > + EFI_SMBIOS_TYPE_INACTIVE, // UINT8 Type > + sizeof (SMBIOS_TABLE_TYPE9), // UINT8 Length > + SMBIOS_HANDLE_PI_RESERVED, > + }, > + 6, // CP1 PCIE2 > + SlotTypePciExpressGen2X1, > + SlotDataBusWidth1X, > + SlotUsageUnknown, > + SlotLengthShort, > + 0, > + {1}, > + {1,0,1}, //PME and SMBUS > + 0, > + 0, > + 0xc, > +}; > + > +STATIC CHAR8 CONST *mArmadaDefaultType9Strings[] = { > + "PCIE0 CP0\0", /* Slot0 */ > + "PCIE1 CP0\0", /* Slot1 */ > + "PCIE2 CP0\0", /* Slot2 */ > + "PCIE0 CP1\0", /* Slot3 */ > + "PCIE1 CP1\0", /* Slot4 */ > + "PCIE2 CP1\0", /* Slot5 */ > + NULL > +}; > + > +// Memory array > +STATIC SMBIOS_TABLE_TYPE16 mArmadaDefaultType16 = { > + { // SMBIOS_STRUCTURE Hdr > + EFI_SMBIOS_TYPE_PHYSICAL_MEMORY_ARRAY, // UINT8 Type > + sizeof (SMBIOS_TABLE_TYPE16), // UINT8 Length > + SMBIOS_HANDLE_MEMORY, > + }, > + MemoryArrayLocationSystemBoard, //on motherboard > + MemoryArrayUseSystemMemory, //system RAM > + MemoryErrorCorrectionNone, //ECC RAM > + 0x1000000, //16GB > + 0xFFFE, //No error information structure > + 0x1, //soldered memory > +}; > + > +STATIC CHAR8 CONST *mArmadaDefaultType16Strings[] = { > + NULL > +}; > + > +// Memory device > +STATIC SMBIOS_TABLE_TYPE17 mArmadaDefaultType17 = { > + { // SMBIOS_STRUCTURE Hdr > + EFI_SMBIOS_TYPE_MEMORY_DEVICE, // UINT8 Type > + sizeof (SMBIOS_TABLE_TYPE17), // UINT8 Length > + SMBIOS_HANDLE_DIMM, > + }, > + SMBIOS_HANDLE_MEMORY, //array to which this module belongs > + 0xFFFE, //no errors > + 64, //single DIMM, no ECC is 64bits (for ecc this would be 72) > + 32, //data width of this device (32-bits) > + 0, //Memory size obtained dynamically > + MemoryFormFactorRowOfChips, //Memory factor > + 0, //Not part of a set > + 1, //Right side of board > + 2, //Bank 0 > + MemoryTypeDdr4, //DDR4 > + {0,0,0,0,0,0,0,0,0,0,0,0,0,0,1}, //unbuffered > + 0, //DRAM speed - requires update > + 0, //varies between diffrent production runs > + 0, //serial > + 0, //asset tag > + 0, //part number > + 0, //rank > +}; > + > +STATIC CHAR8 CONST *mArmadaDefaultType17Strings[] = { > + "RIGHT SIDE\0", /* location */ > + "BANK 0\0", /* bank description */ > + NULL > +}; > + > +// > +// Memory array mapped address, this structure > +// is overridden by SmbiosInstallMemoryStructure. > +// > +STATIC SMBIOS_TABLE_TYPE19 mArmadaDefaultType19 = { > + { // SMBIOS_STRUCTURE Hdr > + EFI_SMBIOS_TYPE_MEMORY_ARRAY_MAPPED_ADDRESS, // UINT8 Type > + sizeof (SMBIOS_TABLE_TYPE19), // UINT8 Length > + SMBIOS_HANDLE_PI_RESERVED, > + }, > + 0xFFFFFFFF, //invalid, look at extended addr field > + 0xFFFFFFFF, > + SMBIOS_HANDLE_DIMM, //handle > + 1, > + 0x080000000, //starting addr of first 2GB > + 0x100000000, //ending addr of first 2GB > +}; > + > +// System boot infomArmadaDefaultType4. > +STATIC SMBIOS_TABLE_TYPE32 mArmadaDefaultType32 = { > + { // SMBIOS_STRUCTURE Hdr > + EFI_SMBIOS_TYPE_SYSTEM_BOOT_INFORMATION, // UINT8 Type > + sizeof (SMBIOS_TABLE_TYPE32), // UINT8 Length > + SMBIOS_HANDLE_PI_RESERVED, > + }, > + {0, 0, 0, 0, 0, 0}, //reserved > + BootInformationStatusNoError, > +}; > + > +STATIC CHAR8 CONST *mArmadaDefaultType32Strings[] = { > + NULL > +}; > + > +STATIC CONST VOID *DefaultCommonTables[][2] = > +{ > + { &mArmadaDefaultType0, mArmadaDefaultType0Strings }, > + { &mArmadaDefaultType1, mArmadaDefaultType1Strings }, > + { &mArmadaDefaultType2, mArmadaDefaultType2Strings }, > + { &mArmadaDefaultType3, mArmadaDefaultType3Strings }, > + { &mArmadaDefaultType4, mArmadaDefaultType4Strings }, > + { &mArmadaDefaultType7_a72_l1i, mArmadaDefaultType7Strings }, > + { &mArmadaDefaultType7_a72_l2, mArmadaDefaultType7Strings }, > + { &mArmadaDefaultType7_a72_l3, mArmadaDefaultType7Strings }, > + { &mArmadaDefaultType9_0, mArmadaDefaultType9Strings }, > + { &mArmadaDefaultType9_1, mArmadaDefaultType9Strings }, > + { &mArmadaDefaultType9_2, mArmadaDefaultType9Strings }, > + { &mArmadaDefaultType9_3, mArmadaDefaultType9Strings }, > + { &mArmadaDefaultType9_4, mArmadaDefaultType9Strings }, > + { &mArmadaDefaultType9_5, mArmadaDefaultType9Strings }, > + { &mArmadaDefaultType16, mArmadaDefaultType16Strings }, > + { &mArmadaDefaultType17, mArmadaDefaultType17Strings }, > + { &mArmadaDefaultType32, mArmadaDefaultType32Strings }, > + { NULL, NULL }, > +}; > + > +/** > + > + Create SMBIOS record. > + > + Converts a fixed SMBIOS structure and an array of pointers to strings into > + an SMBIOS record where the strings are cat'ed on the end of the fixed record > + and terminated via a double NULL and add to SMBIOS table. > + > + SMBIOS_TABLE_TYPE32 gSmbiosType12 = { > + { EFI_SMBIOS_TYPE_SYSTEM_CONFIGURATION_OPTIONS, sizeof (SMBIOS_TABLE_TYPE12), 0 }, > + 1 // StringCount > + }; > + > + CHAR8 *gSmbiosType12Strings[] = { > + "Not Found", > + NULL > + }; > + > + ... > + > + LogSmbiosData ( > + (EFI_SMBIOS_TABLE_HEADER*)&gSmbiosType12, > + gSmbiosType12Strings > + ); > + > + @param Smbios SMBIOS protocol > + @param Template Fixed SMBIOS structure, required. > + @param StringArray Array of strings to convert to an SMBIOS string pack. > + NULL is OK. > +**/ > +STATIC > +EFI_STATUS > +EFIAPI > +LogSmbiosData ( > + IN EFI_SMBIOS_PROTOCOL *Smbios, > + IN EFI_SMBIOS_TABLE_HEADER *Template, > + IN CONST CHAR8 * CONST *StringPack This is called StringArray in doxygen blurb above. > + ) > +{ > + EFI_STATUS Status; > + EFI_SMBIOS_TABLE_HEADER *Record; > + UINTN Index; > + UINTN StringSize; > + UINTN Size; > + CHAR8 *Str; > + > + > + // Calculate the size of the fixed record and optional string pack > + Size = Template->Length; > + if (StringPack == NULL) { > + // At least a double null is required > + Size += 2; > + } else { > + for (Index = 0; StringPack[Index] != NULL; Index++) { > + StringSize = AsciiStrSize (StringPack[Index]); > + Size += StringSize; > + } > + if (StringPack[0] == NULL) { Indentation of if-statement borked. > + // At least a double null is required > + Size += 1; OK, this is just me being slow, but why does a double null require Size to be adjusted by 2 above but 1 here? > + } > + > + // Don't forget the terminating double null > + Size += 1; > + } > + > + // Copy over Template > + Record = (EFI_SMBIOS_TABLE_HEADER *)AllocateZeroPool (Size); > + if (Record == NULL) { > + return EFI_OUT_OF_RESOURCES; > + } > + CopyMem (Record, Template, Template->Length); > + > + // Append string pack > + Str = ((CHAR8 *)Record) + Record->Length; (VOID *)? > + for (Index = 0; StringPack[Index] != NULL; Index++) { > + StringSize = AsciiStrSize (StringPack[Index]); > + CopyMem (Str, StringPack[Index], StringSize); > + Str += StringSize; > + } > + *Str = 0; > + > + Status = Smbios->Add (Smbios, > + NULL, > + &Record->Handle, > + Record); > + ASSERT_EFI_ERROR (Status); > + > + FreePool (Record); > + > + return Status; > +} > + > +/** > + Installs a memory descriptor (type19) for the given address range > + > + @param Smbios SMBIOS protocol This function takes two additional arguments. > + > +**/ > +EFI_STATUS > +SmbiosInstallMemoryStructure ( > + IN EFI_SMBIOS_PROTOCOL *Smbios, > + IN UINT64 StartingAddress, > + IN UINT64 RegionLength > + ) > +{ > + EFI_SMBIOS_HANDLE SmbiosHandle; > + SMBIOS_TABLE_TYPE19 MemoryDescriptor; > + EFI_STATUS Status = EFI_SUCCESS; > + > + CopyMem (&MemoryDescriptor, > + &mArmadaDefaultType19, > + sizeof (SMBIOS_TABLE_TYPE19)); > + > + MemoryDescriptor.ExtendedStartingAddress = StartingAddress; > + MemoryDescriptor.ExtendedEndingAddress = StartingAddress + RegionLength; > + SmbiosHandle = MemoryDescriptor.Hdr.Handle; > + > + Status = Smbios->Add (Smbios, > + NULL, > + &SmbiosHandle, > + (EFI_SMBIOS_TABLE_HEADER*) &MemoryDescriptor); > + > + return Status; > +} > + > +/** > + Install a whole table worth of structructures > + > + @parm @param Also, add actual input arguments :) > +**/ > +EFI_STATUS > +SmbiosInstallStructures ( > + IN EFI_SMBIOS_PROTOCOL *Smbios, > + IN CONST VOID *DefaultTables[][2] > + ) > +{ > + EFI_STATUS Status = EFI_SUCCESS; > + INTN TableEntry; > + > + for (TableEntry = 0; DefaultTables[TableEntry][0] != NULL; TableEntry++) { > + // Omit disabled tables > + if (((EFI_SMBIOS_TABLE_HEADER *)DefaultTables[TableEntry][0])->Type == > + EFI_SMBIOS_TYPE_INACTIVE) { > + continue; > + } > + > + Status = LogSmbiosData (Smbios, > + ((EFI_SMBIOS_TABLE_HEADER *)DefaultTables[TableEntry][0]), > + DefaultTables[TableEntry][1]); > + if (EFI_ERROR (Status)) > + break; > + } > + > + return Status; > +} > + > +/** > + Update memory information basing on the HOB list. > + > + @param Smbios SMBIOS protocol > + > +**/ > +STATIC > +EFI_STATUS > +SmbiosMemoryInstall ( > + IN EFI_SMBIOS_PROTOCOL *Smbios > + ) > +{ > + EFI_PEI_HOB_POINTERS Hob; > + UINT64 MemorySize = 0; Please move initialization somewhere below variable definitions. > + EFI_STATUS Status; > + > + // > + // Get the HOB list for processing > + // > + Hob.Raw = GetHobList (); > + > + // > + // Collect memory ranges > + // > + while (!END_OF_HOB_LIST (Hob)) { > + if (Hob.Header->HobType == EFI_HOB_TYPE_RESOURCE_DESCRIPTOR) { > + if (Hob.ResourceDescriptor->ResourceType == EFI_RESOURCE_SYSTEM_MEMORY) { > + MemorySize += (UINT64)(Hob.ResourceDescriptor->ResourceLength); > + > + Status = SmbiosInstallMemoryStructure (Smbios, > + Hob.ResourceDescriptor->PhysicalStart, > + Hob.ResourceDescriptor->ResourceLength); > + if (EFI_ERROR(Status)) { > + return Status; > + } > + } > + } > + Hob.Raw = GET_NEXT_HOB (Hob); > + } > + > + // TYPE17 fixup And what does the fixup do? > + mArmadaDefaultType17.Size = (UINT16)(MemorySize >> 20); > + > + return EFI_SUCCESS; > +} > + > +/** > + Install all structures from the DefaultTables structure > + > + @param Smbios SMBIOS protocol > + > +**/ > +EFI_STATUS > +SmbiosInstallAllStructures ( > + IN EFI_SMBIOS_PROTOCOL *Smbios > + ) > +{ > + EFI_STATUS Status; > + > + // > + // Fixup Firmware Revision, CPU and DRAM frequencies. Fix up how? > + // > + mArmadaDefaultType0.SystemBiosMajorRelease = (PcdGet32 (PcdFirmwareRevision) >> 16) & 0xFF; > + mArmadaDefaultType0.SystemBiosMinorRelease = PcdGet32 (PcdFirmwareRevision) & 0xFF; Putting the firmware revision in a temporary variable would clean up the above slightly (and make it possible to respect line length). / Leif > + mArmadaDefaultType4.CurrentSpeed = SampleAtResetGetCpuFrequency (); > + mArmadaDefaultType17.Speed = SampleAtResetGetDramFrequency (); > + > + // > + // Generate memory descriptors. > + // > + Status = SmbiosMemoryInstall (Smbios); > + ASSERT_EFI_ERROR (Status); > + > + // > + // Install all tables. > + // > + Status = SmbiosInstallStructures (Smbios, DefaultCommonTables); > + ASSERT_EFI_ERROR (Status); > + > + return EFI_SUCCESS; > +} > + > +/** > + Installs SMBIOS information for Armada platforms > + > + @param ImageHandle Module's image handle > + @param SystemTable Pointer of EFI_SYSTEM_TABLE > + > + @retval EFI_SUCCESS Smbios data successfully installed > + @retval Other Smbios data was not installed > + > +**/ > +EFI_STATUS > +EFIAPI > +SmbiosPlatformDriverEntryPoint ( > + IN EFI_HANDLE ImageHandle, > + IN EFI_SYSTEM_TABLE *SystemTable > + ) > +{ > + EFI_STATUS Status; > + EFI_SMBIOS_PROTOCOL *Smbios; > + > + // > + // Find the SMBIOS protocol > + // > + Status = gBS->LocateProtocol (&gEfiSmbiosProtocolGuid, > + NULL, > + (VOID **)&Smbios); > + if (EFI_ERROR (Status)) { > + return Status; > + } > + > + Status = SmbiosInstallAllStructures (Smbios); > + > + return Status; > +} > -- > 2.7.4 >