Re: [PATCH] ArmPkg/UncachedMemoryAllocationLib: set XP bit via CPU arch protocol

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 15 March 2017 at 10:12, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 15 March 2017 at 09:59, Ryan Harkin <ryan.harkin@linaro.org> wrote:
>> Hi Ard,
>>
>> Thanks for the quick turnaround as always.
>>
>> On 14 March 2017 at 20:20, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>>> On Tue, Mar 14, 2017 at 07:58:15PM +0000, Ard Biesheuvel wrote:
>>>> Commit e7b24ec9785d ("ArmPkg/UncachedMemoryAllocationLib: map uncached
>>>> allocations non-executable") adds code that manipulates the GCD memory
>>>> space attributes of a newly allocated uncached region without checking
>>>> whether this region expose these attributes in its capabilities mask.
>>>>
>>>> Given that the intent is to remove executable permissions from the region,
>>>> this is a fairly pointless exercise to begin with, regardless of whether
>>>> it is correct or not. The reason is that RO/XP memory attributes in the
>>>> GCD memory space map or the UEFI memory map are completely disconnected
>>>> from the actual mapping permissions used in the page tables.
>>>>
>>>> So instead, invoke the CPU arch protocol directly, and add the non-exec
>>>> attributes in the page tables directly.
>>>>
>>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>
>> Tested-by: Ryan Harkin <ryan.harkin@linaro.org>
>>
>> FVPs and TC2 worked before this change, Juno was hanging on boot as
>> per an earlier email thread.
>>
>> I tested this change on FVP Foundation & AEMv8, TC2 and Juno R0/1/2
>> and they all work fine after it's applied to the current HEAD of EDK2
>> (commit 8057622).
>>

Pushed now.