From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail05.groups.io (mail05.groups.io [45.79.224.7]) by spool.mail.gandi.net (Postfix) with ESMTPS id BBB659412BA for ; Tue, 26 Nov 2024 07:48:02 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=9KWpjbAdvoSuVkZOTdHFMvzBjZEjEFtwVikzpXj+0xA=; c=relaxed/simple; d=groups.io; h=From:To:CC:Subject:Thread-Topic:Thread-Index:Date:Message-ID:References:In-Reply-To:Accept-Language:MIME-Version:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Resent-Date:Resent-From:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type; s=20240830; t=1732607282; v=1; x=1732866481; b=t3O2N1Fbh2LeBFV8cCyqlcGQ77k1csQxM643K8lP7O07uxGwFXBn4iaMQGBIxyOD0AAkNcbZ qxrJABOolR78UhRT+Log+QrN571I3Iky3W8kutZjhevqVY8sBWduyQQhkBslS1muIRQX1HEkeSp j0c1zCDwqMor3lEEVZ+x1dkU3Sp4GoadV6B5SVR0aQq56+6pj16CLKb8EGjxx2cLFsIjCDMYqRv 0v748CeUx2KR9mu5YJhp4UA5F7YhLais3rDhc7ilcAet3UR0OghG56AYn5AixTtReM43Xcz0E53 xlkFFY7QtvomzYoTozLT6sLvxE5vy5XLxdz17imqAvLAQ== X-Received: by 127.0.0.2 with SMTP id AwutYY7687511x0j4QDM8pyy; Mon, 25 Nov 2024 23:48:01 -0800 X-Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.15]) by mx.groups.io with SMTP id smtpd.web10.40675.1732607275144744192 for ; Mon, 25 Nov 2024 23:47:55 -0800 X-CSE-ConnectionGUID: ufUpjgfEQ4Ku0iiMKR54KQ== X-CSE-MsgGUID: XO69RQxHT8a29Z95+Lm3Og== X-IronPort-AV: E=McAfee;i="6700,10204,11267"; a="36410348" X-IronPort-AV: E=Sophos;i="6.12,185,1728975600"; d="scan'208,217";a="36410348" X-Received: from fmviesa010.fm.intel.com ([10.60.135.150]) by orvoesa107.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Nov 2024 23:47:55 -0800 X-CSE-ConnectionGUID: CayhZGfcRumKuFh5h2R1Qw== X-CSE-MsgGUID: +k236T8lRQGTA1xKWMa1HA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.12,185,1728975600"; d="scan'208,217";a="91860499" X-Received: from fmsmsx602.amr.corp.intel.com ([10.18.126.82]) by fmviesa010.fm.intel.com with ESMTP/TLS/AES256-GCM-SHA384; 25 Nov 2024 23:47:54 -0800 X-Received: from fmsmsx603.amr.corp.intel.com (10.18.126.83) by fmsmsx602.amr.corp.intel.com (10.18.126.82) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.39; Mon, 25 Nov 2024 23:47:54 -0800 X-Received: from FMSEDG603.ED.cps.intel.com (10.1.192.133) by fmsmsx603.amr.corp.intel.com (10.18.126.83) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.39 via Frontend Transport; Mon, 25 Nov 2024 23:47:54 -0800 X-Received: from NAM10-DM6-obe.outbound.protection.outlook.com (104.47.58.43) by edgegateway.intel.com (192.55.55.68) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.39; Mon, 25 Nov 2024 23:47:53 -0800 X-Received: from PH0PR11MB5048.namprd11.prod.outlook.com (2603:10b6:510:3d::14) by SJ0PR11MB6744.namprd11.prod.outlook.com (2603:10b6:a03:47d::10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.8182.21; Tue, 26 Nov 2024 07:47:50 +0000 X-Received: from PH0PR11MB5048.namprd11.prod.outlook.com ([fe80::67b2:6d4a:c673:54a]) by PH0PR11MB5048.namprd11.prod.outlook.com ([fe80::67b2:6d4a:c673:54a%3]) with mapi id 15.20.8182.019; Tue, 26 Nov 2024 07:47:50 +0000 From: "Zhiguang Liu via groups.io" To: "Kinney, Michael D" , "Zhao, Jason" , "Wu, Jiaxin" , "devel@edk2.groups.io" , "Ni, Ray" , "Tan, Dun" , "Kumar, Rahul R" , Gerd Hoffmann , "Gao, Liming" CC: "Kumar, Chandana C" , "Kuo, Donald" Subject: Re: [edk2-devel] UefiCpuPkg: Proposal to enable/disable AP parallel Thread-Topic: UefiCpuPkg: Proposal to enable/disable AP parallel Thread-Index: Ads7ENWFYT6OL6QbQl+cr51i2qWfsQA6MVzQAAS86RAADhWKAAAPnw2w Date: Tue, 26 Nov 2024 07:47:50 +0000 Message-ID: References: In-Reply-To: Accept-Language: en-US, zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-publictraffictype: Email x-ms-traffictypediagnostic: PH0PR11MB5048:EE_|SJ0PR11MB6744:EE_ x-ms-office365-filtering-correlation-id: 5042de95-d79b-444a-67e9-08dd0deea0c3 x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam-message-info: =?us-ascii?Q?32isOgNOxVq0BKAgUzaGQL4D8uzgvOJU9iA8m95y+8/JKARH1VJEZItpj+GI?= =?us-ascii?Q?OM6ptZiUhB7nYmEL5d+vlVhZ08YcdtCRLQViwXM8pNqBSz5MJoxrCw4GBQs/?= =?us-ascii?Q?qWGYY6Om29njlAbJOVJrv8lCVKd8h5x5Bjc4VZKKcV/InPYxJT+/RVurQXA9?= =?us-ascii?Q?RyYI/s0EMUtNsTvM8ch2YTBqTFCT+mLkDBYvvzh9P9eEfvkrWRPoVlcOuXLl?= =?us-ascii?Q?VBt83VY2PXlFOyxDeNrnKeSkWr9h+UYYmoUnlD8agUEq/5qqlCLAPKctBDhI?= =?us-ascii?Q?DKKEmyeheBYUMNE+dbbLWxs4Jqk5BY4pbmaGSSPYrrA/zN02hyJytXtQsNPH?= =?us-ascii?Q?P8J6WHDS8+PBmwBjQBDn/flhctVRjMhboq/oS+fU+SowNVX6vdMsxO9fQbm3?= =?us-ascii?Q?HLK10yQIiBvX6nxGiaQ9P7x3M3JMK8UONSFhD1LGS3yfq5ls/AOfFQePbWIB?= =?us-ascii?Q?yGD4EpdDdnUQdqW258YxaWtKD3Wo1SPi4p34B7PlstDH6e2aBNpz3iqJ/HxD?= =?us-ascii?Q?vhrPha5GYUad8h7nI5gAbJrLN+mtDH0q09qWpq8Hhd8qHXHdZOQe3BXVCtUU?= =?us-ascii?Q?0Kx9zlSh1+/5bLoaioCQ1hDXILlR5cqXF4p5rMStEEZFKdeM0J0ZV1S4iKDn?= =?us-ascii?Q?2ZkozuxSrpd70yraFLHXLnz1hc0WMLHSMP/g9p4RwB3LF+B0P63tcxIQ/wrL?= =?us-ascii?Q?O9hzwNXx33f1Xsafws7uba9FcDZ8qECmAkpciqmW1en7GGMics2Vb7OaaHW9?= =?us-ascii?Q?6ScpvgacetTUvD6zQAqWu6jn0yZ9pwxRL62W8m/opRAgt12AIUX/v8jf/yIo?= =?us-ascii?Q?SImj7y77Mz+HwLQuqXa+wIdEvRXvTZB+G2kGrJWIw1ZGj4TFHNuxoV7kWzK7?= =?us-ascii?Q?A3GThtuAqq0B106PxsxTHb/QwxGEINGYvwlT+sjmDgr//zeIB4QLySxaKkbl?= =?us-ascii?Q?ScXLiQD4DHaoh9w4W85ChGrV6En8ArxjfSiVIKPlaafmGv9Sdg01YIDZiPY+?= =?us-ascii?Q?4aF3IuxwRE/PfkCW0QtTu/WOFMvf10SX/B4OKSDY/DQ4PIKuVSLJIB4WX14R?= =?us-ascii?Q?LvJmwHRd1kIuvidz6R/Kj4slekCefvn8T9N24BULBF4YPZorn/DNYooNdOkt?= =?us-ascii?Q?nN+rtUH3HMerX4aPfPOt2Qzyuq/G5ICCf76yvY4+YvxYfyZlRPVHSAf5G871?= =?us-ascii?Q?k34YoVNnaEqYDYOxdt16zIuoRyekhWKuznJNxGs8ABSxy+aNCZsohM0rf1gh?= =?us-ascii?Q?FBJeigcl8eqiHw+bl7lidh3X/px/fXjFAclKUVQY8Yb6HNwAwjwvTE2gdOzE?= =?us-ascii?Q?HYZq9USEda5PpRRUxv0R86mqrQvzKsAyypxIPp59fxw3ma+KyWc0w9PyjgtW?= =?us-ascii?Q?0pkGSza7P4oFhki8OiljdBOZxO3pX3I3Y3YeaSQfB9kcUrnMNdoqrAFQV4Tv?= =?us-ascii?Q?GYNn4NTeIOs=3D?= x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?yTar4QB6o5wrrhL4UnUKkGWAHrMWHNSuX6KINR8/LqyagVBt0vqDRlHjLhuC?= =?us-ascii?Q?Po2gU4ZjvjOJifvqbu9vvKalA3ZfCAGOnI2Gid7cy+CXWL5XaJ9j2FuNSucM?= =?us-ascii?Q?awC9T2ak4a81FiaP0h7t8aplDFz7y+We6UUsUmi/+LzSv+BACVTjL8TjF4ib?= =?us-ascii?Q?thXgYH8Xyl9/pjJkjCb7H8A/5mwS3gzNjzLUzawROriEUKLN/XcdqlCdMeXp?= =?us-ascii?Q?B7OZ+oUtjHVibYO2Tm9tt0E3w/W2otdATzSz2vrrJd91GTKTPyrnKGMOQ1n7?= =?us-ascii?Q?Q7k9ESBR1wVSNEZ/lU5xFOSGIG5d8+JLdT8HKDMvrcCS2BEBTpMJ7N4APzTw?= =?us-ascii?Q?TZsX4ZZ275tNTAotWPyv64B6FfO+L4Wm1sHBMvGQx312Fx8fyuteolIZ1C1C?= =?us-ascii?Q?gdItUg7BuGTrWBgzYGC/P1LlqRu3TpgM1NDTKIXOkwF1SqDMhoKbXpGLDDnl?= =?us-ascii?Q?5hrmdHXvj55/TdFphn9dH//RyU3/hooZiGRABdIwFgeAIoWhJBt2rgnxnyId?= =?us-ascii?Q?UuFQeLShnJ+k0dJLDA8IwGhkRRv05C7iUrJTxtfux8CJsCCzsjBXxWFtlw9D?= =?us-ascii?Q?QDsFObYBLyDwdSr7yrqt1GE0JcPgVdgZl8+eTtcnDDWgY31l91Q1JOOl70g4?= =?us-ascii?Q?epgydZNvfL69cdy1cQ9XXT4SljJYrws9hYbZcep+HdEnhfUyaGtmeNH09IjB?= =?us-ascii?Q?2CVZBZJ/tSCc+fn3GCU3wZXYqVHhEgXl1NAYIYt1F/cdg6APtBUnsMpuIGA5?= =?us-ascii?Q?Fi4PqbE/XfuJ1Ke0OpSOt+AufxnhzxjVy3dPEFAzssbWZpeAqGYht2qSsE8w?= =?us-ascii?Q?RJC0C2DUD671SkAnSUDY752VxFgxXToGGCeaOlf1SiIOR0yIaDimMcIDoQ5h?= =?us-ascii?Q?OG6bZDzth8Meg/bzzZRgvn2nM0grrtlfgaNSdnGR33F2IVbp6aBuy9S+WhSJ?= =?us-ascii?Q?xZ7Tl3Z/HgcPchdwRz6LQ5PMp1uofHMGaMFH+G7VB/l7g1tzTjYX4UkP6t1Y?= =?us-ascii?Q?yjywRU8SkHxWXlV8P6w2kFPOTSQ9H2M0S+0qIsowEsQ8Zibheto5GnVe7jcd?= =?us-ascii?Q?AtvGq1j/IhitY5Ry4obge968VWTjkNLSSxI0W4gHbTI/HfC35JeSDWz+4D4o?= =?us-ascii?Q?eTtbLfNSbqrkcYs/44nrw/gFxVmfgmpWFlABn3MSVAc2U3bj4VjWGNb7qMY/?= =?us-ascii?Q?xDi4O6ojd6DRi2cOxta1hCndIWgyqjcCjTxQYJF1JuuKtoHK3Dp0c5/Du1Rk?= =?us-ascii?Q?iRn72PjMOwEq/gbNrUQFIO1rudxilheESFwgZaXx1y8cD43qCTAX4K0CPFth?= =?us-ascii?Q?fIbKafAq2BQqkiJQGdtTSZCWTCb+3HJekPzwuHBogL+Aq7Jb03V3Jnharl+t?= =?us-ascii?Q?aybXqAdUw3QsZmpj4D1Kh8zsPp/Zypm7/bhruwa6LNi/YvP1HSBnmfooznaI?= =?us-ascii?Q?61DHFDjw0iNqO3fS75uhpVGLIqENd+hUKh+nOEQO8rnUJ7mWfiAJxpBy6T9X?= =?us-ascii?Q?iiNiWcOlhEx0UZhZP9R8trKx7X4u1aV/7WhnRZVQOG/tpqT03OqAQtY/kOSz?= =?us-ascii?Q?mbdqg746v4L4sWPNcFZsh5lCzdt8f4oXZOFdBJG1?= MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB5048.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 5042de95-d79b-444a-67e9-08dd0deea0c3 X-MS-Exchange-CrossTenant-originalarrivaltime: 26 Nov 2024 07:47:50.6396 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: lELgkDBblV1SxM0J/rvFeeHhuC833yOZ9S0cNYEOgvZ/G76iArtilZLGOj8HTOiMUd+HIhJ/wsyEB5z13aXz2Q== X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR11MB6744 X-OriginatorOrg: intel.com Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Resent-Date: Mon, 25 Nov 2024 23:47:55 -0800 Resent-From: zhiguang.liu@intel.com Reply-To: devel@edk2.groups.io,zhiguang.liu@intel.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: 40VgyTjKvYRLZ67trVuX29t9x7686176AA= Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_PH0PR11MB5048BFE3E69A64454521EF71902F2PH0PR11MB5048namp_" X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20240830 header.b=t3O2N1Fb; dmarc=pass (policy=none) header.from=groups.io; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 45.79.224.7 as permitted sender) smtp.mailfrom=bounce@groups.io --_000_PH0PR11MB5048BFE3E69A64454521EF71902F2PH0PR11MB5048namp_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Thanks all for your comments. Let me reply one by one. Hi Jason, Yes, we still support enable/disable AP one by one. Caller can still give a= n AP processer ID to disable/enable this AP. Just add a capability to allow= caller give a all Fs value to disable/enable all APs. Hi Jiaxin, Solution 2 will also change the PPI mpservice2. PPI mpservice2 will provide= exact same interface as Protocol mpservice2. Hi Mike, I understand your concern, However, I think it is a very corner case where = AP is disable on purpose and is expected to never be enabled. I am not sure= if we should take effect for such corner case. If we need to consider such case, bitmask may be not a good solution becaus= e we don't know how many cores there are in future. (A UINT64 bit misk can = only handle 64-core CPU) Supportting a non-blocking operation is a good idea for me, let me write th= e solution down in detail. EFI_MP_SERVICES_ENABLEDISABLEAP: (enable case) Send INIT IPI to AP Set AP to state CpuStateAfterInitIpi return EFI_MP_SERVICES_ENABLEDONE: Delay 10ms For loop to check each cores: If this core's state is CpuStateAfterInitIp= i Send SIPI IPI to this core to wake up it. Set AP to state CpuStateIdle The solution needs to add a new interface EnableDone(). After calling EnableDisalbeAp in a for loop to enable APs, caller need to c= all the new interface to really enable cores. To be compatible with original EnableDisalbeAp, we can use BIT63 in the inp= ut ProcessorNumber. If BIT63 is not set, it is a blocking operation like be= fore, and no need to call EnableDone(). If BIT63 is set, it is a non-block = operation and need caller to call EnableDone() later. Another similar idea (Ray also mentioned offline to me before) is that we d= on't provide the new interface. The similar thing should be done in next fi= rst call of MpService. For example, after user calling EnableDisalbeAp() to enable some APs in a f= or loop, another module calls StartupThisAP() to let a specific AP to somet= hing. Inside StartupThisAP(), MpLib first check the AP state, and if any core's s= tate is CpuStateAfterInitIpi, MpLib will delay 10ms, and wake up these APs,= and change the state to CpuStateIdle. After that, MpLib do the original wo= rk. I didn't bring up this idea because I think it is a little complicated. In = PEI phase, we don't know who the last caller of MpService is. For example, = if a caller enables an AP, and then PEI phase ends, how DXE know that? We n= eed additional field in the MP_HAND_OFF Hob to record such state. Mike, please help comment on these two new solutions. Or if you have other = good idea, please share. Thanks Zhiguang From: Kinney, Michael D Sent: Friday, November 22, 2024 2:44 AM To: Zhao, Jason ; Wu, Jiaxin ; L= iu, Zhiguang ; devel@edk2.groups.io; Ni, Ray ; Tan, Dun ; Kumar, Rahul R ; Gerd Hoffmann ; Gao, Liming Cc: Kumar, Chandana C ; Kuo, Donald ; Kinney, Michael D Subject: RE: UefiCpuPkg: Proposal to enable/disable AP parallel If an AP has been disabled on purpose due to a failure and is expected to n= ever be enabled, then a broadcast mechanism would not be a good idea. Instead, we should consider enhancing EFI_MP_SERVICES_ENABLEDISABLEAP to su= pport a non-blocking operation so it can be called in a loop for a group of= target APs. Or a new API that allows a bitmask of APs to enable. Mike From: Zhao, Jason > Sent: Thursday, November 21, 2024 4:03 AM To: Wu, Jiaxin >; Liu, Zhig= uang >; devel@edk2.gr= oups.io; Ni, Ray >; Tan, Dun >; Kum= ar, Rahul R >; Gerd= Hoffmann >; Kinney, Michael D = >; Gao, Limin= g > Cc: Kumar, Chandana C >; Kuo, Donald >= ; Zhao, Jason > Subject: RE: UefiCpuPkg: Proposal to enable/disable AP parallel Hi Zhiguang, May be a fool question. If we follow the 1st solution, do we still have any API to enable/disable A= P one by one? How can we handle any scenario like only enable all Aps with specific core = type (only big cores or only Atoms) if this API is changed to enable all Ap= s. BRs/Jason From: Wu, Jiaxin > Sent: Thursday, November 21, 2024 5:54 PM To: Liu, Zhiguang >; = devel@edk2.groups.io; Ni, Ray >; Tan, Dun >; Kumar, Rahul R >; Gerd Hoffmann >; Kinne= y, Michael D = >; Gao, Liming > Cc: Kumar, Chandana C >; Zhao, Jason >= ; Kuo, Donald > Subject: RE: UefiCpuPkg: Proposal to enable/disable AP parallel Solution 1, it's not kind of spec violating. Instead, it's to add a new cap= ability to the existing interface, and it's a compatible change, no impact = to existing interface usage. I recall we have a guideline that prioritizes = code-first approaches if there are no compatibility issues. Mike and Ray ca= n comment on this. If no objection, I also prefer this way. Solution 2 cannot handle the PPI case, leading to inconsistent behavior bet= ween the protocol and PPI for the mpservice2. Solutions 3 and 4 are more like workarounds to address the specific issue. Thanks, Jiaxin From: Liu, Zhiguang > Sent: Wednesday, November 20, 2024 3:13 PM To: devel@edk2.groups.io; Ni, Ray >; Wu, Jiaxin >; Liu, Zhiguang >; Tan, Dun >; Kum= ar, Rahul R >; Gerd= Hoffmann >; Kinney, Michael D = >; Gao, Limin= g > Cc: Kumar, Chandana C >; Zhao, Jason >= ; Kuo, Donald > Subject: UefiCpuPkg: Proposal to enable/disable AP parallel Hi MdePkg and UefiCpuPkg maintainers and reviewers Recently, we met a performance issue when waking up disabled APs. There is usage where BIOS needs to disable all APs, do something and then e= nable all APs. Now, we are using the MpService PPI/Protocol EnableDisableAP(). This functi= on can only enable/disable one AP each time. To enable one AP, MP service needs to send INIT-SIPI-SIPI, which takes arou= nd 10ms. And now, we will have more than 10 APs in a client platform, and it will ta= ke more than 100ms. The function definition of EnableDisableAP is: typedef EFI_STATUS (EFIAPI *EFI_MP_SERVICES_ENABLEDISABLEAP)( IN EFI_MP_SERVICES_PROTOCOL *This, IN UINTN ProcessorNumber, IN BOOLEAN EnableAP, IN UINT32 *HealthFlag OPTIONAL ); The input parameter ProcessorNumber accepts a range from 0 to the total num= ber of logical processors minus 1. To support enable/disable AP parallel, I have below solutions: Solution1: Let input parameter ProcessorNumber accept a MAX_UINTN also. MAX_UINTN mean= s to enable/disable all APs. The draft PR is at https://github.com/tianocore/edk2/pull/6453 When the par= ameter is MAX_UINTN, EnableDisableAP() will enable/disable APs in a paralle= l way. However, we need to change below header files MdePkg\Include\Protocol\MpService.h MdePkg\Include\Ppi\MpServices.h UefiCpuPkg\Include\Ppi\MpServices2.h The above two follow PI spec. We need to modify the PI spec= first. Solution2: Similar with solution1, but to avoid violating spec, add a = new Protocol named MpServices2. Only change below header files. UefiCpuPkg\Include\Ppi\MpServices2.h UefiCpuPkg\Include\Protocol\MpServices2.h (new) The MdePkg part remains no change. Solution3: MpService create new PPI/Protocol to only contain one funct= ion EnableDisableAllAps(), which will enable/disable all APs in a parallel = way. Solution4: Add PPI/Protocol notify in MpLib. The notify call back func= tion will set WakeUpByInitSipiSipi to True. Similar code is removed in http= s://github.com/tianocore/edk2/pull/6303/commits/f1f8374381019169d421a65a896= ab42ed5338c1e When users need to disable and then enable cores, the flow = will be: 1. Send Init to all APs. (to disable all APs) 2. Do things that user need to do when all APs are disabled 3. Notify callback function 4. Use StartupAllAPs() from existing PPI/Protocol to wake up all APs. The flow is similar with how S3 SMM code take control APs and then give the= control back in old days. Personally, I prefer solution1. It is simpler, but it does violate spec. Please let me know your comments or any new idea, please share. Thanks Zhiguang -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#120843): https://edk2.groups.io/g/devel/message/120843 Mute This Topic: https://groups.io/mt/109680758/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- --_000_PH0PR11MB5048BFE3E69A64454521EF71902F2PH0PR11MB5048namp_ Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable

Thanks all for your comments. Let me reply one by on= e.

 

Hi Jason,

Yes, we still support enable/disable AP one by one. = Caller can still give an AP processer ID to disable/enable this AP. Just ad= d a capability to allow caller give a all Fs value to disable/enable all AP= s.

 

Hi Jiaxin,

Solution 2 will also change the PPI mpservice2. PPI = mpservice2 will provide exact same interface as Protocol mpservice2.

 

Hi Mike,

I understand your concern, However, I think it is a = very corner case where AP is disable on purpose and is expected to never be= enabled. I am not sure if we should take effect for such corner case.=

If we need to consider such case, bitmask may be not= a good solution because we don’t know how many cores there are in fu= ture. (A UINT64 bit misk can only handle 64-core CPU)

Supportting a non-blocking operation is a good idea = for me, let me write the solution down in detail.

 

EFI_MP_SERVICES_ENABLEDISABLEAP: (enable case)<= /o:p>

Send INIT IPI to AP<= /o:p>

Set AP to state CpuStateA= fterInitIpi

return

 

EFI_MP_SERVICES_ENABLEDONE:

        &nbs= p;       Delay 10ms

        &nbs= p;       For loop to check each cores:

        &nbs= p;             =           If this core’s= state is CpuStateAfterInitIpi

