public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Leif Lindholm" <leif@nuviainc.com>
To: Rebecca Cran <rebecca@nuviainc.com>
Cc: devel@edk2.groups.io, "Ard Biesheuvel" <ard.biesheuvel@arm.com>,
	nd@arm.com, "Sami Mujawar" <Sami.Mujawar@arm.com>,
	"Liming Gao" <gaoliming@byosoft.com.cn>,
	"Michael D Kinney" <michael.d.kinney@intel.com>,
	"Zhiguang Liu" <zhiguang.liu@intel.com>,
	"Samer El-Haj-Mahmoud" <Samer.El-Haj-Mahmoud@arm.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>
Subject: Re: [PATCH v7 13/21] ArmPkg: Add Universal/Smbios/ProcessorSubClassDxe
Date: Wed, 3 Feb 2021 15:18:44 +0000	[thread overview]
Message-ID: <20210203151844.GX1664@vanye> (raw)
In-Reply-To: <20210131232511.18340-14-rebecca@nuviainc.com>

I'm afraid I have another few comments on this - but they're pretty
straightforward. If you can address those and then send out a v8 of
just this patch, this set is ready to merge.

On Sun, Jan 31, 2021 at 16:25:03 -0700, Rebecca Cran wrote:
> ProcessorSubClassDxe provides SMBIOS CPU information using generic
> methods combined with calls into OemMiscLib.
> 
> Signed-off-by: Rebecca Cran <rebecca@nuviainc.com>
> Reviewed-by: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
> ---
>  ArmPkg/ArmPkg.dsc                                                         |   2 +
>  ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClassDxe.inf     |  65 ++
>  ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessor.h            | 106 +++
>  ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c          | 766 ++++++++++++++++++++
>  ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorAArch64.c     |  97 +++
>  ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArm.c         | 103 +++
>  ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArmCommon.c   | 250 +++++++
>  ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClassStrings.uni |  24 +
>  8 files changed, 1413 insertions(+)
> 
> diff --git a/ArmPkg/ArmPkg.dsc b/ArmPkg/ArmPkg.dsc
> index 0f77a6da4483..fce86cb6d710 100644
> --- a/ArmPkg/ArmPkg.dsc
> +++ b/ArmPkg/ArmPkg.dsc
> @@ -148,6 +148,8 @@ [Components.common]
>    ArmPkg/Drivers/ArmCrashDumpDxe/ArmCrashDumpDxe.inf
>    ArmPkg/Drivers/ArmScmiDxe/ArmScmiDxe.inf
>  
> +  ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClassDxe.inf
> +
>  [Components.AARCH64]
>    ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
>    ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf
> diff --git a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClassDxe.inf b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClassDxe.inf
> new file mode 100644
> index 000000000000..d74e365375b3
> --- /dev/null
> +++ b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClassDxe.inf
> @@ -0,0 +1,65 @@
> +#/** @file
> +#    ProcessorSubClassDxe.inf
> +#
> +#    Copyright (c) 2021, NUVIA Inc. All rights reserved.
> +#    Copyright (c) 2015, Hisilicon Limited. All rights reserved.
> +#    Copyright (c) 2015, Linaro Limited. All rights reserved.
> +#
> +#    SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +#**/
> +
> +
> +[Defines]
> +  INF_VERSION                    = 1.29
> +  BASE_NAME                      = ProcessorSubClass
> +  FILE_GUID                      = f3fe0e33-ea38-4069-9fb5-be23407207c7
> +  MODULE_TYPE                    = DXE_DRIVER
> +  VERSION_STRING                 = 1.0
> +  ENTRY_POINT                    = ProcessorSubClassEntryPoint
> +
> +[Sources]
> +  SmbiosProcessorArmCommon.c
> +  ProcessorSubClass.c
> +  ProcessorSubClassStrings.uni
> +
> +[Sources.AARCH64]
> +  SmbiosProcessorAArch64.c
> +
> +[Sources.ARM]
> +  SmbiosProcessorArm.c
> +
> +[Packages]
> +  ArmPkg/ArmPkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec
> +
> +[LibraryClasses]
> +  ArmLib
> +  ArmSmcLib
> +  BaseLib
> +  BaseMemoryLib
> +  DebugLib
> +  HiiLib
> +  IoLib
> +  MemoryAllocationLib
> +  OemMiscLib
> +  PcdLib
> +  PrintLib
> +  UefiDriverEntryPoint
> +
> +[Protocols]
> +  gEfiSmbiosProtocolGuid                       # PROTOCOL ALWAYS_CONSUMED
> +
> +[Pcd]
> +  gArmTokenSpaceGuid.PcdProcessorManufacturer
> +  gArmTokenSpaceGuid.PcdProcessorVersion
> +  gArmTokenSpaceGuid.PcdProcessorSerialNumber
> +  gArmTokenSpaceGuid.PcdProcessorAssetTag
> +  gArmTokenSpaceGuid.PcdProcessorPartNumber
> +
> +[Guids]
> +
> +
> +[Depex]
> +  gEfiSmbiosProtocolGuid
> diff --git a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessor.h b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessor.h
> new file mode 100644
> index 000000000000..002fd6cc142b
> --- /dev/null
> +++ b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessor.h
> @@ -0,0 +1,106 @@
> +/** @file
> +  SMBIOS Processor Related Functions.
> +
> +  Copyright (c) 2021, NUVIA Inc. All rights reserved.<BR>
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#ifndef SMBIOS_PROCESSOR_H_
> +#define SMBIOS_PROCESSOR_H_
> +
> +#include <Uefi.h>
> +#include <IndustryStandard/SmBios.h>
> +
> +/** Returns the maximum cache level implemented by the current CPU.
> +
> +    @return The maximum cache level implemented.
> +**/
> +UINT8
> +SmbiosProcessorGetMaxCacheLevel (
> +  VOID
> +  );
> +
> +/** Returns whether or not the specified cache level has separate I/D caches.
> +
> +    @param CacheLevel The cache level (L1, L2 etc.). Zero based.

Could we make all of these not zero-based?
Since we've detached the architectural requirements from the API, it
would help remove several +1 operations from point of use (at the cost
of, I think, one -1 operation).

> +
> +    @return TRUE if the cache level has separate I/D caches, FALSE otherwise.
> +**/
> +BOOLEAN
> +SmbiosProcessorHasSeparateCaches (
> +  UINT8 CacheLevel
> +  );
> +
> +/** Gets the size of the specified cache.
> +
> +    @param CacheLevel       The cache level (L1, L2 etc.). Zero based.
> +    @param InstructionCache Whether the cache is a dedicated instruction cache.
> +    @param DataCache        Whether the cache is a dedicated data cache.
> +    @param UnifiedCache     Whether the cache is a unified cache.
> +
> +    @return The cache size.
> +**/
> +UINT64
> +SmbiosProcessorGetCacheSize (
> +  IN UINT8   CacheLevel,
> +  IN BOOLEAN InstructionCache,
> +  IN BOOLEAN DataCache,
> +  IN BOOLEAN UnifiedCache
> +  );
> +
> +/** Gets the associativity of the specified cache.
> +
> +    @param CacheLevel       The cache level (L1, L2 etc.). Zero based.
> +    @param InstructionCache Whether the cache is a dedicated instruction cache.
> +    @param DataCache        Whether the cache is a dedicated data cache.
> +    @param UnifiedCache     Whether the cache is a unified cache.
> +
> +    @return The cache associativity.
> +**/
> +UINT32
> +SmbiosProcessorGetCacheAssociativity (
> +  IN UINT8   CacheLevel,
> +  IN BOOLEAN InstructionCache,
> +  IN BOOLEAN DataCache,
> +  IN BOOLEAN UnifiedCache
> +  );
> +
> +/** Returns a value for the Processor ID field that conforms to SMBIOS
> +    requirements.
> +
> +    @return Processor ID.
> +**/
> +UINT64
> +SmbiosGetProcessorId (VOID);
> +
> +/** Returns the external clock frequency.
> +
> +    @return The external CPU clock frequency.
> +**/
> +UINTN
> +SmbiosGetExternalClockFrequency (VOID);
> +
> +/** Returns the SMBIOS ProcessorFamily field value.
> +
> +    @return The value for the ProcessorFamily field.
> +**/
> +UINT8
> +SmbiosGetProcessorFamily (VOID);
> +
> +/** Returns the ProcessorFamily2 field value.
> +
> +    @return The value for the ProcessorFamily2 field.
> +**/
> +UINT16
> +SmbiosGetProcessorFamily2 (VOID);
> +
> +/** Returns the SMBIOS Processor Characteristics.
> +
> +    @return Processor Characteristics bitfield.
> +**/
> +PROCESSOR_CHARACTERISTIC_FLAGS
> +SmbiosGetProcessorCharacteristics (VOID);
> +
> +#endif // SMBIOS_PROCESSOR_H_
> diff --git a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c
> new file mode 100644
> index 000000000000..ca55bc019f96
> --- /dev/null
> +++ b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c
> @@ -0,0 +1,766 @@
> +/** @file
> +  ProcessorSubClass.c
> +
> +  Copyright (c) 2021, NUVIA Inc. All rights reserved.<BR>
> +  Copyright (c) 2015, Hisilicon Limited. All rights reserved.
> +  Copyright (c) 2015, Linaro Limited. All rights reserved.
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Uefi.h>
> +#include <Protocol/Smbios.h>
> +#include <IndustryStandard/ArmStdSmc.h>
> +#include <IndustryStandard/SmBios.h>
> +#include <Library/ArmLib.h>
> +#include <Library/ArmSmcLib.h>
> +#include <Library/ArmLib/ArmLibPrivate.h>
> +#include <Library/BaseLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/HiiLib.h>
> +#include <Library/IoLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/OemMiscLib.h>
> +#include <Library/PcdLib.h>
> +#include <Library/PrintLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/UefiLib.h>
> +
> +#include "SmbiosProcessor.h"
> +
> +extern UINT8 ProcessorSubClassStrings[];
> +
> +#define CACHE_SOCKETED_SHIFT       3
> +#define CACHE_LOCATION_SHIFT       5
> +#define CACHE_ENABLED_SHIFT        7
> +#define CACHE_OPERATION_MODE_SHIFT 8
> +
> +typedef enum {
> +  CacheModeWriteThrough = 0,  ///< Cache is write-through
> +  CacheModeWriteBack,         ///< Cache is write-back
> +  CacheModeVariesWithAddress, ///< Cache mode varies by address
> +  CacheModeUnknown,           ///< Cache mode is unknown
> +  CacheModeMax
> +} CACHE_OPERATION_MODE;
> +
> +typedef enum {
> +  CacheLocationInternal = 0, ///< Cache is internal to the processor
> +  CacheLocationExternal,     ///< Cache is external to the processor
> +  CacheLocationReserved,     ///< Reserved
> +  CacheLocationUnknown,      ///< Cache location is unknown
> +  CacheLocationMax
> +} CACHE_LOCATION;
> +
> +EFI_HII_HANDLE       mHiiHandle;
> +
> +EFI_SMBIOS_PROTOCOL  *mSmbios;
> +
> +SMBIOS_TABLE_TYPE4 mSmbiosProcessorTableTemplate = {
> +    {                         // Hdr
> +      EFI_SMBIOS_TYPE_PROCESSOR_INFORMATION, // Type
> +      sizeof (SMBIOS_TABLE_TYPE4), // Length
> +      0                       // Handle
> +    },
> +    1,                        // Socket
> +    CentralProcessor,         // ProcessorType
> +    ProcessorFamilyIndicatorFamily2, // ProcessorFamily
> +    2,                        // ProcessorManufacture
> +    {                         // ProcessorId
> +      {                       // Signature
> +        0
> +      },
> +      {                       // FeatureFlags
> +        0
> +      }
> +    },
> +    3,                        // ProcessorVersion
> +    {                         // Voltage
> +      0
> +    },
> +    0,                        // ExternalClock
> +    0,                        // MaxSpeed
> +    0,                        // CurrentSpeed
> +    0,                        // Status
> +    ProcessorUpgradeUnknown,  // ProcessorUpgrade
> +    0xFFFF,                   // L1CacheHandle
> +    0xFFFF,                   // L2CacheHandle
> +    0xFFFF,                   // L3CacheHandle
> +    4,                        // SerialNumber
> +    5,                        // AssetTag
> +    6,                        // PartNumber
> +    0,                        // CoreCount
> +    0,                        //EnabledCoreCount
> +    0,                        // ThreadCount
> +    0,                        // ProcessorCharacteristics
> +    ProcessorFamilyARM,       // ProcessorFamily2
> +    0,                        // CoreCount2
> +    0,                        // EnabledCoreCount2
> +    0                         // ThreadCount2
> +};
> +
> +/** Sets the HII variable `StringId` is `Pcd` isn't empty.
> +
> +    @param Pcd       The FixedAtBuild PCD that contains the string to fetch.
> +    @param StringId  The string identifier to set.
> +**/
> +#define SET_HII_STRING_IF_PCD_NOT_EMPTY(Pcd, StringId) \
> +  do { \
> +    CHAR16 *Str; \
> +    Str = (CHAR16*)PcdGetPtr (Pcd); \
> +    if (StrLen (Str) > 0) { \
> +      HiiSetString (mHiiHandle, StringId, Str, NULL); \
> +    } \
> +  } while (0)
> +
> +/** Fetches the specified processor's frequency in Hz.
> +
> +  @param ProcessorNumber The processor number
> +
> +  @return The clock frequency in MHz
> +
> +**/
> +UINT16
> +GetCpuFrequency (
> +  IN  UINT8 ProcessorNumber
> +  )
> +{
> +  return (UINT16)(OemGetCpuFreq (ProcessorNumber) / 1000 / 1000);
> +}
> +
> +/** Gets a description of the specified cache.
> +
> +  @param[in] CacheLevel       Zero-based cache level (e.g. L1 cache is 0).
> +  @param[in] InstructionCache Cache is an instruction cache.
> +  @param[in] DataCache        Cache is a data cache.
> +  @param[in] UnifiedCache     Cache is a unified cache.
> +  @param[out] CacheSocketStr  The description of the specified cache
> +
> +  @return The number of Unicode characters in CacheSocketStr not including the
> +          terminating NUL.
> +**/
> +UINTN
> +GetCacheSocketStr (
> +  IN  UINT8   CacheLevel,
> +  IN  BOOLEAN InstructionCache,
> +  IN  BOOLEAN DataCache,
> +  IN  BOOLEAN UnifiedCache,
> +  OUT CHAR16  *CacheSocketStr
> +  )
> +{
> +  UINTN CacheSocketStrLen;
> +
> +  if (CacheLevel == CpuCacheL1 && InstructionCache) {
> +    CacheSocketStrLen = UnicodeSPrint (
> +                          CacheSocketStr,
> +                          SMBIOS_STRING_MAX_LENGTH - 1,
> +                          L"L%x Instruction Cache",
> +                          CacheLevel + 1);
> +  } else if (CacheLevel == CpuCacheL1 && DataCache) {
> +    CacheSocketStrLen = UnicodeSPrint (CacheSocketStr,
> +                          SMBIOS_STRING_MAX_LENGTH - 1,
> +                          L"L%x Data Cache",
> +                          CacheLevel + 1);
> +  } else {
> +    CacheSocketStrLen = UnicodeSPrint (CacheSocketStr,
> +                          SMBIOS_STRING_MAX_LENGTH - 1,
> +                          L"L%x Cache",
> +                          CacheLevel + 1);
> +  }
> +
> +  return CacheSocketStrLen;
> +}
> +
> +/** Fills in the Type 7 record with the cache architecture information
> +    read from the CPU registers.
> +
> +  @param[in]  CacheLevel       Cache level (e.g. L1, L2).
> +  @param[in]  InstructionCache Cache is an instruction cache.
> +  @param[in]  DataCache        Cache is a data cache.
> +  @param[in]  UnifiedCache     Cache is a unified cache.
> +  @param[out] Type7Record      The Type 7 record to fill in.
> +
> +**/
> +VOID
> +ConfigureCacheArchitectureInformation (
> +  IN     UINT8                CacheLevel,
> +  IN     BOOLEAN              InstructionCache,
> +  IN     BOOLEAN              DataCache,
> +  IN     BOOLEAN              UnifiedCache,
> +  OUT    SMBIOS_TABLE_TYPE7   *Type7Record
> +  )
> +{
> +  UINT8        Associativity;
> +  UINT32       CacheSize32;
> +  UINT16       CacheSize16;
> +  UINT64       CacheSize64;
> +
> +  ASSERT (InstructionCache || DataCache || UnifiedCache);
> +
> +  if (InstructionCache) {
> +    Type7Record->SystemCacheType = CacheTypeInstruction;
> +  } else if (DataCache) {
> +    Type7Record->SystemCacheType = CacheTypeData;
> +  } else if (UnifiedCache) {
> +    Type7Record->SystemCacheType = CacheTypeUnified;
> +  } else {
> +    ASSERT(FALSE);
> +  }
> +
> +  CacheSize64 = SmbiosProcessorGetCacheSize (CacheLevel,
> +                                             InstructionCache,
> +                                             DataCache,
> +                                             UnifiedCache
> +                                             );
> +
> +  Associativity = SmbiosProcessorGetCacheAssociativity (CacheLevel,
> +                                                        InstructionCache,
> +                                                        DataCache,
> +                                                        UnifiedCache
> +                                                        );
> +
> +  CacheSize64 /= 1024; // Minimum granularity is 1K
> +
> +  // Encode the cache size into the format SMBIOS wants
> +  if (CacheSize64 < MAX_INT16) {
> +    CacheSize16 = CacheSize64;
> +    CacheSize32 = CacheSize16;
> +  } else if ((CacheSize64 / 64) < MAX_INT16) {
> +    CacheSize16 = (1 << 15) | (CacheSize64 / 64);
> +    CacheSize32 = CacheSize16;
> +  } else {
> +    if ((CacheSize64 / 1024) <= 2047) {
> +      CacheSize32 = CacheSize64;
> +    } else {
> +      CacheSize32 = (1 << 31) | (CacheSize64 / 64);
> +    }
> +
> +    CacheSize16 = -1;
> +  }
> +
> +  Type7Record->Associativity = Associativity + 1;
> +  Type7Record->MaximumCacheSize = CacheSize16;
> +  Type7Record->InstalledSize = CacheSize16;
> +  Type7Record->MaximumCacheSize2 = CacheSize32;
> +  Type7Record->InstalledSize2 = CacheSize32;
> +
> +  switch (Associativity + 1) {

Similarly, could Associativity become 1-based, to get rid of a bunch
of +1?

> +    case 2:
> +      Type7Record->Associativity = CacheAssociativity2Way;
> +      break;
> +    case 4:
> +      Type7Record->Associativity = CacheAssociativity4Way;
> +      break;
> +    case 8:
> +      Type7Record->Associativity = CacheAssociativity8Way;
> +      break;
> +    case 16:
> +      Type7Record->Associativity = CacheAssociativity16Way;
> +      break;
> +    case 12:
> +      Type7Record->Associativity = CacheAssociativity12Way;
> +      break;

Can we sort these according to the visible number rather than what the
enum happens to translate to?

> +    case 24:
> +      Type7Record->Associativity = CacheAssociativity24Way;
> +      break;
> +    case 32:
> +      Type7Record->Associativity = CacheAssociativity32Way;
> +      break;
> +    case 48:
> +      Type7Record->Associativity = CacheAssociativity48Way;
> +      break;
> +    case 64:
> +      Type7Record->Associativity = CacheAssociativity64Way;
> +      break;
> +    case 20:
> +      Type7Record->Associativity = CacheAssociativity20Way;
> +      break;
> +    default:
> +      Type7Record->Associativity = CacheAssociativityOther;
> +      break;
> +  }
> +
> +  Type7Record->CacheConfiguration = (CacheModeUnknown << CACHE_OPERATION_MODE_SHIFT) |
> +                                    (1 << CACHE_ENABLED_SHIFT) |
> +                                    (CacheLocationUnknown << CACHE_LOCATION_SHIFT) |
> +                                    (0 << CACHE_SOCKETED_SHIFT) |
> +                                    CacheLevel;
> +}
> +
> +
> +/** Allocates and initializes an SMBIOS_TABLE_TYPE7 structure.
> +
> +  @param[in]  CacheLevel       The cache level (L1-L7).
> +  @param[in]  InstructionCache Cache is an instruction cache.
> +  @param[in]  DataCache        Cache is a data cache.
> +  @param[in]  UnifiedCache     Cache is a unified cache.
> +
> +  @return A pointer to the Type 7 structure. Returns NULL on failure.
> +**/
> +SMBIOS_TABLE_TYPE7 *
> +AllocateAndInitCacheInformation (
> +  IN UINT8   CacheLevel,
> +  IN BOOLEAN InstructionCache,
> +  IN BOOLEAN DataCache,
> +  IN BOOLEAN UnifiedCache
> +  )
> +{
> +  SMBIOS_TABLE_TYPE7  *Type7Record;
> +  EFI_STRING          CacheSocketStr;
> +  UINTN               CacheSocketStrLen;
> +  UINTN               StringBufferSize;
> +  CHAR8               *OptionalStrStart;
> +  UINTN               TableSize;
> +
> +  // Allocate and fetch the cache description
> +  StringBufferSize = sizeof (CHAR16) * SMBIOS_STRING_MAX_LENGTH;
> +  CacheSocketStr = AllocateZeroPool (StringBufferSize);
> +  if (CacheSocketStr == NULL) {
> +    return NULL;
> +  }
> +
> +  CacheSocketStrLen = GetCacheSocketStr (CacheLevel,
> +                                         InstructionCache,
> +                                         DataCache,
> +                                         UnifiedCache,
> +                                         CacheSocketStr);
> +
> +  TableSize = sizeof (SMBIOS_TABLE_TYPE7) + CacheSocketStrLen + 1 + 1;
> +  Type7Record = AllocateZeroPool (TableSize);
> +  if (Type7Record == NULL) {
> +    FreePool(CacheSocketStr);
> +    return NULL;
> +  }
> +
> +  Type7Record->Hdr.Type = EFI_SMBIOS_TYPE_CACHE_INFORMATION;
> +  Type7Record->Hdr.Length = sizeof (SMBIOS_TABLE_TYPE7);
> +  Type7Record->Hdr.Handle = SMBIOS_HANDLE_PI_RESERVED;
> +
> +  Type7Record->SocketDesignation = 1;
> +
> +  Type7Record->SupportedSRAMType.Unknown = 1;
> +  Type7Record->CurrentSRAMType.Unknown = 1;
> +  Type7Record->CacheSpeed = 0;
> +  Type7Record->ErrorCorrectionType = CacheErrorUnknown;
> +
> +  OptionalStrStart = (CHAR8 *)(Type7Record + 1);
> +  UnicodeStrToAsciiStrS (CacheSocketStr, OptionalStrStart, CacheSocketStrLen + 1);
> +  FreePool (CacheSocketStr);
> +
> +  return Type7Record;
> +}
> +
> +/**
> +  Add Type 7 SMBIOS Record for Cache Information.
> +
> +  @param[in]    ProcessorIndex      Processor number of specified processor.
> +  @param[out]   L1CacheHandle       Pointer to the handle of the L1 Cache SMBIOS record.
> +  @param[out]   L2CacheHandle       Pointer to the handle of the L2 Cache SMBIOS record.
> +  @param[out]   L3CacheHandle       Pointer to the handle of the L3 Cache SMBIOS record.
> +
> +**/
> +VOID
> +AddSmbiosCacheTypeTable (
> +  IN UINTN                  ProcessorIndex,
> +  OUT EFI_SMBIOS_HANDLE     *L1CacheHandle,
> +  OUT EFI_SMBIOS_HANDLE     *L2CacheHandle,
> +  OUT EFI_SMBIOS_HANDLE     *L3CacheHandle
> +  )
> +{
> +  EFI_STATUS                  Status;
> +  SMBIOS_TABLE_TYPE7          *Type7Record;
> +  EFI_SMBIOS_HANDLE           SmbiosHandle;
> +  UINT8                       CacheLevel;
> +  UINT8                       MaxCacheLevel;
> +  BOOLEAN                     DataCacheType;
> +  BOOLEAN                     SeparateCaches;
> +
> +  Status = EFI_SUCCESS;
> +
> +  MaxCacheLevel = 0;
> +
> +  // See if there's an L1 cache present.
> +  MaxCacheLevel = SmbiosProcessorGetMaxCacheLevel ();
> +
> +  if (MaxCacheLevel < 1) {
> +    return;
> +  }
> +
> +  for (CacheLevel = 0; CacheLevel < MaxCacheLevel; CacheLevel++) {
> +    Type7Record = NULL;
> +
> +    SeparateCaches = SmbiosProcessorHasSeparateCaches (CacheLevel);
> +
> +    // At each level of cache, we can have a single type (unified, instruction or data),
> +    // or two types - separate data and instruction caches. If we have separate
> +    // instruction and data caches, then on the first iteration (CacheSubLevel = 0)
> +    // process the instruction cache.
> +    for (DataCacheType = 0; DataCacheType <= 1; DataCacheType++) {
> +      // If there's no separate data/instruction cache, skip the second iteration
> +      if (DataCacheType > 0 && !SeparateCaches) {
> +        continue;
> +      }
> +
> +      Type7Record = AllocateAndInitCacheInformation (CacheLevel,
> +                                                     DataCacheType == 0 && SeparateCaches,
> +                                                     DataCacheType == 1,
> +                                                     DataCacheType == 0 && !SeparateCaches
> +                                                     );

This is a bit too clever for my poor brain.
Could we pass DataCacheType and SeparateCaches and do the logic
conversion in the function?

It feels like this is still somewhat encoding ARM architectural
behaviour in the API? SMBIOS defines
typedef enum {
  CacheTypeOther       = 0x01,
  CacheTypeUnknown     = 0x02,
  CacheTypeInstruction = 0x03,
  CacheTypeData        = 0x04,
  CacheTypeUnified     = 0x05
} CACHE_TYPE_DATA;

OK, let's worry about that aspect later, but please, move the logic
into the functions.

> +      if (Type7Record == NULL) {
> +        continue;
> +      }
> +
> +      ConfigureCacheArchitectureInformation(CacheLevel,
> +                                            (DataCacheType == 0) && SeparateCaches,
> +                                            DataCacheType == 1,
> +                                            (DataCacheType == 0) && !SeparateCaches,
> +                                            Type7Record
> +                                            );

Same again.

> +
> +      // Allow the platform to fill in other information such as speed, SRAM type etc.
> +      if (!OemGetCacheInformation (ProcessorIndex, CacheLevel,
> +                                   DataCacheType == 0, Type7Record)) {

And again.

> +        continue;
> +      }
> +
> +      SmbiosHandle = SMBIOS_HANDLE_PI_RESERVED;
> +      // Finally, install the table
> +      Status = mSmbios->Add (mSmbios, NULL, &SmbiosHandle,
> +                             (EFI_SMBIOS_TABLE_HEADER *)Type7Record);
> +      if (EFI_ERROR (Status)) {
> +        continue;
> +      }
> +
> +      // Config L1/L2/L3 Cache Handle
> +      switch (CacheLevel) {
> +        case CpuCacheL1:
> +          *L1CacheHandle = SmbiosHandle;
> +          break;
> +        case CpuCacheL2:
> +          *L2CacheHandle = SmbiosHandle;
> +          break;
> +        case CpuCacheL3:
> +          *L3CacheHandle = SmbiosHandle;
> +          break;
> +        default:
> +          break;
> +      }
> +    }
> +  }
> +}
> +
> +/** Allocates a Type 4 Processor Information structure and sets the
> +    strings following the data fields.
> +
> +  @param[out] Type4Record    The Type 4 structure to allocate and initialize
> +  @param[in]  ProcessorIndex The index of the processor socket
> +  @param[in]  Populated      Whether the specified processor socket is
> +                             populated.
> +
> +  @retval EFI_SUCCESS          The Type 4 structure was successfully
> +                               allocated and the strings initialized.
> +  @retval EFI_OUT_OF_RESOURCES Could not allocate memory needed.
> +**/
> +EFI_STATUS
> +AllocateType4AndSetProcessorInformationStrings (
> +  SMBIOS_TABLE_TYPE4 **Type4Record,
> +  UINT8 ProcessorIndex,
> +  BOOLEAN Populated
> +  )
> +{
> +  EFI_STATUS      Status;
> +  EFI_STRING_ID   ProcessorManu;
> +  EFI_STRING_ID   ProcessorVersion;
> +  EFI_STRING_ID   SerialNumber;
> +  EFI_STRING_ID   AssetTag;
> +  EFI_STRING_ID   PartNumber;
> +  EFI_STRING      ProcessorSocketStr;
> +  EFI_STRING      ProcessorManuStr;
> +  EFI_STRING      ProcessorVersionStr;
> +  EFI_STRING      SerialNumberStr;
> +  EFI_STRING      AssetTagStr;
> +  EFI_STRING      PartNumberStr;
> +  CHAR8           *OptionalStrStart;
> +  CHAR8           *StrStart;
> +  UINTN           ProcessorSocketStrLen;
> +  UINTN           ProcessorManuStrLen;
> +  UINTN           ProcessorVersionStrLen;
> +  UINTN           SerialNumberStrLen;
> +  UINTN           AssetTagStrLen;
> +  UINTN           PartNumberStrLen;
> +  UINTN           TotalSize;
> +  UINTN           StringBufferSize;
> +
> +  Status = EFI_SUCCESS;
> +
> +  ProcessorManuStr    = NULL;
> +  ProcessorVersionStr = NULL;
> +  SerialNumberStr     = NULL;
> +  AssetTagStr         = NULL;
> +  PartNumberStr       = NULL;
> +
> +  ProcessorManu       = STRING_TOKEN (STR_PROCESSOR_MANUFACTURE);
> +  ProcessorVersion    = STRING_TOKEN (STR_PROCESSOR_VERSION);
> +  SerialNumber        = STRING_TOKEN (STR_PROCESSOR_SERIAL_NUMBER);
> +  AssetTag            = STRING_TOKEN (STR_PROCESSOR_ASSET_TAG);
> +  PartNumber          = STRING_TOKEN (STR_PROCESSOR_PART_NUMBER);
> +
> +  SET_HII_STRING_IF_PCD_NOT_EMPTY (PcdProcessorManufacturer, ProcessorManu);
> +  SET_HII_STRING_IF_PCD_NOT_EMPTY (PcdProcessorVersion, ProcessorVersion);
> +  SET_HII_STRING_IF_PCD_NOT_EMPTY (PcdProcessorSerialNumber, SerialNumber);
> +  SET_HII_STRING_IF_PCD_NOT_EMPTY (PcdProcessorAssetTag, AssetTag);
> +  SET_HII_STRING_IF_PCD_NOT_EMPTY (PcdProcessorPartNumber, PartNumber);
> +
> +  // Processor Socket Designation
> +  StringBufferSize = sizeof (CHAR16) * SMBIOS_STRING_MAX_LENGTH;
> +  ProcessorSocketStr = AllocateZeroPool (StringBufferSize);
> +  if (ProcessorSocketStr == NULL) {
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  ProcessorSocketStrLen = UnicodeSPrint (ProcessorSocketStr, StringBufferSize,
> +                                         L"CPU%02d", ProcessorIndex + 1);
> +
> +  // Processor Manufacture
> +  ProcessorManuStr = HiiGetPackageString (&gEfiCallerIdGuid, ProcessorManu, NULL);
> +  ProcessorManuStrLen = StrLen (ProcessorManuStr);
> +
> +  // Processor Version
> +  ProcessorVersionStr = HiiGetPackageString (&gEfiCallerIdGuid, ProcessorVersion, NULL);
> +  ProcessorVersionStrLen = StrLen (ProcessorVersionStr);
> +
> +  // Serial Number
> +  SerialNumberStr = HiiGetPackageString (&gEfiCallerIdGuid, SerialNumber, NULL);
> +  SerialNumberStrLen = StrLen (SerialNumberStr);
> +
> +  // Asset Tag
> +  AssetTagStr = HiiGetPackageString (&gEfiCallerIdGuid, AssetTag, NULL);
> +  AssetTagStrLen = StrLen (AssetTagStr);
> +
> +  // Part Number
> +  PartNumberStr = HiiGetPackageString (&gEfiCallerIdGuid, PartNumber, NULL);
> +  PartNumberStrLen = StrLen (PartNumberStr);
> +
> +  TotalSize = sizeof (SMBIOS_TABLE_TYPE4) +
> +              ProcessorSocketStrLen  + 1 +
> +              ProcessorManuStrLen    + 1 +
> +              ProcessorVersionStrLen + 1 +
> +              SerialNumberStrLen     + 1 +
> +              AssetTagStrLen         + 1 +
> +              PartNumberStrLen       + 1 + 1;
> +
> +  *Type4Record = AllocateZeroPool (TotalSize);
> +  if (*Type4Record == NULL) {
> +    Status = EFI_OUT_OF_RESOURCES;
> +    goto Exit;
> +  }
> +
> +  CopyMem (*Type4Record, &mSmbiosProcessorTableTemplate, sizeof (SMBIOS_TABLE_TYPE4));
> +
> +  OptionalStrStart = (CHAR8 *)(*Type4Record + 1);
> +  UnicodeStrToAsciiStrS (
> +    ProcessorSocketStr,
> +    OptionalStrStart,
> +    ProcessorSocketStrLen + 1
> +    );
> +
> +  StrStart = OptionalStrStart + ProcessorSocketStrLen + 1;
> +  UnicodeStrToAsciiStrS (
> +    ProcessorManuStr,
> +    StrStart,
> +    ProcessorManuStrLen + 1
> +    );
> +
> +  StrStart += ProcessorManuStrLen + 1;
> +  UnicodeStrToAsciiStrS (
> +    ProcessorVersionStr,
> +    StrStart,
> +    ProcessorVersionStrLen + 1
> +    );
> +
> +  StrStart += ProcessorVersionStrLen + 1;
> +  UnicodeStrToAsciiStrS (
> +    SerialNumberStr,
> +    StrStart,
> +    SerialNumberStrLen + 1
> +    );
> +
> +  StrStart += SerialNumberStrLen + 1;
> +  UnicodeStrToAsciiStrS (
> +    AssetTagStr,
> +    StrStart,
> +    AssetTagStrLen + 1
> +    );
> +
> +  StrStart += AssetTagStrLen + 1;
> +  UnicodeStrToAsciiStrS (
> +    PartNumberStr,
> +    StrStart,
> +    PartNumberStrLen + 1
> +    );
> +
> +Exit:
> +  FreePool (ProcessorSocketStr);
> +  FreePool (ProcessorManuStr);
> +  FreePool (ProcessorVersionStr);
> +  FreePool (SerialNumberStr);
> +  FreePool (AssetTagStr);
> +  FreePool (PartNumberStr);
> +
> +  return Status;
> +}
> +
> +/**
> +  Add Type 4 SMBIOS Record for Processor Information.
> +
> +  @param[in]    ProcessorIndex     Processor index of specified processor.
> +
> +**/
> +EFI_STATUS
> +AddSmbiosProcessorTypeTable (
> +  IN UINTN                  ProcessorIndex
> +  )
> +{
> +  EFI_STATUS                  Status;
> +  SMBIOS_TABLE_TYPE4          *Type4Record;
> +  EFI_SMBIOS_HANDLE           SmbiosHandle;
> +  EFI_SMBIOS_HANDLE           L1CacheHandle;
> +  EFI_SMBIOS_HANDLE           L2CacheHandle;
> +  EFI_SMBIOS_HANDLE           L3CacheHandle;
> +  UINT8                       *LegacyVoltage;
> +  PROCESSOR_STATUS_DATA       ProcessorStatus;
> +  UINT64                      *ProcessorId;
> +  PROCESSOR_CHARACTERISTIC_FLAGS ProcessorCharacteristics;
> +  OEM_MISC_PROCESSOR_DATA     MiscProcessorData;
> +  BOOLEAN                     SocketPopulated;
> +
> +  Type4Record         = NULL;
> +
> +  MiscProcessorData.Voltage         = 0;
> +  MiscProcessorData.CurrentSpeed    = 0;
> +  MiscProcessorData.CoreCount       = 0;
> +  MiscProcessorData.CoresEnabled    = 0;
> +  MiscProcessorData.ThreadCount     = 0;
> +  MiscProcessorData.MaxSpeed        = 0;
> +  L1CacheHandle       = 0xFFFF;
> +  L2CacheHandle       = 0xFFFF;
> +  L3CacheHandle       = 0xFFFF;
> +
> +  SocketPopulated = OemIsSocketPresent(ProcessorIndex);
> +
> +  Status = AllocateType4AndSetProcessorInformationStrings (
> +             &Type4Record,
> +             ProcessorIndex,
> +             SocketPopulated
> +             );
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  OemGetProcessorInformation (ProcessorIndex,
> +                              &ProcessorStatus,
> +                              (PROCESSOR_CHARACTERISTIC_FLAGS*)
> +                                &Type4Record->ProcessorCharacteristics,
> +                              &MiscProcessorData);
> +
> +  if (SocketPopulated) {
> +    AddSmbiosCacheTypeTable (ProcessorIndex, &L1CacheHandle,
> +                             &L2CacheHandle, &L3CacheHandle);
> +  }
> +
> +  LegacyVoltage = (UINT8*)&Type4Record->Voltage;
> +
> +  *LegacyVoltage                    = MiscProcessorData.Voltage;
> +  Type4Record->CurrentSpeed         = MiscProcessorData.CurrentSpeed;
> +  Type4Record->MaxSpeed             = MiscProcessorData.MaxSpeed;
> +  Type4Record->Status               = ProcessorStatus.Data;
> +  Type4Record->L1CacheHandle        = L1CacheHandle;
> +  Type4Record->L2CacheHandle        = L2CacheHandle;
> +  Type4Record->L3CacheHandle        = L3CacheHandle;
> +  Type4Record->CoreCount            = MiscProcessorData.CoreCount;
> +  Type4Record->CoreCount2           = MiscProcessorData.CoreCount;
> +  Type4Record->EnabledCoreCount     = MiscProcessorData.CoresEnabled;
> +  Type4Record->EnabledCoreCount2    = MiscProcessorData.CoresEnabled;
> +  Type4Record->ThreadCount          = MiscProcessorData.ThreadCount;
> +  Type4Record->ThreadCount2         = MiscProcessorData.ThreadCount;
> +
> +  Type4Record->CurrentSpeed = GetCpuFrequency (ProcessorIndex);
> +  Type4Record->ExternalClock =
> +    (UINT16)(SmbiosGetExternalClockFrequency () / 1000 / 1000);
> +
> +  ProcessorId = (UINT64*)&Type4Record->ProcessorId;
> +  *ProcessorId = SmbiosGetProcessorId ();
> +
> +  ProcessorCharacteristics = SmbiosGetProcessorCharacteristics ();
> +  Type4Record->ProcessorCharacteristics |= *((UINT64*)&ProcessorCharacteristics);
> +
> +  Type4Record->ProcessorFamily = SmbiosGetProcessorFamily ();
> +  Type4Record->ProcessorFamily2 = SmbiosGetProcessorFamily2 ();
> +
> +  SmbiosHandle = SMBIOS_HANDLE_PI_RESERVED;
> +  Status = mSmbios->Add (mSmbios, NULL, &SmbiosHandle,
> +                         (EFI_SMBIOS_TABLE_HEADER *)Type4Record);
> +
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Smbios Type04 Table Log Failed! %r \n",
> +            __FUNCTION__, __LINE__, Status));
> +  }
> +  FreePool (Type4Record);
> +
> +  return Status;
> +}
> +
> +/**
> +   Standard EFI driver point.
> +
> +  @param  ImageHandle     Handle for the image of this driver
> +  @param  SystemTable     Pointer to the EFI System Table
> +
> +  @retval  EFI_SUCCESS    The data was successfully stored.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +ProcessorSubClassEntryPoint(
> +  IN EFI_HANDLE         ImageHandle,
> +  IN EFI_SYSTEM_TABLE   *SystemTable
> +  )
> +{
> +  EFI_STATUS    Status;
> +  UINT32        SocketIndex;
> +
> +  //
> +  // Locate dependent protocols
> +  //
> +  Status = gBS->LocateProtocol (&gEfiSmbiosProtocolGuid, NULL, (VOID**)&mSmbios);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "Could not locate SMBIOS protocol.  %r\n", Status));
> +    return Status;
> +  }
> +
> +  //
> +  // Add our default strings to the HII database. They will be modified later.
> +  //
> +  mHiiHandle = HiiAddPackages (&gEfiCallerIdGuid,
> +                               NULL,
> +                               ProcessorSubClassStrings,
> +                               NULL,
> +                               NULL
> +                              );
> +  if (mHiiHandle == NULL) {
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  //
> +  // Add SMBIOS tables for populated sockets.
> +  //
> +  for (SocketIndex = 0; SocketIndex < OemGetProcessorMaxSockets(); SocketIndex++) {
> +    Status = AddSmbiosProcessorTypeTable (SocketIndex);
> +    if (EFI_ERROR (Status)) {
> +      DEBUG ((DEBUG_ERROR, "Add Processor Type Table Failed!  %r.\n", Status));
> +      return Status;
> +    }
> +  }
> +
> +  return Status;
> +}
> diff --git a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorAArch64.c b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorAArch64.c
> new file mode 100644
> index 000000000000..96f739b2abea
> --- /dev/null
> +++ b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorAArch64.c
> @@ -0,0 +1,97 @@
> +/** @file
> +  Functions for AARCH64 processor information
> +
> +  Copyright (c) 2021, NUVIA Inc. All rights reserved.<BR>
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Uefi.h>
> +#include <Library/ArmLib.h>
> +#include <Library/ArmLib/ArmLibPrivate.h>
> +
> +#include "SmbiosProcessor.h"
> +
> +/** Gets the size of the specified cache.
> +
> +    @param CacheLevel       The cache level (L1, L2 etc.). Zero based.
> +    @param InstructionCache Whether the cache is a dedicated instruction cache.
> +    @param DataCache        Whether the cache is a dedicated data cache.
> +    @param UnifiedCache     Whether the cache is a unified cache.
> +
> +    @return The cache size.
> +**/
> +UINT64
> +SmbiosProcessorGetCacheSize (
> +  IN UINT8   CacheLevel,
> +  IN BOOLEAN InstructionCache,
> +  IN BOOLEAN DataCache,
> +  IN BOOLEAN UnifiedCache
> +)
> +{
> +  CCSIDR_DATA Ccsidr;
> +  CSSELR_DATA Csselr;
> +  BOOLEAN     CcidxSupported;
> +  UINT64      CacheSize;
> +
> +  Csselr.Data = 0;
> +  Csselr.Bits.Level = CacheLevel;
> +  Csselr.Bits.InD = InstructionCache;
> +
> +  Ccsidr.Data = ReadCCSIDR (Csselr.Data);
> +
> +  CcidxSupported = ArmHasCcidx ();
> +
> +  if (CcidxSupported) {
> +    CacheSize = (UINT64)(1 << (Ccsidr.BitsCcidxAA64.LineSize + 4)) *
> +                              (Ccsidr.BitsCcidxAA64.Associativity + 1) *
> +                              (Ccsidr.BitsCcidxAA64.NumSets + 1);
> +  } else {
> +    CacheSize = (1 << (Ccsidr.BitsNonCcidx.LineSize + 4)) *
> +                      (Ccsidr.BitsNonCcidx.Associativity + 1) *
> +                      (Ccsidr.BitsNonCcidx.NumSets + 1);
> +  }
> +
> +  return CacheSize;
> +}
> +
> +/** Gets the associativity of the specified cache.
> +
> +    @param CacheLevel       The cache level (L1, L2 etc.). Zero based.
> +    @param InstructionCache Whether the cache is a dedicated instruction cache.
> +    @param DataCache        Whether the cache is a dedicated data cache.
> +    @param UnifiedCache     Whether the cache is a unified cache.
> +
> +    @return The cache associativity.
> +**/
> +UINT32
> +SmbiosProcessorGetCacheAssociativity (
> +  IN UINT8   CacheLevel,
> +  IN BOOLEAN InstructionCache,
> +  IN BOOLEAN DataCache,
> +  IN BOOLEAN UnifiedCache
> +  )
> +{
> +  CCSIDR_DATA Ccsidr;
> +  CSSELR_DATA Csselr;
> +  BOOLEAN     CcidxSupported;
> +  UINT32      Associativity;
> +
> +  Csselr.Data = 0;
> +  Csselr.Bits.Level = CacheLevel;
> +  Csselr.Bits.InD = (InstructionCache | UnifiedCache);
> +
> +  Ccsidr.Data = ReadCCSIDR (Csselr.Data);
> +
> +  CcidxSupported = ArmHasCcidx ();
> +
> +  if (CcidxSupported) {
> +    Associativity = Ccsidr.BitsCcidxAA64.Associativity;

So I would suggest adding +1 here

> +  } else {
> +    Associativity = Ccsidr.BitsNonCcidx.Associativity;

and here.

> +  }
> +
> +  return Associativity;
> +}
> +
> diff --git a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArm.c b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArm.c
> new file mode 100644
> index 000000000000..5222b71f9f68
> --- /dev/null
> +++ b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArm.c
> @@ -0,0 +1,103 @@
> +/** @file
> +  Functions for ARM processor information
> +
> +  Copyright (c) 2021, NUVIA Inc. All rights reserved.<BR>
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Uefi.h>
> +#include <Library/ArmLib.h>
> +#include <Library/ArmLib/ArmLibPrivate.h>
> +
> +#include "SmbiosProcessor.h"
> +
> +/** Gets the size of the specified cache.
> +
> +    @param CacheLevel       The cache level (L1, L2 etc.). Zero based.
> +    @param InstructionCache Whether the cache is a dedicated instruction cache.
> +    @param DataCache        Whether the cache is a dedicated data cache.
> +    @param UnifiedCache     Whether the cache is a unified cache.
> +
> +    @return The cache size.
> +**/
> +UINT64
> +ArmGetCacheSize (
> +  IN UINT8   CacheLevel,
> +  IN BOOLEAN InstructionCache,
> +  IN BOOLEAN DataCache,
> +  IN BOOLEAN UnifiedCache
> +  )
> +{
> +  CCSIDR_DATA  Ccsidr;
> +  CCSIDR2_DATA Ccsidr2;
> +  CSSELR_DATA  Csselr;
> +  BOOLEAN      CcidxSupported;
> +  UINT64       CacheSize;
> +
> +  // Read the CCSIDR register to get the cache architecture
> +  Csselr.Data = 0;
> +  Csselr.Bits.Level = CacheLevel;
> +  Csselr.Bits.InD = (InstructionCache | UnifiedCache);
> +
> +  Ccsidr.Data = ReadCCSIDR (Csselr.Data);
> +
> +  CcidxSupported = ArmHasCcidx ();
> +
> +  if (CcidxSupported) {
> +    Ccsidr2.Data = ReadCCSIDR2 (Csselr.Data);
> +    CacheSize = (UINT64)(1 << (Ccsidr.BitsCcidxAA32.LineSize + 4)) *
> +                              (Ccsidr.BitsCcidxAA32.Associativity + 1) *
> +                              (Ccsidr2.Bits.NumSets + 1);
> +  } else {
> +    CacheSize = (1 << (Ccsidr.BitsNonCcidx.LineSize + 4)) *
> +                      (Ccsidr.BitsNonCcidx.Associativity + 1) *
> +                      (Ccsidr.BitsNonCcidx.NumSets + 1);
> +  }
> +
> +  return CacheSize;
> +}
> +
> +/** Gets the associativity of the specified cache.
> +
> +    @param CacheLevel       The cache level (L1, L2 etc.). Zero based.
> +    @param InstructionCache Whether the cache is a dedicated instruction cache.
> +    @param DataCache        Whether the cache is a dedicated data cache.
> +    @param UnifiedCache     Whether the cache is a unified cache.
> +
> +    @return The cache associativity.
> +**/
> +UINT32
> +ArmGetCacheAssociativity (
> +  IN UINT8   CacheLevel,
> +  IN BOOLEAN InstructionCache,
> +  IN BOOLEAN DataCache,
> +  IN BOOLEAN UnifiedCache
> +  )
> +{
> +  CCSIDR_DATA  Ccsidr;
> +  CCSIDR2_DATA Ccsidr2;
> +  CSSELR_DATA  Csselr;
> +  BOOLEAN      CcidxSupported;
> +  UINT32       Associativity;
> +
> +  // Read the CCSIDR register to get the cache architecture
> +  Csselr.Data = 0;
> +  Csselr.Bits.Level = CacheLevel;
> +  Csselr.Bits.InD = (InstructionCache | UnifiedCache);
> +
> +  Ccsidr.Data = ReadCCSIDR (Csselr.Data);
> +
> +  CcidxSupported = ArmHasCcidx ();
> +
> +  if (CcidxSupported) {
> +    Ccsidr2.Data = ReadCCSIDR2 (Csselr.Data);
> +    Associativity = Ccsidr.BitsCcidxAA32.Associativity;

...and here.

> +  } else {
> +    Associativity = Ccsidr.BitsNonCcidx.Associativity;

and here.

> +  }
> +
> +  return Associativity;
> +}
> +
> diff --git a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArmCommon.c b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArmCommon.c
> new file mode 100644
> index 000000000000..ed6583c19383
> --- /dev/null
> +++ b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArmCommon.c
> @@ -0,0 +1,250 @@
> +/** @file
> +  Functions for processor information common to ARM and AARCH64.
> +
> +  Copyright (c) 2021, NUVIA Inc. All rights reserved.<BR>
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Uefi.h>
> +#include <IndustryStandard/ArmStdSmc.h>
> +#include <IndustryStandard/SmBios.h>
> +#include <Library/ArmLib.h>
> +#include <Library/ArmLib/ArmLibPrivate.h>
> +#include <Library/ArmSmcLib.h>
> +#include <Library/BaseMemoryLib.h>
> +
> +#include "SmbiosProcessor.h"
> +
> +/** Returns the maximum cache level implemented by the current CPU.
> +
> +    @return The maximum cache level implemented.
> +**/
> +UINT8
> +SmbiosProcessorGetMaxCacheLevel (
> +  VOID
> +  )
> +{
> +  CLIDR_DATA Clidr;
> +  UINT8      CacheLevel;
> +  UINT8      MaxCacheLevel;
> +
> +  MaxCacheLevel = 0;
> +
> +  // Read the CLIDR register to find out what caches are present.
> +  Clidr.Data = ReadCLIDR ();
> +
> +  // Get the cache type for the L1 cache. If it's 0, there are no caches.
> +  if (CLIDR_GET_CACHE_TYPE (Clidr.Data, 0) == ClidrCacheTypeNone) {
> +    return 0;
> +  }
> +
> +  for (CacheLevel = 1; CacheLevel < MAX_ARM_CACHE_LEVEL; CacheLevel++) {
> +    if (CLIDR_GET_CACHE_TYPE (Clidr.Data, CacheLevel) == ClidrCacheTypeNone) {
> +      MaxCacheLevel = CacheLevel;
> +      break;
> +    }
> +  }
> +
> +  return MaxCacheLevel;
> +}
> +
> +/** Returns whether or not the specified cache level has separate I/D caches.
> +
> +    @param CacheLevel The cache level (L1, L2 etc.). Zero based.
> +
> +    @return TRUE if the cache level has separate I/D caches, FALSE otherwise.
> +**/
> +BOOLEAN
> +SmbiosProcessorHasSeparateCaches (
> +  UINT8 CacheLevel
> +  )
> +{
> +  CLIDR_CACHE_TYPE CacheType;
> +  CLIDR_DATA       Clidr;
> +  BOOLEAN          SeparateCaches;
> +
> +  SeparateCaches = FALSE;
> +
> +  Clidr.Data = ReadCLIDR ();
> +
> +  CacheType = CLIDR_GET_CACHE_TYPE (Clidr.Data, CacheLevel);
> +
> +  if (CacheType == ClidrCacheTypeSeparate)
> +  {

Bracket on end of line before.

/
    Leif

> +    SeparateCaches = TRUE;
> +  }
> +
> +  return SeparateCaches;
> +}
> +
> +/** Checks if ther ARM64 SoC ID SMC call is supported
> +
> +    @return Whether the ARM64 SoC ID call is supported.
> +**/
> +BOOLEAN
> +HasSmcArm64SocId (
> +  VOID
> +  )
> +{
> +  ARM_SMC_ARGS                   Args;
> +  INT32                          SmcCallStatus;
> +  BOOLEAN                        Arm64SocIdSupported;
> +
> +  Arm64SocIdSupported = FALSE;
> +
> +  Args.Arg0 = SMCCC_VERSION;
> +  ArmCallSmc (&Args);
> +  SmcCallStatus = (INT32)Args.Arg0;
> +
> +  if (SmcCallStatus < 0 || (SmcCallStatus >> 16) >= 1) {
> +    Args.Arg0 = SMCCC_ARCH_FEATURES;
> +    Args.Arg1 = SMCCC_ARCH_SOC_ID;
> +    ArmCallSmc (&Args);
> +
> +    if (Args.Arg0 >= 0) {
> +      Arm64SocIdSupported = TRUE;
> +    }
> +  }
> +
> +  return Arm64SocIdSupported;
> +}
> +
> +/** Fetches the JEP106 code and SoC Revision.
> +
> +    @param Jep106Code  JEP 106 code.
> +    @param SocRevision SoC revision.
> +
> +    @retval EFI_SUCCESS Succeeded.
> +    @retval EFI_UNSUPPORTED Failed.
> +**/
> +EFI_STATUS
> +SmbiosGetSmcArm64SocId (
> +  OUT INT32 *Jep106Code,
> +  OUT INT32 *SocRevision
> +  )
> +{
> +  ARM_SMC_ARGS  Args;
> +  INT32         SmcCallStatus;
> +  EFI_STATUS    Status;
> +
> +  Status = EFI_SUCCESS;
> +
> +  Args.Arg0 = SMCCC_ARCH_SOC_ID;
> +  Args.Arg1 = 0;
> +  ArmCallSmc (&Args);
> +  SmcCallStatus = (INT32)Args.Arg0;
> +
> +  if (SmcCallStatus >= 0) {
> +    *Jep106Code = (INT32)Args.Arg0;
> +  } else {
> +    Status = EFI_UNSUPPORTED;
> +  }
> +
> +  Args.Arg0 = SMCCC_ARCH_SOC_ID;
> +  Args.Arg1 = 1;
> +  ArmCallSmc (&Args);
> +  SmcCallStatus = (INT32)Args.Arg0;
> +
> +  if (SmcCallStatus >= 0) {
> +    *SocRevision = (INT32)Args.Arg0;
> +  } else {
> +    Status = EFI_UNSUPPORTED;
> +  }
> +
> +  return Status;
> +}
> +
> +/** Returns a value for the Processor ID field that conforms to SMBIOS
> +    requirements.
> +
> +    @return Processor ID.
> +**/
> +UINT64
> +SmbiosGetProcessorId (
> +  VOID
> +  )
> +{
> +  INT32  Jep106Code;
> +  INT32  SocRevision;
> +  UINT64 ProcessorId;
> +
> +  if (HasSmcArm64SocId ()) {
> +    SmbiosGetSmcArm64SocId (&Jep106Code, &SocRevision);
> +    ProcessorId = ((UINT64)Jep106Code << 32) | SocRevision;
> +  } else {
> +    ProcessorId = ArmReadMidr ();
> +  }
> +
> +  return ProcessorId;
> +}
> +
> +/** Returns the external clock frequency.
> +
> +    @return The external clock frequency.
> +**/
> +UINTN
> +SmbiosGetExternalClockFrequency (
> +  VOID
> +  )
> +{
> +  return ArmReadCntFrq ();
> +}
> +
> +/** Returns the SMBIOS ProcessorFamily field value.
> +
> +    @return The value for the ProcessorFamily field.
> +**/
> +UINT8
> +SmbiosGetProcessorFamily (
> +  VOID
> +  )
> +{
> +  return ProcessorFamilyIndicatorFamily2;
> +}
> +
> +/** Returns the ProcessorFamily2 field value.
> +
> +    @return The value for the ProcessorFamily2 field.
> +**/
> +UINT16
> +SmbiosGetProcessorFamily2 (
> +  VOID
> +  )
> +{
> +  UINTN  MainIdRegister;
> +  UINT16 ProcessorFamily2;
> +
> +  MainIdRegister = ArmReadMidr ();
> +
> +  if (((MainIdRegister >> 16) & 0xF) < 8) {
> +    ProcessorFamily2 = ProcessorFamilyARM;
> +  } else {
> +    if (sizeof (VOID*) == 4) {
> +      ProcessorFamily2 = ProcessorFamilyARMv7;
> +    } else {
> +      ProcessorFamily2 = ProcessorFamilyARMv8;
> +    }
> +  }
> +
> +  return ProcessorFamily2;
> +}
> +
> +/** Returns the SMBIOS Processor Characteristics.
> +
> +    @return Processor Characteristics bitfield.
> +**/
> +PROCESSOR_CHARACTERISTIC_FLAGS
> +SmbiosGetProcessorCharacteristics (
> +  VOID
> +  )
> +{
> +  PROCESSOR_CHARACTERISTIC_FLAGS Characteristics;
> +
> +  ZeroMem (&Characteristics, sizeof (Characteristics));
> +
> +  Characteristics.ProcessorArm64SocId = HasSmcArm64SocId ();
> +
> +  return Characteristics;
> +}
> diff --git a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClassStrings.uni b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClassStrings.uni
> new file mode 100644
> index 000000000000..22b3c64d9fe2
> --- /dev/null
> +++ b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClassStrings.uni
> @@ -0,0 +1,24 @@
> +/** @file
> +  SMBIOS Type 4 strings
> +
> +  Copyright (c) 2021, NUVIA Inc. All rights reserved.
> +  Copyright (c) 2015, Hisilicon Limited. All rights reserved.
> +  Copyright (c) 2015, Linaro Limited. All rights reserved.
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +/=#
> +
> +#langdef en-US "English"
> +
> +//
> +// Processor Information
> +//
> +#string STR_PROCESSOR_SOCKET_DESIGNATION    #language en-US  "Not Specified"
> +#string STR_PROCESSOR_MANUFACTURE           #language en-US  "Not Specified"
> +#string STR_PROCESSOR_VERSION               #language en-US  "Not Specified"
> +#string STR_PROCESSOR_SERIAL_NUMBER         #language en-US  "Not Specified"
> +#string STR_PROCESSOR_ASSET_TAG             #language en-US  "Not Specified"
> +#string STR_PROCESSOR_PART_NUMBER           #language en-US  "Not Specified"
> +#string STR_PROCESSOR_UNKNOWN               #language en-US  "Unknown"
> -- 
> 2.26.2
> 

  reply	other threads:[~2021-02-03 15:18 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-31 23:24 [PATCH v7 00/21] ArmPkg,MdePkg: Add Universal/Smbios, and related changes Rebecca Cran
2021-01-31 23:24 ` [PATCH v7 01/21] ArmPkg: Add ARM SMC Architecture functions to ArmStdSmc.h Rebecca Cran
2021-01-31 23:24 ` [PATCH v7 02/21] MdePkg: Update IndustryStandard/SmBios.h with processor status data Rebecca Cran
2021-01-31 23:24 ` [PATCH v7 03/21] ArmPkg: Add register encoding definition for MMFR2 Rebecca Cran
2021-01-31 23:24 ` [PATCH v7 04/21] ArmPkg: Add helper to read the Memory Model Features Register 2 Rebecca Cran
2021-01-31 23:24 ` [PATCH v7 05/21] ArmPkg: Add helper function to read the Memory Model Feature Register 4 Rebecca Cran
2021-01-31 23:24 ` [PATCH v7 06/21] ArmPkg: Fix the return type of the ReadCCSIDR function Rebecca Cran
2021-01-31 23:24 ` [PATCH v7 07/21] ArmPkg: Update ArmLibPrivate.h with cache register definitions Rebecca Cran
2021-01-31 23:24 ` [PATCH v7 08/21] ArmPkg: Add definition of the maximum cache level in ARMv8-A Rebecca Cran
2021-01-31 23:24 ` [PATCH v7 09/21] ArmPkg: Add helper to read CCIDX status Rebecca Cran
2021-01-31 23:25 ` [PATCH v7 10/21] ArmPkg: Add helper to read the CCSIDR2 register Rebecca Cran
2021-01-31 23:25 ` [PATCH v7 11/21] ArmPkg: Add Library/OemMiscLib.h Rebecca Cran
2021-01-31 23:25 ` [PATCH v7 12/21] ArmPkg: Add Universal/Smbios/OemMiscLibNull Rebecca Cran
2021-01-31 23:25 ` [PATCH v7 13/21] ArmPkg: Add Universal/Smbios/ProcessorSubClassDxe Rebecca Cran
2021-02-03 15:18   ` Leif Lindholm [this message]
2021-02-08 14:57     ` Rebecca Cran
2021-01-31 23:25 ` [PATCH v7 14/21] ArmPkg: Add Universal/Smbios/SmbiosMiscDxe/Type00 Rebecca Cran
2021-01-31 23:25 ` [PATCH v7 15/21] ArmPkg: Add Universal/Smbios/SmbiosMiscDxe/Type01 Rebecca Cran
2021-01-31 23:25 ` [PATCH v7 16/21] ArmPkg: Add Universal/Smbios/SmbiosMiscDxe/Type02 Rebecca Cran
2021-01-31 23:25 ` [PATCH v7 17/21] ArmPkg: Add Universal/Smbios/SmbiosMiscDxe/Type03 Rebecca Cran
2021-01-31 23:25 ` [PATCH v7 18/21] ArmPkg: Add Universal/Smbios/SmbiosMiscDxe/Type13 Rebecca Cran
2021-01-31 23:25 ` [PATCH v7 19/21] ArmPkg: Add Universal/Smbios/SmbiosMiscDxe/Type32 Rebecca Cran
2021-01-31 23:25 ` [PATCH v7 20/21] ArmPkg: Add SMBIOS PCDs to ArmPkg.dec Rebecca Cran
2021-01-31 23:25 ` [PATCH v7 21/21] ArmPkg: Add Universal/Smbios/SmbiosMiscDxe Rebecca Cran

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=20210203151844.GX1664@vanye \
    --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