From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-in7.apple.com (mail-out7.apple.com [17.151.62.29]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 984151A1DFA for ; Tue, 9 Aug 2016 07:50:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; d=apple.com; s=mailout2048s; c=relaxed/simple; q=dns/txt; i=@apple.com; t=1470754219; x=2334667819; h=From:Sender:Reply-To:Subject:Date:Message-id:To:Cc:MIME-version:Content-type: Content-transfer-encoding:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-reply-to:References:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=S1zn9K6Cb8yXZY/wBKzwY4sEfsiKM1V8D9zQoKU5H0E=; b=htcqjD146xI7b9m2uS17dD4gAzPgytstqQ3saVyicCTUagfY31LMMRYPUDddGwEf tMSJ58NxZjWotp5kjcSF0R4nl7OutLYlnEGPiS/W7ncCcIH6IRxMIXQofvJuky1H sv1RwiuovWNcowDdwoQwAXJ/fFHg2lJJhLhUHifL9p0DmiQS6TFKfYtIwfpTd7QJ /FRj4LoQawpQeT84Ao/E8Dso3s0KKUi9w8WPaL/zhW08p8TAee2BCeECIH8IijCt rr5ZCVlmxsvqow3QltOSPwhRlLxpZ9EM+G1iCYQiBRYc9EvdSEevoWql49TX7KOj /i9TGWYhaAY1XPoWi2vTVg==; Received: from relay2.apple.com (relay2.apple.com [17.128.113.67]) by mail-in7.apple.com (Apple Secure Mail Relay) with SMTP id AE.C7.17422.BADE9A75; Tue, 9 Aug 2016 07:50:19 -0700 (PDT) X-AuditID: 11973e16-f793f6d00000440e-65-57a9edabfd24 Received: from nwk-mmpp-sz12.apple.com (nwk-mmpp-sz12.apple.com [17.128.115.204]) by relay2.apple.com (Apple SCV relay) with SMTP id E7.C5.01452.BADE9A75; Tue, 9 Aug 2016 07:50:19 -0700 (PDT) MIME-version: 1.0 Received: from [17.153.29.18] by nwk-mmpp-sz12.apple.com (Oracle Communications Messaging Server 8.0.1.1.0 64bit (built Jun 15 2016)) with ESMTPSA id <0OBN005EYD7U9S40@nwk-mmpp-sz12.apple.com>; Tue, 09 Aug 2016 07:50:19 -0700 (PDT) Sender: afish@apple.com From: Andrew Fish In-reply-to: <9e979fb5-52ac-efe4-480e-8670d8233a7c@intel.com> Date: Tue, 09 Aug 2016 07:50:18 -0700 Cc: edk2-devel Message-id: References: <776695E1-E864-42F4-A633-B86FB913A510@apple.com> <0C09AFA07DD0434D9E2A0C6AEB048310036A1BCF@shsmsx102.ccr.corp.intel.com> <33692B19-74EE-4C13-8AE3-1C92B74836C3@apple.com> <9e979fb5-52ac-efe4-480e-8670d8233a7c@intel.com> To: "Zeng, Star" X-Mailer: Apple Mail (2.3112) X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrOLMWRmVeSWpSXmKPExsUi2FDorLv67cpwg8839S32HDrKbLGv19qB yWPxnpdMHt2z/7EEMEVx2aSk5mSWpRbp2yVwZRx6P5e94LZWRcfiD4wNjNNVuhg5OSQETCSO vH3PBGGLSVy4t56ti5GLQ0hgL6PEhNc3GbsYOcCK2o7zQcQPMUosa9vDAtLAKyAo8WPyPRaQ GmYBeYmD52VBwswCWhLfH7WyQNS/ZZQ4/2IHO0hCWEBc4t2ZTcwgCWGBWYwSLxbMABvEJqAs sWL+B7AiTgFbiRMHN7KB2CwCqhIb3t1ih5iqIfF19XZ2iMU2Ev8mzGSF2PCGUWL91X1gg0QE 1CT2rt7FDvGOrMS+DQvA3pEQ2MEm8Xr7HeYJjCKzkFw+C+HyWUguX8DIvIpRKDcxM0c3M89c L7GgICdVLzk/dxMjKOSn24ntYHy4yuoQowAHoxIP74XlK8KFWBPLiitzDzFKc7AoifOaLAMK CaQnlqRmp6YWpBbFF5XmpBYfYmTi4JRqYAzUD/jMe2rKdemjytuEv8yJ6o7inH/zVqv7pIsr 004srTYLb7gV5ySx1kLkmte/k5+fT1wbzz1J9W5uN79wrKVTpkOS/j+mix3bruZ8mrRXZB5j 9J9Zl6dcK/lqXCa3aQVPTPOPdzF/FztdKtm5Nn7uhpJ5DZucV7gpLBdY43x96urC1D9zTtgr sRRnJBpqMRcVJwIAzxIoqloCAAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFmpikeLIzCtJLcpLzFFi42IRbCg+o7v67cpwg21tOhZ7Dh1lttjXa+3A 5LF4z0smj+7Z/1gCmKK4bFJSczLLUov07RK4Mg69n8tecFuromPxB8YGxukqXYwcHBICJhJt x/m6GDmBTDGJC/fWs3UxcnEICRxilFjWtocFJMErICjxY/I9FpB6ZgF5iYPnZUHCzAJaEt8f tbJA1L9llDj/Ygc7SEJYQFzi3ZlNzCAJYYFZjBIvFswAG8QmoCyxYv4HsCJOAVuJEwc3soHY LAKqEhve3WKHmKoh8XX1dnaIxTYS/ybMZIXY8IZRYv3VfWCDRATUJPau3sUOcbasxL4NC9gm MArOQnLsLIRjZyE5dgEj8ypGgaLUnMRKI73EgoKcVL3k/NxNjOAQLXTewXhsmdUhRgEORiUe 3gvLV4QLsSaWFVfmAkODg1lJhNfx1cpwId6UxMqq1KL8+KLSnNTiQ4zJQA9MZJYSTc4Hxk9e SbyhiYmBibGxmbGxuYk5acJK4ryP5JeGCwmkJ5akZqemFqQWwWxh4uCUamBUvXrVZaZypXbk 3etv9wdVnPo+S+alSPkfv0slT7bJrHuQN3GrpweDjZjH9ai2t6d/Np1hO7i+k/vs0QOFnwQy 2qpdPr9/b32WY905fjbxujnOq8/9C1K74ucZ23JUqGTF2Yjf5QkLQ+YZs27OOB18peKNh8wD 2RN9n29Mn/R1VeoNFpnUfeYeSizFGYmGWsxFxYkAy+2zeJUCAAA= Subject: Re: [MdeModulePkg] SetVirtualAddressMap() crashed due to DxeReportStatusCodeLib assuming the state of the BootService Memory at runtime. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 09 Aug 2016 14:50:20 -0000 Content-transfer-encoding: 7BIT Content-type: text/plain; CHARSET=US-ASCII > On Aug 8, 2016, at 7:26 PM, Zeng, Star wrote: > > On 2016/8/9 10:07, Andrew Fish wrote: >> >>> On Aug 8, 2016, at 6:21 PM, Zeng, Star wrote: >>> >>> Andrew, >>> >>> Should MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib be used for your case if there are really runtime status code reporting needed? >>> >> >> Star, >> >> If the Library instance does not fully support DXE_RUNTIME_DRIVER, why is it listed in the LIBRARY_CLASS as supported? > > This kind of library can support DXE_RUNTIME_DRIVER at boot time, for example UefiHiiLib, UefiHiiServicesLib, UefiBootServicesTableLib, DxePcdLib and etc. > Star, I understand giving access to Boot Services resources to a Runtime Driver for its constructor. I think the difference here is the DxeReportStatusCodeLib is abstracting a runtime service, but not doing it in a way to really works properly at runtime in all cases. I would expect that a library abstraction a runtime feature would work at runtime. Which is why I ended up with a "bad" mapping in the first place. Thanks, Andrew Fish >> >> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf#L25 >> >> LIBRARY_CLASS = ReportStatusCodeLib|DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SAL_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION UEFI_DRIVER SMM_CORE >> >> Actually I tried to add the UefiRuntimeDriverLib and the build failed as UefiRuntimeDriverLib was not supported for the DXE_CORE type. Maybe the bug is this library instance lists DXE_RUNTIME_DRIVER, DXE_SAL_DRIVER and DXE_SMM_DRIVER when it has special case code to support DXE_CORE? Maybe this library is trying to do too many things? >> >> What ReportStatusCodeLib would you recommend to link with RuntimeDxe driver: https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf > > [LibraryClasses.common.DXE_CORE] > ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf > > [LibraryClasses.common.DXE_RUNTIME_DRIVER] > ReportStatusCodeLib|MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/RuntimeDxeReportStatusCodeLib.inf > > [LibraryClasses.common.UEFI_DRIVER] > ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf > > [LibraryClasses.common.DXE_DRIVER] > ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf > > [LibraryClasses.common.UEFI_APPLICATION] > ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf > > Thanks, > Star > >> >> Thanks, >> >> Andrew Fish >> >>> Thanks, >>> Star >>> -----Original Message----- >>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Andrew Fish >>> Sent: Tuesday, August 9, 2016 7:08 AM >>> To: edk2-devel >>> Subject: [edk2] [MdeModulePkg] SetVirtualAddressMap() crashed due to DxeReportStatusCodeLib assuming the state of the BootService Memory at runtime. >>> >>> I was messing about with an ExitBootServices test that fills boot services memory with 0xAFAFAFAFAFAFAFAF (It was Vincent's idea to use my Initials but it has the handy property of being a non-cononical address and causes on GP fault on X64) and SetVirtualAddressMap() started crashing. >>> >>> It looks like this code is assuming the 1st call to ReportStatus code will not happen at runtime. This is not the case for the RuntimeDxe driver. >>> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c#L43 >>> VOID >>> InternalGetReportStatusCode ( >>> VOID >>> ) >>> { >>> EFI_STATUS Status; >>> >>> if (mReportStatusCodeLibStatusCodeProtocol != NULL) { >>> return; >>> } >>> >>> // >>> // Check gBS just in case ReportStatusCode is called before gBS is initialized. >>> // >>> if (gBS != NULL && gBS->LocateProtocol != NULL) { >>> Status = gBS->LocateProtocol (&gEfiStatusCodeRuntimeProtocolGuid, NULL, (VOID**) &mReportStatusCodeLibStatusCodeProtocol); >>> if (EFI_ERROR (Status)) { >>> mReportStatusCodeLibStatusCodeProtocol = NULL; >>> } >>> } >>> } >>> >>> I'm guessing this seems to work due to:https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c#L803 >>> >>> // >>> // Zero out the Boot Service Table >>> // >>> ZeroMem (gBS, sizeof (EFI_BOOT_SERVICES)); >>> >>> >>> Thus if I'm looking at this code correctly it only looks like it works at Runtime since it is depending on the value of a boot services memory buffer not changing. This is not a valid assumption as that code is owned by the caller of ExitBootServices, so it should be legal for my test to change the value. >>> >>> I wanted to get a few more eyes on this prior to filling a bug? >>> >>> Thanks, >>> >>> Andrew Fish >>> >>> >>> _______________________________________________ >>> edk2-devel mailing list >>> edk2-devel@lists.01.org >>> https://lists.01.org/mailman/listinfo/edk2-devel >>> _______________________________________________ >>> edk2-devel mailing list >>> edk2-devel@lists.01.org >>> https://lists.01.org/mailman/listinfo/edk2-devel > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel