On 29 June 2015 at 16:08, Yao, Jiewen <jiewen....@intel.com> wrote: > Good to know. Thanks! >
Another question from my side: is it guaranteed the the memory map returned by GetMemoryMap() is sorted? Otherwise, it becomes non-trivial for the OS to ensure that all Runtime regions are mapped adjacently. > -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] > Sent: Monday, June 29, 2015 10:06 PM > To: Yao, Jiewen > Cc: edk2-devel@lists.sourceforge.net; Gao, Liming; Laszlo Ersek; Justen, > Jordan L; Kinney, Michael D; Zeng, Star; Zimmer, Vincent; Fleming, Matt > Subject: Re: BUG in properties table feature implementation > > On 29 June 2015 at 15:58, Yao, Jiewen <jiewen....@intel.com> wrote: >> Yes. It seems we need some special handling for AArch64. >> If 64KiB is minimal paging unit, I think we need check 64KiB PE section >> alignment for AArch64. >> >> I confess that I only validated IA32 and X64, and I did not take AArch64 >> into account. >> >> BTW: There is no 64KiB alignment requirement for AArch32, right? I cannot >> find the word. So I just want to double confirm. >> > > No there is not. Only 64-bit ARM can execute with 64 KB page size. > > -- > Ard. > > >> -----Original Message----- >> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] >> Sent: Monday, June 29, 2015 9:42 PM >> To: Yao, Jiewen >> Cc: edk2-devel@lists.sourceforge.net; Gao, Liming; Laszlo Ersek; Justen, >> Jordan L; Kinney, Michael D; Zeng, Star; Zimmer, Vincent; Fleming, Matt >> Subject: Re: BUG in properties table feature implementation >> >> On 29 June 2015 at 14:54, Yao, Jiewen <jiewen....@intel.com> wrote: >>> Thanks for the sharing. Yes. I agree with you that this breaks backward >>> compatibility. >>> Right, I do not think we can boot Win7 or old Win8 with this feature >>> enabled. >>> >>> Both UEFI OS and Firmware need support this UEFI2.5 Properties Table >>> feature. I believe it is known by UEFI forum, when it is added to UEFI >>> spec, and it is published. >>> >> >> Yes, but I don't think it is documented anywhere that the OS needs to map >> all runtime regions adjacently even if it does not actually care about the >> EFI_MEMORY_RO attributes. Essentially, the OS always need to map adjacently >> to be compatible with UEFI 2.5, and the current AArch64 Linux kernel crashes >> badly on any UEFI 2,5 implementation that has this feature enabled. >> >>> UEFI 2.5 properties table is optional feature. It can be enabled or >>> disabled. >>> The benefit of this feature is that: a UEFI2.5 OS can do memory protection >>> based on UEFI memory map. >>> But the old UEFI OS cannot get any benefit. >>> >>> So a platform can made judgment based on its need, as well as the UEFI 2.5 >>> OS availability. >>> >> >> Unfortunately, this makes it an opt-in setting, and system will ship with it >> disabled by default, in order to prevent boot failures on older OSes. That >> is not typically how you want to deploy a security feature. >> >> There is another issue I would like your opinion about: >> for AArch64, MdeModulePkg/Core/Dxe/Mem/Imem.h defines the following >> >> #define EFI_ACPI_RUNTIME_PAGE_ALLOCATION_ALIGNMENT (SIZE_64KB) >> >> and in CoreTerminateMemoryMap(), a final check is done that no Runtime >> regions are aligned incorrectly. >> Yet, with the Properties Table feature enabled, memory regions are split >> without taking this into account. Strangely enough, CoreTerminateMemoryMap >> () does not complain about this. >> >> This is documented in the UEFI spec 2.3.6: >> """ >> All 4KiB memory pages allocated for use by runtime services (of types >> EfiRuntimeServicesCode, EfiRuntimeServicesData and >> EfiACPIMemoryNVS) must use identical ARM Memory Page Attributes (as >> described in Table 8) within the same physical 64KiB page. Mixed attribute >> mappings within a larger page are not allowed. >> """ >> >> So I suppose the Properties table feature should use 64 KB not 4 KB when >> executing on AArch64. However, the documentation for the Properties Table >> feature mentions 4 KB explicitly, so that should go into the errata as well. >> >> >> Regards, >> Ard. >> >> >>> That is why we do not enable this PE/COFF 4K alignment in BaseTool as >>> standard configuration. >>> So in other word, it is disabled by tool by default. A platform may also >>> decide to not publish PropertiesTable by below PCD: >>> ## Publish PropertiesTable or not. >>> # >>> # If this PCD is TRUE, DxeCore publishs PropertiesTable. >>> # DxeCore evaluates if all runtime drivers has 4K aligned PE sections. If >>> all >>> # PE sections in runtime drivers are 4K aligned, DxeCore sets BIT0 in >>> # PropertiesTable. Or DxeCore clears BIT0 in PropertiesTable. >>> # If this PCD is FALSE, DxeCore does not publish PropertiesTable. >>> # >>> # If PropertiesTable has BIT0 set, DxeCore uses below policy in UEFI >>> memory map: >>> # 1) Use EfiRuntimeServicesCode for runtime driver PE image code >>> section and >>> # use EfiRuntimeServicesData for runtime driver PE image header and >>> other section. >>> # 2) Set EfiRuntimeServicesCode to be EFI_MEMORY_RO. >>> # 3) Set EfiRuntimeServicesData to be EFI_MEMORY_XP. >>> # 4) Set EfiMemoryMappedIO and EfiMemoryMappedIOPortSpace to be >>> EFI_MEMORY_XP. >>> # >>> # NOTE: Platform need gurantee this PCD is set correctly. Platform should >>> set >>> # this PCD to be TURE if and only if all runtime driver has seperated >>> Code/Data >>> # section. If PE code/data sections are merged, the result is >>> unpredictable. >>> # >>> # @Prompt Publish UEFI PropertiesTable. >>> >>> gEfiMdeModulePkgTokenSpaceGuid.PropertiesTableEnable|TRUE|BOOLEAN|0x00 >>> 00006e >>> >>> Thank you >>> Yao Jiewen >>> >>> >>> -----Original Message----- >>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] >>> Sent: Monday, June 29, 2015 8:44 PM >>> To: Yao, Jiewen >>> Cc: edk2-devel@lists.sourceforge.net; Gao, Liming; Laszlo Ersek; >>> Justen, Jordan L; Kinney, Michael D; Zeng, Star >>> Subject: Re: BUG in properties table feature implementation >>> >>> On 29 June 2015 at 14:34, Yao, Jiewen <jiewen....@intel.com> wrote: >>>> Hi Ard >>>> Yes, your are right. We do observe similar odd behavior in old >>>> Win7/Win8/Win8.1, and we have to disable it. >>>> Win8 and Win8.1 have OS patch to set adjacent virtual memory map, to >>>> resolve this issue. >>> >>> I see this as a big problem with the feature, as it breaks backward >>> compatibility. IOW, you won't be able to boot Windows 7 on a UEFI 2.5 >>> system unless you manually disable the feature (in the setup screen, I >>> suppose?) >>> >>>> We also tested Win10, and Suse 11 GM. They are good. >>>> >>>> Would you please share the information on which OS you are using? >>>> >>> >>> I am using the upstream Linux kernel for AArch64. I am not using a >>> distribution. >>> >>>> All in all, if OS cannot guarantee the virtual memory map is adjacent, we >>>> have to disable this feature. >>>> >>> >>> This is backwards: the feature should be implemented such that it cannot be >>> trivially broken by a well-behaving OS. Note that nothing in the >>> SetVirtualAddressMap () implementation suggests that the regions should be >>> adjacent. >>> >>> Perhaps it would have been better to introduce a new memory region >>> type EfiRuntimeServicesPecoffData, so the OS at least knows which >>> regions it should keep adjacent, (i.e., Code and PecoffData regions >>> should be kept together) >>> >>> So does PE/COFF even allow the memory image to deviate from the file image? >>> If so, the correct way is to update the PE/COFF loader and the relocation >>> logic can deal with sections being laid out differently in memory than they >>> are in the file. >>> >>> -- >>> Ard. >>> >>>> -----Original Message----- >>>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] >>>> Sent: Monday, June 29, 2015 6:46 PM >>>> To: edk2-devel@lists.sourceforge.net; Yao, Jiewen; Gao, Liming; >>>> Laszlo Ersek; Justen, Jordan L; Kinney, Michael D; Zeng, Star >>>> Subject: BUG in properties table feature implementation >>>> >>>> Hello all, >>>> >>>> I am running into another problem with the implementation of the UEFI >>>> 2.5 Properties Table feature. It splits PE/COFF images into separate but >>>> adjacent memory regions, only to be able to assign different permissions >>>> to .text and .data sections. This is working fine at boot time. >>>> >>>> However, at runtime, after calling virtual address map, this breaks >>>> down completely. Since the virtual mapping supplied to >>>> SetVirtualAddressMap() does not have to guarantee adjacency between code >>>> and data regions (of which the OS does not know whether they belong >>>> together or not), reapplying the relocations corrupts the memory image and >>>> breaks the runtime services. >>>> >>>> For example, this region >>>> >>>> 0x00005eeb1000-0x00005eeb6fff [Runtime Code] >>>> 0x00005eeb7000-0x00005eec0fff [Runtime Data] >>>> >>>> is mapped on AARCH64 as >>>> >>>> EFI remap 0x000000005eeb1000 => 00000000440a1000 >>>> EFI remap 0x000000005eeb7000 => 00000000440b7000 >>>> >>>> which retains the relative alignment, but adds a 64 KB offset to the >>>> second regions so that the regions can still be mapped with different >>>> permissions when the OS is executing with a 64 KB page size. >>>> >>>> As far as I can tell, this is in accordance with the spec, and was working >>>> fine until I tried to enable the properties table feature. >>>> With that enabled, the two above regions could actually describe one >>>> single PE/COFF image in memory, and the 64 KB offset results in the >>>> relocations to be applied incorrectly. >>>> >>>> I looked at PeCoffLoaderRelocateImageForRuntime () but to me, it is >>>> not very obvious how to solve this. Obviously, our PE/COFF >>>> implementation is not complete since it assumes file offset == memory >>>> offset for sections, but this does not hold anymore for UEFI 2.5 >>>> >>>> I would also like to point out again that this is another result of the >>>> fact that this series was pushed through with any review or testing >>>> outside of the Intel firmware team. For features of this magnitude and >>>> complexity, more scrutiny and testing is obviously required. >>>> >>>> Kind regards, >>>> Ard. ------------------------------------------------------------------------------ Monitor 25 network devices or servers for free with OpManager! OpManager is web-based network management software that monitors network devices and physical & virtual servers, alerts via email & sms for fault. Monitor 25 devices for free with no restriction. Download now http://ad.doubleclick.net/ddm/clk/292181274;119417398;o _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
