From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id 9A920740032 for ; Mon, 22 Jan 2024 23:30:41 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=DPg+n09DGw0w76hNTSmLU136C+MWl39YliByDiRxyVk=; c=relaxed/simple; d=groups.io; h=ARC-Seal:ARC-Message-Signature:ARC-Authentication-Results: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:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type:Content-Transfer-Encoding; s=20140610; t=1705966240; v=1; b=YD14EFgCN7PKIwGqkRBsuNsaEolfD3Z33eZMAu1tCvUGaScYg+awzOgFdqM9bqKD4avfvWYi K7KKEALXw/zwNEwOPaaZ/RbeVEGAPj2dIrTVADZ6NOtihcXsWgK8qJQhL+R9ryoRwdH7SQNGAVK A2NzDZAlpNqCmiS/45XlG7m4= X-Received: by 127.0.0.2 with SMTP id 4wwoYY7687511xnQeZAGBq56; Mon, 22 Jan 2024 15:30:40 -0800 X-Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.11]) by mx.groups.io with SMTP id smtpd.web10.8501.1705966239467183396 for ; Mon, 22 Jan 2024 15:30:39 -0800 X-IronPort-AV: E=McAfee;i="6600,9927,10961"; a="8012057" X-IronPort-AV: E=Sophos;i="6.05,212,1701158400"; d="scan'208";a="8012057" X-Received: from orviesa001.jf.intel.com ([10.64.159.141]) by orvoesa103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 22 Jan 2024 15:30:38 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.05,212,1701158400"; d="scan'208";a="34197326" X-Received: from orsmsx601.amr.corp.intel.com ([10.22.229.14]) by orviesa001.jf.intel.com with ESMTP/TLS/AES256-GCM-SHA384; 22 Jan 2024 15:30:38 -0800 X-Received: from orsmsx611.amr.corp.intel.com (10.22.229.24) by ORSMSX601.amr.corp.intel.com (10.22.229.14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.35; Mon, 22 Jan 2024 15:30:37 -0800 X-Received: from orsedg603.ED.cps.intel.com (10.7.248.4) by orsmsx611.amr.corp.intel.com (10.22.229.24) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.35 via Frontend Transport; Mon, 22 Jan 2024 15:30:37 -0800 X-Received: from NAM11-CO1-obe.outbound.protection.outlook.com (104.47.56.168) by edgegateway.intel.com (134.134.137.100) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.35; Mon, 22 Jan 2024 15:30:36 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ge3EqLHuOTRApJxQoyhh1RD1J5rzR8sM6EcHr6tOxil+YC+3Ksn5GyOvVRk/7GTqlC4X7ztemjJhBx2nfKJka5lvUsNVRPHvXUnvk40J1Zhc4c841+Q0vE4vAfYzNBQ1PzI7FtorE3zrTnOfHzZexsIrZFUAePejIYH1EYJ5mvYPa3/n98RI20sXeNvm+xFxy4M2osoZxb+FcInOZAZprfyCoFBKRUARDaUw2d7VkvSL4wn+cCNpUW6bLWe3Ix2XGZTuKRssCFZKRfOG1bkufQFuaUfbx2VGwHCPBJI45DF678HoGmjeq1ClYCSDw6Z08R10PY3rzy2rsorIyGuoYA== 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=4w8eJuTZSzBPfcrYL+PSZBkMzn9lfCtk5wLMhqN53lY=; b=aB2m+cSU6pjhuF0aJSMVo0YA2CTf7SXOdZKjOK0SymEqeFZ5P3CfGRLRF6kBTLfDTW//4LpQUihAEOWv8hESIIq1+6PYxcx09MzGT6wO/EHbw+zeZzzm/hNDwAnY8/9xDe9CLA75+I0yIkX8//abvBKD3N4nAE3Q1dw9QBwQb8evhJW29+t9AVQV0zOWjbRWrH3KqhhdsDQ039gnFDEcMQsS5djIoGwzub/GsMn+zW1ZZ+NjzOhEau2NNP12RusgrPtKPpvmWnakNnF/f8KbCrdiThVgH5llWntCL95FTGqBy8TqFIanlXI6b16fehyBmrs3EfyTpahAJZNUpqMnxA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none X-Received: from SA2PR11MB4938.namprd11.prod.outlook.com (2603:10b6:806:fb::14) by LV2PR11MB6021.namprd11.prod.outlook.com (2603:10b6:408:17e::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7202.31; Mon, 22 Jan 2024 23:30:34 +0000 X-Received: from SA2PR11MB4938.namprd11.prod.outlook.com ([fe80::af34:7981:454b:39eb]) by SA2PR11MB4938.namprd11.prod.outlook.com ([fe80::af34:7981:454b:39eb%4]) with mapi id 15.20.7202.034; Mon, 22 Jan 2024 23:30:34 +0000 From: "Michael D Kinney" To: "Ni, Ray" , Nhi Pham , "devel@edk2.groups.io" , "Andrew Fish (afish@apple.com)" CC: "ardb+tianocore@kernel.org" , "sami.mujawar@arm.com" , "lersek@redhat.com" , "Kinney, Michael D" Subject: Re: [edk2-devel] [PATCH 1/1] StandaloneMmPkg/Core: Remove optimization for depex evaluation Thread-Topic: [PATCH 1/1] StandaloneMmPkg/Core: Remove optimization for depex evaluation Thread-Index: AQHaTN/aWBI6E4ucKUSceUStJuUnybDmdj2g Date: Mon, 22 Jan 2024 23:30:34 +0000 Message-ID: References: <20240119045646.3896430-1-nhi@os.amperecomputing.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-publictraffictype: Email x-ms-traffictypediagnostic: SA2PR11MB4938:EE_|LV2PR11MB6021:EE_ x-ms-office365-filtering-correlation-id: 3061b50f-282b-45c5-0315-08dc1ba2219f x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam-message-info: Ecv9z/IofR762sPzv3NrIMYJk3HcC9MknORULiVHu5GzVbKfyq6rTPOxGWVDZmhCn2qRljdTRulpoemDKwQhb9AGFKvOJxgKYUVPXsRYkSOpJWrb32L+s8vc6A5XC6evZBIp+7281EqQhjitpmRjobv6WrPer5Vy4h3WGV7GM/2e2DiLbHbGrI1JMVRePL58Jn+Gq4AdifQqUdI5ttSbcNOROc0e/cXF9zmP5uf8NBV07R8sDlNXuqQYEZFYfIzvDWcqOxjICgmJIm1W7l4Z4kTO8BG21zRx+zik9J2kxSp/VshzSBxDc60iPsuuuRcaqQ5J0F2MNzG0sSXjjdd2NMfBeOCdtWmJSi4hLagZaMVOyU/lIMEPFXPV+AKya368Occ8TJu61NiL3e7hpTGoavVKvxjDKID1HyxzAv1kkUQb7d7Mjbyw6GYykrD+094sXDdQJrozoDL9NO/Y/OoN8DZplsfdqFiqxbLeHTKZ1ow/s7FWmV/JZRcSm1Gx9vmkLAApqXp3nk4JXHj/yiD8u3TPCAyqIIvdIFBQYpq/82J0ApI8LVMLv61/MfUoiNoFiQg8GitgmgtME4wRc51KHHQBJm1Xkwg546eMSelUsdM= x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?5CNFIRQxvjfg/qqMrYg1VBAIvqMI6Xd/2QTQ+H/hHESPB73l3HqJ8lcdgMSb?= =?us-ascii?Q?WE+SBUM8vahQXsDZOe12HljJdcPSgpTwNpG/sonu0oOD+CSEHBxPaPsAY8Qi?= =?us-ascii?Q?P0mzR2X1zbIE1pQ6ArfOtMwsXJT5PHJG+epsVpZ1InYaqnrO+YNSQx+fVOYr?= =?us-ascii?Q?L7kH5DU9QVvIp+qxk8bHVVMwZTynJ29nZb0GdQtxzd87d+4W708KG8cOdKfT?= =?us-ascii?Q?0b0P2fE0M4SCn9N7HfXkQXKrs3+AoBelEwXCRGwa9fTwPnLKQYHByFiTj1xj?= =?us-ascii?Q?SkDYfnSuHbDzMWsOtwY97I+k8fEmCBmIrIFGmJgOT566nZELywUdOwvabpB8?= =?us-ascii?Q?65Gsx3T63BZFg3OFxJYUR142xjIclrV8SKhazqjZ0Do3ORAMlq9KHOwCTsND?= =?us-ascii?Q?wFKQ27B/2XHhixkLzQ2m2XsZI3npEdLJpNK9/MHWy5qnwEThj+yS/avUYk7+?= =?us-ascii?Q?j2lCEI0nl6SWjF3QBUlMYoGc4rtJ2Ew6X1rxfHCdyM8URzGw+DR9y+D2Q4ZR?= =?us-ascii?Q?HbbELvrcjDzQJnWBi5L0OAFOitDntk7Xp+QExybLngRT2PipLlFApg43asR6?= =?us-ascii?Q?K4vuh7g6C4e08CYS/EjFFVuzh7QlQaWDUH1ktjmrdFwPMlnxCH4V389fmo/L?= =?us-ascii?Q?CMAP7CdjhV41p9MdSBZPInl7kDIa5cnT03na6+2RLkTR/fZ/P8JG5zCmsVQa?= =?us-ascii?Q?s675HAO4uvKrLlITIqDSIK77kiZJc2WuOOBoUHJbW+1DcAbGq4NbMjRVOEvC?= =?us-ascii?Q?WK1rmAaTRdyuhg/RS7MIAT6mWqpsYlmKocILTj7UTeoY+dpFhEhqXthiPj31?= =?us-ascii?Q?gYHpsH6mSfUs+v0HLdG90HVW/7p7wWSiY8yk0h5/sPkZgc5VfoYpNcfD6Wmr?= =?us-ascii?Q?V0NLW0mN9zodPMAQuy2BSiVMgKk+6dPP078+GvDRw66/tM7ayY7eTFYQ1Q3t?= =?us-ascii?Q?IlDFHhg67tzPJu3kSZExjFfnkMlw45QJK0JIrLCx0SecridTz6X8kI6OupIw?= =?us-ascii?Q?c7VcOVexyYCfstIIXfqz8xJyAYVmfZOpC/+0pIr/VRb8XxyxUoCEda7qO2aQ?= =?us-ascii?Q?y9+RiAI3AjHftKf+XyXdJM9uJBoSew/sXNm6iBtDJJztVMJupfFDtUHpjITU?= =?us-ascii?Q?5Mj+Co49OONCWGdUhKb7I1bKJKJJgYQvxHroUl+bcoDXeppbdRsJ0yR/kdKz?= =?us-ascii?Q?mDu9OfQ4n/Uao4rTx1OYleV9pSUwn3UJUyEMJjoGFuhZSk7gtB5+xKkO5vqy?= =?us-ascii?Q?5m0LgTwMHFrhu2mPZngbDyPOz8M0sZUFpPMg4E5uchcLCVGnacfcEZIiRDUm?= =?us-ascii?Q?wea0GFlItdBdp+HI0UGDQ5FXeMyZkdCk8+9hyeNLxlyBLFUMuQo1BYvf0Ppa?= =?us-ascii?Q?LUGthlKaPnThauFnF/5GyfojtTX9q7k+bpgjbgohAUMRppEZfKiw890QRJdr?= =?us-ascii?Q?GcfF+ZjTLFXfW5oTPhmT7BvydOVYmKZkpxS9bwM+EdYb3MaFEzkFtDfmT64q?= =?us-ascii?Q?QWenfaCGpCmP5McAcyzhUkCrzZxqLS6JWKbpBfU/2XTx8HAoA+TEFxyDiKGW?= =?us-ascii?Q?e1OOVlv8293o9cfPSY3CTtbNwSFeZF80Eq3oPLFg?= MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: SA2PR11MB4938.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 3061b50f-282b-45c5-0315-08dc1ba2219f X-MS-Exchange-CrossTenant-originalarrivaltime: 22 Jan 2024 23:30:34.2366 (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: kRm3XWK0CD6b+aGkjbUdCRu96HxEvT2BkH/BoghXh//PVx/0T7r2SUa7uqy271FxLc3rr+wtrYT2dFbHMzLDQGGOatCufRYm6e7ajjyjjGY= X-MS-Exchange-Transport-CrossTenantHeadersStamped: LV2PR11MB6021 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 Reply-To: devel@edk2.groups.io,michael.d.kinney@intel.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: pidAEKVre49pMKE1qj8kavDpx7686176AA= Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=YD14EFgC; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=intel.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io; arc=reject ("signature check failed: fail, {[1] = sig:microsoft.com:reject}") Hi Ray, +Andrew Fish That optimization was imported into git history 17 years ago, so it has effectively always been there. I do not recall the performance improvement at the time the optimization was originally implemented. The difference in behavior is that caching the result may miss an uninstall later before dispatch. Always evaluating right before=20 dispatch is safer because is guarantees that the expression is TRUE and all conditions met when the image is started. Your example is possible, but not likely to appear in practice for the types of protocols used in dependency expressions. Protocols that are uninstalled are more typically associated with=20 the UEFI Driver model for buses/devices that can be added/removed. If CoreLocateProtocol() was optimized, then perhaps this optimization would have never been implemented. I see no harm in removing the optimization, especially for only Standalone MM. If there is a need to treat DEPEX section of all images as const, then there are other places that the cached evaluation could be stored to enable this specific optimization for all image types. Best regards, Mike > -----Original Message----- > From: Ni, Ray > Sent: Sunday, January 21, 2024 7:05 PM > To: Nhi Pham ; devel@edk2.groups.io; Kinney, > Michael D > Cc: ardb+tianocore@kernel.org; sami.mujawar@arm.com; lersek@redhat.com > Subject: RE: [PATCH 1/1] StandaloneMmPkg/Core: Remove optimization for > depex evaluation >=20 > Reviewed-by: Ray Ni >=20 > PI spec does not define the REPLACE_TRUE op-code. > Existing dispatchers (DXE, SMM and Standalone MM) use REPLACE_TRUE to > optimize the protocol evaluation. PEI dispatcher does not use this > optimization as the Depex binary might be in flash. >=20 > Now this patch only modifies standalone MM version to not use the > optimization. I think it's a right way. >=20 >=20 >=20 > Because the optimization cannot handle a case: > 1. driver A installs protocol #a. > 2. driver B depends on protocol #a. > 3. driver A uninstalls the protocol #a in a callback (protocol callback, > timer callback, or a service provided by A that driver B invokes) > 4. driver B gets dispatched after the callback. <--- problem here. The > protocol #a does not exist!! >=20 > @Kinney, Michael D, do you have history data of which optimization > result it achieved? Do you think that the optimization can be in > CoreLocateProtocol() instead of in the callers? >=20 > Thanks, > Ray > > -----Original Message----- > > From: Nhi Pham > > Sent: Friday, January 19, 2024 12:57 PM > > To: devel@edk2.groups.io > > Cc: ardb+tianocore@kernel.org; Ni, Ray ; > > sami.mujawar@arm.com; lersek@redhat.com; Nhi Pham > > > > Subject: [PATCH 1/1] StandaloneMmPkg/Core: Remove optimization for > > depex evaluation > > > > From: Laszlo Ersek > > > > The current dependency evaluator violates the memory access permission > > when patching depex grammar directly in the read-only depex memory > area. > > > > Laszlo pointed out the optimization issue in the thread (1) "Memory > > Attribute for depex section" and provided suggested patch to remove > the > > perf optimization. > > > > In my testing, removing the optimization does not make significant > perf > > reduction. That makes sense that StandaloneMM dispatcher only searches > > in MM protocol database and does not depend on UEFI/DXE protocol > > database. Also, we don't have many protocols in StandaloneMM like > > UEFI/DXE. > > > > From Laszlo, > > > > "The patch removes the EFI_DEP_REPLACE_TRUE handling altogether, plus > it > > CONST-ifies the Iterator pointer (which points into the DEPEX > section), > > so that the compiler catch any possible accesses at *build time* that > > would write to the write-protected DEPEX memory area." > > > > (1) https://edk2.groups.io/g/devel/message/113531 > > > > Signed-off-by: Nhi Pham > > --- > > StandaloneMmPkg/Core/Dependency.c | 37 ++++---------------- > > 1 file changed, 7 insertions(+), 30 deletions(-) > > > > diff --git a/StandaloneMmPkg/Core/Dependency.c > > b/StandaloneMmPkg/Core/Dependency.c > > index 440fe3e45238..2bcb07d34666 100644 > > --- a/StandaloneMmPkg/Core/Dependency.c > > +++ b/StandaloneMmPkg/Core/Dependency.c > > @@ -13,16 +13,6 @@ > > > > > > #include "StandaloneMmCore.h" > > > > > > > > -/// > > > > -/// EFI_DEP_REPLACE_TRUE - Used to dynamically patch the dependency > > expression > > > > -/// to save time. A EFI_DEP_PUSH is > > evaluated one an > > > > -/// replaced with EFI_DEP_REPLACE_TRUE. If > > PI spec's Vol 2 > > > > -/// Driver Execution Environment Core > > Interface use 0xff > > > > -/// as new DEPEX opcode. > > EFI_DEP_REPLACE_TRUE should be > > > > -/// defined to a new value that is not > > conflicting with PI spec. > > > > -/// > > > > -#define EFI_DEP_REPLACE_TRUE 0xff > > > > - > > > > /// > > > > /// Define the initial size of the dependency expression evaluation > stack > > > > /// > > > > @@ -170,12 +160,12 @@ MmIsSchedulable ( > > IN EFI_MM_DRIVER_ENTRY *DriverEntry > > > > ) > > > > { > > > > - EFI_STATUS Status; > > > > - UINT8 *Iterator; > > > > - BOOLEAN Operator; > > > > - BOOLEAN Operator2; > > > > - EFI_GUID DriverGuid; > > > > - VOID *Interface; > > > > + EFI_STATUS Status; > > > > + CONST UINT8 *Iterator; > > > > + BOOLEAN Operator; > > > > + BOOLEAN Operator2; > > > > + EFI_GUID DriverGuid; > > > > + VOID *Interface; > > > > > > > > Operator =3D FALSE; > > > > Operator2 =3D FALSE; > > > > @@ -253,8 +243,7 @@ MmIsSchedulable ( > > Status =3D PushBool (FALSE); > > > > } else { > > > > DEBUG ((DEBUG_DISPATCH, " PUSH GUID(%g) =3D TRUE\n", > > &DriverGuid)); > > > > - *Iterator =3D EFI_DEP_REPLACE_TRUE; > > > > - Status =3D PushBool (TRUE); > > > > + Status =3D PushBool (TRUE); > > > > } > > > > > > > > if (EFI_ERROR (Status)) { > > > > @@ -356,18 +345,6 @@ MmIsSchedulable ( > > DEBUG ((DEBUG_DISPATCH, " RESULT =3D %a\n", Operator ? > > "TRUE" : "FALSE")); > > > > return Operator; > > > > > > > > - case EFI_DEP_REPLACE_TRUE: > > > > - CopyMem (&DriverGuid, Iterator + 1, sizeof (EFI_GUID)); > > > > - DEBUG ((DEBUG_DISPATCH, " PUSH GUID(%g) =3D TRUE\n", > > &DriverGuid)); > > > > - Status =3D PushBool (TRUE); > > > > - if (EFI_ERROR (Status)) { > > > > - DEBUG ((DEBUG_DISPATCH, " RESULT =3D FALSE (Unexpected > > error)\n")); > > > > - return FALSE; > > > > - } > > > > - > > > > - Iterator +=3D sizeof (EFI_GUID); > > > > - break; > > > > - > > > > default: > > > > DEBUG ((DEBUG_DISPATCH, " RESULT =3D FALSE (Unknown > > opcode)\n")); > > > > goto Done; > > > > -- > > 2.25.1 -=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 (#114156): https://edk2.groups.io/g/devel/message/114156 Mute This Topic: https://groups.io/mt/103824815/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/19134562= 12/xyzzy [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-