public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH V2 1/1] ArmPlatformPkg/TZASC: Allow specifying subregions to be disabled
@ 2016-12-26 21:15 Bhupesh Sharma
  2017-01-16 17:20 ` Leif Lindholm
  0 siblings, 1 reply; 3+ messages in thread
From: Bhupesh Sharma @ 2016-12-26 21:15 UTC (permalink / raw)
  To: linaro-uefi, edk2-devel; +Cc: Bhupesh Sharma, Leif Lindholm, Ard Biesheuvel

ARM TZASC-380 IP provides a mechanism to split memory regions being
protected via it into eight equal-sized sub-regions. A bit-setting
allows the corresponding subregion to be disabled.

Several NXP/FSL SoCs support the TZASC-380 IP block and allow
the DDR connected via the TZASC to be partitioned into regions
having different security settings and also allow subregions
to be disabled.

This patch enables this support and can be used for SoCs which
support such a partition of DDR regions.

Details of the 'subregion_disable' register can be viewed here:
http://infocenter.arm.com/help/topic/com.arm.doc.ddi0431c/CHDIGDCI.html

Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Bhupesh Sharma <bhupesh.linux@gmail.com>
Contributed-under: TianoCore Contribution Agreement 1.0
---
 .../Library/ArmVExpressSecLibCTA9x4/CTA9x4Sec.c            | 14 +++++++-------
 ArmPlatformPkg/Drivers/ArmTrustZone/ArmTrustZone.c         | 10 ++++++++--
 ArmPlatformPkg/Include/Drivers/ArmTrustzone.h              |  3 ++-
 3 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9x4Sec.c b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9x4Sec.c
index 6fa0774f59f8..42d731ea98c9 100644
--- a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9x4Sec.c
+++ b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9x4Sec.c
@@ -72,18 +72,18 @@ ArmPlatformSecTrustzoneInit (
   // NOR Flash 0 non secure (BootMon)
   TZASCSetRegion(ARM_VE_TZASC_BASE,1,TZASC_REGION_ENABLED,
       ARM_VE_SMB_NOR0_BASE,0,
-      TZASC_REGION_SIZE_64MB, TZASC_REGION_SECURITY_NSRW);
+      TZASC_REGION_SIZE_64MB, TZASC_REGION_SECURITY_NSRW, 0);
 
   // NOR Flash 1. The first half of the NOR Flash1 must be secure for the secure firmware (sec_uefi.bin)
   if (PcdGetBool (PcdTrustzoneSupport) == TRUE) {
     //Note: Your OS Kernel must be aware of the secure regions before to enable this region
     TZASCSetRegion(ARM_VE_TZASC_BASE,2,TZASC_REGION_ENABLED,
         ARM_VE_SMB_NOR1_BASE + SIZE_32MB,0,
-        TZASC_REGION_SIZE_32MB, TZASC_REGION_SECURITY_NSRW);
+        TZASC_REGION_SIZE_32MB, TZASC_REGION_SECURITY_NSRW, 0);
   } else {
     TZASCSetRegion(ARM_VE_TZASC_BASE,2,TZASC_REGION_ENABLED,
         ARM_VE_SMB_NOR1_BASE,0,
-        TZASC_REGION_SIZE_64MB, TZASC_REGION_SECURITY_NSRW);
+        TZASC_REGION_SIZE_64MB, TZASC_REGION_SECURITY_NSRW, 0);
   }
 
   // Base of SRAM. Only half of SRAM in Non Secure world
@@ -92,22 +92,22 @@ ArmPlatformSecTrustzoneInit (
     //Note: Your OS Kernel must be aware of the secure regions before to enable this region
     TZASCSetRegion(ARM_VE_TZASC_BASE,3,TZASC_REGION_ENABLED,
         ARM_VE_SMB_SRAM_BASE,0,
-        TZASC_REGION_SIZE_16MB, TZASC_REGION_SECURITY_NSRW);
+        TZASC_REGION_SIZE_16MB, TZASC_REGION_SECURITY_NSRW, 0);
   } else {
     TZASCSetRegion(ARM_VE_TZASC_BASE,3,TZASC_REGION_ENABLED,
         ARM_VE_SMB_SRAM_BASE,0,
-        TZASC_REGION_SIZE_32MB, TZASC_REGION_SECURITY_NSRW);
+        TZASC_REGION_SIZE_32MB, TZASC_REGION_SECURITY_NSRW, 0);
   }
 
   // Memory Mapped Peripherals. All in non secure world
   TZASCSetRegion(ARM_VE_TZASC_BASE,4,TZASC_REGION_ENABLED,
       ARM_VE_SMB_PERIPH_BASE,0,
-      TZASC_REGION_SIZE_64MB, TZASC_REGION_SECURITY_NSRW);
+      TZASC_REGION_SIZE_64MB, TZASC_REGION_SECURITY_NSRW, 0);
 
   // MotherBoard Peripherals and On-chip peripherals.
   TZASCSetRegion(ARM_VE_TZASC_BASE,5,TZASC_REGION_ENABLED,
       ARM_VE_SMB_MB_ON_CHIP_PERIPH_BASE,0,
-      TZASC_REGION_SIZE_256MB, TZASC_REGION_SECURITY_NSRW);
+      TZASC_REGION_SIZE_256MB, TZASC_REGION_SECURITY_NSRW, 0);
 }
 
 /**
diff --git a/ArmPlatformPkg/Drivers/ArmTrustZone/ArmTrustZone.c b/ArmPlatformPkg/Drivers/ArmTrustZone/ArmTrustZone.c
index 070c0dcb5d4d..c99c16d4c442 100644
--- a/ArmPlatformPkg/Drivers/ArmTrustZone/ArmTrustZone.c
+++ b/ArmPlatformPkg/Drivers/ArmTrustZone/ArmTrustZone.c
@@ -87,20 +87,26 @@ TZASCSetRegion (
   IN  UINTN LowAddress,
   IN  UINTN HighAddress,
   IN  UINTN Size,
-  IN  UINTN Security
+  IN  UINTN Security,
+  IN  UINTN SubregionDisableMask
   )
 {
   UINT32*     Region;
+  UINT32      RegionAttributes;
 
   if (RegionId > TZASCGetNumRegions(TzascBase)) {
     return EFI_INVALID_PARAMETER;
   }
 
+  RegionAttributes = ((Security & 0xF) << 28) |
+                     ((SubregionDisableMask & 0xFF) << 8) |
+                     ((Size & 0x3F) << 1) | (Enabled & 0x1);
+
   Region = (UINT32*)((UINTN)TzascBase + TZASC_REGIONS_REG + (RegionId * 0x10));
 
   MmioWrite32((UINTN)(Region), LowAddress&0xFFFF8000);
   MmioWrite32((UINTN)(Region+1), HighAddress);
-  MmioWrite32((UINTN)(Region+2), ((Security & 0xF) <<28) | ((Size & 0x3F) << 1) | (Enabled & 0x1));
+  MmioWrite32((UINTN)(Region+2), RegionAttributes);
 
   return EFI_SUCCESS;
 }
diff --git a/ArmPlatformPkg/Include/Drivers/ArmTrustzone.h b/ArmPlatformPkg/Include/Drivers/ArmTrustzone.h
index 78e98aad535f..1ba963d7b6c5 100644
--- a/ArmPlatformPkg/Include/Drivers/ArmTrustzone.h
+++ b/ArmPlatformPkg/Include/Drivers/ArmTrustzone.h
@@ -82,7 +82,8 @@ TZASCSetRegion (
   IN  UINTN LowAddress,
   IN  UINTN HighAddress,
   IN  UINTN Size,
-  IN  UINTN Security
+  IN  UINTN Security,
+  IN  UINTN SubregionDisableMask
   );
 
 #endif
-- 
2.7.4



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

* Re: [PATCH V2 1/1] ArmPlatformPkg/TZASC: Allow specifying subregions to be disabled
  2016-12-26 21:15 [PATCH V2 1/1] ArmPlatformPkg/TZASC: Allow specifying subregions to be disabled Bhupesh Sharma
@ 2017-01-16 17:20 ` Leif Lindholm
  2017-01-19  5:57   ` Bhupesh SHARMA
  0 siblings, 1 reply; 3+ messages in thread
From: Leif Lindholm @ 2017-01-16 17:20 UTC (permalink / raw)
  To: Bhupesh Sharma; +Cc: linaro-uefi, edk2-devel, Ard Biesheuvel

On Tue, Dec 27, 2016 at 02:45:42AM +0530, Bhupesh Sharma wrote:
> ARM TZASC-380 IP provides a mechanism to split memory regions being
> protected via it into eight equal-sized sub-regions. A bit-setting
> allows the corresponding subregion to be disabled.
> 
> Several NXP/FSL SoCs support the TZASC-380 IP block and allow
> the DDR connected via the TZASC to be partitioned into regions
> having different security settings and also allow subregions
> to be disabled.
> 
> This patch enables this support and can be used for SoCs which
> support such a partition of DDR regions.

So, I am tempted to take this patch, but I should warn you that you
are possibly now the only consumer of this driver. (Other platforms
use ARM Trusted Firmware.)

> Details of the 'subregion_disable' register can be viewed here:
> http://infocenter.arm.com/help/topic/com.arm.doc.ddi0431c/CHDIGDCI.html
>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Signed-off-by: Bhupesh Sharma <bhupesh.linux@gmail.com>

Please add Signed-off-by: Bhupesh Sharma <bhupesh.sharma@nxp.com>
as was in the v1 submission, then add your gmail.com submission below.

> Contributed-under: TianoCore Contribution Agreement 1.0

Contributed-under: goes before Signed-off-by:.

> ---
>  .../Library/ArmVExpressSecLibCTA9x4/CTA9x4Sec.c            | 14 +++++++-------
>  ArmPlatformPkg/Drivers/ArmTrustZone/ArmTrustZone.c         | 10 ++++++++--
>  ArmPlatformPkg/Include/Drivers/ArmTrustzone.h              |  3 ++-
>  3 files changed, 17 insertions(+), 10 deletions(-)
> 
> diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9x4Sec.c b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9x4Sec.c
> index 6fa0774f59f8..42d731ea98c9 100644
> --- a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9x4Sec.c
> +++ b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9x4Sec.c
> @@ -72,18 +72,18 @@ ArmPlatformSecTrustzoneInit (
>    // NOR Flash 0 non secure (BootMon)
>    TZASCSetRegion(ARM_VE_TZASC_BASE,1,TZASC_REGION_ENABLED,
>        ARM_VE_SMB_NOR0_BASE,0,
> -      TZASC_REGION_SIZE_64MB, TZASC_REGION_SECURITY_NSRW);
> +      TZASC_REGION_SIZE_64MB, TZASC_REGION_SECURITY_NSRW, 0);
>  
>    // NOR Flash 1. The first half of the NOR Flash1 must be secure for the secure firmware (sec_uefi.bin)
>    if (PcdGetBool (PcdTrustzoneSupport) == TRUE) {
>      //Note: Your OS Kernel must be aware of the secure regions before to enable this region
>      TZASCSetRegion(ARM_VE_TZASC_BASE,2,TZASC_REGION_ENABLED,
>          ARM_VE_SMB_NOR1_BASE + SIZE_32MB,0,
> -        TZASC_REGION_SIZE_32MB, TZASC_REGION_SECURITY_NSRW);
> +        TZASC_REGION_SIZE_32MB, TZASC_REGION_SECURITY_NSRW, 0);
>    } else {
>      TZASCSetRegion(ARM_VE_TZASC_BASE,2,TZASC_REGION_ENABLED,
>          ARM_VE_SMB_NOR1_BASE,0,
> -        TZASC_REGION_SIZE_64MB, TZASC_REGION_SECURITY_NSRW);
> +        TZASC_REGION_SIZE_64MB, TZASC_REGION_SECURITY_NSRW, 0);
>    }
>  
>    // Base of SRAM. Only half of SRAM in Non Secure world
> @@ -92,22 +92,22 @@ ArmPlatformSecTrustzoneInit (
>      //Note: Your OS Kernel must be aware of the secure regions before to enable this region
>      TZASCSetRegion(ARM_VE_TZASC_BASE,3,TZASC_REGION_ENABLED,
>          ARM_VE_SMB_SRAM_BASE,0,
> -        TZASC_REGION_SIZE_16MB, TZASC_REGION_SECURITY_NSRW);
> +        TZASC_REGION_SIZE_16MB, TZASC_REGION_SECURITY_NSRW, 0);
>    } else {
>      TZASCSetRegion(ARM_VE_TZASC_BASE,3,TZASC_REGION_ENABLED,
>          ARM_VE_SMB_SRAM_BASE,0,
> -        TZASC_REGION_SIZE_32MB, TZASC_REGION_SECURITY_NSRW);
> +        TZASC_REGION_SIZE_32MB, TZASC_REGION_SECURITY_NSRW, 0);
>    }
>  
>    // Memory Mapped Peripherals. All in non secure world
>    TZASCSetRegion(ARM_VE_TZASC_BASE,4,TZASC_REGION_ENABLED,
>        ARM_VE_SMB_PERIPH_BASE,0,
> -      TZASC_REGION_SIZE_64MB, TZASC_REGION_SECURITY_NSRW);
> +      TZASC_REGION_SIZE_64MB, TZASC_REGION_SECURITY_NSRW, 0);
>  
>    // MotherBoard Peripherals and On-chip peripherals.
>    TZASCSetRegion(ARM_VE_TZASC_BASE,5,TZASC_REGION_ENABLED,
>        ARM_VE_SMB_MB_ON_CHIP_PERIPH_BASE,0,
> -      TZASC_REGION_SIZE_256MB, TZASC_REGION_SECURITY_NSRW);
> +      TZASC_REGION_SIZE_256MB, TZASC_REGION_SECURITY_NSRW, 0);
>  }
>  
>  /**
> diff --git a/ArmPlatformPkg/Drivers/ArmTrustZone/ArmTrustZone.c b/ArmPlatformPkg/Drivers/ArmTrustZone/ArmTrustZone.c
> index 070c0dcb5d4d..c99c16d4c442 100644
> --- a/ArmPlatformPkg/Drivers/ArmTrustZone/ArmTrustZone.c
> +++ b/ArmPlatformPkg/Drivers/ArmTrustZone/ArmTrustZone.c
> @@ -87,20 +87,26 @@ TZASCSetRegion (
>    IN  UINTN LowAddress,
>    IN  UINTN HighAddress,
>    IN  UINTN Size,
> -  IN  UINTN Security
> +  IN  UINTN Security,
> +  IN  UINTN SubregionDisableMask
>    )
>  {
>    UINT32*     Region;
> +  UINT32      RegionAttributes;
>  
>    if (RegionId > TZASCGetNumRegions(TzascBase)) {
>      return EFI_INVALID_PARAMETER;
>    }
>  
> +  RegionAttributes = ((Security & 0xF) << 28) |
> +                     ((SubregionDisableMask & 0xFF) << 8) |
> +                     ((Size & 0x3F) << 1) | (Enabled & 0x1);
> +

While I realise this is mostly a refactoring of the modified line
below - could you possibly get rid of those magic values with some
judicial use of #defines?

Regards,

Leif

>    Region = (UINT32*)((UINTN)TzascBase + TZASC_REGIONS_REG + (RegionId * 0x10));
>  
>    MmioWrite32((UINTN)(Region), LowAddress&0xFFFF8000);
>    MmioWrite32((UINTN)(Region+1), HighAddress);
> -  MmioWrite32((UINTN)(Region+2), ((Security & 0xF) <<28) | ((Size & 0x3F) << 1) | (Enabled & 0x1));
> +  MmioWrite32((UINTN)(Region+2), RegionAttributes);
>  
>    return EFI_SUCCESS;
>  }
> diff --git a/ArmPlatformPkg/Include/Drivers/ArmTrustzone.h b/ArmPlatformPkg/Include/Drivers/ArmTrustzone.h
> index 78e98aad535f..1ba963d7b6c5 100644
> --- a/ArmPlatformPkg/Include/Drivers/ArmTrustzone.h
> +++ b/ArmPlatformPkg/Include/Drivers/ArmTrustzone.h
> @@ -82,7 +82,8 @@ TZASCSetRegion (
>    IN  UINTN LowAddress,
>    IN  UINTN HighAddress,
>    IN  UINTN Size,
> -  IN  UINTN Security
> +  IN  UINTN Security,
> +  IN  UINTN SubregionDisableMask
>    );
>  
>  #endif
> -- 
> 2.7.4
> 


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

* Re: [PATCH V2 1/1] ArmPlatformPkg/TZASC: Allow specifying subregions to be disabled
  2017-01-16 17:20 ` Leif Lindholm
