+Wenyi On Mon, Jun 14, 2021 at 12:57:49 -0600, Rebecca Cran wrote: > Many of the cache definitions in ArmLibPrivate.h are being used outside > of ArmLib, in Universal/Smbios. Move them into ArmCache.h to make them > public, and remove the include of ArmLibPrivate.h from files in > Universal/Smbios. > > Signed-off-by: Rebecca Cran <rebe...@nuviainc.com>
Reviewed-by: Leif Lindholm <l...@nuviainc.com> Pushed as a63914d3f603. Thanks! I will note that this change breaks Silicon/Hisilicon/Hi1616 and Silicon/Hisilicon/Hi1620, which use some of the macros moved by this patch. However, I am unable to build these anyway with recent iasl/gcc. Wenyi: can you have a look at making these platforms build? This will now also involve dropping use of Library/ArmLib/ArmLibPrivate.h. / Leif > --- > ArmPkg/Include/IndustryStandard/ArmCache.h | > 112 ++++++++++++++++++ > ArmPkg/Include/Library/ArmLib.h | > 36 +++++- > ArmPkg/Library/ArmLib/ArmLibPrivate.h | > 123 -------------------- > ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c | > 2 +- > ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorAArch64.c | > 2 +- > ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArm.c | > 2 +- > ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArmCommon.c | > 2 +- > 7 files changed, 148 insertions(+), 131 deletions(-) > > diff --git a/ArmPkg/Include/IndustryStandard/ArmCache.h > b/ArmPkg/Include/IndustryStandard/ArmCache.h > new file mode 100644 > index 000000000000..f9de46b5bffd > --- /dev/null > +++ b/ArmPkg/Include/IndustryStandard/ArmCache.h > @@ -0,0 +1,112 @@ > +/** @file > + > + Copyright (c) 2020 - 2021, NUVIA Inc. All rights reserved.<BR> > + > + SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > + > +#ifndef ARM_CACHE_H_ > +#define ARM_CACHE_H_ > + > +#include <Uefi/UefiBaseType.h> > + > +// The ARM Architecture Reference Manual for ARMv8-A defines up > +// to 7 levels of cache, L1 through L7. > +#define MAX_ARM_CACHE_LEVEL 7 > + > +/// Defines the structure of the CSSELR (Cache Size Selection) register > +typedef union { > + struct { > + UINT32 InD :1; ///< Instruction not Data bit > + UINT32 Level :3; ///< Cache level (zero based) > + UINT32 TnD :1; ///< Allocation not Data bit > + UINT32 Reserved :27; ///< Reserved, RES0 > + } Bits; ///< Bitfield definition of the register > + UINT32 Data; ///< The entire 32-bit value > +} CSSELR_DATA; > + > +/// The cache type values for the InD field of the CSSELR register > +typedef enum > +{ > + /// Select the data or unified cache > + CsselrCacheTypeDataOrUnified = 0, > + /// Select the instruction cache > + CsselrCacheTypeInstruction, > + CsselrCacheTypeMax > +} CSSELR_CACHE_TYPE; > + > +/// Defines the structure of the CCSIDR (Current Cache Size ID) register > +typedef union { > + struct { > + UINT64 LineSize :3; ///< Line size (Log2(Num bytes in > cache) - 4) > + UINT64 Associativity :10; ///< Associativity - 1 > + UINT64 NumSets :15; ///< Number of sets in the cache -1 > + UINT64 Unknown :4; ///< Reserved, UNKNOWN > + UINT64 Reserved :32; ///< Reserved, RES0 > + } BitsNonCcidx; ///< Bitfield definition of the register when FEAT_CCIDX > is not supported. > + struct { > + UINT64 LineSize :3; ///< Line size (Log2(Num bytes in > cache) - 4) > + UINT64 Associativity :21; ///< Associativity - 1 > + UINT64 Reserved1 :8; ///< Reserved, RES0 > + UINT64 NumSets :24; ///< Number of sets in the cache -1 > + UINT64 Reserved2 :8; ///< Reserved, RES0 > + } BitsCcidxAA64; ///< Bitfield definition of the register when FEAT_IDX is > supported. > + struct { > + UINT64 LineSize : 3; > + UINT64 Associativity : 21; > + UINT64 Reserved : 8; > + UINT64 Unallocated : 32; > + } BitsCcidxAA32; > + UINT64 Data; ///< The entire 64-bit value > +} CCSIDR_DATA; > + > +/// Defines the structure of the AARCH32 CCSIDR2 register. > +typedef union { > + struct { > + UINT32 NumSets :24; ///< Number of sets in the cache - 1 > + UINT32 Reserved :8; ///< Reserved, RES0 > + } Bits; ///< Bitfield definition of the register > + UINT32 Data; ///< The entire 32-bit value > +} CCSIDR2_DATA; > + > +/** Defines the structure of the CLIDR (Cache Level ID) register. > + * > + * The lower 32 bits are the same for both AARCH32 and AARCH64 > + * so we can use the same structure for both. > +**/ > +typedef union { > + struct { > + UINT32 Ctype1 : 3; ///< Level 1 cache type > + UINT32 Ctype2 : 3; ///< Level 2 cache type > + UINT32 Ctype3 : 3; ///< Level 3 cache type > + UINT32 Ctype4 : 3; ///< Level 4 cache type > + UINT32 Ctype5 : 3; ///< Level 5 cache type > + UINT32 Ctype6 : 3; ///< Level 6 cache type > + UINT32 Ctype7 : 3; ///< Level 7 cache type > + UINT32 LoUIS : 3; ///< Level of Unification Inner Shareable > + UINT32 LoC : 3; ///< Level of Coherency > + UINT32 LoUU : 3; ///< Level of Unification Uniprocessor > + UINT32 Icb : 3; ///< Inner Cache Boundary > + } Bits; ///< Bitfield definition of the register > + UINT32 Data; ///< The entire 32-bit value > +} CLIDR_DATA; > + > +/// The cache types reported in the CLIDR register. > +typedef enum { > + /// No cache is present > + ClidrCacheTypeNone = 0, > + /// There is only an instruction cache > + ClidrCacheTypeInstructionOnly, > + /// There is only a data cache > + ClidrCacheTypeDataOnly, > + /// There are separate data and instruction caches > + ClidrCacheTypeSeparate, > + /// There is a unified cache > + ClidrCacheTypeUnified, > + ClidrCacheTypeMax > +} CLIDR_CACHE_TYPE; > + > +#define CLIDR_GET_CACHE_TYPE(x, level) ((x >> (3 * (level))) & 0b111) > + > +#endif /* ARM_CACHE_H_ */ > diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h > index 5c232d779c83..79ea755777a9 100644 > --- a/ArmPkg/Include/Library/ArmLib.h > +++ b/ArmPkg/Include/Library/ArmLib.h > @@ -2,7 +2,7 @@ > > Copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR> > Copyright (c) 2011 - 2016, ARM Ltd. All rights reserved.<BR> > - Copyright (c) 2020, NUVIA Inc. All rights reserved.<BR> > + Copyright (c) 2020 - 2021, NUVIA Inc. All rights reserved.<BR> > > SPDX-License-Identifier: BSD-2-Clause-Patent > > @@ -109,9 +109,37 @@ typedef enum { > #define GET_MPID(ClusterId, CoreId) (((ClusterId) << 8) | (CoreId)) > #define PRIMARY_CORE_ID (PcdGet32(PcdArmPrimaryCore) & ARM_CORE_MASK) > > -// The ARM Architecture Reference Manual for ARMv8-A defines up > -// to 7 levels of cache, L1 through L7. > -#define MAX_ARM_CACHE_LEVEL 7 > +/** Reads the CCSIDR register for the specified cache. > + > + @param CSSELR The CSSELR cache selection register value. > + > + @return The contents of the CCSIDR_EL1 register for the specified cache, > when in AARCH64 mode. > + Returns the contents of the CCSIDR register in AARCH32 mode. > +**/ > +UINTN > +ReadCCSIDR ( > + IN UINT32 CSSELR > + ); > + > +/** Reads the CCSIDR2 for the specified cache. > + > + @param CSSELR The CSSELR cache selection register value > + > + @return The contents of the CCSIDR2 register for the specified cache. > +**/ > +UINT32 > +ReadCCSIDR2 ( > + IN UINT32 CSSELR > + ); > + > +/** Reads the Cache Level ID (CLIDR) register. > + > + @return The contents of the CLIDR_EL1 register. > +**/ > +UINT32 > +ReadCLIDR ( > + VOID > + ); > > UINTN > EFIAPI > diff --git a/ArmPkg/Library/ArmLib/ArmLibPrivate.h > b/ArmPkg/Library/ArmLib/ArmLibPrivate.h > index 5db83d620bfc..668aefd6a088 100644 > --- a/ArmPkg/Library/ArmLib/ArmLibPrivate.h > +++ b/ArmPkg/Library/ArmLib/ArmLibPrivate.h > @@ -52,101 +52,6 @@ > #define CACHE_ARCHITECTURE_UNIFIED (0UL) > #define CACHE_ARCHITECTURE_SEPARATE (1UL) > > - > -/// Defines the structure of the CSSELR (Cache Size Selection) register > -typedef union { > - struct { > - UINT32 InD :1; ///< Instruction not Data bit > - UINT32 Level :3; ///< Cache level (zero based) > - UINT32 TnD :1; ///< Allocation not Data bit > - UINT32 Reserved :27; ///< Reserved, RES0 > - } Bits; ///< Bitfield definition of the register > - UINT32 Data; ///< The entire 32-bit value > -} CSSELR_DATA; > - > -/// The cache type values for the InD field of the CSSELR register > -typedef enum > -{ > - /// Select the data or unified cache > - CsselrCacheTypeDataOrUnified = 0, > - /// Select the instruction cache > - CsselrCacheTypeInstruction, > - CsselrCacheTypeMax > -} CSSELR_CACHE_TYPE; > - > -/// Defines the structure of the CCSIDR (Current Cache Size ID) register > -typedef union { > - struct { > - UINT64 LineSize :3; ///< Line size (Log2(Num bytes in > cache) - 4) > - UINT64 Associativity :10; ///< Associativity - 1 > - UINT64 NumSets :15; ///< Number of sets in the cache -1 > - UINT64 Unknown :4; ///< Reserved, UNKNOWN > - UINT64 Reserved :32; ///< Reserved, RES0 > - } BitsNonCcidx; ///< Bitfield definition of the register when FEAT_CCIDX > is not supported. > - struct { > - UINT64 LineSize :3; ///< Line size (Log2(Num bytes in > cache) - 4) > - UINT64 Associativity :21; ///< Associativity - 1 > - UINT64 Reserved1 :8; ///< Reserved, RES0 > - UINT64 NumSets :24; ///< Number of sets in the cache -1 > - UINT64 Reserved2 :8; ///< Reserved, RES0 > - } BitsCcidxAA64; ///< Bitfield definition of the register when FEAT_IDX is > supported. > - struct { > - UINT64 LineSize : 3; > - UINT64 Associativity : 21; > - UINT64 Reserved : 8; > - UINT64 Unallocated : 32; > - } BitsCcidxAA32; > - UINT64 Data; ///< The entire 64-bit value > -} CCSIDR_DATA; > - > -/// Defines the structure of the AARCH32 CCSIDR2 register. > -typedef union { > - struct { > - UINT32 NumSets :24; ///< Number of sets in the cache - 1 > - UINT32 Reserved :8; ///< Reserved, RES0 > - } Bits; ///< Bitfield definition of the register > - UINT32 Data; ///< The entire 32-bit value > -} CCSIDR2_DATA; > - > -/** Defines the structure of the CLIDR (Cache Level ID) register. > - * > - * The lower 32 bits are the same for both AARCH32 and AARCH64 > - * so we can use the same structure for both. > -**/ > -typedef union { > - struct { > - UINT32 Ctype1 : 3; ///< Level 1 cache type > - UINT32 Ctype2 : 3; ///< Level 2 cache type > - UINT32 Ctype3 : 3; ///< Level 3 cache type > - UINT32 Ctype4 : 3; ///< Level 4 cache type > - UINT32 Ctype5 : 3; ///< Level 5 cache type > - UINT32 Ctype6 : 3; ///< Level 6 cache type > - UINT32 Ctype7 : 3; ///< Level 7 cache type > - UINT32 LoUIS : 3; ///< Level of Unification Inner Shareable > - UINT32 LoC : 3; ///< Level of Coherency > - UINT32 LoUU : 3; ///< Level of Unification Uniprocessor > - UINT32 Icb : 3; ///< Inner Cache Boundary > - } Bits; ///< Bitfield definition of the register > - UINT32 Data; ///< The entire 32-bit value > -} CLIDR_DATA; > - > -/// The cache types reported in the CLIDR register. > -typedef enum { > - /// No cache is present > - ClidrCacheTypeNone = 0, > - /// There is only an instruction cache > - ClidrCacheTypeInstructionOnly, > - /// There is only a data cache > - ClidrCacheTypeDataOnly, > - /// There are separate data and instruction caches > - ClidrCacheTypeSeparate, > - /// There is a unified cache > - ClidrCacheTypeUnified, > - ClidrCacheTypeMax > -} CLIDR_CACHE_TYPE; > - > -#define CLIDR_GET_CACHE_TYPE(x, level) ((x >> (3 * (level))) & 0b111) > - > VOID > CPSRMaskInsert ( > IN UINT32 Mask, > @@ -158,32 +63,4 @@ CPSRRead ( > VOID > ); > > -/** Reads the CCSIDR register for the specified cache. > - > - @param CSSELR The CSSELR cache selection register value. > - > - @return The contents of the CCSIDR_EL1 register for the specified cache, > when in AARCH64 mode. > - Returns the contents of the CCSIDR register in AARCH32 mode. > -**/ > -UINTN > -ReadCCSIDR ( > - IN UINT32 CSSELR > - ); > - > -/** Reads the CCSIDR2 for the specified cache. > - > - @param CSSELR The CSSELR cache selection register value > - > - @return The contents of the CCSIDR2 register for the specified cache. > -**/ > -UINT32 > -ReadCCSIDR2 ( > - IN UINT32 CSSELR > - ); > - > -UINT32 > -ReadCLIDR ( > - VOID > - ); > - > #endif // ARM_LIB_PRIVATE_H_ > diff --git a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c > b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c > index 0cb56c53975e..fb484086a457 100644 > --- a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c > +++ b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c > @@ -10,11 +10,11 @@ > > #include <Uefi.h> > #include <Protocol/Smbios.h> > +#include <IndustryStandard/ArmCache.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> > diff --git > a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorAArch64.c > b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorAArch64.c > index ddd774b16f83..6fbb95afb215 100644 > --- a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorAArch64.c > +++ b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorAArch64.c > @@ -8,8 +8,8 @@ > **/ > > #include <Uefi.h> > +#include <IndustryStandard/ArmCache.h> > #include <Library/ArmLib.h> > -#include <Library/ArmLib/ArmLibPrivate.h> > > #include "SmbiosProcessor.h" > > diff --git > a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArm.c > b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArm.c > index c78bd41a7e06..7616fca425fd 100644 > --- a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArm.c > +++ b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArm.c > @@ -8,8 +8,8 @@ > **/ > > #include <Uefi.h> > +#include <IndustryStandard/ArmCache.h> > #include <Library/ArmLib.h> > -#include <Library/ArmLib/ArmLibPrivate.h> > > #include "SmbiosProcessor.h" > > diff --git > a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArmCommon.c > b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArmCommon.c > index bccb21cfbb41..292f10bf97eb 100644 > --- a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArmCommon.c > +++ b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArmCommon.c > @@ -8,10 +8,10 @@ > **/ > > #include <Uefi.h> > +#include <IndustryStandard/ArmCache.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> > > -- > 2.26.2 > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#76769): https://edk2.groups.io/g/devel/message/76769 Mute This Topic: https://groups.io/mt/83538484/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-