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 043F2D8019A for ; Fri, 18 Apr 2025 22:40:38 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=rPpC1HYCUcqq4Q4DdNGCLRMDL9pWGEA0oi3zeiG1rys=; c=relaxed/simple; d=groups.io; h=From:To:CC:Subject:Thread-Topic:Thread-Index:Date:Message-ID:References:In-Reply-To:Accept-Language:msip_labels: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=1745016038; v=1; x=1745275237; b=kaDyZMjUQdIosxQlI9IZtbxaHfOOq/OqmkYs+IFHY/3sOG6+RPBVgdqEi1W+2BDfCN08aQck oXq8fFYGBls6A7c59dYCUgtOJjnx2RX0CHuJg+MU0LA09Kv3kaPQMJk0Og3yJdDVsdKarIE4pBr XetXjDZZcJ4uiCr1yuE57/lF22gEuDMCUGhskpgHFDVgjtiLpuHGh4vL1TTAHDEJZgmkquJqs/H tNDSLkVERdRP5RvARt7Jabb/dcfkl8GkmO7v+ZFgilp4UiIDzbpiTUCtH7iFDIBa+RkWI3HQehb TS87iMl9ARjzSaAfNsSbd28YmAsppn0YBZ1fqJ0DKP8yw== X-Received: by 127.0.0.2 with SMTP id 9Y2cYY7687511xOilKnsKgIb; Fri, 18 Apr 2025 15:40:37 -0700 X-Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.17]) by mx.groups.io with SMTP id smtpd.web10.1147.1745016036588126325 for ; Fri, 18 Apr 2025 15:40:36 -0700 X-CSE-ConnectionGUID: YsiI1MXaRH23q4L0VSeVBg== X-CSE-MsgGUID: DDPFBMBTTcCVc+rVuMhaYA== X-IronPort-AV: E=McAfee;i="6700,10204,11407"; a="46531247" X-IronPort-AV: E=Sophos;i="6.15,222,1739865600"; d="scan'208,217";a="46531247" X-Received: from orviesa007.jf.intel.com ([10.64.159.147]) by fmvoesa111.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Apr 2025 15:40:35 -0700 X-CSE-ConnectionGUID: 6n1uigmVRTGW6BsX012R8g== X-CSE-MsgGUID: Dcl6vbBXQq+zTJXeLK6B6w== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.15,222,1739865600"; d="scan'208,217";a="131796746" X-Received: from orsmsx901.amr.corp.intel.com ([10.22.229.23]) by orviesa007.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Apr 2025 15:37:30 -0700 X-Received: from ORSMSX902.amr.corp.intel.com (10.22.229.24) by ORSMSX901.amr.corp.intel.com (10.22.229.23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1544.14; Fri, 18 Apr 2025 15:37:29 -0700 X-Received: from orsedg603.ED.cps.intel.com (10.7.248.4) by ORSMSX902.amr.corp.intel.com (10.22.229.24) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1544.14 via Frontend Transport; Fri, 18 Apr 2025 15:37:29 -0700 X-Received: from NAM11-BN8-obe.outbound.protection.outlook.com (104.47.58.176) 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.44; Fri, 18 Apr 2025 15:37:29 -0700 X-Received: from CO1PR11MB4929.namprd11.prod.outlook.com (2603:10b6:303:6d::19) by CH3PR11MB8209.namprd11.prod.outlook.com (2603:10b6:610:15d::20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.8632.33; Fri, 18 Apr 2025 22:37:06 +0000 X-Received: from CO1PR11MB4929.namprd11.prod.outlook.com ([fe80::a886:6510:729d:f9d0]) by CO1PR11MB4929.namprd11.prod.outlook.com ([fe80::a886:6510:729d:f9d0%7]) with mapi id 15.20.8655.022; Fri, 18 Apr 2025 22:37:06 +0000 From: "Michael D Kinney via groups.io" To: Kun Qin , "Ni, Ray" CC: "devel@edk2.groups.io" , "Ni, Ray" , "Kinney, Michael D" Subject: Re: [edk2-devel] A bug in the SmmCommunication V3 logic Thread-Topic: A bug in the SmmCommunication V3 logic Thread-Index: AQHbsCxo5nQBH4RMRk+1qEepe02xqLOpBL4QgACO8lCAAComQIAAMy/AgAAC4LCAAAuzgIAAAzIggAABLYA= Date: Fri, 18 Apr 2025 22:37:05 +0000 Message-ID: References: In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: msip_labels: MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_ActionId=7199950a-a9af-4e88-86e5-b41f15734340;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_ContentBits=0;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Enabled=true;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Method=Standard;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Name=Internal;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_SetDate=2025-04-18T07:21:47Z;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_SiteId=72f988bf-86f1-41af-91ab-2d7cd011db47;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Tag=10, 3, 0, 1; x-ms-publictraffictype: Email x-ms-traffictypediagnostic: CO1PR11MB4929:EE_|CH3PR11MB8209:EE_ x-ms-office365-filtering-correlation-id: 2e041472-1c2f-489b-0c82-08dd7ec98c1b x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam-message-info: =?us-ascii?Q?HEp5UkK+Sw09jSuyvkNLIOTyF+7Qx5j3BNRMijqnqE1D7qgUhOgkNxhYPkot?= =?us-ascii?Q?Bzk/DUMlP6bLjLHN/3X9PD7ho272w3MYlZFASZ6ehV413yzU8U4b3DklKE4G?= =?us-ascii?Q?b3ogqk/JB4lXhNlZSDCc3oGIusbtw6NmaEkFkalMt39m8gmIueFCbQ2KESJU?= =?us-ascii?Q?GHTrAFTATGnY2WNX07EWRhGFFYW9tb61ZOU4u6hyCmnfYekSCuGv+A+1473c?= =?us-ascii?Q?/YTqotiTY1tdAVpDvoUskXQ9TONhFK2xxWWigHBlsEjfPv7WUh0jKZ6Wnwx1?= =?us-ascii?Q?8nTWowhMv1sBwEEWtwdDwq9Ue3azK8XHVMLZVHNF8IkA95LvnpWNh3MS1B6M?= =?us-ascii?Q?sud/u5qZW+UNZYB5KC8413ZcB2SStSAfjs/PECl+yQjREmd4vlpkfMR3HP+g?= =?us-ascii?Q?1tUHLjlOvH/uUeKDXePHjVl0y5D7yFdNceGsaZm+NKjwrB+vMf/8OWq9MdDk?= =?us-ascii?Q?VqCCg0lLappYSVOVZqb0xkvsKH8sv+A9lMlLZarZpUdDq7ru2P7PRSr3dZ+M?= =?us-ascii?Q?PoInvZzfcc0OkMWqiP5O2MIeXUIwPkLpXHF87Ou8KpK0ZaDX6d6vwpBtk1fT?= =?us-ascii?Q?A4YvbQLD7dtim1ireSubCa+0yoSvKJYk7Er0k/4tvn//gYjqDVRgL6Hv+5mL?= =?us-ascii?Q?VebVy3PIK0yhnK3eCTL4nDI7n4L12NsaHpDX5dq9LxQhKoxOHA7FQqpIYhAm?= =?us-ascii?Q?gaED1Df41f+9TJ7Em+iXhGUdSzWPrGvM0+dpSyo+TAOXxzcRRJkBIymdddy5?= =?us-ascii?Q?lVuJ2dtmIKzGiKC4w+0nHkjMsovSldgvG4+I7FKlmUUiY6CSpBcx3zTLCaqa?= =?us-ascii?Q?g4oxeFwsW3gDwYqVHHCJsYq52WcGZQr5ddOEhBzjt1QSMhjBIFpZsE+rssIc?= =?us-ascii?Q?QP5FN94JdJszSsLmLN2Y0vGRhBGM8d2x/UKDW40wIrtVLEQ/+gjhL6X2xsCT?= =?us-ascii?Q?p3oDIJweL+eRC7U33FG6TnaKoEAgSL+j2SfRqZl7LCfGZ87AVnKTHNVavlT2?= =?us-ascii?Q?PyoIRXtPQAxIbyxGf1iRoC7rtvKmu+EqhfiutPQcFADIC9WNCy92y/Y7rcXy?= =?us-ascii?Q?iRB63Y7NIPqn2w3qlRMKSC3JMxzWTcdHn6hfzO90ZbRLngjHJxYKTBD5IoJN?= =?us-ascii?Q?zQuPKe6N5iHGMRGnfXmo6sl5whZ8jX3vmJvPWrO9zz8os4/wP3Ga14vpEVPl?= =?us-ascii?Q?PBGfjihZp9GiiiCnW9b+UPfa0noP5uqk1thQs4N5b9SARwpSd1y2v7dPZqxk?= =?us-ascii?Q?Wl+e+umc/RTl/g9UCNUmtKKqfd8vqq17iE56echx1qRKVSKxDZ/IZgYQRkOR?= =?us-ascii?Q?x1ViGuqFntEX5icgXn69zqbm56RmKlVh7L867eW0bWnI6KFWuv5qNDQHvmdB?= =?us-ascii?Q?EqEgjwkzkfpWpH2KVzcjIDUZi4Hi27Or4gD5f+ZLCNyTjYTjFyDV3kZW4N7p?= =?us-ascii?Q?/Ap7UiPjxHZ07NSorH+iOlVkjkDGmw5b7CnnZXjhS82r7W2X4OYdaA9cr9XH?= =?us-ascii?Q?NupRMv/SN3pm8nU=3D?= x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?kO5fMEvcg8/3q74MIqp3ICawCaPVmNuYlyLs+UFOK/yEE31neJy3dEZk5OcB?= =?us-ascii?Q?pi/CBA66zq5J5jPrfXhnM8j9ukB6d9uJOk29B8uLCiubl7dMeJeZr9jQRFxZ?= =?us-ascii?Q?CVKltc6QCHOinVCIKSFamo+xHALuFeK3VpB2770T08HkQHeyi///H0p3BeZA?= =?us-ascii?Q?Ea5xYgb8oIFC/AiKMnrNb909cQoIMyXXzZc+fA4ITiyQoQYDnaKqRYPil3vZ?= =?us-ascii?Q?YxWH7e7tV8p3F6ZnPo+5qEEDdINSMKFrbaWG26KyP5XlwbQf/NN4MteIe7Vw?= =?us-ascii?Q?pcnmY3gfdntkYnM7kuX1ST8ML8FSiBMvyaUftqr7sPYnB0cRqFV+ByZUNQ8V?= =?us-ascii?Q?mJ9l+RpfpS02nZjyDDQMdtGbhpada/f2+gR7h+qluelI7RWiueMGjCM8ADYJ?= =?us-ascii?Q?2oEu2PmBO395LdM2Y5UFZeLoRIYAUPXg/GGm8EWbEJXzrdpHRS2ge9n4uXdl?= =?us-ascii?Q?AE+7a9r20Jevw1CqexbYKwqKdr35HPM3qFE9xF4NsJxWO1tlxd7L9d6VVD/O?= =?us-ascii?Q?3udRvaVU7dO23hFIl6QHnKyUeysWGA9yw460aY9TqzBpuW5SYma58tSXZCZi?= =?us-ascii?Q?acpdwfeRmRDbe+EjpT2JSLL+3dn5FLZSnREizvvgEWhbWoGqe31LLGbnH0c5?= =?us-ascii?Q?wbpvwgWFLHjlRfxxYDreKBQOYQRUCb+slu3VTlKRULtmHoqdWEkx1DID6w15?= =?us-ascii?Q?ikTN3BC/26oxQZ2755WqQWbMPtv7FjnxDO7HhWfHSKNnZ4WQxXBvTvYoX9Ak?= =?us-ascii?Q?gVLgYlZokU0HdQU2IIRYzVejLXvVLW18loyitqxT+Dra8vVp0JWLTnFVqPue?= =?us-ascii?Q?R9AhfoJ/fsVEKYjXLfScPQg1tvvJZ7Lq4VOWC1EXEMKQA4Hs3KwFSrK6787W?= =?us-ascii?Q?AfnkveBWfbX3T6TUyapg8fc4n/YDOEHgVGlAqQCKe4+sv1YZLdOPvIdFWrHt?= =?us-ascii?Q?4KHvSIMcISjOHrs/38S49KSgyjFdFZYOp3h4mndx3U83zP2+XEF6qxNCnErk?= =?us-ascii?Q?XSnhkuBC3sh3sNl5r22aWmu3W/E+xtxTnJgP9e4SqlxpMiqvBgXPg7TRW4CC?= =?us-ascii?Q?jF2ouypLYh4u83QCdHEKhHVH9ScxKFJfZBlhxgMXykx67tw/eN6YqhQWDkNB?= =?us-ascii?Q?5BHkdodD2H3+KYXlvuhwlbstpJG1zniI/cHd2btOVtHyEzZ9VEYFg5TyIqPe?= =?us-ascii?Q?W/msbF9f7pVk+MzCDC1xU4lx2KNIsGRkDRMY8x6IkxdeZsN+dlofCr9ypbQG?= =?us-ascii?Q?XbD7r7kQPnBNTkTDniKekIXAMzWaFQJiJoE3ifRuezlQLMKKRknWhxqGwWrK?= =?us-ascii?Q?wg72PQoC5FxxzChe20D3ME8mtltlN10pLvR6sop39sAOlMb/LorDnBeoetf8?= =?us-ascii?Q?gLhZAl8VQlBlBbOhk/KlIAX4Nv/5K2nXiOJkIYIsjOUzaqIg/P0XB06ZdzAw?= =?us-ascii?Q?7jzHaQ1IWqUQbNXgzVEMex3g0mzH/n4xQKNtUcDkFJwCVPxYWNse3KH+jwlL?= =?us-ascii?Q?144/0o3n37Lxn1YPIfqjv5fIyzelk9kqjnlrrW9T2PP4q9W2CgjprLdbZk8K?= =?us-ascii?Q?MeT8fgnoJ66xfDk6Gd4HmT6CqLRL9i4xojxGqB3r/ng2unY7FnDt5oUXT76g?= =?us-ascii?Q?pQ=3D=3D?= MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: CO1PR11MB4929.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 2e041472-1c2f-489b-0c82-08dd7ec98c1b X-MS-Exchange-CrossTenant-originalarrivaltime: 18 Apr 2025 22:37:06.0041 (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: 8ro6MsAuNPsKkgIy+lbuZSiBvCk+up6VqQIUGIc/JeMkWoyJ4OSqjYobegsL+8lpR0Wj8Wowi37LFwT5DEw2DYqMRMPYjJnjw65QK23VQcQ= X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH3PR11MB8209 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: Fri, 18 Apr 2025 15:40:36 -0700 Resent-From: michael.d.kinney@intel.com Reply-To: devel@edk2.groups.io,michael.d.kinney@intel.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: KiUGL0k0x1mSOcbY3yrLSdNox7686176AA= Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_CO1PR11MB49291986F0F5E0F31A821927D2BF2CO1PR11MB4929namp_" X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20240830 header.b=kaDyZMjU; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 45.79.224.7 as permitted sender) smtp.mailfrom=bounce@groups.io; dmarc=pass (policy=none) header.from=groups.io --_000_CO1PR11MB49291986F0F5E0F31A821927D2BF2CO1PR11MB4929namp_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Take a look at this branch with one commit that implements my proposal so f= ar https://github.com/mdkinney/edk2/commits/MdeModulePkg_Core_PiSmmCore_Virtua= l_Address_Fix/ What version of MM Communicate is used with STMM? Mike From: Kun Qin Sent: Friday, April 18, 2025 3:34 PM To: Kinney, Michael D ; Ni, Ray Cc: devel@edk2.groups.io; Ni, Ray Subject: RE: A bug in the SmmCommunication V3 logic Okay, I can change that. On the other hand, how do we want handle the difference between MM communic= ate for stmm and traditional MM? stmm expects the incoming to be virtual an= d will always touch the buffer. But traditional mm lucked out by never touc= hing the incoming. Regards, Kun From: Kinney, Michael D > Sent: Friday, April 18, 2025 3:23 PM To: Kun Qin >; Ni, Ray = > Cc: devel@edk2.groups.io; Ni, Ray >; Kinney, Michael D > Subject: [EXTERNAL] RE: A bug in the SmmCommunication V3 logic I think my proposal is better to have separate logic. One for V1/V2 and an= other for V3. I agree that V1 must pass in non NULL CommSize and we can do spec change fo= r that since NULL CommSize is not implementable. V2 can support CommSize optional because it has phys and virt. V3 does not have CommSize. Mike From: Kun Qin > Sent: Friday, April 18, 2025 3:18 PM To: Kinney, Michael D >; Ni, Ray > Cc: devel@edk2.groups.io; Ni, Ray > Subject: RE: A bug in the SmmCommunication V3 logic Hi Mike, I created a PR on edk2 site but I was thinking about redoing it... Because = of the v1 implementation is a bit chaotic. The original form are still broken if we back out the change, according to = our discussion below. Specifically for v1, we cannot touch the incoming buf= fer after going virtual at all, because we need the incoming buffer to be p= hysical to populate the gSmmCorePrivate->CommunicationBuffer pointer. Other= wise, the validation inside SMM will work pass. On the StMm side, we are partially covered because MM communicate on standa= lone mm will always intake the incoming and copy it over to the static buff= er `CopyMem ((VOID *)(UINTN)mMmCommonBuffer.PhysicalStart, CommBuffer, Buff= erSize);`. However, this bears the assumption that we will need to pass in = virtual address when invoking Stmm backed MM communicate v1 to make it work= , which is a contradictory expectation compared to traditional MM communica= te protocol. One solution is to make comm size a requirement for v1, and v2/3 can check = the incoming content through the virtual address. Please let me know your t= houghts. Thanks, Kun From: Kinney, Michael D > Sent: Friday, April 18, 2025 2:34 PM To: Kun Qin >; Ni, Ray = > Cc: devel@edk2.groups.io; Ni, Ray >; Kinney, Michael D > Subject: [EXTERNAL] RE: A bug in the SmmCommunication V3 logic Hi Kun, I have a proposed solution that puts SmmCommunicationCommunicate() back to = its original form, so there are no changes in behavior for SmmCommunication= Communicate() or SmmCommunicationMmCommunicate2(). I then pull the V3 only logic into MmCommunicationMmCommunicate3(). It has some code duplication, but makes it easier to understand and uses th= e virtual and physical pointers in the right places for the V3 implementati= on. What were you thinking? Mike From: Kun Qin > Sent: Friday, April 18, 2025 11:26 AM To: Kinney, Michael D >; Ni, Ray > Cc: devel@edk2.groups.io; Ni, Ray > Subject: RE: A bug in the SmmCommunication V3 logic Hi Mike, Thanks for the explanation. That makes sense. I confirmed the issue on PiSm= mIpl side. A patch should be available some time today after I test both tr= aditional MM and Standalone MM paths. Regards, Kun From: Kinney, Michael D > Sent: Friday, April 18, 2025 8:59 AM To: Kun Qin >; Ni, Ray = > Cc: devel@edk2.groups.io; Ni, Ray >; Kinney, Michael D > Subject: [EXTERNAL] RE: A bug in the SmmCommunication V3 logic Hi Kun, I think the reason it may have always worked is for the case where SmmCommu= nicationCommunicate() at runtime after SetVirtualAddressMap() and CommSize = is not NULL. In that case, CommunicateHeader that points to CommBuffer is = never dereferenced and a call to mSmmControl2->Trigger() is made. If CommSize is NULL, then MessageLength field of CommunicateHeader would be= dereferenced as a physical address at OS runtime after SetVirtualAddressMa= p() and a page fault would occur. My guess is that CommSize is never NULL,= and that is why we have been lucky. With the addition of the check for the V3 GUID, the HeaderGuid field of Com= municateHeader is always dereferenced. So if CommunicateHeader is a physic= al address at OS Runtime after SetVirtualAddressMap(), then a page fault wi= ll always occur. No more luck. Mike From: Kun Qin > Sent: Friday, April 18, 2025 12:30 AM To: Ni, Ray > Cc: Kinney, Michael D >; devel@edk2.groups.io; Ni, Ray > Subject: RE: A bug in the SmmCommunication V3 logic Hi Ray, I will verify the code first thing tomorrow morning. But I just looked at t= he code flow before the change, it seems that SmmCommunicationMmCommunicate= 2 is also using physical address, and the common routine will deference the= pointer to read message length as well. I just checked variable runtime dr= iver did not convert the input physical pointer. Would that cause the same = issue? Did I miss something or we have lucked out all along? Regards, Kun From: Ni, Ray > Sent: Thursday, April 17, 2025 11:45 PM To: Kun Qin > Cc: Kinney, Michael D >; devel@edk2.groups.io; Ni, Ray > Subject: [EXTERNAL] A bug in the SmmCommunication V3 logic Hi Qin, I think there is a bug in the SmmCommunication protocol implementation. All 3 communication protocol calls go to the same communicate() function th= at tests the HeaderGuid against the V3 GUID. But when the call is from runtime, reading the HeaderGuid using the physica= l address of communication buffer would cause page fault. The virtual addre= ss should be used. The bug was not there without your patch because the communicate routines h= appened not to read any bytes from the communication buffer but simply pass= the address to SMM. SMM expects the physical address because the virtual-t= o-physical mapping in SMM is identical. The bug exists in both the SmmIpl.c in MdeModulePkg and the MmCommunication= Dxe.c in StandaloneMmPkg. The bug would cause OS boot failure if there is any communication protocol = invocation after ExitBootService. I guess the bug might not be there in your first version of patch, but was = introduced when I asked you to consolidate the logic together. Can you kindly reproduce it locally and send out a fix after confirming? Thanks, Ray -=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 (#121269): https://edk2.groups.io/g/devel/message/121269 Mute This Topic: https://groups.io/mt/112327494/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_CO1PR11MB49291986F0F5E0F31A821927D2BF2CO1PR11MB4929namp_ Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable

Take a look at this= branch with one commit that implements my proposal so far

 

https://github.com/mdkinney/edk2/commits/MdeModulePkg_Core_PiSmmCor= e_Virtual_Address_Fix/

 

What version of MM = Communicate is used with STMM?

 

Mike

 

From: Kun Qin <Kun.Qin@microsoft.= com>
Sent: Friday, April 18, 2025 3:34 PM
To: Kinney, Michael D <michael.d.kinney@intel.com>; Ni, Ray &l= t;ray.ni@intel.com>
Cc: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>
Subject: RE: A bug in the SmmCommunication V3 logic

 

Okay, I can change that.

 

On the other hand, how do we want handle the differ= ence between MM communicate for stmm and traditional MM? stmm expects the i= ncoming to be virtual and will always touch the buffer. But traditional mm lucked out by never touching the incoming.=

 

Regards,

Kun

 

From: Kinney, Michael D <michael.d.kinney@intel.com>
Sent: Friday, April 18, 2025 3:23 PM
To: Kun Qin <Kun.Qin@mic= rosoft.com>; Ni, Ray <ray.ni@= intel.com>
Cc: devel@edk2.groups.io= ; Ni, Ray <ray.ni@intel.com>;= Kinney, Michael D <michae= l.d.kinney@intel.com>
Subject: [EXTERNAL] RE: A bug in the SmmCommunication V3 logic<= /o:p>

 

