1. Patchew
  2. EDK2
  3. [edk2-devel] [PATCH v2 0/2] ArmPkg,ArmVirtPkg: Add support EFI_MP_SERVICES_PROTOCOL on AARCH64
  4. View patch

[edk2-devel] [PATCH v2 1/2] ArmPkg: Replace CoreId and ClusterId with Mpidr in ARM_CORE_INFO struct

Rebecca Cran posted 2 patches 4 years, 1 month ago
There is a newer version of this series
[edk2-devel] [PATCH v2 1/2] ArmPkg: Replace CoreId and ClusterId with Mpidr in ARM_CORE_INFO struct
Posted by Rebecca Cran 4 years, 1 month ago 
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                                | 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)
 
 /** 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)) {
       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 (#84928): https://edk2.groups.io/g/devel/message/84928
Mute This Topic: https://groups.io/mt/87749223/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2 1/2] ArmPkg: Replace CoreId and ClusterId with Mpidr in ARM_CORE_INFO struct
Posted by Ard Biesheuvel 4 years, 1 month ago 
Hi Rebecca,

Some nits below.

On Wed, 15 Dec 2021 at 18:46, Rebecca Cran <rebecca@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 <rebecca@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/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2 1/2] ArmPkg: Replace CoreId and ClusterId with Mpidr in ARM_CORE_INFO struct
Posted by Rebecca Cran 4 years, 1 month ago 
On 12/15/21 10:53 AM, Ard Biesheuvel wrote:
> On Wed, 15 Dec 2021 at 18:46, Rebecca Cran <rebecca@nuviainc.com> wrote:
>>   #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.

Nope, that was a mistake I made when trying to integrate my changes 
following the Uncrustify work.

>     // 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.


Will do.


-- 
Rebecca Cran



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#84934): https://edk2.groups.io/g/devel/message/84934
Mute This Topic: https://groups.io/mt/87749223/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.