Hi Rebecca,
Some nits below.
On Wed, 15 Dec 2021 at 18:46, Rebecca Cran <rebe...@nuviainc.com> wrote:
>
> Remove the ClusterId and CoreId fields in the ARM_CORE_INFO structure in
> favor of a new Mpidr field. Update code in
> ArmPlatformPkg/PrePeiCore/MainMPCore and ArmPlatformPkg/PrePi/MainMPCore.c
> to use the new field and call new macros GET_MPIDR_AFF0 and GET_MPIDR_AFF1
> instead.
>
> Signed-off-by: Rebecca Cran <rebe...@nuviainc.com>
> ---
> ArmPkg/Include/Guid/ArmMpCoreInfo.h | 3 +--
> ArmPkg/Include/Library/ArmLib.h | 10
> +++++++---
> ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.c | 8 ++++----
> ArmPlatformPkg/PrePeiCore/MainMPCore.c | 2 +-
> ArmPlatformPkg/PrePi/MainMPCore.c | 2 +-
> 5 files changed, 14 insertions(+), 11 deletions(-)
>
> diff --git a/ArmPkg/Include/Guid/ArmMpCoreInfo.h
> b/ArmPkg/Include/Guid/ArmMpCoreInfo.h
> index 06f9326ca021..43f0848e78b8 100644
> --- a/ArmPkg/Include/Guid/ArmMpCoreInfo.h
> +++ b/ArmPkg/Include/Guid/ArmMpCoreInfo.h
> @@ -14,8 +14,7 @@
> #define MPIDR_U_BIT_MASK 0x40000000
>
> typedef struct {
> - UINT32 ClusterId;
> - UINT32 CoreId;
> + UINT64 Mpidr;
>
> // MP Core Mailbox
> EFI_PHYSICAL_ADDRESS MailboxSetAddress;
> diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h
> index e4d0476090c7..5287bdfc8684 100644
> --- a/ArmPkg/Include/Library/ArmLib.h
> +++ b/ArmPkg/Include/Library/ArmLib.h
> @@ -108,10 +108,14 @@ typedef enum {
>
> #define ARM_CORE_MASK ARM_CORE_AFF0
> #define ARM_CLUSTER_MASK ARM_CORE_AFF1
> -#define GET_CORE_ID(MpId) ((MpId) & ARM_CORE_MASK)
> -#define GET_CLUSTER_ID(MpId) (((MpId) & ARM_CLUSTER_MASK) >> 8)
> +#define GET_CORE_ID(MpId) ((MpId) & ARM_CORE_MASK)
> +#define GET_CLUSTER_ID(MpId) (((MpId) & ARM_CLUSTER_MASK) >> 8)
> #define GET_MPID(ClusterId, CoreId) (((ClusterId) << 8) | (CoreId))
> -#define PRIMARY_CORE_ID (PcdGet32(PcdArmPrimaryCore) & ARM_CORE_MASK)
> +#define GET_MPIDR_AFF0(MpId) ((MpId) & ARM_CORE_AFF0)
> +#define GET_MPIDR_AFF1(MpId) (((MpId) & ARM_CORE_AFF1) >> 8)
> +#define GET_MPIDR_AFF2(MpId) (((MpId) & ARM_CORE_AFF2) >> 16)
> +#define GET_MPIDR_AFF3(MpId) (((MpId) & ARM_CORE_AFF3) >> 32)
> +#define PRIMARY_CORE_ID (PcdGet32(PcdArmPrimaryCore) & ARM_CORE_MASK)
>
Any reason in particular for these whitespace changes? If not, please
omit them - reviewing changes in unfamiliar code is challenging enough
without having to figure out which hunks actually matter and which
ones don't.
> /** Reads the CCSIDR register for the specified cache.
>
> diff --git a/ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.c
> b/ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.c
> index eeab58805e67..852275f44fc3 100644
> --- a/ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.c
> +++ b/ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.c
> @@ -14,7 +14,7 @@
> ARM_CORE_INFO mArmPlatformNullMpCoreInfoTable[] = {
> {
> // Cluster 0, Core 0
> - 0x0, 0x0,
> + 0x0,
>
> // MP Core MailBox Set/Get/Clear Addresses and Clear Value
> (EFI_PHYSICAL_ADDRESS)0,
> @@ -24,7 +24,7 @@ ARM_CORE_INFO mArmPlatformNullMpCoreInfoTable[] = {
> },
> {
> // Cluster 0, Core 1
> - 0x0, 0x1,
> + 0x1,
>
> // MP Core MailBox Set/Get/Clear Addresses and Clear Value
> (EFI_PHYSICAL_ADDRESS)0,
> @@ -34,7 +34,7 @@ ARM_CORE_INFO mArmPlatformNullMpCoreInfoTable[] = {
> },
> {
> // Cluster 0, Core 2
> - 0x0, 0x2,
> + 0x2,
>
> // MP Core MailBox Set/Get/Clear Addresses and Clear Value
> (EFI_PHYSICAL_ADDRESS)0,
> @@ -44,7 +44,7 @@ ARM_CORE_INFO mArmPlatformNullMpCoreInfoTable[] = {
> },
> {
> // Cluster 0, Core 3
> - 0x0, 0x3,
> + 0x3,
>
> // MP Core MailBox Set/Get/Clear Addresses and Clear Value
> (EFI_PHYSICAL_ADDRESS)0,
> diff --git a/ArmPlatformPkg/PrePeiCore/MainMPCore.c
> b/ArmPlatformPkg/PrePeiCore/MainMPCore.c
> index 0b8e5dfb3f30..e6cb75157053 100644
> --- a/ArmPlatformPkg/PrePeiCore/MainMPCore.c
> +++ b/ArmPlatformPkg/PrePeiCore/MainMPCore.c
> @@ -68,7 +68,7 @@ SecondaryMain (
>
> // Find the core in the ArmCoreTable
> for (Index = 0; Index < ArmCoreCount; Index++) {
> - if ((ArmCoreInfoTable[Index].ClusterId == ClusterId) &&
> (ArmCoreInfoTable[Index].CoreId == CoreId)) {
> + if ((GET_MPIDR_AFF1 (ArmCoreInfoTable[Index].Mpidr) == ClusterId) &&
> (GET_MPIDR_AFF0 (ArmCoreInfoTable[Index].Mpidr) == CoreId)) {
Please rewrap overly long lines.
> break;
> }
> }
> diff --git a/ArmPlatformPkg/PrePi/MainMPCore.c
> b/ArmPlatformPkg/PrePi/MainMPCore.c
> index ce53cea6367c..0433fd0f2e75 100644
> --- a/ArmPlatformPkg/PrePi/MainMPCore.c
> +++ b/ArmPlatformPkg/PrePi/MainMPCore.c
> @@ -67,7 +67,7 @@ SecondaryMain (
>
> // Find the core in the ArmCoreTable
> for (Index = 0; Index < ArmCoreCount; Index++) {
> - if ((ArmCoreInfoTable[Index].ClusterId == ClusterId) &&
> (ArmCoreInfoTable[Index].CoreId == CoreId)) {
> + if ((GET_MPIDR_AFF1 (ArmCoreInfoTable[Index].Mpidr) == ClusterId) &&
> (GET_MPIDR_AFF0 (ArmCoreInfoTable[Index].Mpidr) == CoreId)) {
> break;
> }
> }
> --
> 2.31.1
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#84930): https://edk2.groups.io/g/devel/message/84930
Mute This Topic: https://groups.io/mt/87749223/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
