Re: [PATCH v2 14/17] Silicon/Ampere: Update ArmPlatformLib to work with changed ARM_CORE_INFO

["Nhi Pham" <nhi@os.amperecomputing.com>]

On 31/01/2022 19:08, Leif Lindholm wrote:
> On Sun, Jan 30, 2022 at 11:36:51 +0100, Ard Biesheuvel wrote:
>> On Sun, 19 Dec 2021 at 04:36, Nhi Pham <nhi@os.amperecomputing.com> wrote:
>>> Hi Rebecca,
>>>
>>> Leif is merging the rest of Altra port to the edk2-platforms which has
>>> SRAT ACPI table consuming the CPU Core Info table. Therefore, we will
>>> need to fix the SRAT too. I would defer the fix until the Altra port is
>>> fully merged.
>>>
>> This seems to be stalled so to make progress, I am going to merge this.
> Thanks, that was the right call.
>
> Apologies for radio silence, catching up on backlog (including
> remainder of Altra port) now.

Thanks much, Leif.

-Nhi

>
> /
>      Leif
>
>>> On 17/12/2021 05:07, Rebecca Cran wrote:
>>>> The ARM_CORE_INFO struct has been updated so the MPIDR is now a single
>>>> field instead of separate cluster/core fields. Update ArmPlatformLib.
>>>>
>>>> Signed-off-by: Rebecca Cran <rebecca@nuviainc.com>
>>>> ---
>>>>    Silicon/Ampere/AmpereAltraPkg/Library/ArmPlatformLib/ArmPlatformLib.c | 5 ++---
>>>>    1 file changed, 2 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/Silicon/Ampere/AmpereAltraPkg/Library/ArmPlatformLib/ArmPlatformLib.c b/Silicon/Ampere/AmpereAltraPkg/Library/ArmPlatformLib/ArmPlatformLib.c
>>>> index 5b4be0e55516..f2ec923d6f8d 100644
>>>> --- a/Silicon/Ampere/AmpereAltraPkg/Library/ArmPlatformLib/ArmPlatformLib.c
>>>> +++ b/Silicon/Ampere/AmpereAltraPkg/Library/ArmPlatformLib/ArmPlatformLib.c
>>>> @@ -108,9 +108,8 @@ PrePeiCoreGetMpCoreInfo (
>>>>        }
>>>>        SocketId = SOCKET_ID (Index);
>>>>        ClusterId = CLUSTER_ID (Index);
>>>> -    mArmPlatformMpCoreInfoTable[mArmPlatformCoreCount].ClusterId = SocketId;
>>>> -    mArmPlatformMpCoreInfoTable[mArmPlatformCoreCount].CoreId =
>>>> -      (ClusterId << 8) | (Index % PLATFORM_CPU_NUM_CORES_PER_CPM);
>>>> +    mArmPlatformMpCoreInfoTable[mArmPlatformCoreCount].Mpidr = GET_MPID (
>>>> +      SocketId, (ClusterId << 8) | (Index % PLATFORM_CPU_NUM_CORES_PER_CPM));
>>> For Ampere Altra, the correct MPIDR encoding is SocketId << 32 |
>>> ClusterId << 16 | (Index % PLATFORM_CPU_NUM_CORES_PER_CPM) << 8
>>>
>>> It would be the same what
>>> Platform/Ampere/JadePkg/Drivers/AcpiPlatformDxe/AcpiMadt.c (not
>>> available yet - being merged in) is describing.
>>>
>> Feel free to follow up with a patch that changes this into the correct
>> representation, but this patch does not make it less correct than it
>> already is; it just stores the socket ID in the cluster ID field in a
>> different way.