Re: [edk2-devel] [PATCH] DynamicTablesPkg: Fix ETE _UID Creation

["Ashish Singhal via groups.io" <ashishsingha=nvidia.com@groups.io>]

[-- Attachment #1: Type: text/plain, Size: 3847 bytes --]



________________________________
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

?

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

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
>               );



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111404): https://edk2.groups.io/g/devel/message/111404
Mute This Topic: https://groups.io/mt/102598848/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 7832 bytes --]