Re: [PATCH 1/2] ArmPkg: Replace CoreId and ClusterId with Mpidr in ARM_CORE_INFO struct

["Sami Mujawar" <sami.mujawar@arm.com>]

Hi Rebecca,

Thank you for this patch.

This patch looks good to me.

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar

On 18/10/2021 04:39 PM, Rebecca Cran 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 <rebecca@nuviainc.com>
> ---
>   ArmPkg/Include/Guid/ArmMpCoreInfo.h                            | 3 +--
>   ArmPkg/Include/Library/ArmLib.h                                | 4 ++++
>   ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.c | 8 ++++----
>   ArmPlatformPkg/PrePeiCore/MainMPCore.c                         | 2 +-
>   ArmPlatformPkg/PrePi/MainMPCore.c                              | 2 +-
>   5 files changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/ArmPkg/Include/Guid/ArmMpCoreInfo.h b/ArmPkg/Include/Guid/ArmMpCoreInfo.h
> index b810767879ae..04c44650ebf5 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 79ea755777a9..d764cc670970 100644
> --- a/ArmPkg/Include/Library/ArmLib.h
> +++ b/ArmPkg/Include/Library/ArmLib.h
> @@ -107,6 +107,10 @@ typedef enum {
>   #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 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)
>
>   /** Reads the CCSIDR register for the specified cache.
> diff --git a/ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.c b/ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.c
> index 1cd9ec30a977..6630cb68452c 100644
> --- a/ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.c
> +++ b/ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.c
> @@ -15,7 +15,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,
> @@ -25,7 +25,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,
> @@ -35,7 +35,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,
> @@ -45,7 +45,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 859f1adf2078..5f8a268bf664 100644
> --- a/ArmPlatformPkg/PrePeiCore/MainMPCore.c
> +++ b/ArmPlatformPkg/PrePeiCore/MainMPCore.c
> @@ -65,7 +65,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;
>       }
>     }
> diff --git a/ArmPlatformPkg/PrePi/MainMPCore.c b/ArmPlatformPkg/PrePi/MainMPCore.c
> index 091464df2aa9..6497a1ad27fd 100644
> --- a/ArmPlatformPkg/PrePi/MainMPCore.c
> +++ b/ArmPlatformPkg/PrePi/MainMPCore.c
> @@ -64,7 +64,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;
>       }
>     }

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.