public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] ArmPkg/UncachedMemoryAllocationLib: set XP bit via CPU arch protocol
@ 2017-03-14 19:58 Ard Biesheuvel
  2017-03-14 20:20 ` Leif Lindholm
  0 siblings, 1 reply; 5+ messages in thread
From: Ard Biesheuvel @ 2017-03-14 19:58 UTC (permalink / raw)
  To: edk2-devel, leif.lindholm, ryan.harkin; +Cc: Ard Biesheuvel

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>
---
 ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.c   | 36 ++++++++++++++++----
 ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.inf |  8 ++++-
 2 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.c b/ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.c
index b4fbfbcb362b..2530175bab7a 100644
--- a/ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.c
+++ b/ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.c
@@ -27,6 +27,10 @@
 #include <Library/DxeServicesTableLib.h>
 #include <Library/CacheMaintenanceLib.h>
 
+#include <Protocol/Cpu.h>
+
+STATIC EFI_CPU_ARCH_PROTOCOL    *mCpu;
+
 VOID *
 UncachedInternalAllocatePages (
   IN EFI_MEMORY_TYPE  MemoryType,
@@ -150,15 +154,19 @@ AllocatePagesFromList (
 
   Status = gDS->GetMemorySpaceDescriptor (Memory, &Descriptor);
   if (EFI_ERROR (Status)) {
-    gBS->FreePages (Memory, Pages);
-    return Status;
+    goto FreePages;
   }
 
   Status = gDS->SetMemorySpaceAttributes (Memory, EFI_PAGES_TO_SIZE (Pages),
-                  EFI_MEMORY_WC | EFI_MEMORY_XP);
+                  EFI_MEMORY_WC);
   if (EFI_ERROR (Status)) {
-    gBS->FreePages (Memory, Pages);
-    return Status;
+    goto FreePages;
+  }
+
+  Status = mCpu->SetMemoryAttributes (mCpu, Memory, EFI_PAGES_TO_SIZE (Pages),
+                   EFI_MEMORY_XP);
+  if (EFI_ERROR (Status)) {
+    goto FreePages;
   }
 
   InvalidateDataCacheRange ((VOID *)(UINTN)Memory, EFI_PAGES_TO_SIZE (Pages));
@@ -166,8 +174,8 @@ AllocatePagesFromList (
   NewNode = AllocatePool (sizeof (FREE_PAGE_NODE));
   if (NewNode == NULL) {
     ASSERT (FALSE);
-    gBS->FreePages (Memory, Pages);
-    return EFI_OUT_OF_RESOURCES;
+    Status = EFI_OUT_OF_RESOURCES;
+    goto FreePages;
   }
 
   NewNode->Base       = Memory;
@@ -181,6 +189,10 @@ AllocatePagesFromList (
 
   *Allocation = NewNode->Allocation;
   return EFI_SUCCESS;
+
+FreePages:
+  gBS->FreePages (Memory, Pages);
+  return Status;
 }
 
 /**
@@ -238,6 +250,16 @@ FreePagesFromList (
  */
 EFI_STATUS
 EFIAPI
+UncachedMemoryAllocationLibConstructor (
+  IN EFI_HANDLE        ImageHandle,
+  IN EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  return gBS->LocateProtocol (&gEfiCpuArchProtocolGuid, NULL, (VOID **)&mCpu);
+}
+
+EFI_STATUS
+EFIAPI
 UncachedMemoryAllocationLibDestructor (
   IN EFI_HANDLE        ImageHandle,
   IN EFI_SYSTEM_TABLE  *SystemTable
diff --git a/ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.inf b/ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.inf
index d7a0f2f792a1..c637430c9020 100644
--- a/ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.inf
+++ b/ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.inf
@@ -22,7 +22,7 @@ [Defines]
   MODULE_TYPE                    = DXE_DRIVER
   VERSION_STRING                 = 1.0
   LIBRARY_CLASS                  = UncachedMemoryAllocationLib
-
+  CONSTRUCTOR                    = UncachedMemoryAllocationLibConstructor
   DESTRUCTOR                     = UncachedMemoryAllocationLibDestructor
 
 [Sources.common]
@@ -42,3 +42,9 @@ [LibraryClasses]
 
 [Pcd]
   gArmTokenSpaceGuid.PcdArmFreeUncachedMemorySizeThreshold
+
+[Protocols]
+  gEfiCpuArchProtocolGuid
+
+[Depex]
+  gEfiCpuArchProtocolGuid
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] ArmPkg/UncachedMemoryAllocationLib: set XP bit via CPU arch protocol
  2017-03-14 19:58 [PATCH] ArmPkg/UncachedMemoryAllocationLib: set XP bit via CPU arch protocol Ard Biesheuvel
@ 2017-03-14 20:20 ` Leif Lindholm
  2017-03-15  9:59   ` Ryan Harkin
  0 siblings, 1 reply; 5+ messages in thread
From: Leif Lindholm @ 2017-03-14 20:20 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: edk2-devel, ryan.harkin

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>
> ---
>  ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.c   | 36 ++++++++++++++++----
>  ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.inf |  8 ++++-
>  2 files changed, 36 insertions(+), 8 deletions(-)
> 
> diff --git a/ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.c b/ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.c
> index b4fbfbcb362b..2530175bab7a 100644
> --- a/ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.c
> +++ b/ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.c
> @@ -27,6 +27,10 @@
>  #include <Library/DxeServicesTableLib.h>
>  #include <Library/CacheMaintenanceLib.h>
>  
> +#include <Protocol/Cpu.h>
> +
> +STATIC EFI_CPU_ARCH_PROTOCOL    *mCpu;
> +
>  VOID *
>  UncachedInternalAllocatePages (
>    IN EFI_MEMORY_TYPE  MemoryType,
> @@ -150,15 +154,19 @@ AllocatePagesFromList (
>  
>    Status = gDS->GetMemorySpaceDescriptor (Memory, &Descriptor);
>    if (EFI_ERROR (Status)) {
> -    gBS->FreePages (Memory, Pages);
> -    return Status;
> +    goto FreePages;
>    }
>  
>    Status = gDS->SetMemorySpaceAttributes (Memory, EFI_PAGES_TO_SIZE (Pages),
> -                  EFI_MEMORY_WC | EFI_MEMORY_XP);
> +                  EFI_MEMORY_WC);
>    if (EFI_ERROR (Status)) {
> -    gBS->FreePages (Memory, Pages);
> -    return Status;
> +    goto FreePages;
> +  }
> +
> +  Status = mCpu->SetMemoryAttributes (mCpu, Memory, EFI_PAGES_TO_SIZE (Pages),
> +                   EFI_MEMORY_XP);

So, all other changes here are straightforward, but this one relies on
the fact that we don't care about EFI_MEMORY_RO being cleared when we
flip to _XP. As discussed on irc, if you can add a small note on that
(and wait for Ryan's Tested-by):

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> +  if (EFI_ERROR (Status)) {
> +    goto FreePages;
>    }
>  
>    InvalidateDataCacheRange ((VOID *)(UINTN)Memory, EFI_PAGES_TO_SIZE (Pages));
> @@ -166,8 +174,8 @@ AllocatePagesFromList (
>    NewNode = AllocatePool (sizeof (FREE_PAGE_NODE));
>    if (NewNode == NULL) {
>      ASSERT (FALSE);
> -    gBS->FreePages (Memory, Pages);
> -    return EFI_OUT_OF_RESOURCES;
> +    Status = EFI_OUT_OF_RESOURCES;
> +    goto FreePages;
>    }
>  
>    NewNode->Base       = Memory;
> @@ -181,6 +189,10 @@ AllocatePagesFromList (
>  
>    *Allocation = NewNode->Allocation;
>    return EFI_SUCCESS;
> +
> +FreePages:
> +  gBS->FreePages (Memory, Pages);
> +  return Status;
>  }
>  
>  /**
> @@ -238,6 +250,16 @@ FreePagesFromList (
>   */
>  EFI_STATUS
>  EFIAPI
> +UncachedMemoryAllocationLibConstructor (
> +  IN EFI_HANDLE        ImageHandle,
> +  IN EFI_SYSTEM_TABLE  *SystemTable
> +  )
> +{
> +  return gBS->LocateProtocol (&gEfiCpuArchProtocolGuid, NULL, (VOID **)&mCpu);
> +}
> +
> +EFI_STATUS
> +EFIAPI
>  UncachedMemoryAllocationLibDestructor (
>    IN EFI_HANDLE        ImageHandle,
>    IN EFI_SYSTEM_TABLE  *SystemTable
> diff --git a/ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.inf b/ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.inf
> index d7a0f2f792a1..c637430c9020 100644
> --- a/ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.inf
> +++ b/ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.inf
> @@ -22,7 +22,7 @@ [Defines]
>    MODULE_TYPE                    = DXE_DRIVER
>    VERSION_STRING                 = 1.0
>    LIBRARY_CLASS                  = UncachedMemoryAllocationLib
> -
> +  CONSTRUCTOR                    = UncachedMemoryAllocationLibConstructor
>    DESTRUCTOR                     = UncachedMemoryAllocationLibDestructor
>  
>  [Sources.common]
> @@ -42,3 +42,9 @@ [LibraryClasses]
>  
>  [Pcd]
>    gArmTokenSpaceGuid.PcdArmFreeUncachedMemorySizeThreshold
> +
> +[Protocols]
> +  gEfiCpuArchProtocolGuid
> +
> +[Depex]
> +  gEfiCpuArchProtocolGuid
> -- 
> 2.7.4
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ArmPkg/UncachedMemoryAllocationLib: set XP bit via CPU arch protocol
  2017-03-14 20:20 ` Leif Lindholm
