________________________________
From: Leif Lindholm <quic_llindhol@quicinc.com>
Sent: Wednesday, November 15, 2023 10:20 AM
To: devel@edk2.groups.io <devel@edk2.groups.io>; Ashish Singhal <ashishsingha@nvidia.com>
Cc: ardb+tianocore@kernel.org <ardb+tianocore@kernel.org>; sami.mujawar@arm.com <sami.mujawar@arm.com>; Jeff Brasen <jbrasen@nvidia.com>
Subject: Re: [edk2-devel] [PATCH] DynamicTablesPkg: Fix ETE _UID Creation

External email: Use caution opening links or attachments


On Wed, Nov 15, 2023 at 16:24:46 +0000, Ashish Singhal via groups.io wrote:
> On Tue, Nov 14, 2023 at 20:19:04 -0700, Ashish Singhal 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(-)
> >
> > 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.
>
> Can that replace both uses of CpuName in the function (so it can be
> dropped), or does
>
>   Status = WriteAslName ('E', CpuName, AslName);
>
> have other requirements?
>
> /
>     Leif
>
> Hello Leif,
>
> CPU Name can be more logical, and you may have the same CPU name in
> different clusters for example. _UID however needs to be unique.

Sure, makes sense.
I just dislike functions that take too many arguments, so wanted to
make sure we weren't missing an opportunity to drop one as we were
adding this new one.
Never mind me :)

Thanks,

Leif

No worries at all. Please let me know if you have any other questions before this patch can be accepted.

Thanks
Ashish

> Thanks
> Ashish
>
> >    @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
> >               );
> > --
> > 2.17.1
> >
>
>
> 
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111282): https://edk2.groups.io/g/devel/message/111282
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]
-=-=-=-=-=-=-=-=-=-=-=-