diff mbox series

[edk2,4/4] ArmPkg/ArmMmuLib: AARCH64: add support for modifying only permissions

Message ID 1486661891-7888-5-git-send-email-ard.biesheuvel@linaro.org
State New
Headers show
Series ArmPkg: add groundwork for DXE image protection | expand

Commit Message

Ard Biesheuvel Feb. 9, 2017, 5:38 p.m. UTC 
Since the new DXE page protection for PE/COFF images may invoke
EFI_CPU_ARCH_PROTOCOL.SetMemoryAttributes() with only permission
attributes set, add support for this in the AARCH64 MMU code.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

---
 ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 73 +++++++++++++++-----
 1 file changed, 56 insertions(+), 17 deletions(-)

-- 
2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Leif Lindholm Feb. 10, 2017, 6:16 p.m. UTC | #1 
On Thu, Feb 09, 2017 at 05:38:11PM +0000, Ard Biesheuvel wrote:
> Since the new DXE page protection for PE/COFF images may invoke

> EFI_CPU_ARCH_PROTOCOL.SetMemoryAttributes() with only permission

> attributes set, add support for this in the AARCH64 MMU code.

> 

> Contributed-under: TianoCore Contribution Agreement 1.0

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---

>  ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 73 +++++++++++++++-----

>  1 file changed, 56 insertions(+), 17 deletions(-)

> 

> diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c

> index 6aa970bc0514..764e54b5d747 100644

> --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c

> +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c

> @@ -28,6 +28,10 @@

>  // We use this index definition to define an invalid block entry

>  #define TT_ATTR_INDX_INVALID    ((UINT32)~0)

>  

> +#define EFI_MEMORY_CACHETYPE_MASK   (EFI_MEMORY_UC | EFI_MEMORY_WC | \

> +                                     EFI_MEMORY_WT | EFI_MEMORY_WB | \

> +                                     EFI_MEMORY_UCE)

> +


This is already duplicated between

ArmPkg/Drivers/CpuDxe/CpuDxe.h
and
UefiCpuPkg/CpuDxe/CpuDxe.h

Can we avoid adding more?

>  STATIC

>  UINT64