I think my proposal= is better to have separate logic.  One for V1/V2 and another for V3.<= o:p>

 

I agree that V1 mus= t pass in non NULL CommSize and we can do spec change for that since NULL C= ommSize is not implementable.

 

V2 can support Comm= Size optional because it has phys and virt. 

 

V3 does not have Co= mmSize.

 

Mike

 

 

 

From: Kun Qin <Kun.Qin@microsoft.com>
Sent: Friday, April 18, 2025 3:18 PM
To: Kinney, Michael D <michael.d.kinney@intel.com>; Ni, Ray <ray.ni@intel.com>
Cc: devel@edk2.groups.io= ; Ni, Ray <ray.ni@intel.com><= br> Subject: RE: A bug in the SmmCommunication V3 logic

 

Hi Mike,

 

I created a PR on edk2 site but I was thinking abou= t redoing it… Because of the v1 implementation is a bit chaotic.=

 

The original form are still broken if we back out t= he change, according to our discussion below. Specifically for v1, we canno= t touch the incoming buffer after going virtual at all, because we need the incoming buffer to be physical to popul= ate the gSmmCorePrivate->CommunicationBuffer pointer. Otherwise, the val= idation inside SMM will work pass.

 

On the StMm side, we are partially covered because = MM communicate on standalone mm will always intake the incoming and copy it= over to the static buffer `CopyMem ((VOID *)(UINTN)mMmCommonBuffer.Physica= lStart, CommBuffer, BufferSize);`. However, this bears the assumption that we will= need to pass in virtual address when invoking Stmm backed MM communicate v1 to make = it work, which is a contradictory expectation compared to traditional MM co= mmunicate protocol.

 