Send SIP= I IPI to this core to wake up it.

Set AP t= o state CpuStateIdle

        &nbs= p;      

The solution needs to add a new interface EnableDone= ().

After calling EnableDisalbeAp in a for loop to enabl= e APs, caller need to call the new interface to really enable cores.

To be compatible with original EnableDisalbeAp, we c= an use BIT63 in the input ProcessorNumber. If BIT63 is not set, it is a blo= cking operation like before, and no need to call EnableDone(). If BIT63 is = set, it is a non-block operation and need caller to call EnableDone() later.

 

Another similar idea (Ray also mentioned offline to = me before) is that we don’t provide the new interface. The similar th= ing should be done in next first call of MpService.

For example, after user calling EnableDisalbeAp() to= enable some APs in a for loop, another module calls StartupThisAP() to let= a specific AP to something.

Inside StartupThisAP(), MpLib first check the AP sta= te, and if any core’s state is CpuStateAfterInitIpi, MpLib will delay= 10ms, and wake up these APs, and change the state to CpuStateIdle. After t= hat, MpLib do the original work.

I didn’t bring up this idea because I think it= is a little complicated. In PEI phase, we don’t know who the last ca= ller of MpService is. For example, if a caller enables an AP, and then PEI = phase ends, how DXE know that? We need additional field in the MP_HAND_OFF Hob to record such state.

 