@ 2017-01-19  5:57   ` Bhupesh SHARMA
  0 siblings, 0 replies; 3+ messages in thread
From: Bhupesh SHARMA @ 2017-01-19  5:57 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: linaro-uefi, edk2-devel, Ard Biesheuvel

Hi Leif,

Thanks for the review.

On Mon, Jan 16, 2017 at 10:50 PM, Leif Lindholm
<leif.lindholm@linaro.org> wrote:
> On Tue, Dec 27, 2016 at 02:45:42AM +0530, Bhupesh Sharma wrote:
>> ARM TZASC-380 IP provides a mechanism to split memory regions being
>> protected via it into eight equal-sized sub-regions. A bit-setting
>> allows the corresponding subregion to be disabled.
>>
>> Several NXP/FSL SoCs support the TZASC-380 IP block and allow
>> the DDR connected via the TZASC to be partitioned into regions
>> having different security settings and also allow subregions
>> to be disabled.
>>
>> This patch enables this support and can be used for SoCs which
>> support such a partition of DDR regions.
>
> So, I am tempted to take this patch, but I should warn you that you
> are possibly now the only consumer of this driver. (Other platforms
> use ARM Trusted Firmware.)

Gulp. Understood :)

>> Details of the 'subregion_disable' register can be viewed here:
>> http://infocenter.arm.com/help/topic/com.arm.doc.ddi0431c/CHDIGDCI.html
>>
>> Cc: Leif Lindholm <leif.lindholm@linaro.org>
>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Signed-off-by: Bhupesh Sharma <bhupesh.linux@gmail.com>
>
> Please add Signed-off-by: Bhupesh Sharma <bhupesh.sharma@nxp.com>
> as was in the v1 submission, then add your gmail.com submission below.

Sure.

>> Contributed-under: TianoCore Contribution Agreement 1.0
>
> Contributed-under: goes before Signed-off-by:.
>
>> ---
>>  .../Library/ArmVExpressSecLibCTA9x4/CTA9x4Sec.c            | 14 +++++++-------
>>  ArmPlatformPkg/Drivers/ArmTrustZone/ArmTrustZone.c         | 10 ++++++++--
>>  ArmPlatformPkg/Include/Drivers/ArmTrustzone.h              |  3 ++-
>>  3 files changed, 17 insertions(+), 10 deletions(-)
>>
>> diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9x4Sec.c b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9x4Sec.c
>> index 6fa0774f59f8..42d731ea98c9 100644
>> --- a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9x4Sec.c
>> +++ b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9x4Sec.c
>> @@ -72,18 +72,18 @@ ArmPlatformSecTrustzoneInit (
>>    // NOR Flash 0 non secure (BootMon)
>>    TZASCSetRegion(ARM_VE_TZASC_BASE,1,TZASC_REGION_ENABLED,
>>        ARM_VE_SMB_NOR0_BASE,0,
>> -      TZASC_REGION_SIZE_64MB, TZASC_REGION_SECURITY_NSRW);
>> +      TZASC_REGION_SIZE_64MB, TZASC_REGION_SECURITY_NSRW, 0);
>>
>>    // NOR Flash 1. The first half of the NOR Flash1 must be secure for the secure firmware (sec_uefi.bin)
>>    if (PcdGetBool (PcdTrustzoneSupport) == TRUE) {
>>      //Note: Your OS Kernel must be aware of the secure regions before to enable this region
>>      TZASCSetRegion(ARM_VE_TZASC_BASE,2,TZASC_REGION_ENABLED,
>>          ARM_VE_SMB_NOR1_BASE + SIZE_32MB,0,
>> -        TZASC_REGION_SIZE_32MB, TZASC_REGION_SECURITY_NSRW);
>> +        TZASC_REGION_SIZE_32MB, TZASC_REGION_SECURITY_NSRW, 0);
>>    } else {
>>      TZASCSetRegion(ARM_VE_TZASC_BASE,2,TZASC_REGION_ENABLED,
>>          ARM_VE_SMB_NOR1_BASE,0,
>> -        TZASC_REGION_SIZE_64MB, TZASC_REGION_SECURITY_NSRW);
>> +        TZASC_REGION_SIZE_64MB, TZASC_REGION_SECURITY_NSRW, 0);
>>    }
>>
>>    // Base of SRAM. Only half of SRAM in Non Secure world
>> @@ -92,22 +92,22 @@ ArmPlatformSecTrustzoneInit (
>>      //Note: Your OS Kernel must be aware of the secure regions before to enable this region
>>      TZASCSetRegion(ARM_VE_TZASC_BASE,3,TZASC_REGION_ENABLED,
>>          ARM_VE_SMB_SRAM_BASE,0,
>> -        TZASC_REGION_SIZE_16MB, TZASC_REGION_SECURITY_NSRW);
>> +        TZASC_REGION_SIZE_16MB, TZASC_REGION_SECURITY_NSRW, 0);
>>    } else {
>>      TZASCSetRegion(ARM_VE_TZASC_BASE,3,TZASC_REGION_ENABLED,
>>          ARM_VE_SMB_SRAM_BASE,0,
>> -        TZASC_REGION_SIZE_32MB, TZASC_REGION_SECURITY_NSRW);
>> +        TZASC_REGION_SIZE_32MB, TZASC_REGION_SECURITY_NSRW, 0);
>>    }
>>
>>    // Memory Mapped Peripherals. All in non secure world
>>    TZASCSetRegion(ARM_VE_TZASC_BASE,4,TZASC_REGION_ENABLED,
>>        ARM_VE_SMB_PERIPH_BASE,0,
>> -      TZASC_REGION_SIZE_64MB, TZASC_REGION_SECURITY_NSRW);
>> +      TZASC_REGION_SIZE_64MB, TZASC_REGION_SECURITY_NSRW, 0);
>>
>>    // MotherBoard Peripherals and On-chip peripherals.
>>    TZASCSetRegion(ARM_VE_TZASC_BASE,5,TZASC_REGION_ENABLED,
>>        ARM_VE_SMB_MB_ON_CHIP_PERIPH_BASE,0,
>> -      TZASC_REGION_SIZE_256MB, TZASC_REGION_SECURITY_NSRW);
>> +      TZASC_REGION_SIZE_256MB, TZASC_REGION_SECURITY_NSRW, 0);
>>  }
>>
>>  /**
>> diff --git a/ArmPlatformPkg/Drivers/ArmTrustZone/ArmTrustZone.c b/ArmPlatformPkg/Drivers/ArmTrustZone/ArmTrustZone.c
>> index 070c0dcb5d4d..c99c16d4c442 100644
>> --- a/ArmPlatformPkg/Drivers/ArmTrustZone/ArmTrustZone.c
>> +++ b/ArmPlatformPkg/Drivers/ArmTrustZone/ArmTrustZone.c
>> @@ -87,20 +87,26 @@ TZASCSetRegion (
>>    IN  UINTN LowAddress,
>>    IN  UINTN HighAddress,
>>    IN  UINTN Size,
>> -  IN  UINTN Security
>> +  IN  UINTN Security,
>> +  IN  UINTN SubregionDisableMask
>>    )
>>  {
>>    UINT32*     Region;
>> +  UINT32      RegionAttributes;
>>
>>    if (RegionId > TZASCGetNumRegions(TzascBase)) {
>>      return EFI_INVALID_PARAMETER;
>>    }
>>
>> +  RegionAttributes = ((Security & 0xF) << 28) |
>> +                     ((SubregionDisableMask & 0xFF) << 8) |
>> +                     ((Size & 0x3F) << 1) | (Enabled & 0x1);
>> +
>
> While I realise this is mostly a refactoring of the modified line
> below - could you possibly get rid of those magic values with some
> judicial use of #defines?

Ok, will add those in the V3.

Many thanks,
Bhupesh

> Regards,
>
> Leif
>
>>    Region = (UINT32*)((UINTN)TzascBase + TZASC_REGIONS_REG + (RegionId * 0x10));
>>
>>    MmioWrite32((UINTN)(Region), LowAddress&0xFFFF8000);
>>    MmioWrite32((UINTN)(Region+1), HighAddress);
>> -  MmioWrite32((UINTN)(Region+2), ((Security & 0xF) <<28) | ((Size & 0x3F) << 1) | (Enabled & 0x1));
>> +  MmioWrite32((UINTN)(Region+2), RegionAttributes);
>>
>>    return EFI_SUCCESS;
>>  }
>> diff --git a/ArmPlatformPkg/Include/Drivers/ArmTrustzone.h b/ArmPlatformPkg/Include/Drivers/ArmTrustzone.h
>> index 78e98aad535f..1ba963d7b6c5 100644
>> --- a/ArmPlatformPkg/Include/Drivers/ArmTrustzone.h
>> +++ b/ArmPlatformPkg/Include/Drivers/ArmTrustzone.h
>> @@ -82,7 +82,8 @@ TZASCSetRegion (
>>    IN  UINTN LowAddress,
>>    IN  UINTN HighAddress,
>>    IN  UINTN Size,
>> -  IN  UINTN Security
>> +  IN  UINTN Security,
>> +  IN  UINTN SubregionDisableMask
>>    );
>>
>>  #endif
>> --
>> 2.7.4
>>


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

end of thread, other threads:[~2017-01-19  5:57 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-26 21:15 [PATCH V2 1/1] ArmPlatformPkg/TZASC: Allow specifying subregions to be disabled Bhupesh Sharma
2017-01-16 17:20 ` Leif Lindholm
2017-01-19  5:57   ` Bhupesh SHARMA

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