External email: Use caution opening links or attachments


On 11/17/23 17:37, Ashish Singhal wrote:
>
>
> ------------------------------------------------------------------------
> *From:* Laszlo Ersek <lersek@redhat.com>
> *Sent:* Friday, November 17, 2023 2:20 AM
> *To:* devel@edk2.groups.io <devel@edk2.groups.io>; Ashish Singhal
> <ashishsingha@nvidia.com>; quic_llindhol@quicinc.com
> <quic_llindhol@quicinc.com>; ardb+tianocore@kernel.org
> <ardb+tianocore@kernel.org>; sami.mujawar@arm.com
> <sami.mujawar@arm.com>; Jeff Brasen <jbrasen@nvidia.com>
> *Cc:* Michael Kinney <michael.d.kinney@intel.com>; Liming Gao (Byosoft
> address) <gaoliming@byosoft.com.cn>
> *Subject:* Re: [edk2-devel] [PATCH] DynamicTablesPkg: Fix ETE _UID Creation
>
> External email: Use caution opening links or attachments
>
>
> On 11/15/23 04:19, Ashish Singhal via groups.io wrote:
>> Just like CPU _UID, ETE UID also needs to be unique so
>> use AcpiProcessorUid instead of CpuName
>>
>> Signed-off-by: Ashish Singhal <ashishsingha@nvidia.com>
>> ---
>>  .../Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> Is this a fixup for the recent feature
>
> [PATCH v3 00/11] Update MADT for ACPI 6.5, and add TRBE & ETE support
> https://edk2.groups.io/g/devel/message/108996
> <https://edk2.groups.io/g/devel/message/108996>
>
> ?
>
> If so, then I *think* this qualifies to be merged during the hard
> feature freeze (+Liming +Mike), but:
>
> - I think we should have a "Fixes:" tag in the commit message (for
> pointing out the commit that should have contained the code being
> added/updated now)
>
> - I think we should have a BZ too (also linked into the commit message).
>
> Laszlo
>
> Hello Laszlo,
>
> The issue was indeed introduced in the patch series you pointed to and
> precisely in the commit
> https://github.com/tianocore/edk2/commit/3ee23713e1ce09faa6fa66ee6799e3e336deb58b <https://github.com/tianocore/edk2/commit/3ee23713e1ce09faa6fa66ee6799e3e336deb58b>. This is indeed a bug and should ideally be fixed as soon as possible. Do you need me to file BZ bug and link that in commit message?

A BZ ticket would be great, yes. It helps with the release notes
(determining the contents of a release).

If/when you file the new BZ, please add the old BZ's URL to the See Also
field. ... Oh, wait, the original series didn't have a BZ. That's a pity.

Thanks
Laszlo

>
> Thanks
> Ashish
>
>>
>> diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
>> index 8228c7845a..724f33c660 100644
>> --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
>> +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
>> @@ -359,6 +359,7 @@ CreateAmlCpcNode (
>>
>>    @param [in]  Generator    The SSDT Cpu Topology generator.
>>    @param [in]  ParentNode   Parent node to attach the Cpu node to.
>> +  @param [in]  GicCInfo     CM_ARM_GICC_INFO object used to create the node.
>>    @param [in]  CpuName      Value used to generate the node name.
>>    @param [out] EtNodePtr   If not NULL, return the created Cpu node.
>>
>> @@ -372,6 +373,7 @@ EFIAPI
>>  CreateAmlEtd (
>>    IN   ACPI_CPU_TOPOLOGY_GENERATOR  *Generator,
>>    IN   AML_NODE_HANDLE              ParentNode,
>> +  IN   CM_ARM_GICC_INFO             *GicCInfo,
>>    IN   UINT32                       CpuName,
>>    OUT  AML_OBJECT_NODE_HANDLE       *EtNodePtr OPTIONAL
>>    )
>> @@ -397,7 +399,7 @@ CreateAmlEtd (
>>
>>    Status = AmlCodeGenNameInteger (
>>               "_UID",
>> -             CpuName,
>> +             GicCInfo->AcpiProcessorUid,
>>               EtNode,
>>               NULL
>>               );
>> @@ -474,6 +476,7 @@ CreateAmlEtNode (
>>    Status = CreateAmlEtd (
>>               Generator,
>>               Node,
>> +             GicCInfo,
>>               CpuName,
>>               NULL
>>               );
>