Mike, please help comment on these two new solutions= . Or if you have other good idea, please share.

 

Thanks

Zhiguang

 

 

From: Kinney, Michael D <michael.d.kinney@intel.com>
Sent: Friday, November 22, 2024 2:44 AM
To: Zhao, Jason <jason.zhao@intel.com>; Wu, Jiaxin <jiaxin.= wu@intel.com>; Liu, Zhiguang <zhiguang.liu@intel.com>; devel@edk2.= groups.io; Ni, Ray <ray.ni@intel.com>; Tan, Dun <dun.tan@intel.com= >; Kumar, Rahul R <rahul.r.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Gao, Liming <gaoliming@byosoft.com.cn> Cc: Kumar, Chandana C <chandana.c.kumar@intel.com>; Kuo, Donal= d <donald.kuo@intel.com>; Kinney, Michael D <michael.d.kinney@inte= l.com>
Subject: RE: UefiCpuPkg: Proposal to enable/disable AP parallel=

 

If an AP has been d= isabled on purpose due to a failure and is expected to never be enabled, th= en a broadcast mechanism would not be a good idea.

 

Instead, we should = consider enhancing EFI_MP_SERVICES_ENABLEDISABLEAP to support a non-blocking operation = so it can be called in a loop for a group of target APs.  Or a new API= that allows a bitmask of APs to enable.

 