>  ArmMemoryAttributeToPageAttribute (

> @@ -101,25 +105,46 @@ PageAttributeToGcdAttribute (

>    return GcdAttributes;

>  }

>  

> -ARM_MEMORY_REGION_ATTRIBUTES

> -GcdAttributeToArmAttribute (

> +STATIC

> +UINT64

> +GcdAttributeToPageAttribute (

>    IN UINT64 GcdAttributes

>    )

>  {

> -  switch (GcdAttributes & 0xFF) {

> +  UINT64 PageAttributes;

> +

> +  switch (GcdAttributes & EFI_MEMORY_CACHETYPE_MASK) {

>    case EFI_MEMORY_UC:

> -    return ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;

> +    PageAttributes = TT_ATTR_INDX_DEVICE_MEMORY;

> +    break;

>    case EFI_MEMORY_WC:

> -    return ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED;

> +    PageAttributes = TT_ATTR_INDX_MEMORY_NON_CACHEABLE;

> +    break;

>    case EFI_MEMORY_WT:

> -    return ARM_MEMORY_REGION_ATTRIBUTE_WRITE_THROUGH;

> +    PageAttributes = TT_ATTR_INDX_MEMORY_WRITE_THROUGH | TT_SH_INNER_SHAREABLE;


These TT_SH additions look like a bugfix - should they be a separate commit?

> +    break;

>    case EFI_MEMORY_WB:

> -    return ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;

> +    PageAttributes = TT_ATTR_INDX_MEMORY_WRITE_BACK | TT_SH_INNER_SHAREABLE;

> +    break;

>    default:

> -    DEBUG ((EFI_D_ERROR, "GcdAttributeToArmAttribute: 0x%lX attributes is not supported.\n", GcdAttributes));

> -    ASSERT (0);

> -    return ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;

> +    PageAttributes = TT_ATTR_INDX_MASK;


OK, so you're doing the same thing here as in the ARM code.
This is a substantial change in behaviour (old behaviour: if unknown,
set to DEVICE; new behaviour: if unknown, set "all types permitted").
Am I missing something?

> +    break;

>    }

> +

> +  if ((GcdAttributes & EFI_MEMORY_XP) != 0 ||

> +      (GcdAttributes & EFI_MEMORY_CACHETYPE_MASK) == EFI_MEMORY_UC) {

> +    if (ArmReadCurrentEL () == AARCH64_EL2) {

> +      PageAttributes |= TT_XN_MASK;

> +    } else {

> +      PageAttributes |= TT_UXN_MASK | TT_PXN_MASK;

> +    }

> +  }

> +

> +  if ((GcdAttributes & EFI_MEMORY_RO) != 0) {

> +    PageAttributes |= TT_AP_RO_RO;

> +  }

> +

> +  return PageAttributes | TT_AF;

>  }

>  

>  #define MIN_T0SZ        16

> @@ -434,17 +459,31 @@ SetMemoryAttributes (

>    )

>  {

>    RETURN_STATUS                Status;

> -  ARM_MEMORY_REGION_DESCRIPTOR MemoryRegion;

>    UINT64                      *TranslationTable;

> -

> -  MemoryRegion.PhysicalBase = BaseAddress;

> -  MemoryRegion.VirtualBase = BaseAddress;

> -  MemoryRegion.Length = Length;

> -  MemoryRegion.Attributes = GcdAttributeToArmAttribute (Attributes);

> +  UINT64                       PageAttributes;

> +  UINT64                       PageAttributeMask;

> +

> +  PageAttributes = GcdAttributeToPageAttribute (Attributes);

> +  PageAttributeMask = 0;

> +

> +  if ((Attributes & EFI_MEMORY_CACHETYPE_MASK) == 0) {

> +    //

> +    // No memory type was set in Attributes, so we are going to update the

> +    // permissions only.

> +    //

> +    PageAttributes &= TT_AP_MASK | TT_UXN_MASK | TT_PXN_MASK;

> +    PageAttributeMask = ~(TT_ADDRESS_MASK_BLOCK_ENTRY | TT_AP_MASK |

> +                          TT_PXN_MASK | TT_XN_MASK);

> +  }

>  

>    TranslationTable = ArmGetTTBR0BaseAddress ();

>  

> -  Status = FillTranslationTable (TranslationTable, &MemoryRegion);

> +  Status = UpdateRegionMapping (

> +             TranslationTable,

> +             BaseAddress,

> +             Length,

> +             PageAttributes,

> +             PageAttributeMask);

>    if (RETURN_ERROR (Status)) {

>      return Status;

>    }

> -- 

> 2.7.4

> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel Feb. 10, 2017, 6:23 p.m. UTC | #2 
On 10 February 2017 at 18:16, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Thu, Feb 09, 2017 at 05:38:11PM +0000, Ard Biesheuvel wrote:

>> Since the new DXE page protection for PE/COFF images may invoke

>> EFI_CPU_ARCH_PROTOCOL.SetMemoryAttributes() with only permission

>> attributes set, add support for this in the AARCH64 MMU code.

>>

>> Contributed-under: TianoCore Contribution Agreement 1.0

>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

>> ---

>>  ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 73 +++++++++++++++-----

>>  1 file changed, 56 insertions(+), 17 deletions(-)

>>

>> diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c

>> index 6aa970bc0514..764e54b5d747 100644

>> --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c

>> +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c

>> @@ -28,6 +28,10 @@

>>  // We use this index definition to define an invalid block entry

>>  #define TT_ATTR_INDX_INVALID    ((UINT32)~0)

>>

>> +#define EFI_MEMORY_CACHETYPE_MASK   (EFI_MEMORY_UC | EFI_MEMORY_WC | \

>> +                                     EFI_MEMORY_WT | EFI_MEMORY_WB | \

>> +                                     EFI_MEMORY_UCE)

>> +

>

> This is already duplicated between

>

> ArmPkg/Drivers/CpuDxe/CpuDxe.h

> and

> UefiCpuPkg/CpuDxe/CpuDxe.h

>

> Can we avoid adding more?

>


Good point. Mind if I move it to ArmMmuLib.h? (and keep the UefiCpuPkg
one alone)

>>  STATIC

>>  UINT64

>>  ArmMemoryAttributeToPageAttribute (

>> @@ -101,25 +105,46 @@ PageAttributeToGcdAttribute (

>>    return GcdAttributes;

>>  }

>>

>> -ARM_MEMORY_REGION_ATTRIBUTES

>> -GcdAttributeToArmAttribute (

>> +STATIC

>> +UINT64

>> +GcdAttributeToPageAttribute (

>>    IN UINT64 GcdAttributes

>>    )

>>  {

>> -  switch (GcdAttributes & 0xFF) {

>> +  UINT64 PageAttributes;

>> +

>> +  switch (GcdAttributes & EFI_MEMORY_CACHETYPE_MASK) {

>>    case EFI_MEMORY_UC:

>> -    return ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;

>> +    PageAttributes = TT_ATTR_INDX_DEVICE_MEMORY;

>> +    break;

>>    case EFI_MEMORY_WC:

>> -    return ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED;

>> +    PageAttributes = TT_ATTR_INDX_MEMORY_NON_CACHEABLE;

>> +    break;

>>    case EFI_MEMORY_WT:

>> -    return ARM_MEMORY_REGION_ATTRIBUTE_WRITE_THROUGH;

>> +    PageAttributes = TT_ATTR_INDX_MEMORY_WRITE_THROUGH | TT_SH_INNER_SHAREABLE;

>

> These TT_SH additions look like a bugfix - should they be a separate commit?

>


No, it's the diff that is confusing here: GcdAttributeToArmAttribute()
is removed completely, and replaced with
GcdAttributeToPageAttribute(). Due to the case labels, these line up,
but they are completely unrelated.

>> +    break;

>>    case EFI_MEMORY_WB:

>> -    return ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;

>> +    PageAttributes = TT_ATTR_INDX_MEMORY_WRITE_BACK | TT_SH_INNER_SHAREABLE;

>> +    break;

>>    default:

>> -    DEBUG ((EFI_D_ERROR, "GcdAttributeToArmAttribute: 0x%lX attributes is not supported.\n", GcdAttributes));

>> -    ASSERT (0);

>> -    return ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;

>> +    PageAttributes = TT_ATTR_INDX_MASK;

>

> OK, so you're doing the same thing here as in the ARM code.

> This is a substantial change in behaviour (old behaviour: if unknown,

> set to DEVICE; new behaviour: if unknown, set "all types permitted").

> Am I missing something?

>


Again, completely different function

>> +    break;

>>    }

>> +

>> +  if ((GcdAttributes & EFI_MEMORY_XP) != 0 ||

>> +      (GcdAttributes & EFI_MEMORY_CACHETYPE_MASK) == EFI_MEMORY_UC) {

>> +    if (ArmReadCurrentEL () == AARCH64_EL2) {

>> +      PageAttributes |= TT_XN_MASK;

>> +    } else {

>> +      PageAttributes |= TT_UXN_MASK | TT_PXN_MASK;

>> +    }

>> +  }

>> +

>> +  if ((GcdAttributes & EFI_MEMORY_RO) != 0) {

>> +    PageAttributes |= TT_AP_RO_RO;

>> +  }

>> +

>> +  return PageAttributes | TT_AF;

>>  }

>>

>>  #define MIN_T0SZ        16

>> @@ -434,17 +459,31 @@ SetMemoryAttributes (

>>    )

>>  {

>>    RETURN_STATUS                Status;

>> -  ARM_MEMORY_REGION_DESCRIPTOR MemoryRegion;

>>    UINT64                      *TranslationTable;

>> -

>> -  MemoryRegion.PhysicalBase = BaseAddress;

>> -  MemoryRegion.VirtualBase = BaseAddress;

>> -  MemoryRegion.Length = Length;

>> -  MemoryRegion.Attributes = GcdAttributeToArmAttribute (Attributes);

>> +  UINT64                       PageAttributes;

>> +  UINT64                       PageAttributeMask;

>> +

>> +  PageAttributes = GcdAttributeToPageAttribute (Attributes);

>> +  PageAttributeMask = 0;

>> +

>> +  if ((Attributes & EFI_MEMORY_CACHETYPE_MASK) == 0) {

>> +    //

>> +    // No memory type was set in Attributes, so we are going to update the

>> +    // permissions only.

>> +    //

>> +    PageAttributes &= TT_AP_MASK | TT_UXN_MASK | TT_PXN_MASK;

>> +    PageAttributeMask = ~(TT_ADDRESS_MASK_BLOCK_ENTRY | TT_AP_MASK |

>> +                          TT_PXN_MASK | TT_XN_MASK);

>> +  }

>>

>>    TranslationTable = ArmGetTTBR0BaseAddress ();

>>

>> -  Status = FillTranslationTable (TranslationTable, &MemoryRegion);

>> +  Status = UpdateRegionMapping (

>> +             TranslationTable,

>> +             BaseAddress,

>> +             Length,

>> +             PageAttributes,

>> +             PageAttributeMask);

>>    if (RETURN_ERROR (Status)) {

>>      return Status;

>>    }

>> --

>> 2.7.4

>>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Leif Lindholm Feb. 11, 2017, 2:35 p.m. UTC | #3 
On Fri, Feb 10, 2017 at 06:23:23PM +0000, Ard Biesheuvel wrote:
> On 10 February 2017 at 18:16, Leif Lindholm <leif.lindholm@linaro.org> wrote:

> > On Thu, Feb 09, 2017 at 05:38:11PM +0000, Ard Biesheuvel wrote:

> >> Since the new DXE page protection for PE/COFF images may invoke

> >> EFI_CPU_ARCH_PROTOCOL.SetMemoryAttributes() with only permission

> >> attributes set, add support for this in the AARCH64 MMU code.

> >>

> >> Contributed-under: TianoCore Contribution Agreement 1.0

> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> >> ---

> >>  ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 73 +++++++++++++++-----

> >>  1 file changed, 56 insertions(+), 17 deletions(-)

> >>

> >> diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c

> >> index 6aa970bc0514..764e54b5d747 100644

> >> --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c

> >> +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c

> >> @@ -28,6 +28,10 @@

> >>  // We use this index definition to define an invalid block entry

> >>  #define TT_ATTR_INDX_INVALID    ((UINT32)~0)

> >>

> >> +#define EFI_MEMORY_CACHETYPE_MASK   (EFI_MEMORY_UC | EFI_MEMORY_WC | \

> >> +                                     EFI_MEMORY_WT | EFI_MEMORY_WB | \

> >> +                                     EFI_MEMORY_UCE)

> >> +

> >

> > This is already duplicated between

> >

> > ArmPkg/Drivers/CpuDxe/CpuDxe.h

> > and

> > UefiCpuPkg/CpuDxe/CpuDxe.h

> >

> > Can we avoid adding more?

> >

> 

> Good point. Mind if I move it to ArmMmuLib.h? (and keep the UefiCpuPkg

> one alone)


That's fine for now.
They need to be squashed at some point, but that doesn't have to be now.

> >>  STATIC

> >>  UINT64

> >>  ArmMemoryAttributeToPageAttribute (

> >> @@ -101,25 +105,46 @@ PageAttributeToGcdAttribute (

> >>    return GcdAttributes;

> >>  }

> >>

> >> -ARM_MEMORY_REGION_ATTRIBUTES

> >> -GcdAttributeToArmAttribute (

> >> +STATIC

> >> +UINT64

> >> +GcdAttributeToPageAttribute (

> >>    IN UINT64 GcdAttributes

> >>    )

> >>  {

> >> -  switch (GcdAttributes & 0xFF) {

> >> +  UINT64 PageAttributes;

> >> +

> >> +  switch (GcdAttributes & EFI_MEMORY_CACHETYPE_MASK) {

> >>    case EFI_MEMORY_UC:

> >> -    return ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;

> >> +    PageAttributes = TT_ATTR_INDX_DEVICE_MEMORY;

> >> +    break;

> >>    case EFI_MEMORY_WC:

> >> -    return ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED;

> >> +    PageAttributes = TT_ATTR_INDX_MEMORY_NON_CACHEABLE;

> >> +    break;

> >>    case EFI_MEMORY_WT:

> >> -    return ARM_MEMORY_REGION_ATTRIBUTE_WRITE_THROUGH;

> >> +    PageAttributes = TT_ATTR_INDX_MEMORY_WRITE_THROUGH | TT_SH_INNER_SHAREABLE;

> >

> > These TT_SH additions look like a bugfix - should they be a separate commit?

> >

> 

> No, it's the diff that is confusing here: GcdAttributeToArmAttribute()

> is removed completely, and replaced with

> GcdAttributeToPageAttribute(). Due to the case labels, these line up,

> but they are completely unrelated.


Aaaah...

> 

> >> +    break;

> >>    case EFI_MEMORY_WB:

> >> -    return ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;

> >> +    PageAttributes = TT_ATTR_INDX_MEMORY_WRITE_BACK | TT_SH_INNER_SHAREABLE;

> >> +    break;

> >>    default:

> >> -    DEBUG ((EFI_D_ERROR, "GcdAttributeToArmAttribute: 0x%lX attributes is not supported.\n", GcdAttributes));

> >> -    ASSERT (0);

> >> -    return ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;

> >> +    PageAttributes = TT_ATTR_INDX_MASK;

> >

> > OK, so you're doing the same thing here as in the ARM code.

> > This is a substantial change in behaviour (old behaviour: if unknown,

> > set to DEVICE; new behaviour: if unknown, set "all types permitted").

> > Am I missing something?

> >

> 

> Again, completely different function


Sneaky :)

OK, I have no issues then.

/
    Leif

> >> +    break;

> >>    }

> >> +

> >> +  if ((GcdAttributes & EFI_MEMORY_XP) != 0 ||

> >> +      (GcdAttributes & EFI_MEMORY_CACHETYPE_MASK) == EFI_MEMORY_UC) {

> >> +    if (ArmReadCurrentEL () == AARCH64_EL2) {

> >> +      PageAttributes |= TT_XN_MASK;

> >> +    } else {

> >> +      PageAttributes |= TT_UXN_MASK | TT_PXN_MASK;

> >> +    }

> >> +  }

> >> +

> >> +  if ((GcdAttributes & EFI_MEMORY_RO) != 0) {

> >> +    PageAttributes |= TT_AP_RO_RO;

> >> +  }

> >> +

> >> +  return PageAttributes | TT_AF;

> >>  }

> >>

> >>  #define MIN_T0SZ        16

> >> @@ -434,17 +459,31 @@ SetMemoryAttributes (

> >>    )

> >>  {

> >>    RETURN_STATUS                Status;

> >> -  ARM_MEMORY_REGION_DESCRIPTOR MemoryRegion;

> >>    UINT64                      *TranslationTable;

> >> -

> >> -  MemoryRegion.PhysicalBase = BaseAddress;

> >> -  MemoryRegion.VirtualBase = BaseAddress;

> >> -  MemoryRegion.Length = Length;

> >> -  MemoryRegion.Attributes = GcdAttributeToArmAttribute (Attributes);

> >> +  UINT64                       PageAttributes;

> >> +  UINT64                       PageAttributeMask;

> >> +

> >> +  PageAttributes = GcdAttributeToPageAttribute (Attributes);

> >> +  PageAttributeMask = 0;

> >> +

> >> +  if ((Attributes & EFI_MEMORY_CACHETYPE_MASK) == 0) {

> >> +    //

> >> +    // No memory type was set in Attributes, so we are going to update the

> >> +    // permissions only.

> >> +    //

> >> +    PageAttributes &= TT_AP_MASK | TT_UXN_MASK | TT_PXN_MASK;

> >> +    PageAttributeMask = ~(TT_ADDRESS_MASK_BLOCK_ENTRY | TT_AP_MASK |

> >> +                          TT_PXN_MASK | TT_XN_MASK);

> >> +  }

> >>

> >>    TranslationTable = ArmGetTTBR0BaseAddress ();

> >>

> >> -  Status = FillTranslationTable (TranslationTable, &MemoryRegion);

> >> +  Status = UpdateRegionMapping (

> >> +             TranslationTable,

> >> +             BaseAddress,

> >> +             Length,

> >> +             PageAttributes,

> >> +             PageAttributeMask);

> >>    if (RETURN_ERROR (Status)) {

> >>      return Status;

> >>    }

> >> --

> >> 2.7.4

> >>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
diff mbox series

Patch

diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
index 6aa970bc0514..764e54b5d747 100644
--- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
@@ -28,6 +28,10 @@ 
 // We use this index definition to define an invalid block entry
 #define TT_ATTR_INDX_INVALID    ((UINT32)~0)
 
+#define EFI_MEMORY_CACHETYPE_MASK   (EFI_MEMORY_UC | EFI_MEMORY_WC | \
+                                     EFI_MEMORY_WT | EFI_MEMORY_WB | \
+                                     EFI_MEMORY_UCE)
+
 STATIC
 UINT64
 ArmMemoryAttributeToPageAttribute (
@@ -101,25 +105,46 @@  PageAttributeToGcdAttribute (
   return GcdAttributes;
 }
 
-ARM_MEMORY_REGION_ATTRIBUTES
-GcdAttributeToArmAttribute (
+STATIC
+UINT64
+GcdAttributeToPageAttribute (
   IN UINT64 GcdAttributes
   )
 {
-  switch (GcdAttributes & 0xFF) {
+  UINT64 PageAttributes;
+
+  switch (GcdAttributes & EFI_MEMORY_CACHETYPE_MASK) {
   case EFI_MEMORY_UC:
-    return ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
+    PageAttributes = TT_ATTR_INDX_DEVICE_MEMORY;
+    break;
   case EFI_MEMORY_WC:
-    return ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED;
+    PageAttributes = TT_ATTR_INDX_MEMORY_NON_CACHEABLE;
+    break;
   case EFI_MEMORY_WT:
-    return ARM_MEMORY_REGION_ATTRIBUTE_WRITE_THROUGH;
+    PageAttributes = TT_ATTR_INDX_MEMORY_WRITE_THROUGH | TT_SH_INNER_SHAREABLE;
+    break;
   case EFI_MEMORY_WB:
-    return ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
+    PageAttributes = TT_ATTR_INDX_MEMORY_WRITE_BACK | TT_SH_INNER_SHAREABLE;
+    break;
   default:
-    DEBUG ((EFI_D_ERROR, "GcdAttributeToArmAttribute: 0x%lX attributes is not supported.\n", GcdAttributes));
-    ASSERT (0);
-    return ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
+    PageAttributes = TT_ATTR_INDX_MASK;
+    break;
   }
+
+  if ((GcdAttributes & EFI_MEMORY_XP) != 0 ||
+      (GcdAttributes & EFI_MEMORY_CACHETYPE_MASK) == EFI_MEMORY_UC) {
+    if (ArmReadCurrentEL () == AARCH64_EL2) {
+      PageAttributes |= TT_XN_MASK;
+    } else {
+      PageAttributes |= TT_UXN_MASK | TT_PXN_MASK;
+    }
+  }
+
+  if ((GcdAttributes & EFI_MEMORY_RO) != 0) {
+    PageAttributes |= TT_AP_RO_RO;
+  }
+
+  return PageAttributes | TT_AF;
 }
 
 #define MIN_T0SZ        16
@@ -434,17 +459,31 @@  SetMemoryAttributes (
   )
 {
   RETURN_STATUS                Status;
-  ARM_MEMORY_REGION_DESCRIPTOR MemoryRegion;
   UINT64                      *TranslationTable;
-
-  MemoryRegion.PhysicalBase = BaseAddress;
-  MemoryRegion.VirtualBase = BaseAddress;
-  MemoryRegion.Length = Length;
-  MemoryRegion.Attributes = GcdAttributeToArmAttribute (Attributes);
+  UINT64                       PageAttributes;
+  UINT64                       PageAttributeMask;
+
+  PageAttributes = GcdAttributeToPageAttribute (Attributes);
+  PageAttributeMask = 0;
+
+  if ((Attributes & EFI_MEMORY_CACHETYPE_MASK) == 0) {
+    //
+    // No memory type was set in Attributes, so we are going to update the
+    // permissions only.
+    //
+    PageAttributes &= TT_AP_MASK | TT_UXN_MASK | TT_PXN_MASK;
+    PageAttributeMask = ~(TT_ADDRESS_MASK_BLOCK_ENTRY | TT_AP_MASK |
+                          TT_PXN_MASK | TT_XN_MASK);
+  }
 
   TranslationTable = ArmGetTTBR0BaseAddress ();
 
-  Status = FillTranslationTable (TranslationTable, &MemoryRegion);
+  Status = UpdateRegionMapping (
+             TranslationTable,
+             BaseAddress,
+             Length,
+             PageAttributes,
+             PageAttributeMask);
   if (RETURN_ERROR (Status)) {
     return Status;
   }