From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM12-MW2-obe.outbound.protection.outlook.com (NAM12-MW2-obe.outbound.protection.outlook.com [40.107.244.41]) by mx.groups.io with SMTP id smtpd.web11.18652.1676304636311247762 for ; Mon, 13 Feb 2023 08:10:36 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@nvidia.com header.s=selector2 header.b=lXCVwKSA; spf=permerror, err=parse error for token &{10 18 %{i}._ip.%{h}._ehlo.%{d}._spf.vali.email}: invalid domain name (domain: nvidia.com, ip: 40.107.244.41, mailfrom: jbrasen@nvidia.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=BYofJc23sAe1+F5DvtQNQWD0D4AoETyp5Ni6kPlDrN+JQfx4yAKSrMP1vet4RlFo6u4tZB9ODUs2MI4dNqINN3xLdvjh22Umhs9+KYA+GZTApoqm4F7oqZ2R2sxJqzZS9ef9Zl7rouhz6rpcwbCoQe5qPDrlMptzARePynblNp1buU+1pedhhDRlKojKRGWkrp54NqS9Opy32Oe3QJl6D3+6Ot7f6j8TCGFisZR5utUGtX1mho+Rebc58RAVMG9c7CsLDhfPgPjQ7Ta+TkJPA7+yaaQo2S2sdXYTXEZaczw/hk5xOlcXDJmThU6f6QRu1vr+B2sDDvmcL1vW3wqV4g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=cdmGxPj6Se5Njd5NT1OGJ2N4bIoMVXoabbc1bHgg0PA=; b=Fwo0LobrtjQ1rgG4RkGTPgUb214fKOI4WMEALYQtjJNjG25B/rBrk6NiAj5G7xhXXzoXY5RU26W1nir9VZ1lGpBRqeuNxmUzLaNrfbguH/IiidB1q/G5MwRCpKHiZkGQ0nOKrNVlBO2ouwFywXsqGEX96HMq76SXXyK1+diTgoLWbC5JuzV9SD/LhXWwI8k5hmNI2Bz8UYtsMdjwIdl3UsvZh6QGsTVLvVRtggHhoUi6FXBZGABK7wbRza/qHXxUbDlbzzMPCOAhH9DCGxdWFER4rq5DQHTwsNXRttCjnfeEFXAvwXbjPLpbL6YEuoY0/twXWzVWsavDq0+losJ+gg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=nvidia.com; dmarc=pass action=none header.from=nvidia.com; dkim=pass header.d=nvidia.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Nvidia.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=cdmGxPj6Se5Njd5NT1OGJ2N4bIoMVXoabbc1bHgg0PA=; b=lXCVwKSA9zdXWj7YbbkiPN8wSwH7W1YHgKQ2qX4B/M+fqWwMdopp6SGCQHv2yBG9M83WteEzOgOcvITClYawSmoWHVbbK+HyKXwMvUmqc1L/AkflXGjXK2lY35Z4RvWvmue6W4pfrmK06yh4XWogrj7ilbP3h1vgdRC03d1ynZ4qD3YxVnVZZG+AHZhR0gy/u0VLHrxXtuWMnPe2Uf6SU4lceowxI5vtA3Z+obwO1GZfJ5Eo+BJ7XPlQOvGqCdCh31rwdAey/BQP85w5S6i5IMnuObEbDtuaIdmRgiAnlTMbzPXAC8Fb40xjfrTwn1qAbKr2QAwYB0unuwWjR7sapQ== Received: from DS7PR12MB5789.namprd12.prod.outlook.com (2603:10b6:8:74::21) by PH7PR12MB7819.namprd12.prod.outlook.com (2603:10b6:510:27f::5) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6086.23; Mon, 13 Feb 2023 16:10:33 +0000 Received: from DS7PR12MB5789.namprd12.prod.outlook.com ([fe80::3d2e:1f83:5259:9893]) by DS7PR12MB5789.namprd12.prod.outlook.com ([fe80::3d2e:1f83:5259:9893%3]) with mapi id 15.20.6086.024; Mon, 13 Feb 2023 16:10:33 +0000 From: "Jeff Brasen" To: Pierre Gondois , "devel@edk2.groups.io" CC: "Sami.Mujawar@arm.com" , "Alexei.Fedorov@arm.com" , "quic_llindhol@quicinc.com" , "ardb+tianocore@kernel.org" Subject: Re: [PATCH v2] DynamicTablesPkg: Allow multiple top level physical nodes Thread-Topic: [PATCH v2] DynamicTablesPkg: Allow multiple top level physical nodes Thread-Index: AQHZN/rej1ovjPMb7EWLRPpmsGL7FK7BqokAgAtw2MA= Date: Mon, 13 Feb 2023 16:10:33 +0000 Message-ID: References: <2fbd84095cc52b908f0a59d98358f36a396c319b.1675447806.git.jbrasen@nvidia.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=nvidia.com; x-ms-publictraffictype: Email x-ms-traffictypediagnostic: DS7PR12MB5789:EE_|PH7PR12MB7819:EE_ x-ms-office365-filtering-correlation-id: 759278e2-c6aa-46f6-a6f9-08db0ddcd5db x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: jC+cFGf9EJpTptKjfUUQiP6R+dR++T1z57r6PrNcWD1n9azPqTFWHgP5aE4Aqzz7HSUPYGHr/ZnyrFMp1iaGwedJZy/hf9x9eAl6/DsyNlZdhztLWe8u3vqXxaZau+fT2sXefwptkcmHO/Gcd0lt8LH9WZhgUMBVq0MIxYX/vnzMB6LokYBBzTFGpygW2YYjJcbVo5IL++Puy0lklS16pXV4WfzXL4hXOR41zD5qgje6Yzog5qt4KXykrnKmH+sXYmWR5DBiMqBKyVc9lKI8wzRXUfFex9fffHnEqMQkYxDpykAgGXbnnWTmj+dXrZvktzQ4PC9Kz2aGGhRYGM7UuwYrPe8HTqhxpc/DJlVd0nVri4vLrCy9kDElXVtw1y+BfVVbGGAXtKmQmw1ZUhBLUXOYGOvoOzFJw0JD+sln83huEhksgEXG2aLTdorhQ7UjoHgwzrlhdaiDiQdJQFP/jVQXmXiFaws/F0C91idj7e6hjFSpRcjgiF73xH+GOnoEH8nxjhYBcH9EaWp6bQenxKaWYDRD09JGAeiyu6nyHXYgJ/ZCLyUxJNBnaTzhvRW9hIh6ee6lg/H7eih6VSVurabmyUz8FvJlWtkLf9tfqFKVwKW1WyMozkzmkp4oBzHvo92FIMswSGQmH5sdP0I065GgZQAvjfXzoxXczdJKw2Y5o3nOyQuYUj39n92lg4llNnqCEqDYAH7Wvn8R2ObTrg== x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:DS7PR12MB5789.namprd12.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230025)(4636009)(346002)(396003)(366004)(39860400002)(376002)(136003)(451199018)(41300700001)(110136005)(54906003)(316002)(64756008)(4326008)(52536014)(66556008)(66476007)(8676002)(66946007)(66446008)(76116006)(38100700002)(55016003)(86362001)(33656002)(122000001)(38070700005)(7696005)(45080400002)(53546011)(71200400001)(186003)(6506007)(26005)(9686003)(5660300002)(2906002)(8936002)(478600001)(966005)(83380400001);DIR:OUT;SFP:1101; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?pnqOz5WqRFDPOXF7voBufSNtZ1CgA08oEGFm0lybcPytmhZ7KJnl4xOBKkBo?= =?us-ascii?Q?FHA5RzO7ODtyrKK/4ApKGrxSwmfPt7QIKlDcVwQtmQ0bEZzRqeTWJYe5D0MO?= =?us-ascii?Q?tY4kzSGPYK+8yDehLZ+tKlvbuFvVfFtWKPAuSEdA9ubsDD7deUhBsmt+1lo8?= =?us-ascii?Q?M899vQn7a1AnMK1VpRLD8yES/3M9X5LY2C7jEOX5wo0fn9Q933NyX4eSPu4F?= =?us-ascii?Q?l90VBe4BrbRzM5fkSWQNvH0jd+iJrIR11DeYbFb+/apxqlw9ExqBspIe5oVG?= =?us-ascii?Q?9dsMZ045yfWwIz2CbOjHOzGXUUL4kltDFbHltDuw7O9I6ijtqu2hm3XfBMXO?= =?us-ascii?Q?dgDkc4L95JabuSOc5kOU7AZ7KGR3plNeQukTD5nwmpqpcq1XugWYn5RB5G9Q?= =?us-ascii?Q?FVlmKZkz+ERZrBIIMKAAZN2CIHWve1AI/zfeXZWbojo+ZQ60rECWouL8FDoP?= =?us-ascii?Q?IG4ldyPaXRKg9LYJ8Wl6PLzYBWYP2FvB1MpgBe2SngnI1UtjOvwlwQWeGXug?= =?us-ascii?Q?Ac4LekmHWEoVKGQbu45GKywes/IrXErPQVEXaB2J5ksjP22L2XW7ugwG3QlQ?= =?us-ascii?Q?qiOH1+W0JYa5PW8S708dY9pPPt5m0nlgYL+g6jFd/Ra0BCwT8KkUJW/jJ9Ae?= =?us-ascii?Q?5Kn17qrrglmX9Nq2wpCcD+JjVHG4M262S7LC5bS2ZpHpSPBZuNL3CyO81fMK?= =?us-ascii?Q?a7HhuKTsZtsVF6LH143hWRyeajW7NxBiLwJ17MM+PjdPBaVJjjkfzLY7YFeZ?= =?us-ascii?Q?CU4ecM5tQGelImAMtHb4sehMFvkqU7XjDsJIOUR+Vy7r2GeuSuMKAz2HS9iv?= =?us-ascii?Q?iogxqFXV/yrluKReNwMAdXqOxMWcVyIUKLmu2BTWX59QIOxq1Ft8G0NbFa7j?= =?us-ascii?Q?hPSNJzYrZcIrn0fKlmX5uYITIpXdkKpr6slrWRhWbQwE6AdecV0TStQ+LgnX?= =?us-ascii?Q?J6STVga6CC8LWwraVUnwJcuUW5hOMiDY0btkHlEGIVsK4fXampQrvWjxxtBy?= =?us-ascii?Q?ogcnQEksvnkc+vBbCkjZvaWUwNXTermUCJ4BV27aAKHKt9fvp8xTnDr7NFOH?= =?us-ascii?Q?wYPWKs0WRnjOkncV0i1QyRPT5ta0/Tu1UZV2j19V3sIT28sP5hjv3l+blCoX?= =?us-ascii?Q?l0HsTVSW7FQgBbij9lLmroyfDmhw2tMgGufUPbq0ZGcTgnLY9edQy2cbv9aF?= =?us-ascii?Q?EkpXo0RL8viWrfANNEAll3IELqYT4g3KZ05oQ0g5tggFfUebFA6grMKAMy0r?= =?us-ascii?Q?TOs5/FoZboSqMUq2wYHH9vNR9qFotvN3rGOwQTZIul3Qblj56YKysH/QKh44?= =?us-ascii?Q?MtR5sCRMo9WLPluqkWyQehzpqmwOxCM8blqViAw0YsrhubLuZSkBPbAee3aZ?= =?us-ascii?Q?Qcjw2rsc6GI8fJuvXr/Ms1u5MdADSzup4JxOgrILI/5P6yKZuNKfiJElNYL5?= =?us-ascii?Q?Fa7q+y0Ki2qcCzl4DnDSn6Pi5LkzODno0ITB5neJ5lr5sIR4m4s9bOs4L7qn?= =?us-ascii?Q?hg2a4FPzUYvXDLbBYaghG0BLwvjyp4923xS0j/qprX1ZKMyVLns4PL9f73bp?= =?us-ascii?Q?zYNL3DD1qNcvnRFFHnbRIKQE8AZLmPHUTuQoE/Jp?= MIME-Version: 1.0 X-OriginatorOrg: Nvidia.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: DS7PR12MB5789.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 759278e2-c6aa-46f6-a6f9-08db0ddcd5db X-MS-Exchange-CrossTenant-originalarrivaltime: 13 Feb 2023 16:10:33.4809 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 43083d15-7273-40c1-b7db-39efd9ccc17a X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: foFrpxxJITbj2dIIUDAju4JJwHbq8h602OpmH6ZiBREGJiWfdXqpObELrZ0MR0YIPMDfG+dPSFv0dJsP9PllWA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR12MB7819 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable The changes on your branch seem pretty good to me > -----Original Message----- > From: Pierre Gondois > Sent: Monday, February 6, 2023 2:28 AM > To: Jeff Brasen ; devel@edk2.groups.io > Cc: Sami.Mujawar@arm.com; Alexei.Fedorov@arm.com; > quic_llindhol@quicinc.com; ardb+tianocore@kernel.org > Subject: Re: [PATCH v2] DynamicTablesPkg: Allow multiple top level physic= al > nodes >=20 > External email: Use caution opening links or attachments >=20 >=20 > Hello Jeff, > Thanks for the v2. Also cf the first discussion at: >=20 > https://nam11.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2Fedk > 2.groups.io%2Fg%2Fdevel%2Ftopic%2F96680589%2399612&data=3D05%7C01% > 7Cjbrasen%40nvidia.com%7Ccee1a4886a754ba2d28508db08246448%7C43083 > d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638112724625353615%7CUnkn > own%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik > 1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=3Dur6vlVCBpt%2BQdid > 3xJKglx3trDZb4kxajkAWFXsr920%3D&reserved=3D0 > - I think it would be good to extract a function that does all the checks= as > there are many possibilities for the flags/parent combinations. > - I think it would also be nice to reset the index of ProcContainers for = each > new level (i.e. not to have the same incrementing index for > clusters/packages) >=20 > I created a branch based on your work at: > https://nam11.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2Fgith > ub.com%2Fpierregondois%2Fedk2%2Ftree%2Fpg%2Ftop_level_pnode_Wip > &data=3D05%7C01%7Cjbrasen%40nvidia.com%7Ccee1a4886a754ba2d28508db0 > 8246448%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C63811272462 > 5353615%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV > 2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=3DYdj > RbbboKTyVmi2rr2bvu0ARx9uey5mLYtWikbkP7Ek%3D&reserved=3D0 >=20 > Regards, > Pierre >=20 > On 2/3/23 19:10, Jeff Brasen wrote: > > In SSDT CPU topology generator allow for multiple top level physical > > nodes as would be seen with a multi-socket system. This will create a > > top level processor container for all systems. > > > > Signed-off-by: Jeff Brasen > > --- > > .../SsdtCpuTopologyGenerator.c | 43 ++++++------------= - > > 1 file changed, 12 insertions(+), 31 deletions(-) > > > > diff --git > > > a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCp > uT > > opologyGenerator.c > > > b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCp > uT > > opologyGenerator.c > > index c24da8ec71..46b757e0b2 100644 > > --- > > > a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCp > uT > > opologyGenerator.c > > +++ > b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/Ssdt > > +++ CpuTopologyGenerator.c > > @@ -814,7 +814,8 @@ CreateAmlProcessorContainer ( > > Protocol Interface. > > @param [in] NodeToken Token of the > CM_ARM_PROC_HIERARCHY_INFO > > currently handled. > > - Cannot be CM_NULL_TOKEN. > > + CM_NULL_TOKEN if top level conta= iner > > + should be created. > > @param [in] ParentNode Parent node to attach the creat= ed > > node to. > > @param [in,out] ProcContainerIndex Pointer to the current > > processor container @@ -841,12 +842,12 @@ CreateAmlCpuTopologyTree > ( > > AML_OBJECT_NODE_HANDLE ProcContainerNode; > > UINT32 Uid; > > UINT16 Name; > > + UINT32 NodeFlags; > > > > ASSERT (Generator !=3D NULL); > > ASSERT (Generator->ProcNodeList !=3D NULL); > > ASSERT (Generator->ProcNodeCount !=3D 0); > > ASSERT (CfgMgrProtocol !=3D NULL); > > - ASSERT (NodeToken !=3D CM_NULL_TOKEN); > > ASSERT (ParentNode !=3D NULL); > > ASSERT (ProcContainerIndex !=3D NULL); > > > > @@ -893,8 +894,14 @@ CreateAmlCpuTopologyTree ( > > } else { > > // If this is not a Cpu, then this is a processor container. > > > > + NodeFlags =3D Generator->ProcNodeList[Index].Flags; > > + // Allow physical property for top level nodes > > + if (NodeToken =3D=3D CM_NULL_TOKEN) { > > + NodeFlags &=3D ~EFI_ACPI_6_3_PPTT_PACKAGE_PHYSICAL; > > + } > > + >=20 > I think that if (NodeToken =3D=3D CM_NULL_TOKEN) and doesn't have the > Physical Package flag, no error will be triggered even though this is not= a valid > configuration. >=20 > > // Acpi processor Id for clusters is not handled. > > - if ((Generator->ProcNodeList[Index].Flags & > PPTT_PROCESSOR_MASK) !=3D > > + if ((NodeFlags & PPTT_PROCESSOR_MASK) !=3D > > PPTT_CLUSTER_PROCESSOR_MASK) > > { > > DEBUG (( > > @@ -974,8 +981,6 @@ CreateTopologyFromProcHierarchy ( > > ) > > { > > EFI_STATUS Status; > > - UINT32 Index; > > - UINT32 TopLevelProcNodeIndex; > > UINT32 ProcContainerIndex; > > > > ASSERT (Generator !=3D NULL); > > @@ -984,8 +989,7 @@ CreateTopologyFromProcHierarchy ( > > ASSERT (CfgMgrProtocol !=3D NULL); > > ASSERT (ScopeNode !=3D NULL); > > > > - TopLevelProcNodeIndex =3D MAX_UINT32; > > - ProcContainerIndex =3D 0; > > + ProcContainerIndex =3D 0; > > > > Status =3D TokenTableInitialize (Generator, Generator->ProcNodeCoun= t); > > if (EFI_ERROR (Status)) { > > @@ -993,33 +997,10 @@ CreateTopologyFromProcHierarchy ( > > return Status; > > } > > > > - // It is assumed that there is one unique > > CM_ARM_PROC_HIERARCHY_INFO > > - // structure with no ParentToken and the > > EFI_ACPI_6_3_PPTT_PACKAGE_PHYSICAL > > - // flag set. All other CM_ARM_PROC_HIERARCHY_INFO are non-physical > > and > > - // have a ParentToken. > > - for (Index =3D 0; Index < Generator->ProcNodeCount; Index++) { > > - if ((Generator->ProcNodeList[Index].ParentToken =3D=3D > CM_NULL_TOKEN) && > > - (Generator->ProcNodeList[Index].Flags & > > - EFI_ACPI_6_3_PPTT_PACKAGE_PHYSICAL)) > > - { > > - if (TopLevelProcNodeIndex !=3D MAX_UINT32) { > > - DEBUG (( > > - DEBUG_ERROR, > > - "ERROR: SSDT-CPU-TOPOLOGY: Top level > CM_ARM_PROC_HIERARCHY_INFO " > > - "must be unique\n" > > - )); > > - ASSERT (0); > > - goto exit_handler; > > - } > > - > > - TopLevelProcNodeIndex =3D Index; > > - } > > - } // for > > - > > Status =3D CreateAmlCpuTopologyTree ( > > Generator, > > CfgMgrProtocol, > > - Generator->ProcNodeList[TopLevelProcNodeIndex].Token, > > + CM_NULL_TOKEN, > > ScopeNode, > > &ProcContainerIndex > > );