Mike

 

From: Zhao, Jason <jason.zhao@intel.com>
Sent: Thursday, November 21, 2024 4:03 AM
To: Wu, Jiaxin <jiaxin.wu@intel.com>; Liu, Zhiguang <zhiguang.liu@intel.com>; devel@ed= k2.groups.io; Ni, Ray <ray.ni@intel.com>; Tan, Dun <dun.tan@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn> Cc: Kumar, Chandana C <chandana.c.kumar@intel.com>; Kuo, Donald <donald.kuo@intel.com>; Zhao, Jason <jason.zhao@intel.com>
Subject: RE: UefiCpuPkg: Proposal to enable/disable AP parallel=

 

Hi Zhiguang,

 

May be a fool quest= ion.

If we follow the 1<= sup>st solution, do we still have any API to enable/disable AP one by= one?

How can we handle a= ny scenario like only enable all Aps with specific core type (only big core= s or only Atoms) if this API is changed to enable all Aps.

 

BRs/Jason

From: Wu, Jiaxin <jiaxin.wu@intel.com>
Sent: Thursday, November 21, 2024 5:54 PM
To: Liu, Zhiguang <zhiguang.liu@intel.com>; devel@ed= k2.groups.io; Ni, Ray <ray.ni@intel.com>; Tan, Dun <dun.tan@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn> Cc: Kumar, Chandana C <chandana.c.kumar@intel.com>; Zhao, Jason <jason.zhao@intel.com>; Kuo, Donald <donald.kuo@intel.com>
Subject: RE: UefiCpuPkg: Proposal to enable/disable AP parallel=

 

