From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by mx.groups.io with SMTP id smtpd.web10.11475.1623337510999978301 for ; Thu, 10 Jun 2021 08:05:11 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=cpAyAV6k; spf=pass (domain: kernel.org, ip: 198.145.29.99, mailfrom: ardb@kernel.org) Received: by mail.kernel.org (Postfix) with ESMTPSA id AAB43613DD for ; Thu, 10 Jun 2021 15:05:09 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1623337509; bh=tmLukQfmFGuvQzGNJoet3oV/rgF6Mtu3u8UFpKJ1gzI=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=cpAyAV6kwYMV5RZQZk82fI35d3ZSG1tg7Ps5VYEHQ4HWExQOG3gjpjbpVquFxXNVU h9XouDMp5jAiHNX/M+0tWFry10IbsjZRmshfXSvmm6KPrULw8BtVcwMTpA1mOiMMTG hVYfyFwkzymIC1wNNQ+zDeWCx2IbBHWqu598e5Oz6h55ZXyDCfLOyzvEHx1IKCrGic tYQ6sERZ0oINbBLHyoyA/PXfAzt3GWcl4ONtfD3VnCDd59uKVuOSwoAxrZhZI/ZOMg RPF4EsLgVwB2tiGa8fhPKt7EifufwzbWTxcu/z44FwCRrXvSzs8HbOryl90Y6eRl4v +bTXdCrtpzrQg== Received: by mail-ot1-f50.google.com with SMTP id w23-20020a9d5a970000b02903d0ef989477so23381934oth.9 for ; Thu, 10 Jun 2021 08:05:09 -0700 (PDT) X-Gm-Message-State: AOAM533Y+fbw6FGplPH+Xfy5TyGULf8YgzuNrpY/ml/+Lvd5++YcO/fn H3vtBQh9P1dq81qPL1NRdxLxVT82uncv6QwAqYI= X-Google-Smtp-Source: ABdhPJwkArmbpMjGt4DHOhJrQ1ijGDRd/E5QmMBum+/wYslyPQQZx7B+bCVxK0OHArW4VwfVus4tHOcEA8TVCz839B4= X-Received: by 2002:a9d:4004:: with SMTP id m4mr2757131ote.108.1623337508923; Thu, 10 Jun 2021 08:05:08 -0700 (PDT) MIME-Version: 1.0 References: <20210608135432.23420-1-rebecca@nuviainc.com> In-Reply-To: <20210608135432.23420-1-rebecca@nuviainc.com> From: "Ard Biesheuvel" Date: Thu, 10 Jun 2021 17:04:57 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 1/1] ArmPkg: Move cache defs used in Universal/Smbios into ArmLib.h To: Rebecca Cran Cc: edk2-devel-groups-io , Leif Lindholm , Ard Biesheuvel Content-Type: text/plain; charset="UTF-8" On Tue, 8 Jun 2021 at 15:54, Rebecca Cran wrote: > > Many of the cache definitions in ArmLibPrivate.h are being used outside > of ArmLib, in Universal/Smbios. Move them into ArmLib.h to make them > public, and remove the include of ArmLibPrivate.h from files in > Universal/Smbios. > > Signed-off-by: Rebecca Cran Hi Rebecca, If these definitions describe anything more than the software interface exposed by the library, they really belong under IndustryStandard/ not Library. > --- > ArmPkg/Include/Library/ArmLib.h | 128 +++++++++++++++++++- > ArmPkg/Library/ArmLib/ArmLibPrivate.h | 123 ------------------- > ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c | 1 - > ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorAArch64.c | 1 - > ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArm.c | 1 - > ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArmCommon.c | 1 - > 6 files changed, 127 insertions(+), 128 deletions(-) > > diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h > index 5c232d779c83..0052adaa70aa 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.
> Copyright (c) 2011 - 2016, ARM Ltd. All rights reserved.
> - Copyright (c) 2020, NUVIA Inc. All rights reserved.
> + Copyright (c) 2020 - 2021, NUVIA Inc. All rights reserved.
> > SPDX-License-Identifier: BSD-2-Clause-Patent > > @@ -113,6 +113,132 @@ typedef enum { > // 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) > + > +/** 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 > ArmDataCacheLineLength ( > 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..02297871c92c 100644 > --- a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c > +++ b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c > @@ -14,7 +14,6 @@ > #include > #include > #include > -#include > #include > #include > #include > diff --git a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorAArch64.c b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorAArch64.c > index ddd774b16f83..e5309ff598f9 100644 > --- a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorAArch64.c > +++ b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorAArch64.c > @@ -9,7 +9,6 @@ > > #include > #include > -#include > > #include "SmbiosProcessor.h" > > diff --git a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArm.c b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArm.c > index c78bd41a7e06..8176409b24e6 100644 > --- a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArm.c > +++ b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArm.c > @@ -9,7 +9,6 @@ > > #include > #include > -#include > > #include "SmbiosProcessor.h" > > diff --git a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArmCommon.c b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArmCommon.c > index bccb21cfbb41..07e18b3c04ea 100644 > --- a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArmCommon.c > +++ b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArmCommon.c > @@ -11,7 +11,6 @@ > #include > #include > #include > -#include > #include > #include > > -- > 2.26.2 >