One solution is to make comm size a requirement for= v1, and v2/3 can check the incoming content through the virtual address. P= lease let me know your thoughts.

 

Thanks,

Kun

 

From: Kinney, Michael D <michael.d.kinney@intel.com>
Sent: Friday, April 18, 2025 2:34 PM
To: Kun Qin <Kun.Qin@mic= rosoft.com>; Ni, Ray <ray.ni@= intel.com>
Cc: devel@edk2.groups.io= ; Ni, Ray <ray.ni@intel.com>;= Kinney, Michael D <michae= l.d.kinney@intel.com>
Subject: [EXTERNAL] RE: A bug in the SmmCommunication V3 logic<= /o:p>

 

Hi Kun,<= /span>

 

I have a proposed s= olution that puts SmmCommunicationCommunicate() back to its original form, = so there are no changes in behavior for SmmCommunicationCommunicate() or Sm= mCommunicationMmCommunicate2().

 

I then pull the V3 = only logic into MmCommunicationMmCommunicate3().

 

It has some code du= plication, but makes it easier to understand and uses the virtual and physi= cal pointers in the right places for the V3 implementation.

 

What were you think= ing?

 

Mike

 

 

From: Kun Qin <Kun.Qin@microsoft.com>
Sent: Friday, April 18, 2025 11:26 AM
To: Kinney, Michael D <michael.d.kinney@intel.com>; Ni, Ray <ray.ni@intel.com>
Cc: devel@edk2.group= s.io; Ni, Ray <r= ay.ni@intel.com>
Subject: RE: A bug in the SmmCommunication V3 logic

 

Hi Mike,

 

Thanks for the explanation. That makes sense. I con= firmed the issue on PiSmmIpl side. A patch should be available some time to= day after I test both traditional MM and Standalone MM paths.

 

Regards,

Kun

 

From: Kinney, Michael D <<= a href=3D"mailto:michael.d.kinney@intel.com">michael.d.kinney@intel.com>
Sent: Friday, April 18, 2025 8:59 AM
To: Kun Qin <Kun= .Qin@microsoft.com>; Ni, Ray <ray.ni@intel.com>
Cc: devel@edk2.group= s.io; Ni, Ray <r= ay.ni@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: [EXTERNAL] RE: A bug in the SmmCommunication V3 logic<= /o:p>

 

Hi Kun,<= /span>

 

I think the reason = it may have always worked is for the case where SmmCommunicationCommunicate= () at runtime after SetVirtualAddressMap() and CommSize is not NULL.  = In that case, CommunicateHeader that points to CommBuffer is never dereferenced and a call to mSmmControl2->Trigger= () is made.

 

If CommSize is NULL= , then MessageLength field of CommunicateHeader would be dereferenced as a = physical address at OS runtime after SetVirtualAddressMap() and a page faul= t would occur.  My guess is that CommSize is never NULL, and that is why we have been lucky.

 

With the addition o= f the check for the V3 GUID, the HeaderGuid field of CommunicateHeader is a= lways dereferenced.  So if CommunicateHeader is a physical address at = OS Runtime after SetVirtualAddressMap(), then a page fault will always occur.  No more luck.=

 

Mike

 

From: Kun Qin <Kun.Qin@microsoft.com>
Sent: Friday, April 18, 2025 12:30 AM
To: Ni, Ray <ray.ni@i= ntel.com>
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io= ; Ni, Ray <ray.ni@int= el.com>
Subject: RE: A bug in the SmmCommunication V3 logic

 

Hi Ray,

 

I will verify the code first thing tomorrow morning= . But I just looked at the code flow before the change, it seems that SmmCo= mmunicationMmCommunicate2 is also using physical address, and the common routine will deference the pointer to read message= length as well. I just checked variable runtime driver did not convert the= input physical pointer. Would that cause the same issue? Did I miss someth= ing or we have lucked out all along?

 

Regards,

Kun

 

From: Ni, Ray <ray.ni@intel.com>
Sent: Thursday, April 17, 2025 11:45 PM
To: Kun Qin <Kun= .Qin@microsoft.com>
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io= ; Ni, Ray <ray.ni@int= el.com>
Subject: [EXTERNAL] A bug in the SmmCommunication V3 logic

 

Hi Qin,=

I think= there is a bug in the SmmCommunication protocol implementation.=

&n= bsp;

All 3 c= ommunication protocol calls go to the same communicate() function that test= s the HeaderGuid against the V3 GUID.

But whe= n the call is from runtime, reading the HeaderGuid using the physical addre= ss of communication buffer would cause page fault. The virtual address shou= ld be used.

The bug= was not there without your patch because the communicate routines happened= not to read any bytes from the communication buffer but simply pass the ad= dress to SMM. SMM expects the physical address because the virtual-to-physical mapping in SMM is identical.<= /o:p>

&n= bsp;

The bug= exists in both the SmmIpl.c in MdeModulePkg and the MmCommunicationDxe.c i= n StandaloneMmPkg.

The bug= would cause OS boot failure if there is any communication protocol invocat= ion after ExitBootService.

&n= bsp;

I guess= the bug might not be there in your first version of patch, but was introdu= ced when I asked you to consolidate the logic together.

&n= bsp;

Can you= kindly reproduce it locally and send out a fix after confirming?

&n= bsp;

Thanks,=

Ray

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

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

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

_._,_._,_
--_000_CO1PR11MB49291986F0F5E0F31A821927D2BF2CO1PR11MB4929namp_--