From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f43.google.com (mail-wm1-f43.google.com [209.85.128.43]) by mx.groups.io with SMTP id smtpd.web09.7311.1613042891428187923 for ; Thu, 11 Feb 2021 03:28:11 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20150623.gappssmtp.com header.s=20150623 header.b=rntlNyhS; spf=pass (domain: nuviainc.com, ip: 209.85.128.43, mailfrom: leif@nuviainc.com) Received: by mail-wm1-f43.google.com with SMTP id o24so5361991wmh.5 for ; Thu, 11 Feb 2021 03:28:11 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nuviainc-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=UOrBFarTQIjP+XNooLwlw1tL6vTN9Mty1AiHx/P5ezU=; b=rntlNyhS1qcE3sQ6s2KQJX4+FXzEQAcXBddrQzlZAqkPsXXNlHLh5IRH7y74o1wfA+ U7fstnfyz5IVI7by39YA+LqrhJxzrTDPq0+ts9YkgbkTal37iW3HhFeR3F4MQ55w2eVN fTnb9NqXJJShCQjvMvIyi09etD0rMKmR5G7WiWtkP6d743IaI8JbeAG9RAvjyT2QIPmv NPs37nfmtthtNxETdcinC/6p+BIqrvBpkopYTWJFaUjb/sVceH7Haqk7MN1ATQv4aPIw mXwEgimAbu87Y66xt4olxgqwYp9sF3AApipntfW/5C8KIONlusa6gHvsyOE2wMNZLtWj cQCQ== 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=UOrBFarTQIjP+XNooLwlw1tL6vTN9Mty1AiHx/P5ezU=; b=huYdR94sjGAf09hD/N18neM9IAQzQDXCZYNfFPrvDvhJA3b3AyEq1eF33gftRIfm9m KZBsWx9NZtdiKSNuePolIppPuthFb8IT6PMuY4DGKB+6GN8ycTDYdYthIDyCsu5no2v+ AfKX1s9rlJkEHaFNTLk9aIIfgo7Kgd8pBjq8W1DvUYqgxSPN4rzD2YBQwZMxh6bXjEoB ZtbAlgd9Hl9VL9GmIua1yHpWrzzGV8TdiuS7jXA5UP+jctTPBqQkf4A9iJhWE1sxfHCp aT9sCvUyrw6glfmvx6IuzY+e60oV3XD/FctwtUFLzHhoSN61mz8Xs6SxNVlfdmN3nsIR M8dg== X-Gm-Message-State: AOAM531ygvz8H0x2gNwwLmpAFwli2nRlTtXPQ1HVexSAmlNSILm6qx53 mhET7faLs0vI6Gxg2Wt9N9vGXg== X-Google-Smtp-Source: ABdhPJy5BZNJYQulP2od0giJvVsptHRj9WX9PqYE39VbD1p+pa00eKBdP569rhkzcy6F8xR9usfgQg== X-Received: by 2002:a7b:c856:: with SMTP id c22mr4968430wml.5.1613042889990; Thu, 11 Feb 2021 03:28:09 -0800 (PST) Return-Path: Received: from vanye (cpc1-cmbg19-2-0-cust915.5-4.cable.virginm.net. [82.27.183.148]) by smtp.gmail.com with ESMTPSA id x13sm9260004wmc.27.2021.02.11.03.28.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 11 Feb 2021 03:28:09 -0800 (PST) Date: Thu, 11 Feb 2021 11:28:07 +0000 From: "Leif Lindholm" To: Rebecca Cran Cc: devel@edk2.groups.io, Ard Biesheuvel , Graeme Gregory , Radoslaw Biernacki , Tanmay Jagdale Subject: Re: [edk2-platforms PATCH 1/1] Platform/Qemu/SbsaQemu: Add SMBIOS tables Message-ID: <20210211112807.GX1664@vanye> References: <20210210032442.32658-1-rebecca@nuviainc.com> MIME-Version: 1.0 In-Reply-To: <20210210032442.32658-1-rebecca@nuviainc.com> User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Tue, Feb 09, 2021 at 20:24:42 -0700, Rebecca Cran wrote: > o Add SMBIOS 3.4.0 tables using ArmPkg/Universal/Smbios. > o Bump the PcdSmbiosVersion PCD from 0x300 to 0x304 to indicate support > for SMBIOS 3.4.0, as is required by SBBR. > o Add an implementation of OemMiscLib that provides the system > information. The serial numbers, asset tags etc. are currently all > fixed strings, to allow fwts to pass without errors. > o Add SMBIOS PCDs to identify the platform. The processor serial > number, asset tag and part number are populated because otherwise > fwts reports errors. > > Signed-off-by: Rebecca Cran > --- > Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.c | 278 ++++++++++++++++++++ > Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.inf | 37 +++ > Platform/Qemu/SbsaQemu/SbsaQemu.dsc | 33 ++- > Platform/Qemu/SbsaQemu/SbsaQemu.fdf | 7 + > 4 files changed, 354 insertions(+), 1 deletion(-) > > diff --git a/Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.c b/Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.c > new file mode 100644 > index 000000000000..e736096dc607 > --- /dev/null > +++ b/Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.c > @@ -0,0 +1,278 @@ > +/** @file > +* OemMiscLib.c > +* > +* Copyright (c) 2021, NUVIA Inc. All rights reserved. > +* Copyright (c) 2020, Linaro Ltd. All rights reserved. > +* > +* SPDX-License-Identifier: BSD-2-Clause-Patent > +* > +**/ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +/** Returns whether the specified processor is present or not. > + > + @param ProcessIndex The processor index to check. > + > + @return TRUE if the processor is present, FALSE otherwise. > +**/ > +BOOLEAN > +OemIsSocketPresent ( > + UINTN ProcessorIndex This gets a bit confusing - mixing socket and processor terminology. > + ) > +{ > + if (ProcessorIndex == 0) { And this means that we only end up creating entries for cpu0, although the sbsa-ref platform defaults to 4. > + return TRUE; > + } > + > + return FALSE; > +} > + > +/** Gets the CPU frequency of the specified processor. > + > + @param ProcessorIndex Index of the processor to get the frequency for. > + > + @return CPU frequency in Hz > +**/ > +UINTN OemGetCpuFreq ( > + UINT8 ProcessorIndex > + ) > +{ > + return 2000000000; // 2 GHz > +} > + > + > +/** Walks through the Device Tree created by Qemu and counts the number > + of CPUs present in it. > + > + Copied from Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c Instead of copying this, could we break it out into a new helper lib? Say Silicon/Qemu/SbsaQemu/Library/FdtHelperLib ? And invoke this in both SbsaQemuAcpiDxe and here? > + > + @return The number of CPUs present. > +**/ > +UINT16 > +CountCpusFromFdt ( > + VOID > +) > +{ > + VOID *DeviceTreeBase; > + INT32 Node; > + INT32 Prev; > + INT32 CpuNode; > + INT32 CpuCount; > + > + DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeBaseAddress); > + ASSERT (DeviceTreeBase != NULL); > + > + // Make sure we have a valid device tree blob > + ASSERT (fdt_check_header (DeviceTreeBase) == 0); > + > + CpuNode = fdt_path_offset (DeviceTreeBase, "/cpus"); > + if (CpuNode <= 0) { > + DEBUG ((DEBUG_ERROR, "Unable to locate /cpus in device tree\n")); > + return 0; > + } > + > + CpuCount = 0; > + > + // Walk through /cpus node and count the number of subnodes. > + // The count of these subnodes corresponds to the number of > + // CPUs created by Qemu. > + Prev = fdt_first_subnode (DeviceTreeBase, CpuNode); > + while (1) { > + CpuCount++; > + Node = fdt_next_subnode (DeviceTreeBase, Prev); > + if (Node < 0) { > + break; > + } > + Prev = Node; > + } > + > + return CpuCount; > +} > + > +/** Gets information about the specified processor and stores it in > + the structures provided. > + > + @param ProcessorIndex Index of the processor to get the information for. > + @param ProcessorStatus Processor status. > + @param ProcessorCharacteristics Processor characteritics. > + @param MiscProcessorData Miscellaneous processor information. > + > + @return TRUE on success, FALSE on failure. > +**/ > +BOOLEAN > +OemGetProcessorInformation ( > + IN UINTN ProcessorIndex, > + IN OUT PROCESSOR_STATUS_DATA *ProcessorStatus, > + IN OUT PROCESSOR_CHARACTERISTIC_FLAGS *ProcessorCharacteristics, > + IN OUT OEM_MISC_PROCESSOR_DATA *MiscProcessorData > + ) > +{ > + UINT16 CoreCount = CountCpusFromFdt (); > + > + if (ProcessorIndex == 0) { > + ProcessorStatus->Bits.CpuStatus = 1; // CPU enabled > + ProcessorStatus->Bits.Reserved1 = 0; > + ProcessorStatus->Bits.SocketPopulated = 1; > + ProcessorStatus->Bits.Reserved2 = 0; > + } else { > + ProcessorStatus->Bits.CpuStatus = 0; // CPU disabled > + ProcessorStatus->Bits.Reserved1 = 0; > + ProcessorStatus->Bits.SocketPopulated = 0; > + ProcessorStatus->Bits.Reserved2 = 0; > + } > + > + ProcessorCharacteristics->ProcessorReserved1 = 0; > + ProcessorCharacteristics->ProcessorUnknown = 0; > + ProcessorCharacteristics->Processor64BitCapable = 1; > + ProcessorCharacteristics->ProcessorMultiCore = 1; > + ProcessorCharacteristics->ProcessorHardwareThread = 0; > + ProcessorCharacteristics->ProcessorExecuteProtection = 1; > + ProcessorCharacteristics->ProcessorEnhancedVirtualization = 0; > + ProcessorCharacteristics->ProcessorPowerPerformanceCtrl = 0; > + ProcessorCharacteristics->Processor128BitCapable = 0; > + ProcessorCharacteristics->ProcessorArm64SocId = 1; > + ProcessorCharacteristics->ProcessorReserved2 = 0; > + > + MiscProcessorData->CurrentSpeed = 2000; > + MiscProcessorData->MaxSpeed = 2000; > + MiscProcessorData->CoreCount = CoreCount; > + MiscProcessorData->CoresEnabled = CoreCount; > + MiscProcessorData->ThreadCount = 1; > + > + return TRUE; > +} > + > +/** Gets the maximum number of sockets supported by the platform. > + > + @return The maximum number of sockets. > +**/ > +UINT8 > +OemGetProcessorMaxSockets ( > + VOID > + ) > +{ > + return 1; This ought to be a Pcd. Or a global variable set from FDT. At some point we will want to support multi-socket configurations. > +} > + > +/** Gets information about the cache at the specified cache level. > + > + @param ProcessorIndex The processor to get information for. > + @param CacheLevel The cache level to get information for. > + @param DataCache Whether the cache is a data cache. > + @param UnifiedCache Whether the cache is a unified cache. > + @param SmbiosCacheTable The SMBIOS Type7 cache information structure. > + > + @return TRUE on success, FALSE on failure. > +**/ > +EFIAPI > +BOOLEAN > +OemGetCacheInformation ( > + IN UINT8 ProcessorIndex, > + IN UINT8 CacheLevel, > + IN BOOLEAN DataCache, > + IN BOOLEAN UnifiedCache, > + IN OUT SMBIOS_TABLE_TYPE7 *SmbiosCacheTable > + ) > +{ > + SmbiosCacheTable->CacheConfiguration = CacheLevel - 1; > + > + if (CacheLevel == 1 && !DataCache && !UnifiedCache) { > + // Unknown operational mode > + SmbiosCacheTable->CacheConfiguration |= (3 << 8); > + } else { > + // Write back operational mode > + SmbiosCacheTable->CacheConfiguration |= (1 << 8); > + } > + > + return TRUE; > +} > + > +/** Gets the type of chassis for the system. > + > + @param ChassisType The type of the chassis. > + > + @retval EFI_SUCCESS The chassis type was fetched successfully. > +**/ > +EFI_STATUS > +EFIAPI > +OemGetChassisType ( > + UINT8 *ChassisType > + ) > +{ > + *ChassisType = MiscChassisTypeUnknown; > + return EFI_SUCCESS; > +} > + > +/** Updates the HII string for the specified field. > + > + @param mHiiHandle The HII handle. > + @param TokenToUpdate The string to update. > + @param Offset The field to get information about. > +**/ > +VOID > +OemUpdateSmbiosInfo ( > + IN EFI_HII_HANDLE mHiiHandle, > + IN EFI_STRING_ID TokenToUpdate, > + IN OEM_MISC_SMBIOS_HII_STRING_FIELD Offset > + ) > +{ > + // These values are fixed for now, but should be configurable via > + // something like an emulated SCP. > + switch (Offset) { > + case SystemManufacturerType01: > + HiiSetString (mHiiHandle, TokenToUpdate, L"QEMU", NULL); These strings would also be good to have as Pcds. / Leif > + break; > + case SerialNumType01: > + HiiSetString (mHiiHandle, TokenToUpdate, L"SN0000", NULL); > + break; > + case SkuNumberType01: > + HiiSetString (mHiiHandle, TokenToUpdate, L"SK0000", NULL); > + break; > + case FamilyType01: > + HiiSetString (mHiiHandle, TokenToUpdate, L"ArmVirt", NULL); > + break; > + case AssertTagType02: > + HiiSetString (mHiiHandle, TokenToUpdate, L"AT0000", NULL); > + break; > + case SerialNumberType02: > + HiiSetString (mHiiHandle, TokenToUpdate, L"SN0000", NULL); > + break; > + case BoardManufacturerType02: > + HiiSetString (mHiiHandle, TokenToUpdate, L"QEMU", NULL); > + break; > + case SkuNumberType02: > + HiiSetString (mHiiHandle, TokenToUpdate, L"SK0000", NULL); > + break; > + case ChassisLocationType02: > + HiiSetString (mHiiHandle, TokenToUpdate, L"Bottom", NULL); > + break; > + case SerialNumberType03: > + HiiSetString (mHiiHandle, TokenToUpdate, L"SN0000", NULL); > + break; > + case VersionType03: > + HiiSetString (mHiiHandle, TokenToUpdate, L"1.0", NULL); > + break; > + case ManufacturerType03: > + HiiSetString (mHiiHandle, TokenToUpdate, L"QEMU", NULL); > + break; > + case AssetTagType03: > + HiiSetString (mHiiHandle, TokenToUpdate, L"AT0000", NULL); > + break; > + case SkuNumberType03: > + HiiSetString (mHiiHandle, TokenToUpdate, L"SK0000", NULL); > + break; > + default: > + break; > + } > +} > + > diff --git a/Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.inf b/Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.inf > new file mode 100644 > index 000000000000..ad6e1453906b > --- /dev/null > +++ b/Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.inf > @@ -0,0 +1,37 @@ > +#/** @file > +# OemMiscLib.inf > +# > +# Copyright (c) 2021, NUVIA Inc. All rights reserved. > +# Copyright (c) 2018, Hisilicon Limited. All rights reserved. > +# Copyright (c) 2018, Linaro Limited. All rights reserved. > +# > +# SPDX-License-Identifier: BSD-2-Clause-Patent > +# > +#**/ > + > +[Defines] > + INF_VERSION = 1.29 > + BASE_NAME = OemMiscLib > + FILE_GUID = 958caf90-9e55-4e2a-86e0-71da21485e2c > + MODULE_TYPE = BASE > + VERSION_STRING = 1.0 > + LIBRARY_CLASS = OemMiscLib > + > +[Sources.common] > + OemMiscLib.c > + > +[Packages] > + ArmPkg/ArmPkg.dec > + EmbeddedPkg/EmbeddedPkg.dec > + MdeModulePkg/MdeModulePkg.dec > + MdePkg/MdePkg.dec > + Silicon/Qemu/SbsaQemu/SbsaQemu.dec > + > +[LibraryClasses] > + BaseMemoryLib > + FdtLib > + IoLib > + PcdLib > + > +[Pcd] > + gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdDeviceTreeBaseAddress > diff --git a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc > index f6af3f9111ee..b178d11127b4 100644 > --- a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc > +++ b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc > @@ -1,4 +1,5 @@ > # > +# Copyright (c) 2021, NUVIA Inc. All rights reserved. > # Copyright (c) 2019, Linaro Limited. All rights reserved. > # > # SPDX-License-Identifier: BSD-2-Clause-Patent > @@ -121,6 +122,8 @@ DEFINE NETWORK_HTTP_BOOT_ENABLE = FALSE > # ARM PL011 UART Driver > PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf > > + OemMiscLib|Platform/Qemu/SbsaQemu/OemMiscLib/OemMiscLib.inf > + > # Debug Support > PeCoffExtraActionLib|ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.inf > DebugAgentLib|MdeModulePkg/Library/DebugAgentLibNull/DebugAgentLibNull.inf > @@ -484,6 +487,23 @@ DEFINE NETWORK_HTTP_BOOT_ENABLE = FALSE > # enumeration to complete before installing ACPI tables. > gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration|FALSE > > + gArmTokenSpaceGuid.PcdSystemProductName|L"QEMU SBSA-REF Machine" > + gArmTokenSpaceGuid.PcdSystemVersion|L"1.0" > + gArmTokenSpaceGuid.PcdBaseBoardManufacturer|L"QEMU" > + gArmTokenSpaceGuid.PcdBaseBoardProductName|L"SBSA-REF" > + gArmTokenSpaceGuid.PcdBaseBoardVersion|L"1.0" > + > + # These values are fixed for now, but should be configurable via > + # something like an emulated SCP. > + gArmTokenSpaceGuid.PcdProcessorManufacturer|L"QEMU" > + gArmTokenSpaceGuid.PcdProcessorVersion|L"arm-virt" > + gArmTokenSpaceGuid.PcdProcessorSerialNumber|L"SN0000" > + gArmTokenSpaceGuid.PcdProcessorAssetTag|L"AT0000" > + gArmTokenSpaceGuid.PcdProcessorPartNumber|L"PN0000" > + > + gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor|L"EFI Development Kit II / SbsaQemu" > + gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"1.0" > + > [PcdsDynamicDefault.common] > gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3 > > @@ -508,9 +528,12 @@ DEFINE NETWORK_HTTP_BOOT_ENABLE = FALSE > # > # SMBIOS entry point version > # > - gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosVersion|0x0300 > + gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosVersion|0x0304 > gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosDocRev|0x0 > > + gArmTokenSpaceGuid.PcdSystemBiosRelease|0x0100 > + gArmTokenSpaceGuid.PcdEmbeddedControllerFirmwareRelease|0x0100 > + > ################################################################################ > # > # Components Section - list of all EDK II Modules needed by this Platform > @@ -668,6 +691,14 @@ DEFINE NETWORK_HTTP_BOOT_ENABLE = FALSE > MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf > MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf > > + # > + # SMBIOS support > + # > + ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClassDxe.inf > + ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf > + EmbeddedPkg/Library/FdtLib/FdtLib.inf > + MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf > + > # > # PCI support > # > diff --git a/Platform/Qemu/SbsaQemu/SbsaQemu.fdf b/Platform/Qemu/SbsaQemu/SbsaQemu.fdf > index 3bcf0bf0040a..c35e3ed44054 100644 > --- a/Platform/Qemu/SbsaQemu/SbsaQemu.fdf > +++ b/Platform/Qemu/SbsaQemu/SbsaQemu.fdf > @@ -236,6 +236,13 @@ READ_LOCK_STATUS = TRUE > INF RuleOverride = ACPITABLE Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf > INF MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf > > + # > + # SMBIOS support > + # > + INF ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClassDxe.inf > + INF ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf > + INF MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf > + > # > # PCI support > # > -- > 2.26.2 >