Solution 1, it’s not kind of spec violating. I= nstead, it’s to add a new capability to the existing interface, and i= t’s a compatible change, no impact to existing interface usage. I rec= all we have a guideline that prioritizes code-first approaches if there are no compatibility issues. Mike and Ray can comment = on this. If no objection, I also prefer this way.

 

Solution 2 cannot handle the PPI case, leading to in= consistent behavior between the protocol and PPI for the mpservice2.

 

Solutions 3 and 4 are more like workarounds to addre= ss the specific issue.

 

Thanks,<= /span>

Jiaxin <= /span>

 

From: Liu, Zhiguang <zhiguang.liu@intel.com= >
Sent: Wednesday, November 20, 2024 3:13 PM
To: devel@edk2.groups.io; Ni, Ray <ray= .ni@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com= >; Liu, Zhiguang <zhiguang.liu@intel.com>; Tan, Dun <dun.tan@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn> Cc: Kumar, Chandana C <chandana.c.kumar@intel.com>; Zhao, Jason <jason.zhao@intel.com>; Kuo, Donald <donald.kuo@intel.com>
Subject: UefiCpuPkg: Proposal to enable/disable AP parallel

 

Hi MdePkg and UefiCpuPkg maintainers and reviewers

 

Recently, we met a performance issue when waking up = disabled APs.

