ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c | 25 +- ArmPkg/Drivers/CpuDxe/Arm/Mmu.c | 96 +++++-- ArmPkg/Drivers/CpuDxe/CpuDxe.c | 2 + ArmPkg/Drivers/CpuDxe/CpuDxe.h | 17 ++ ArmPkg/Drivers/CpuDxe/CpuDxe.inf | 2 + ArmPkg/Drivers/CpuDxe/MemoryAttribute.c | 271 ++++++++++++++++++++ ArmPkg/Include/Chipset/ArmV7Mmu.h | 88 +++---- ArmPkg/Include/Library/ArmMmuLib.h | 34 +++ ArmPkg/Library/ArmLib/Arm/ArmV7Support.S | 2 + ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 68 ++++- ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibConvert.c | 8 +- ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c | 2 +- ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c | 173 +++++++++++-- ArmVirtPkg/ArmVirt.dsc.inc | 2 + MdePkg/Include/Protocol/MemoryAttribute.h | 142 ++++++++++ MdePkg/MdePkg.dec | 3 + 16 files changed, 833 insertions(+), 102 deletions(-) create mode 100644 ArmPkg/Drivers/CpuDxe/MemoryAttribute.c create mode 100644 MdePkg/Include/Protocol/MemoryAttribute.h
v4: - major cleanup of the 32-bit ARM code - add support for EFI_MEMORY_RP using the access flag - enable stack guard in ArmVirtPkg (which uses EFI_MEMORY_RP) - incorporate optimization from other series [0] to avoid splitting block entries unnecessarily v3: - fix ARM32 bug in attribute conversion - add Liming's ack to patch #1 - include draft patch (NOT FOR MERGE) used to test the changes v2: - drop patch to bump exposed UEFI revision to v2.10 - add missing permitted return values to protocol definition [0] https://edk2.groups.io/g/devel/message/99801 Cc: Michael Kinney <michael.d.kinney@intel.com> Cc: Liming Gao <gaoliming@byosoft.com.cn> Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Michael Kubacki <michael.kubacki@microsoft.com> Cc: Sean Brogan <sean.brogan@microsoft.com> Cc: Rebecca Cran <quic_rcran@quicinc.com> Cc: Leif Lindholm <quic_llindhol@quicinc.com> Cc: Sami Mujawar <sami.mujawar@arm.com> Cc: Taylor Beebe <t@taylorbeebe.com> Ard Biesheuvel (11): ArmPkg/ArmMmuLib ARM: Remove half baked large page support ArmPkg/ArmMmuLib ARM: Split off XN page descriptor bit from type field ArmPkg/CpuDxe ARM: Fix page-to-section attribute conversion ArmPkg/ArmMmuLib ARM: Isolate the access flag from AP mask ArmPkg/ArmMmuLib ARM: Clear individual permission bits ArmPkg/ArmMmuLib: Implement EFI_MEMORY_RP using access flag ArmVirtPkg: Enable stack guard ArmPkg/ArmMmuLib: Avoid splitting block entries if possible ArmPkg/CpuDxe: Expose unified region-to-EFI attribute conversion MdePkg: Add Memory Attribute Protocol definition ArmPkg/CpuDxe: Implement EFI memory attributes protocol ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c | 25 +- ArmPkg/Drivers/CpuDxe/Arm/Mmu.c | 96 +++++-- ArmPkg/Drivers/CpuDxe/CpuDxe.c | 2 + ArmPkg/Drivers/CpuDxe/CpuDxe.h | 17 ++ ArmPkg/Drivers/CpuDxe/CpuDxe.inf | 2 + ArmPkg/Drivers/CpuDxe/MemoryAttribute.c | 271 ++++++++++++++++++++ ArmPkg/Include/Chipset/ArmV7Mmu.h | 88 +++---- ArmPkg/Include/Library/ArmMmuLib.h | 34 +++ ArmPkg/Library/ArmLib/Arm/ArmV7Support.S | 2 + ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 68 ++++- ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibConvert.c | 8 +- ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c | 2 +- ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c | 173 +++++++++++-- ArmVirtPkg/ArmVirt.dsc.inc | 2 + MdePkg/Include/Protocol/MemoryAttribute.h | 142 ++++++++++ MdePkg/MdePkg.dec | 3 + 16 files changed, 833 insertions(+), 102 deletions(-) create mode 100644 ArmPkg/Drivers/CpuDxe/MemoryAttribute.c create mode 100644 MdePkg/Include/Protocol/MemoryAttribute.h -- 2.39.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#99885): https://edk2.groups.io/g/devel/message/99885 Mute This Topic: https://groups.io/mt/96853149/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Hey Ard, Once the Memory Attribute Protocol is made available, Windows will have some expectations about its functionality. Can you run this test app created by me and Jiewen to ensure it meets the Windows requirements? Part of the test needed an AARCH64 implementation which I just added - let me know if it doesn't work. Thanks :) https://github.com/TaylorBeebe/mu_basecore/tree/update_map_test_app/MdePkg/Test/ShellTest/MemoryAttributeProtocolFuncTestApp On 2/9/2023 5:59 AM, Ard Biesheuvel wrote: > v4: > > - major cleanup of the 32-bit ARM code > > - add support for EFI_MEMORY_RP using the access flag > > - enable stack guard in ArmVirtPkg (which uses EFI_MEMORY_RP) > > - incorporate optimization from other series [0] to avoid splitting > > block entries unnecessarily > > > > v3: > > - fix ARM32 bug in attribute conversion > > - add Liming's ack to patch #1 > > - include draft patch (NOT FOR MERGE) used to test the changes > > > > v2: > > - drop patch to bump exposed UEFI revision to v2.10 > > - add missing permitted return values to protocol definition > > > > [0] https://edk2.groups.io/g/devel/message/99801 > > > > Cc: Michael Kinney <michael.d.kinney@intel.com> > > Cc: Liming Gao <gaoliming@byosoft.com.cn> > > Cc: Jiewen Yao <jiewen.yao@intel.com> > > Cc: Michael Kubacki <michael.kubacki@microsoft.com> > > Cc: Sean Brogan <sean.brogan@microsoft.com> > > Cc: Rebecca Cran <quic_rcran@quicinc.com> > > Cc: Leif Lindholm <quic_llindhol@quicinc.com> > > Cc: Sami Mujawar <sami.mujawar@arm.com> > > Cc: Taylor Beebe <t@taylorbeebe.com> > > > > Ard Biesheuvel (11): > > ArmPkg/ArmMmuLib ARM: Remove half baked large page support > > ArmPkg/ArmMmuLib ARM: Split off XN page descriptor bit from type field > > ArmPkg/CpuDxe ARM: Fix page-to-section attribute conversion > > ArmPkg/ArmMmuLib ARM: Isolate the access flag from AP mask > > ArmPkg/ArmMmuLib ARM: Clear individual permission bits > > ArmPkg/ArmMmuLib: Implement EFI_MEMORY_RP using access flag > > ArmVirtPkg: Enable stack guard > > ArmPkg/ArmMmuLib: Avoid splitting block entries if possible > > ArmPkg/CpuDxe: Expose unified region-to-EFI attribute conversion > > MdePkg: Add Memory Attribute Protocol definition > > ArmPkg/CpuDxe: Implement EFI memory attributes protocol > > > > ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c | 25 +- > > ArmPkg/Drivers/CpuDxe/Arm/Mmu.c | 96 +++++-- > > ArmPkg/Drivers/CpuDxe/CpuDxe.c | 2 + > > ArmPkg/Drivers/CpuDxe/CpuDxe.h | 17 ++ > > ArmPkg/Drivers/CpuDxe/CpuDxe.inf | 2 + > > ArmPkg/Drivers/CpuDxe/MemoryAttribute.c | 271 ++++++++++++++++++++ > > ArmPkg/Include/Chipset/ArmV7Mmu.h | 88 +++---- > > ArmPkg/Include/Library/ArmMmuLib.h | 34 +++ > > ArmPkg/Library/ArmLib/Arm/ArmV7Support.S | 2 + > > ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 68 ++++- > > ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibConvert.c | 8 +- > > ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c | 2 +- > > ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c | 173 +++++++++++-- > > ArmVirtPkg/ArmVirt.dsc.inc | 2 + > > MdePkg/Include/Protocol/MemoryAttribute.h | 142 ++++++++++ > > MdePkg/MdePkg.dec | 3 + > > 16 files changed, 833 insertions(+), 102 deletions(-) > > create mode 100644 ArmPkg/Drivers/CpuDxe/MemoryAttribute.c > > create mode 100644 MdePkg/Include/Protocol/MemoryAttribute.h > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#100039): https://edk2.groups.io/g/devel/message/100039 Mute This Topic: https://groups.io/mt/96853149/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On Sat, 11 Feb 2023 at 01:56, Taylor Beebe <t@taylorbeebe.com> wrote: > > Hey Ard, > > Once the Memory Attribute Protocol is made available, Windows will have > some expectations about its functionality. Can you run this test app > created by me and Jiewen to ensure it meets the Windows requirements? > Part of the test needed an AARCH64 implementation which I just added - > let me know if it doesn't work. > Thanks, this is rather helpful. There appears to be an issue related to DEBUG_PROPERTY_DEBUG_CLEAR_MEMORY_ENABLED so I had to disable that to run these tests, as otherwise, the DXE core tries to clear freed pages before restoring the memory attributes. With that out of the way, the only test that fails is 'New EfiLoaderCode buffer attributes expected' because this firmware build maps loader code RWX, as existing boot stages for Linux are relying on this (including the kernel itself at this point) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#100043): https://edk2.groups.io/g/devel/message/100043 Mute This Topic: https://groups.io/mt/96853149/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 2/11/2023 2:05 AM, Ard Biesheuvel wrote: > On Sat, 11 Feb 2023 at 01:56, Taylor Beebe <t@taylorbeebe.com> wrote: >> >> Hey Ard, >> >> Once the Memory Attribute Protocol is made available, Windows will have >> some expectations about its functionality. Can you run this test app >> created by me and Jiewen to ensure it meets the Windows requirements? >> Part of the test needed an AARCH64 implementation which I just added - >> let me know if it doesn't work. >> > > Thanks, this is rather helpful. > > There appears to be an issue related to > DEBUG_PROPERTY_DEBUG_CLEAR_MEMORY_ENABLED so I had to disable that to > run these tests, as otherwise, the DXE core tries to clear freed pages > before restoring the memory attributes. > > With that out of the way, the only test that fails is 'New > EfiLoaderCode buffer attributes expected' because this firmware build > maps loader code RWX, as existing boot stages for Linux are relying on > this (including the kernel itself at this point) It makes sense that the NewEfiLoaderCode test fails, but I am surprised the FreePagesWithProtectionAttributesTestCase passes. The test ensures that a page with EFI_MEMORY_RP and/or EFI_MEMORY_RO has those attributes cleared before attempting to free the page within the FreePage routine and is related to the concern Marvin had. Did you make a change to the core or is there an execution path I'm not seeing which allows that test to pass? -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#100610): https://edk2.groups.io/g/devel/message/100610 Mute This Topic: https://groups.io/mt/96853149/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On Wed, 1 Mar 2023 at 21:43, Taylor Beebe <t@taylorbeebe.com> wrote: > > > > On 2/11/2023 2:05 AM, Ard Biesheuvel wrote: > > On Sat, 11 Feb 2023 at 01:56, Taylor Beebe <t@taylorbeebe.com> wrote: > >> > >> Hey Ard, > >> > >> Once the Memory Attribute Protocol is made available, Windows will have > >> some expectations about its functionality. Can you run this test app > >> created by me and Jiewen to ensure it meets the Windows requirements? > >> Part of the test needed an AARCH64 implementation which I just added - > >> let me know if it doesn't work. > >> > > > > Thanks, this is rather helpful. > > > > There appears to be an issue related to > > DEBUG_PROPERTY_DEBUG_CLEAR_MEMORY_ENABLED so I had to disable that to > > run these tests, as otherwise, the DXE core tries to clear freed pages > > before restoring the memory attributes. > > > > With that out of the way, the only test that fails is 'New > > EfiLoaderCode buffer attributes expected' because this firmware build > > maps loader code RWX, as existing boot stages for Linux are relying on > > this (including the kernel itself at this point) > > It makes sense that the NewEfiLoaderCode test fails, but I am surprised > the FreePagesWithProtectionAttributesTestCase passes. The test ensures > that a page with EFI_MEMORY_RP and/or EFI_MEMORY_RO has those attributes > cleared before attempting to free the page within the FreePage routine > and is related to the concern Marvin had. > > Did you make a change to the core or is there an execution path I'm not > seeing which allows that test to pass? No, I didn't make any additional changes to the core afair. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#100611): https://edk2.groups.io/g/devel/message/100611 Mute This Topic: https://groups.io/mt/96853149/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
My mistake - the DEBUG_PROPERTY_DEBUG_CLEAR_MEMORY_ENABLED feature is why FreePagesWithProtectionAttributesTestCase might fail. To make the clear memory feature more compatible with the memory attribute protocol, can you add a check to DebugClearMemoryEnabled() on free and clear EFI_MEMORY_RP and EFI_MEMORY_RO if they're set? -Taylor On 3/1/2023 1:57 PM, Ard Biesheuvel wrote: > On Wed, 1 Mar 2023 at 21:43, Taylor Beebe <t@taylorbeebe.com> wrote: >> >> >> >> On 2/11/2023 2:05 AM, Ard Biesheuvel wrote: >>> On Sat, 11 Feb 2023 at 01:56, Taylor Beebe <t@taylorbeebe.com> wrote: >>>> >>>> Hey Ard, >>>> >>>> Once the Memory Attribute Protocol is made available, Windows will have >>>> some expectations about its functionality. Can you run this test app >>>> created by me and Jiewen to ensure it meets the Windows requirements? >>>> Part of the test needed an AARCH64 implementation which I just added - >>>> let me know if it doesn't work. >>>> >>> >>> Thanks, this is rather helpful. >>> >>> There appears to be an issue related to >>> DEBUG_PROPERTY_DEBUG_CLEAR_MEMORY_ENABLED so I had to disable that to >>> run these tests, as otherwise, the DXE core tries to clear freed pages >>> before restoring the memory attributes. >>> >>> With that out of the way, the only test that fails is 'New >>> EfiLoaderCode buffer attributes expected' because this firmware build >>> maps loader code RWX, as existing boot stages for Linux are relying on >>> this (including the kernel itself at this point) >> >> It makes sense that the NewEfiLoaderCode test fails, but I am surprised >> the FreePagesWithProtectionAttributesTestCase passes. The test ensures >> that a page with EFI_MEMORY_RP and/or EFI_MEMORY_RO has those attributes >> cleared before attempting to free the page within the FreePage routine >> and is related to the concern Marvin had. >> >> Did you make a change to the core or is there an execution path I'm not >> seeing which allows that test to pass? > > No, I didn't make any additional changes to the core afair. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#100876): https://edk2.groups.io/g/devel/message/100876 Mute This Topic: https://groups.io/mt/96853149/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On Wed, 8 Mar 2023 at 18:24, Taylor Beebe <t@taylorbeebe.com> wrote: > > My mistake - the DEBUG_PROPERTY_DEBUG_CLEAR_MEMORY_ENABLED feature is > why FreePagesWithProtectionAttributesTestCase might fail. > > To make the clear memory feature more compatible with the memory > attribute protocol, can you add a check to DebugClearMemoryEnabled() on > free and clear EFI_MEMORY_RP and EFI_MEMORY_RO if they're set? > I think the best strategy here is to simply apply the default permissions first, and only then actually free the pages, clear them etc etc That also fixes the existing issue where we may remap fewer pages than what we actually freed. I'll add a patch to my series for this. > > On 3/1/2023 1:57 PM, Ard Biesheuvel wrote: > > On Wed, 1 Mar 2023 at 21:43, Taylor Beebe <t@taylorbeebe.com> wrote: > >> > >> > >> > >> On 2/11/2023 2:05 AM, Ard Biesheuvel wrote: > >>> On Sat, 11 Feb 2023 at 01:56, Taylor Beebe <t@taylorbeebe.com> wrote: > >>>> > >>>> Hey Ard, > >>>> > >>>> Once the Memory Attribute Protocol is made available, Windows will have > >>>> some expectations about its functionality. Can you run this test app > >>>> created by me and Jiewen to ensure it meets the Windows requirements? > >>>> Part of the test needed an AARCH64 implementation which I just added - > >>>> let me know if it doesn't work. > >>>> > >>> > >>> Thanks, this is rather helpful. > >>> > >>> There appears to be an issue related to > >>> DEBUG_PROPERTY_DEBUG_CLEAR_MEMORY_ENABLED so I had to disable that to > >>> run these tests, as otherwise, the DXE core tries to clear freed pages > >>> before restoring the memory attributes. > >>> > >>> With that out of the way, the only test that fails is 'New > >>> EfiLoaderCode buffer attributes expected' because this firmware build > >>> maps loader code RWX, as existing boot stages for Linux are relying on > >>> this (including the kernel itself at this point) > >> > >> It makes sense that the NewEfiLoaderCode test fails, but I am surprised > >> the FreePagesWithProtectionAttributesTestCase passes. The test ensures > >> that a page with EFI_MEMORY_RP and/or EFI_MEMORY_RO has those attributes > >> cleared before attempting to free the page within the FreePage routine > >> and is related to the concern Marvin had. > >> > >> Did you make a change to the core or is there an execution path I'm not > >> seeing which allows that test to pass? > > > > No, I didn't make any additional changes to the core afair. > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#101097): https://edk2.groups.io/g/devel/message/101097 Mute This Topic: https://groups.io/mt/96853149/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.