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 92A0378003C for ; Sat, 19 Apr 2025 03:32:16 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=fnz8J6fpfX1ESZHapZaLSIeeVtTKTLEYZiS3T74vuxY=; 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=1745033536; v=1; x=1745292735; b=H/LXuezlEjW0BVdLlTswOZwWRqnwy0W44YqrqRoG/fibm1XTCiylEty69ysIeJmXYemfE3Xp nOHtbIEhbJL5smFwtD9sNzUG8foGKLpAJUgxeUxPZWCOcj2lovaoVMmxDhhuaRfTPOS4dpE9yCQ AcbTzWswDK/5+tYXxGxOLT7yFIGyJR0bDI3hQLZ6YGfxqJVHFK67Fn8ZVHENmTKeczB6mVDLSho LDEZ3c5kQ9ELjqNfpqEr76JhMIKvbZzxox8GB0KpoHBIlqI8BCaJNAv+qCH84qVBrFSCggI+k8v LDuxxcnc0/M0yE9aE/0f0FgOT745CUyDj00YaOn9jtExQ== X-Received: by 127.0.0.2 with SMTP id M0bAYY7687511xCoz9telP0m; Fri, 18 Apr 2025 20:32:15 -0700 X-Received: from SJ2PR03CU002.outbound.protection.outlook.com (SJ2PR03CU002.outbound.protection.outlook.com [52.101.44.123]) by mx.groups.io with SMTP id smtpd.web11.1207.1745016368950772925 for ; Fri, 18 Apr 2025 15:46:09 -0700 X-Received: from CH3PR21MB4517.namprd21.prod.outlook.com (2603:10b6:610:21b::17) by CH4PR21MB4243.namprd21.prod.outlook.com (2603:10b6:610:228::16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.8678.10; Fri, 18 Apr 2025 22:46:06 +0000 X-Received: from CH3PR21MB4517.namprd21.prod.outlook.com ([fe80::7bdd:bb65:8eb1:f082]) by CH3PR21MB4517.namprd21.prod.outlook.com ([fe80::7bdd:bb65:8eb1:f082%4]) with mapi id 15.20.8678.011; Fri, 18 Apr 2025 22:46:06 +0000 From: "Kun Qin via groups.io" To: "Kinney, Michael D" , "Ni, Ray" CC: "devel@edk2.groups.io" , "Ni, Ray" Subject: Re: [edk2-devel] A bug in the SmmCommunication V3 logic Thread-Topic: A bug in the SmmCommunication V3 logic Thread-Index: AQHbsCxo5nQBH4RMRk+1qEepe02xqLOpBL4QgACO8lCAAComQIAAMy/AgAAC4LCAAAuzgIAAAzIggAABLYCAAAK58A== Date: Fri, 18 Apr 2025 22:46:06 +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: CH3PR21MB4517:EE_|CH4PR21MB4243:EE_ x-ms-office365-filtering-correlation-id: b69bd85c-d8be-4311-77cf-08dd7ecace27 x-ld-processed: 72f988bf-86f1-41af-91ab-2d7cd011db47,ExtAddr x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam-message-info: =?us-ascii?Q?4Owd0m9vGEhg6gYiLCPBb5w3PUIkUqgPGih4UiFCrvdEd5ascMw3UNcQzJzM?= =?us-ascii?Q?bvO8v7RVVUmcVeYP84qA4C5gouTfQ4cyNbKO/QOmIA7Q2nthJ9Az07J/7wg+?= =?us-ascii?Q?3kWZYcogeWOpVOEPfhB9GxDfjnlMZ4DWofw/DPvrAvzPuKmtn4wSZU7Kx8Dg?= =?us-ascii?Q?xTUsIlm0gCj/OrfTAgxqofP/yoO0Ivzz7MTlsXg8zjAF0pjo5ZqMG9KfOb6d?= =?us-ascii?Q?A7gqWxSq8RS/G/5cJQpMmyS/vHhukkoozQQMw6AoLs9Wu82AUKaKi7WnjaBg?= =?us-ascii?Q?+GDPN7O2DXRUOB/Rbb2H0PrNF6W+rLU+lQM7q7hMXZtGzkPof1VAArggbvrR?= =?us-ascii?Q?RRqFiT8O26Lv4jnb6YvVsP2JA8CEzyp52sXKTX4or7bEW3qE7Ms/84FcCLyP?= =?us-ascii?Q?iAJcdByqdw1oi/lIo0KYWNuHlMOHMyZw8qAdAaU/MnqgYRsYB1BxZK+Rd1PG?= =?us-ascii?Q?dD/Pp6xSq5Mp1a40XCvPwXwpafky4XT/F7Pp2hyof9CbHA9ubv/5OOdZObtX?= =?us-ascii?Q?quSSwKdYXLyYMDeaBi22wHYkJ0cedbf8ns5GO2JpdvStBnckgMObVtHUCN5T?= =?us-ascii?Q?RMZ3DZAC3GBCHanzN7lLaBZoArNRvOuFwa5NNljGG0e+H0BkkvJUhpzM5ZhJ?= =?us-ascii?Q?3V05P6zurevn6yfTA0ZyXw6UQvPHIrCw+TxkqYgXb20ENDJ6519DPu5dDzDI?= =?us-ascii?Q?DNbTi+EflAlKbH61go+dSNhNLQfq+Tm9mBoH/bFtzztmG3rsivN71Hwkwv4E?= =?us-ascii?Q?qw1HhEWC7tuxtKOP1XQLhubU1PtRZuj9RTuhkdBiecpNJfSjKavB0yCWnire?= =?us-ascii?Q?3QRi3If6BYCbS1RWCVv8mvT1M7gGWJuoxI5yX+OKNt+6LjzlreJoRHVg4TOo?= =?us-ascii?Q?EZ6wgGMUvTWskLxnHCTNR9KrU3h3XfTfLNCCKZTTg4xxEKaJx4cRPe/FAOX+?= =?us-ascii?Q?fbe7v2CJVo8EzBvtaAvjK5lnbpqE6q7o1rIrVOjS7tkQVH7i4kHcT2DEK2+y?= =?us-ascii?Q?IsOmugcTT5Kw1eP9Kvb0iz7Ar2Yi4Gc0VlY2xwwDDj/24vEWQa3WhtWorvWZ?= =?us-ascii?Q?pZp0MzKsslxetPVa9MoOkVd9JFOS6UH+KSHX3NtYhhQz0okcW0cbnbgI7Kwt?= =?us-ascii?Q?5qxp109bUQPkeIQ6uNOjp8jr4cshEBVXXyWXMsPzI6QIVtmBk4ROQ489xoGt?= =?us-ascii?Q?C237eZlAS6ukFnhfUf8T8sAfTMrZagLbTGZULjSnjIOzSFLJMcpsvib91N+Y?= =?us-ascii?Q?mCiv2oLB66zkbHrnawCNTLZhZpm++xks1OTGltXFp1v+jromCrdI01oe6/fk?= =?us-ascii?Q?jYjoGXqF7j7FHlQldKdgWKXdQMB3qXoIaWk6KG3+yGK9y/ZGp7OS9rzqmHIx?= =?us-ascii?Q?shoW36PHgBdEg8Kmk5ofmSky4JZfVUjCxMKSBoCyg6lOusvzzlt4vOzGDphV?= =?us-ascii?Q?UIkqgH+b+SBJVgv2BuDj8sLOSwz7oqMeSktAmQmw1U1rsuBzjUQ/iorDzo+B?= =?us-ascii?Q?Yv5NL14nLXrAp/Y=3D?= x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?9IyODZfGo92wdvltjgWejQgMDLRJ/6QOTn7Ama0SblciCXwtibHAikLtfvsa?= =?us-ascii?Q?u7GQG98VZ1nhGdJkQsQ/LuV5GxN9+RDjel3PhFtuR3t3b9OSu5utpMbbBqJT?= =?us-ascii?Q?xaZwSWX9OZTcCPPWeCZV7GXiTd5V44iL1W3xObMgls3IzUKMeYUHFsVjyXGn?= =?us-ascii?Q?/UkHMPtUouWkGRWZ1OBi7nfOVwUaQa8q+fOUdqscDabOB7au+vCbE3FVu/F4?= =?us-ascii?Q?S8TEIfVOtJCCB+M+lXjDhhheBuSOUadvv4K19yuu0Uhjd1/qDSbIzCdqQjpr?= =?us-ascii?Q?PD/kO4UgreokZojbqba1xhCfiFGoAR2bDZgeubi69KsSXpNB11BLnHuBXbno?= =?us-ascii?Q?GUX4YNsqh3QM4wFwQAEY62A4+eXPQWEhMUndxzbImud9/11wKUjRr9bvx6EH?= =?us-ascii?Q?6uJ6fZweAh+W3yHrNhycLZnv71W58tkmrvzILOqLGTVTwgYCS+szLM5e//Wi?= =?us-ascii?Q?J97lesAAGh5L7oSV7aryHZgenSjdxsmcKCzCgaU46PEVqLDisdcnJkQfZ2/j?= =?us-ascii?Q?mdBNINpq6rILYqXdLuN/d8O5AZrz4yOzuwK8JYLE3jwq/24YLtvDTIg2Oye/?= =?us-ascii?Q?5jMHUO7LoyXlvk5dhqCNVhVVUjfOIIEIZV8H/ZXpPQahmqO8LYzZw4tDIk5p?= =?us-ascii?Q?29MXN0NWRzDlidEDPEwZzpcaCoU3tS1L5SsLnOjiVrnfB8Q8ITa85qRtW3BQ?= =?us-ascii?Q?L9XhnCnviLTVDehkk8JACgYGU8cHRXcxmWHCY5Exsy9BQqhK7BAsTy31kdNi?= =?us-ascii?Q?8V6AD/yrALwvdvZFhpwagezlxUyup0jYaB1lpFiMt3oiqNoHUMB31UEe9Akp?= =?us-ascii?Q?H/PVQAmw/JivOXLffkEO4I7nmKeig/rA9iweXUijr6nKOe8IRBObHfekUhYm?= =?us-ascii?Q?PeW/zkX9Fu9pmSJE97tTJTY61bgBCVmFc1TlG/uqKZZKvZXYHxK6t/z4sDBW?= =?us-ascii?Q?H1UnotKBxK+8uw1d4g/jLGmmyW5opa/xgGM9pViLiCbygnZlHMUsQjMZqEK+?= =?us-ascii?Q?rfa9k8CDlvnsl8ZVeVgQWDIpBnK4N95VSWGKSFP9b4uJZ9ih0EU5+1NoCP/f?= =?us-ascii?Q?3FBqO+RqJbgo7F8kBhuWB1sOZ3UQ1YF7Eio1HWgQni8XCP2lFgBYpoS+82o0?= =?us-ascii?Q?m8VX1tbYhQwSJpXjZQrSlkzVhCu3CEiTSzSbH4XL6lQTphr/EklnfF1+uFUR?= =?us-ascii?Q?7Vqn8MtBF97pSQXyo2Zfd41y2/M21TpI7N4X55NY4izIoZnHCKe2EUvq0aVH?= =?us-ascii?Q?XHB46cTU53nc5/+QGhQEa3T3Fz7pFc02M/u0s7JpQItSUl+zI+agRlJRH+uG?= =?us-ascii?Q?e1I7YzBGkpO5/UiAQLbTbx0sccrq+VVc9BDV0JAFuqKCK//0bHL3ohkA90P0?= =?us-ascii?Q?6ZtGFO5XtBHNYgjjcPgMPx3FxaN6SakUkM6DxAh9ohGvpSj6nMkD8dPRuFPB?= =?us-ascii?Q?h8AYG02wwnxBvwjX6zg45gzlKoDz6SdBDLpjW2niV+qQwhpc/9P7cPT3EPP6?= =?us-ascii?Q?eHoyAe3QXkMHd+jEW9/AQPIeCscl2FlMEpt9D5Qeu7/GRjtBiDa8cK8KLHip?= =?us-ascii?Q?Ed61ZWaBH5sKkk8/xxQSoCJ4f4sFZhNNyqhBT8XMwm8zkxPWDJ0qjpw9rpmV?= =?us-ascii?Q?pOg2ixSogcVs4CfTHd8Pv3Vu2c7M+7SoJ5M/M+2S4r60?= MIME-Version: 1.0 X-OriginatorOrg: microsoft.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: CH3PR21MB4517.namprd21.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: b69bd85c-d8be-4311-77cf-08dd7ecace27 X-MS-Exchange-CrossTenant-originalarrivaltime: 18 Apr 2025 22:46:06.3548 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 72f988bf-86f1-41af-91ab-2d7cd011db47 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: wYxY5F4JzBvbo6yOphPvRHBNFAnNsvmdljLUMufMgbVch7mvx0GTsL6k9X0Z6c26GGIK1WhqSqV+xuSCPxz9/A== X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH4PR21MB4243 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 20:32:11 -0700 Resent-From: Kun.Qin@microsoft.com Reply-To: devel@edk2.groups.io,Kun.Qin@microsoft.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: tx219sY4BqIuDOwwBTGNYmuEx7686176AA= Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_CH3PR21MB4517798A8BCD2A7082A9E1FEE9BF2CH3PR21MB4517namp_" X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20240830 header.b="H/LXuezl"; 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_CH3PR21MB4517798A8BCD2A7082A9E1FEE9BF2CH3PR21MB4517namp_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Current MM communicate DXE driver supports all 3 versions. I guess v1 is ne= ver invoked in Stmm based solutions. I will update the IPL code for SMM sho= rtly. Regards, Kun From: Kinney, Michael D Sent: Friday, April 18, 2025 3:37 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 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 (#121273): https://edk2.groups.io/g/devel/message/121273 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_CH3PR21MB4517798A8BCD2A7082A9E1FEE9BF2CH3PR21MB4517namp_ Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable

Current MM communicate DXE driver supports all 3 ve= rsions. I guess v1 is never invoked in Stmm based solutions. I will update = the IPL code for SMM shortly.

 

Regards,

Kun

 

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

 

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 <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

 

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 (#121273) | =20 | Mute= This Topic | New Topic
Your Subscriptio= n | Contact Group Owner | Unsubscribe [rebecca@openfw.io]

_._,_._,_
--_000_CH3PR21MB4517798A8BCD2A7082A9E1FEE9BF2CH3PR21MB4517namp_--