From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f51.google.com (mail-wm1-f51.google.com [209.85.128.51]) by mx.groups.io with SMTP id smtpd.web11.9776.1624031669928392295 for ; Fri, 18 Jun 2021 08:54:30 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20150623.gappssmtp.com header.s=20150623 header.b=aLx7tnXo; spf=pass (domain: nuviainc.com, ip: 209.85.128.51, mailfrom: leif@nuviainc.com) Received: by mail-wm1-f51.google.com with SMTP id l18-20020a1ced120000b029014c1adff1edso9008039wmh.4 for ; Fri, 18 Jun 2021 08:54:29 -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=yRw/e6XoC+Aqq0M9XZdpHXMVyUGPaIMRlzIgcwrQFQ0=; b=aLx7tnXoKAtMTC0QslwVHQppe3No7+vh1/sXOdkpvdt9pBLsYFzmAJYmBb7azvjgcT y+BpZkv6LQo//hI4K8caL4TeDPGo19hQONer/AG71JKxUWeNUmDskz6INHTluccC4NaS bzbC+tDBAtM/+t0Qm38aKw2BYluNUbgAXQAqI3Bbqwt/DpEJ/Jszsj54MLgAuHxIQ1Cb /qQGxFwVLPjp2+i8/OE7I1ezV4uCtSFxFTPAvexJ59xJSNYrWvMZ3zeU81tbHqISaD/a VFdK5An8yaQgAcA48vsEfBnKp4k+MuBn2+djKin3Vm3u+VgbjeTD8vskdFbHx8gzmXNd xttw== 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=yRw/e6XoC+Aqq0M9XZdpHXMVyUGPaIMRlzIgcwrQFQ0=; b=nmHEc3A/aqO+6SoML0Vrp8Wo44BQY082IY4k/6x+j/VJZipaezGNUBOLj4skikPlRW yCmjgeLDOVW+F/wle3rO8excXFDDNUS4AZbFmvTZcL0Mv0sfpwKjTKlNqyVKCOPPqB8V mvqYsgoZwwpogLnbfPR2d+NLNuIryYjbGqXOyWVBoipoho+QsC1nnRv3lpAKKqbWmBoP +4JzwxlgbuGltVvm/z+uv3G+iozNcYc/66MIQxdVbMtNTJ1gbB0imWlaIQruZLYSuaaA IOR2z9slqSzGamqenw2sZvBpNvfRjz5b7QrNZHgOq4r5FGvHesWcDHKhAkITMdC4FT/i LIgw== X-Gm-Message-State: AOAM5338Rl+HZsK/ZBHqdgYh+CUq7r7r3hplYOqoQG/iHspX+RPlwj46 UOC9ZJRe5EVCAhybINEhuj4JsQ== X-Google-Smtp-Source: ABdhPJwtBTvQrpEHibAr6uJ4IXDbbwzE0Mtl4XRwvE0eJsF9B1kgv/bzlvjjBq2E1Uxq/E6OXpxd7Q== X-Received: by 2002:a1c:f60f:: with SMTP id w15mr12316384wmc.71.1624031668231; Fri, 18 Jun 2021 08:54:28 -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 j34sm7869062wms.7.2021.06.18.08.54.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 18 Jun 2021 08:54:27 -0700 (PDT) Date: Fri, 18 Jun 2021 16:54:26 +0100 From: "Leif Lindholm" To: Rebecca Cran Cc: Ard Biesheuvel , devel@edk2.groups.io, Wenyi Xie Subject: Re: [PATCH v2 1/1] ArmPkg: Move cache defs used in Universal/Smbios into ArmCache.h Message-ID: <20210618155426.bb42gb3hl2h4uszw@leviathan> References: <20210614185749.12474-1-rebecca@nuviainc.com> <20210618155338.b4kufjiydlsot74k@leviathan> MIME-Version: 1.0 In-Reply-To: <20210618155338.b4kufjiydlsot74k@leviathan> Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Agh. *Actually* cc Wenyi. 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 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 > >