From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from szxga08-in.huawei.com (szxga08-in.huawei.com [45.249.212.255]) by mx.groups.io with SMTP id smtpd.web08.40962.1624238630323083459 for ; Sun, 20 Jun 2021 18:23:51 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: huawei.com, ip: 45.249.212.255, mailfrom: xiewenyi2@huawei.com) Received: from dggemv704-chm.china.huawei.com (unknown [172.30.72.56]) by szxga08-in.huawei.com (SkyGuard) with ESMTP id 4G7WrH476bz1BPsN; Mon, 21 Jun 2021 09:18:39 +0800 (CST) Received: from dggpemm000003.china.huawei.com (7.185.36.128) by dggemv704-chm.china.huawei.com (10.3.19.47) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2176.2; Mon, 21 Jun 2021 09:23:46 +0800 Received: from [10.174.154.21] (10.174.154.21) by dggpemm000003.china.huawei.com (7.185.36.128) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2176.2; Mon, 21 Jun 2021 09:23:46 +0800 Subject: Re: [PATCH v2 1/1] ArmPkg: Move cache defs used in Universal/Smbios into ArmCache.h To: Leif Lindholm , Rebecca Cran CC: Ard Biesheuvel , References: <20210614185749.12474-1-rebecca@nuviainc.com> <20210618155338.b4kufjiydlsot74k@leviathan> <20210618155426.bb42gb3hl2h4uszw@leviathan> From: "wenyi,xie" Message-ID: <40fdb6a5-8e94-5e57-6f28-dc0d9b14ef01@huawei.com> Date: Mon, 21 Jun 2021 09:23:40 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.0.1 MIME-Version: 1.0 In-Reply-To: <20210618155426.bb42gb3hl2h4uszw@leviathan> X-Originating-IP: [10.174.154.21] X-ClientProxiedBy: dggems702-chm.china.huawei.com (10.3.19.179) To dggpemm000003.china.huawei.com (7.185.36.128) X-CFilter-Loop: Reflected Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: quoted-printable OK=EF=BC=8CI will handle this later. Thanks Wenyi On 2021/6/18 23:54, Leif Lindholm wrote: > Agh. *Actually* cc Wenyi. >=20 > On Fri, Jun 18, 2021 at 16:53:38 +0100, Leif Lindholm wrote: >> +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 outsi= de >>> of ArmLib, in Universal/Smbios. Move them into ArmCache.h to make the= m >>> 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/SmbiosProcessorArmCommo= n.c | 2 +- >>> 7 files changed, 148 insertions(+), 131 deletions(-) >>> >>> diff --git a/ArmPkg/Include/IndustryStandard/ArmCache.h b/ArmPkg/Incl= ude/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) regis= ter >>> +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 =3D 0, >>> + /// Select the instruction cache >>> + CsselrCacheTypeInstruction, >>> + CsselrCacheTypeMax >>> +} CSSELR_CACHE_TYPE; >>> + >>> +/// Defines the structure of the CCSIDR (Current Cache Size ID) regi= ster >>> +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 cac= he -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 cac= he -1 >>> + UINT64 Reserved2 :8; ///< Reserved, RES0 >>> + } BitsCcidxAA64; ///< Bitfield definition of the register when FEA= T_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 cac= he - 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 Shareabl= e >>> + 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 =3D 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 @@ >>> =20 >>> 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.
>>> =20 >>> SPDX-License-Identifier: BSD-2-Clause-Patent >>> =20 >>> @@ -109,9 +109,37 @@ typedef enum { >>> #define GET_MPID(ClusterId, CoreId) (((ClusterId) << 8) | (CoreId)= ) >>> #define PRIMARY_CORE_ID (PcdGet32(PcdArmPrimaryCore) & ARM_COR= E_MASK) >>> =20 >>> -// 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 mod= e. >>> +**/ >>> +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 cac= he. >>> +**/ >>> +UINT32 >>> +ReadCCSIDR2 ( >>> + IN UINT32 CSSELR >>> + ); >>> + >>> +/** Reads the Cache Level ID (CLIDR) register. >>> + >>> + @return The contents of the CLIDR_EL1 register. >>> +**/ >>> +UINT32 >>> +ReadCLIDR ( >>> + VOID >>> + ); >>> =20 >>> UINTN >>> EFIAPI >>> diff --git a/ArmPkg/Library/ArmLib/ArmLibPrivate.h b/ArmPkg/Library/A= rmLib/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) >>> =20 >>> - >>> -/// Defines the structure of the CSSELR (Cache Size Selection) regis= ter >>> -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 =3D 0, >>> - /// Select the instruction cache >>> - CsselrCacheTypeInstruction, >>> - CsselrCacheTypeMax >>> -} CSSELR_CACHE_TYPE; >>> - >>> -/// Defines the structure of the CCSIDR (Current Cache Size ID) regi= ster >>> -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 cac= he -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 cac= he -1 >>> - UINT64 Reserved2 :8; ///< Reserved, RES0 >>> - } BitsCcidxAA64; ///< Bitfield definition of the register when FEA= T_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 cac= he - 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 Shareabl= e >>> - 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 =3D 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 >>> ); >>> =20 >>> -/** 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 mod= e. >>> -**/ >>> -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 cac= he. >>> -**/ >>> -UINT32 >>> -ReadCCSIDR2 ( >>> - IN UINT32 CSSELR >>> - ); >>> - >>> -UINT32 >>> -ReadCLIDR ( >>> - VOID >>> - ); >>> - >>> #endif // ARM_LIB_PRIVATE_H_ >>> diff --git a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSu= bClass.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 @@ >>> =20 >>> #include >>> #include >>> +#include >>> #include >>> #include >>> #include >>> #include >>> -#include >>> #include >>> #include >>> #include >>> diff --git a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProce= ssorAArch64.c b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProces= sorAArch64.c >>> index ddd774b16f83..6fbb95afb215 100644 >>> --- a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorAAr= ch64.c >>> +++ b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorAAr= ch64.c >>> @@ -8,8 +8,8 @@ >>> **/ >>> =20 >>> #include >>> +#include >>> #include >>> -#include >>> =20 >>> #include "SmbiosProcessor.h" >>> =20 >>> diff --git a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProce= ssorArm.c b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorA= rm.c >>> index c78bd41a7e06..7616fca425fd 100644 >>> --- a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArm= .c >>> +++ b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArm= .c >>> @@ -8,8 +8,8 @@ >>> **/ >>> =20 >>> #include >>> +#include >>> #include >>> -#include >>> =20 >>> #include "SmbiosProcessor.h" >>> =20 >>> diff --git a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProce= ssorArmCommon.c b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProc= essorArmCommon.c >>> index bccb21cfbb41..292f10bf97eb 100644 >>> --- a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArm= Common.c >>> +++ b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArm= Common.c >>> @@ -8,10 +8,10 @@ >>> **/ >>> =20 >>> #include >>> +#include >>> #include >>> #include >>> #include >>> -#include >>> #include >>> #include >>> =20 >>> --=20 >>> 2.26.2 >>> > . >=20