From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f53.google.com (mail-wm1-f53.google.com [209.85.128.53]) by mx.groups.io with SMTP id smtpd.web12.9894.1624031622378937233 for ; Fri, 18 Jun 2021 08:53:42 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20150623.gappssmtp.com header.s=20150623 header.b=xB24PK72; spf=pass (domain: nuviainc.com, ip: 209.85.128.53, mailfrom: leif@nuviainc.com) Received: by mail-wm1-f53.google.com with SMTP id y13-20020a1c4b0d0000b02901c20173e165so6093694wma.0 for ; Fri, 18 Jun 2021 08:53:42 -0700 (PDT) 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; bh=GlsH44xYAiIfZuXnE+UE012j2N9t4x2/ofd6+s6lgyg=; b=xB24PK72w79jxRzR8uMLymf/9NrzBVA/hGKbH2FevCJmWzFGCU4upQ1/5xiGFZ+9wM J/TlX23sJgAElhAaPz9ZlJcMLPX/HLBFzV6w8/NqEfNgG2LMWMJ6XhdE/fZC4ZfCkQgk 7Aw4nvLhceqKvl1KRr0LvKz7F+JFKnLg24k/N3q5i9Bm49SNphmB9jHOn6vEVRPTGz9a U6BQZMuC1Co+H2yhaYmqnO+XcRHu3j6XW8ZexurHgGJ0hqc1HHZt0FjBnLQDKPAAcdfU 7rYOUqII5FEf6OOET9RfWM0bfxDokreRrdEJXjXQHf6WWRt0ShCSAaelngspWMA3dkID /FWQ== 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; bh=GlsH44xYAiIfZuXnE+UE012j2N9t4x2/ofd6+s6lgyg=; b=N3OWseMtFy19jnyR49aE0J7eiiFDo/q48UZ9h3I01Ig0T6+V6kYHUk79zy/ad1YPju upjx9W4jgvcOdL7+BskybzIkJHNkAmqQ0aezFT9tfZhgc92L83CViD24jCnTOjXYHoPg Za72fh1ZweYlEMXnO3Efx5Qu+XqfuiEkeKo5kCbTJF0T3nreNns+B6G/aGh2pJbPfICV XJujaGokvzTjIIHsYj/q2tXHtmr8e6qsHB3JZLFTnuVBaHSt8daTs06j6mrDCeQC13IA eKbj/q/l13qz+yWq+9wiPBFU3h0ZHBFe+aIDhtYIG0sBEE9P1/NbmCymWAyx2vXMceyK kbxw== X-Gm-Message-State: AOAM531LmwqF6MHEl46AXej9OvZfAW62+D3EAfv/cqUV6jQjbgIXdPYx 1H1/h7WdYSAybU6S4mtayMCsJg== X-Google-Smtp-Source: ABdhPJxGDpg+bbvCLcjCBzb+68tkdsDEThRu/PecwsP1a5E6MdPDRKuK69brEXh4roYN1D10dcupeQ== X-Received: by 2002:a1c:ed0a:: with SMTP id l10mr12268487wmh.151.1624031620758; Fri, 18 Jun 2021 08:53:40 -0700 (PDT) Return-Path: Received: from leviathan (cpc1-cmbg19-2-0-cust915.5-4.cable.virginm.net. [82.27.183.148]) by smtp.gmail.com with ESMTPSA id w23sm12052269wmi.0.2021.06.18.08.53.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 18 Jun 2021 08:53:39 -0700 (PDT) Date: Fri, 18 Jun 2021 16:53:38 +0100 From: "Leif Lindholm" To: Rebecca Cran Cc: Ard Biesheuvel , devel@edk2.groups.io Subject: Re: [PATCH v2 1/1] ArmPkg: Move cache defs used in Universal/Smbios into ArmCache.h Message-ID: <20210618155338.b4kufjiydlsot74k@leviathan> References: <20210614185749.12474-1-rebecca@nuviainc.com> MIME-Version: 1.0 In-Reply-To: <20210614185749.12474-1-rebecca@nuviainc.com> Content-Type: text/plain; charset=us-ascii Content-Disposition: inline +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 Reviewed-by: Leif Lindholm 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.
> + > + SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > + > +#ifndef ARM_CACHE_H_ > +#define ARM_CACHE_H_ > + > +#include > + > +// 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.
> 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 > > @@ -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 > #include > +#include > #include > #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..6fbb95afb215 100644 > --- a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorAArch64.c > +++ b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorAArch64.c > @@ -8,8 +8,8 @@ > **/ > > #include > +#include > #include > -#include > > #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 > +#include > #include > -#include > > #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 > +#include > #include > #include > #include > -#include > #include > #include > > -- > 2.26.2 >