@ 2017-03-15  9:59   ` Ryan Harkin
  2017-03-15 10:12     ` Ard Biesheuvel
  0 siblings, 1 reply; 5+ messages in thread
From: Ryan Harkin @ 2017-03-15  9:59 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: Ard Biesheuvel, edk2-devel@lists.01.org

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).


>> ---
>>  ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.c   | 36 ++++++++++++++++----
>>  ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.inf |  8 ++++-
>>  2 files changed, 36 insertions(+), 8 deletions(-)
>>
>> diff --git a/ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.c b/ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.c
>> index b4fbfbcb362b..2530175bab7a 100644
>> --- a/ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.c
>> +++ b/ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.c
>> @@ -27,6 +27,10 @@
>>  #include <Library/DxeServicesTableLib.h>
>>  #include <Library/CacheMaintenanceLib.h>
>>
>> +#include <Protocol/Cpu.h>
>> +
>> +STATIC EFI_CPU_ARCH_PROTOCOL    *mCpu;
>> +
>>  VOID *
>>  UncachedInternalAllocatePages (
>>    IN EFI_MEMORY_TYPE  MemoryType,
>> @@ -150,15 +154,19 @@ AllocatePagesFromList (
>>
>>    Status = gDS->GetMemorySpaceDescriptor (Memory, &Descriptor);
>>    if (EFI_ERROR (Status)) {
>> -    gBS->FreePages (Memory, Pages);
>> -    return Status;
>> +    goto FreePages;
>>    }
>>
>>    Status = gDS->SetMemorySpaceAttributes (Memory, EFI_PAGES_TO_SIZE (Pages),
>> -                  EFI_MEMORY_WC | EFI_MEMORY_XP);
>> +                  EFI_MEMORY_WC);
>>    if (EFI_ERROR (Status)) {
>> -    gBS->FreePages (Memory, Pages);
>> -    return Status;
>> +    goto FreePages;
>> +  }
>> +
>> +  Status = mCpu->SetMemoryAttributes (mCpu, Memory, EFI_PAGES_TO_SIZE (Pages),
>> +                   EFI_MEMORY_XP);
>
> So, all other changes here are straightforward, but this one relies on
> the fact that we don't care about EFI_MEMORY_RO being cleared when we
> flip to _XP. As discussed on irc, if you can add a small note on that
> (and wait for Ryan's Tested-by):
>
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>
>> +  if (EFI_ERROR (Status)) {
>> +    goto FreePages;
>>    }
>>
>>    InvalidateDataCacheRange ((VOID *)(UINTN)Memory, EFI_PAGES_TO_SIZE (Pages));
>> @@ -166,8 +174,8 @@ AllocatePagesFromList (
>>    NewNode = AllocatePool (sizeof (FREE_PAGE_NODE));
>>    if (NewNode == NULL) {
>>      ASSERT (FALSE);
>> -    gBS->FreePages (Memory, Pages);
>> -    return EFI_OUT_OF_RESOURCES;
>> +    Status = EFI_OUT_OF_RESOURCES;
>> +    goto FreePages;
>>    }
>>
>>    NewNode->Base       = Memory;
>> @@ -181,6 +189,10 @@ AllocatePagesFromList (
>>
>>    *Allocation = NewNode->Allocation;
>>    return EFI_SUCCESS;
>> +
>> +FreePages:
>> +  gBS->FreePages (Memory, Pages);
>> +  return Status;
>>  }
>>
>>  /**
>> @@ -238,6 +250,16 @@ FreePagesFromList (
>>   */
>>  EFI_STATUS
>>  EFIAPI
>> +UncachedMemoryAllocationLibConstructor (
>> +  IN EFI_HANDLE        ImageHandle,
>> +  IN EFI_SYSTEM_TABLE  *SystemTable
>> +  )
>> +{
>> +  return gBS->LocateProtocol (&gEfiCpuArchProtocolGuid, NULL, (VOID **)&mCpu);
>> +}
>> +
>> +EFI_STATUS
>> +EFIAPI
>>  UncachedMemoryAllocationLibDestructor (
>>    IN EFI_HANDLE        ImageHandle,
>>    IN EFI_SYSTEM_TABLE  *SystemTable
>> diff --git a/ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.inf b/ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.inf
>> index d7a0f2f792a1..c637430c9020 100644
>> --- a/ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.inf
>> +++ b/ArmPkg/Library/UncachedMemoryAllocationLib/UncachedMemoryAllocationLib.inf
>> @@ -22,7 +22,7 @@ [Defines]
>>    MODULE_TYPE                    = DXE_DRIVER
>>    VERSION_STRING                 = 1.0
>>    LIBRARY_CLASS                  = UncachedMemoryAllocationLib
>> -
>> +  CONSTRUCTOR                    = UncachedMemoryAllocationLibConstructor
>>    DESTRUCTOR                     = UncachedMemoryAllocationLibDestructor
>>
>>  [Sources.common]
>> @@ -42,3 +42,9 @@ [LibraryClasses]
>>
>>  [Pcd]
>>    gArmTokenSpaceGuid.PcdArmFreeUncachedMemorySizeThreshold
>> +
>> +[Protocols]
>> +  gEfiCpuArchProtocolGuid
>> +
>> +[Depex]
>> +  gEfiCpuArchProtocolGuid
>> --
>> 2.7.4
>>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ArmPkg/UncachedMemoryAllocationLib: set XP bit via CPU arch protocol
  2017-03-15  9:59   ` Ryan Harkin
@ 2017-03-15 10:12     ` Ard Biesheuvel
  2017-03-15 19:37       ` Ard Biesheuvel
  0 siblings, 1 reply; 5+ messages in thread
From: Ard Biesheuvel @ 2017-03-15 10:12 UTC (permalink / raw)
  To: Ryan Harkin; +Cc: Leif Lindholm, edk2-devel@lists.01.org

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).
>

Thanks!


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ArmPkg/UncachedMemoryAllocationLib: set XP bit via CPU arch protocol
  2017-03-15 10:12     ` Ard Biesheuvel
@ 2017-03-15 19:37       ` Ard Biesheuvel
  0 siblings, 0 replies; 5+ messages in thread
From: Ard Biesheuvel @ 2017-03-15 19:37 UTC (permalink / raw)
  To: Ryan Harkin; +Cc: Leif Lindholm, edk2-devel@lists.01.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.


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-03-15 19:37 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-14 19:58 [PATCH] ArmPkg/UncachedMemoryAllocationLib: set XP bit via CPU arch protocol Ard Biesheuvel
2017-03-14 20:20 ` Leif Lindholm
2017-03-15  9:59   ` Ryan Harkin
2017-03-15 10:12     ` Ard Biesheuvel
2017-03-15 19:37       ` Ard Biesheuvel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox