Re: [PATCH v4 00/11] ArmPkg: implement EFI memory attributes protocol

["Taylor Beebe" <t@taylorbeebe.com>]

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.