Re: [edk2] [RESEND PATCH 1/1] ArmPlatformPkg/ArmTrustZone: Add support for specifying Subregions to be disabled

2016-10-26 Thread Bhupesh Sharma 
Hi Ard,

> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Monday, October 17, 2016 7:46 PM
> 
> On 17 October 2016 at 14:25, Leif Lindholm 
> wrote:
> > On Mon, Oct 17, 2016 at 10:18:01AM +, Bhupesh Sharma wrote:
> >> Hi Ard, Leif,
> >>
> >> Any comments on this patch ?
> >
> > You didn't cc me before :)
> >
> > But more importantly, I don't really have any platform to test this
> > on, so I could use a Tested-by: from someone who does. Evan, do you?
> >
> >> > From: Bhupesh Sharma [mailto:bhupesh.sha...@nxp.com]
> >> > Sent: Friday, October 14, 2016 4:40 PM
> >> >
> >> > ARM TZASC-380 IP provides a mechanism to split memory regions
> being
> >> > protected via it into eight equal-sized sub-regions, with a bit
> >> > setting allowing 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.
> >> >
> >> > This patch enables this support and can be used for SoCs which
> >> > support such partition of DDR regions.
> >> >
> >> > Details of the 'subregion_disable'
> >
> > This is not actually what the register is called in the link you're
> > providing.
> >
> >> > register can be viewed here:
> >> >
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0431
> >> > c/CJ
> >> > ABCFHB.html
> >> >
> >> > Contributed-under: TianoCore Contribution Agreement 1.0
> >> > Signed-off-by: Bhupesh Sharma 
> >> > Cc: Ard Biesheuvel 
> >> > ---
> >> >  .../Library/ArmVExpressSecLibCTA9x4/CTA9x4Sec.c | 21
> >> > ++---
> >> >  ArmPlatformPkg/Drivers/ArmTrustZone/ArmTrustZone.c  |  5 +++--
> >> >  ArmPlatformPkg/Include/Drivers/ArmTrustzone.h   |  3 ++-
> >> >  3 files changed, 19 insertions(+), 10 deletions(-)
> >> >
> >> > diff --git
> >> >
> a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA
> >> > 9x4S
> >> > ec.c
> >> >
> b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA
> >> > 9x4S
> >> > ec.c
> >> > index 6fa0774..d358d65 100644
> >> > ---
> >> >
> a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA
> >> > 9x4S
> >> > ec.c
> >> > +++
> >> >
> b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA
> >> > 9
> >> > +++ x4Sec.c
> >> > @@ -72,18 +72,21 @@ 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);
> >
> > TAB used (convert to spaces).
> >
> >> >} 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);
> >
> > TAB used (convert to spaces).
> >
> >> >}
> >> >
> >> >// Base of SRAM. Only half of SRAM in Non Secure world @@ -
> 92,22
> >> > +95,26 @@ 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);
> >
> > TAB used (convert to spaces).
> >
> >> >} 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);
> >
> > TAB used (convert to spaces).
> >
> > (These are all found by BaseTools/Scripts/PatchCheck.py, which also
> > points out that the subject line is too long.)
> >
> >> >}
> >> >
> >> >// 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);
> >> > +

Re: [edk2] [RESEND PATCH 1/1] ArmPlatformPkg/ArmTrustZone: Add support for specifying Subregions to be disabled

2016-10-26 Thread Bhupesh Sharma 
Hi Leif,

Thanks for the review.

Please see my replies in-line.

> From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
> Sent: Monday, October 17, 2016 6:55 PM
> 
> On Mon, Oct 17, 2016 at 10:18:01AM +, Bhupesh Sharma wrote:
> > Hi Ard, Leif,
> >
> > Any comments on this patch ?
> 
> You didn't cc me before :)
> 
> But more importantly, I don't really have any platform to test this on,
> so I could use a Tested-by: from someone who does. Evan, do you?
> 
> > > From: Bhupesh Sharma [mailto:bhupesh.sha...@nxp.com]
> > > Sent: Friday, October 14, 2016 4:40 PM
> > >
> > > ARM TZASC-380 IP provides a mechanism to split memory regions being
> > > protected via it into eight equal-sized sub-regions, with a bit
> > > setting allowing 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.
> > >
> > > This patch enables this support and can be used for SoCs which
> > > support such partition of DDR regions.
> > >
> > > Details of the 'subregion_disable'
> 
> This is not actually what the register is called in the link you're
> providing.
> 
> > > register can be viewed here:
> > >
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0431c
> > > /CJ
> > > ABCFHB.html
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.0
> > > Signed-off-by: Bhupesh Sharma 
> > > Cc: Ard Biesheuvel 
> > > ---
> > >  .../Library/ArmVExpressSecLibCTA9x4/CTA9x4Sec.c | 21
> > > ++---
> > >  ArmPlatformPkg/Drivers/ArmTrustZone/ArmTrustZone.c  |  5 +++--
> > >  ArmPlatformPkg/Include/Drivers/ArmTrustzone.h   |  3 ++-
> > >  3 files changed, 19 insertions(+), 10 deletions(-)
> > >
> > > diff --git
> > >
> a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9
> > > x4S
> > > ec.c
> > >
> b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9
> > > x4S
> > > ec.c
> > > index 6fa0774..d358d65 100644
> > > ---
> > >
> a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9
> > > x4S
> > > ec.c
> > > +++
> > >
> b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9
> > > +++ x4Sec.c
> > > @@ -72,18 +72,21 @@ 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);
> 
> TAB used (convert to spaces).

Ok.
 
> > >} 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);
> 
> TAB used (convert to spaces).

Ok.
 
> > >}
> > >
> > >// Base of SRAM. Only half of SRAM in Non Secure world @@ -92,22
> > > +95,26 @@ 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);
> 
> TAB used (convert to spaces).

Ok.
 
> > >} 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);
> 
> TAB used (convert to spaces).

Ok.
> (These are all found by BaseTools/Scripts/PatchCheck.py, which also
> points out that the subject line is too long.)

Thanks for the pointer. I was looking for a checkpatch.pl kind of
equivalent for EDK2. Looks like I found one now :)

> 
> > >}
> > >
> > >// 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,
> > > +

Re: [edk2] [RESEND PATCH 1/1] ArmPlatformPkg/ArmTrustZone: Add support for specifying Subregions to be disabled

2016-10-17 Thread Ard Biesheuvel 
On 17 October 2016 at 14:25, Leif Lindholm  wrote:
> On Mon, Oct 17, 2016 at 10:18:01AM +, Bhupesh Sharma wrote:
>> Hi Ard, Leif,
>>
>> Any comments on this patch ?
>
> You didn't cc me before :)
>
> But more importantly, I don't really have any platform to test this
> on, so I could use a Tested-by: from someone who does. Evan, do you?
>
>> > From: Bhupesh Sharma [mailto:bhupesh.sha...@nxp.com]
>> > Sent: Friday, October 14, 2016 4:40 PM
>> >
>> > ARM TZASC-380 IP provides a mechanism to split memory regions being
>> > protected via it into eight equal-sized sub-regions, with a bit setting
>> > allowing 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.
>> >
>> > This patch enables this support and can be used for SoCs which support
>> > such partition of DDR regions.
>> >
>> > Details of the 'subregion_disable'
>
> This is not actually what the register is called in the link you're
> providing.
>
>> > register can be viewed here:
>> > http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0431c/CJ
>> > ABCFHB.html
>> >
>> > Contributed-under: TianoCore Contribution Agreement 1.0
>> > Signed-off-by: Bhupesh Sharma 
>> > Cc: Ard Biesheuvel 
>> > ---
>> >  .../Library/ArmVExpressSecLibCTA9x4/CTA9x4Sec.c | 21
>> > ++---
>> >  ArmPlatformPkg/Drivers/ArmTrustZone/ArmTrustZone.c  |  5 +++--
>> >  ArmPlatformPkg/Include/Drivers/ArmTrustzone.h   |  3 ++-
>> >  3 files changed, 19 insertions(+), 10 deletions(-)
>> >
>> > diff --git
>> > a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9x4S
>> > ec.c
>> > b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9x4S
>> > ec.c
>> > index 6fa0774..d358d65 100644
>> > ---
>> > a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9x4S
>> > ec.c
>> > +++
>> > b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9
>> > +++ x4Sec.c
>> > @@ -72,18 +72,21 @@ 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);
>
> TAB used (convert to spaces).
>
>> >} 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);
>
> TAB used (convert to spaces).
>
>> >}
>> >
>> >// Base of SRAM. Only half of SRAM in Non Secure world @@ -92,22
>> > +95,26 @@ 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);
>
> TAB used (convert to spaces).
>
>> >} 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);
>
> TAB used (convert to spaces).
>
> (These are all found by BaseTools/Scripts/PatchCheck.py, which also
> points out that the subject line is too long.)
>
>> >}
>> >
>> >// 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);
>> >  }
>> >
>> >  /**
>> >

Re: [edk2] [RESEND PATCH 1/1] ArmPlatformPkg/ArmTrustZone: Add support for specifying Subregions to be disabled

2016-10-17 Thread Leif Lindholm 
On Mon, Oct 17, 2016 at 10:18:01AM +, Bhupesh Sharma wrote:
> Hi Ard, Leif,
> 
> Any comments on this patch ?

You didn't cc me before :)

But more importantly, I don't really have any platform to test this
on, so I could use a Tested-by: from someone who does. Evan, do you?

> > From: Bhupesh Sharma [mailto:bhupesh.sha...@nxp.com]
> > Sent: Friday, October 14, 2016 4:40 PM
> > 
> > ARM TZASC-380 IP provides a mechanism to split memory regions being
> > protected via it into eight equal-sized sub-regions, with a bit setting
> > allowing 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.
> > 
> > This patch enables this support and can be used for SoCs which support
> > such partition of DDR regions.
> > 
> > Details of the 'subregion_disable'

This is not actually what the register is called in the link you're
providing.

> > register can be viewed here:
> > http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0431c/CJ
> > ABCFHB.html
> > 
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Bhupesh Sharma 
> > Cc: Ard Biesheuvel 
> > ---
> >  .../Library/ArmVExpressSecLibCTA9x4/CTA9x4Sec.c | 21
> > ++---
> >  ArmPlatformPkg/Drivers/ArmTrustZone/ArmTrustZone.c  |  5 +++--
> >  ArmPlatformPkg/Include/Drivers/ArmTrustzone.h   |  3 ++-
> >  3 files changed, 19 insertions(+), 10 deletions(-)
> > 
> > diff --git
> > a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9x4S
> > ec.c
> > b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9x4S
> > ec.c
> > index 6fa0774..d358d65 100644
> > ---
> > a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9x4S
> > ec.c
> > +++
> > b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9
> > +++ x4Sec.c
> > @@ -72,18 +72,21 @@ 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);

TAB used (convert to spaces).

> >} 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);

TAB used (convert to spaces).

> >}
> > 
> >// Base of SRAM. Only half of SRAM in Non Secure world @@ -92,22
> > +95,26 @@ 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);

TAB used (convert to spaces).

> >} 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);

TAB used (convert to spaces).

(These are all found by BaseTools/Scripts/PatchCheck.py, which also
points out that the subject line is too long.)

> >}
> > 
> >// 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 070c0dc..5cd41ef 100644
> > ---

Re: [edk2] [RESEND PATCH 1/1] ArmPlatformPkg/ArmTrustZone: Add support for specifying Subregions to be disabled

2016-10-17 Thread Bhupesh Sharma 
Hi Ard, Leif,

Any comments on this patch ?

> From: Bhupesh Sharma [mailto:bhupesh.sha...@nxp.com]
> Sent: Friday, October 14, 2016 4:40 PM
> 
> ARM TZASC-380 IP provides a mechanism to split memory regions being
> protected via it into eight equal-sized sub-regions, with a bit setting
> allowing 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.
> 
> This patch enables this support and can be used for SoCs which support
> such partition of DDR regions.
> 
> Details of the 'subregion_disable' register can be viewed here:
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0431c/CJ
> ABCFHB.html
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Bhupesh Sharma 
> Cc: Ard Biesheuvel 
> ---
>  .../Library/ArmVExpressSecLibCTA9x4/CTA9x4Sec.c | 21
> ++---
>  ArmPlatformPkg/Drivers/ArmTrustZone/ArmTrustZone.c  |  5 +++--
>  ArmPlatformPkg/Include/Drivers/ArmTrustzone.h   |  3 ++-
>  3 files changed, 19 insertions(+), 10 deletions(-)
> 
> diff --git
> a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9x4S
> ec.c
> b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9x4S
> ec.c
> index 6fa0774..d358d65 100644
> ---
> a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9x4S
> ec.c
> +++
> b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9
> +++ x4Sec.c
> @@ -72,18 +72,21 @@ 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
> +95,26 @@ 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 070c0dc..5cd41ef 100644
> --- a/ArmPlatformPkg/Drivers/ArmTrustZone/ArmTrustZone.c
> +++ b/ArmPlatformPkg/Drivers/ArmTrustZone/ArmTrustZone.c
> @@ -87,7 +87,8 @@ TZASCSetRegion (
>IN  UINTN LowAddress,
>IN  UINTN HighAddress,
>IN  UINTN Size,
> -  IN  UINTN Security
> +  IN  UINTN Security,
> +  IN  UINTN SubregionDisableMask
>)
>  {
>UINT32* Region;
> @@ -100,7 +101,7 @@ TZASCSetRegion (
> 
>MmioWrite32((UINTN)(Region), LowAddress&0x8000);
>MmioWrite32((UINTN)(Region+1), HighAddress);
> -  MmioWrite32((UINTN)(Region+2), ((Security & 0xF) <<28) | ((Size &
> 0x3F) << 1) | (Enabled & 0x1));
> +  MmioWrite32((UINTN)(Region+2), ((Security & 0xF) <<28) |
> + ((SubregionDisableMask & 0xFF) << 8) | ((Size & 0x3F) << 1) |
> (Enabled
> + & 0x1));
> 
>return EFI_SUCCESS;
>  }
> diff --git

[edk2] [RESEND PATCH 1/1] ArmPlatformPkg/ArmTrustZone: Add support for specifying Subregions to be disabled

2016-10-14 Thread Bhupesh Sharma 
ARM TZASC-380 IP provides a mechanism to split memory regions being
protected via it into eight equal-sized sub-regions,
with a bit setting allowing 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.

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

Details of the 'subregion_disable' register can be viewed here:
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0431c/CJABCFHB.html

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Bhupesh Sharma 
Cc: Ard Biesheuvel 
---
 .../Library/ArmVExpressSecLibCTA9x4/CTA9x4Sec.c | 21 ++---
 ArmPlatformPkg/Drivers/ArmTrustZone/ArmTrustZone.c  |  5 +++--
 ArmPlatformPkg/Include/Drivers/ArmTrustzone.h   |  3 ++-
 3 files changed, 19 insertions(+), 10 deletions(-)

diff --git 
a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9x4Sec.c 
b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9x4Sec.c
index 6fa0774..d358d65 100644
--- a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9x4Sec.c
+++ b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9x4Sec.c
@@ -72,18 +72,21 @@ 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 +95,26 @@ 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 070c0dc..5cd41ef 100644
--- a/ArmPlatformPkg/Drivers/ArmTrustZone/ArmTrustZone.c
+++ b/ArmPlatformPkg/Drivers/ArmTrustZone/ArmTrustZone.c
@@ -87,7 +87,8 @@ TZASCSetRegion (
   IN  UINTN LowAddress,
   IN  UINTN HighAddress,
   IN  UINTN Size,
-  IN  UINTN Security
+  IN  UINTN Security,
+  IN  UINTN SubregionDisableMask
   )
 {
   UINT32* Region;
@@ -100,7 +101,7 @@ TZASCSetRegion (
 
   MmioWrite32((UINTN)(Region), LowAddress&0x8000);
   MmioWrite32((UINTN)(Region+1), HighAddress);
-  MmioWrite32((UINTN)(Region+2), ((Security & 0xF) <<28) | ((Size & 0x3F) << 
1) | (Enabled & 0x1));
+  MmioWrite32((UINTN)(Region+2), ((Security & 0xF) <<28) | 
((SubregionDisableMask & 0xFF) << 8) | ((Size & 0x3F) << 1) | (Enabled & 0x1));
 
   return EFI_SUCCESS;
 }
diff --git a/ArmPlatformPkg/Include/Drivers/ArmTrustzone.h 
b/ArmPlatformPkg/Include/Drivers/ArmTrustzone.h
index 78e98aa..1ba963d 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