There is usage where BIOS needs to disable all APs, = do something and then enable all APs.

Now, we are using the MpService PPI/Protocol EnableD= isableAP(). This function can only enable/disable one AP each time.

To enable one AP, MP service needs to send INIT-SIPI= -SIPI, which takes around 10ms.

And now, we will have more than 10 APs in a client p= latform, and it will take more than 100ms.

The function definition of EnableDisableAP is:<= /o:p>

typedef

EFI_STATUS

(EFIAPI *EFI_MP_SERVICES= _ENABLEDISABLEAP)(

  IN  EFI_MP_S= ERVICES_PROTOCOL  *This,

  IN  UINTN &n= bsp;                   Process= orNumber,

  IN  BOOLEAN =                   EnableAP,

  IN  UINT32 &= nbsp;                  *Health= Flag OPTIONAL

  );

The input parameter ProcessorNumber accepts a range = from 0 to the total number of logical processors minus 1.

 

To support enable/disable AP parallel, I have below = solutions:

 

Solution1:

Let input parameter Proce= ssorNumber accept a MAX_UINTN also. MAX_UINTN means to enable/disable all A= Ps.

The draft PR is at https://github.com/tianocore/edk2/pull/6453 When the parameter is MAX_U= INTN, EnableDisableAP() will enable/disable APs in a parallel way.

        &nbs= p;       However, we need to change below hea= der files

        &nbs= p;            &= nbsp;          MdePkg\Include\= Protocol\MpService.h

        &nbs= p;            &= nbsp;          MdePkg\Include\= Ppi\MpServices.h

        &nbs= p;            &= nbsp;          UefiCpuPkg\Incl= ude\Ppi\MpServices2.h

        &nbs= p;       The above two follow PI spec. We nee= d to modify the PI spec first.

 

Solution2:

        &nbs= p;       Similar with solution1, but to avoid= violating spec, add a new Protocol named MpServices2. Only change below he= ader files.

        &nbs= p;            &= nbsp;          UefiCpuPkg\Incl= ude\Ppi\MpServices2.h

UefiCpuP= kg\Include\Protocol\MpServices2.h (new)

        &nbs= p;       The MdePkg part remains no change.

 

Solution3:

        &nbs= p;       MpService create new PPI/Protocol to= only contain one function EnableDisableAllAps(), which will enable/disable= all APs in a parallel way.

 

Solution4:

        &nbs= p;       Add PPI/Protocol notify in MpLib. Th= e notify call back function will set WakeUpByInitSipiSipi to True. Similar = code is removed in https://github.com/tianocore/edk2/pull/6303/commits/f1f8374381019169d421a65= a896ab42ed5338c1e

        &nbs= p;       When users need to disable and then = enable cores, the flow will be:

  1. Send Init to all APs. (to disable all APs)
  2. D= o things that user need to do when all APs are disabled
  3. Notify callback function
  4. Use StartupAllAPs() fro= m existing PPI/Protocol to wake up all APs.

The flow is similar with = how S3 SMM code take control APs and then give the control back in old days= .

 

Personally, I prefer solution1. It is simpler, but i= t does violate spec.

Please let me know your comments or any new idea, pl= ease share.

 

Thanks

Zhiguang       &n= bsp;  

 

_._,_._,_
Groups.io Links:

=20 You receive all messages sent to this group. =20 =20

View/Reply Online (#120843) | =20 | Mute= This Topic | New Topic
Your Subscriptio= n | Contact Group Owner | Unsubscribe [rebecca@openfw.io]

_._,_._,_
--_000_PH0PR11MB5048BFE3E69A64454521EF71902F2PH0PR11MB5048namp_--