* [RESEND PATCH 1/1] ArmPlatformPkg/ArmTrustZone: Add support for specifying Subregions to be disabled @ 2016-10-14 11:09 Bhupesh Sharma 2016-10-17 10:18 ` Bhupesh Sharma 0 siblings, 1 reply; 6+ messages in thread From: Bhupesh Sharma @ 2016-10-14 11:09 UTC (permalink / raw) To: edk2-devel; +Cc: linaro-uefi, Bhupesh Sharma, Ard Biesheuvel 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 <bhupesh.sharma@nxp.com> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- .../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&0xFFFF8000); 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 SubregionDisableMask ); #endif -- 1.9.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH 1/1] ArmPlatformPkg/ArmTrustZone: Add support for specifying Subregions to be disabled 2016-10-14 11:09 [RESEND PATCH 1/1] ArmPlatformPkg/ArmTrustZone: Add support for specifying Subregions to be disabled Bhupesh Sharma @ 2016-10-17 10:18 ` Bhupesh Sharma 2016-10-17 13:25 ` Leif Lindholm 0 siblings, 1 reply; 6+ messages in thread From: Bhupesh Sharma @ 2016-10-17 10:18 UTC (permalink / raw) To: edk2-devel@ml01.01.org, Ard Biesheuvel, Leif Lindholm Cc: linaro-uefi@lists.linaro.org, Bhupesh Sharma Hi Ard, Leif, Any comments on this patch ? > From: Bhupesh Sharma [mailto:bhupesh.sharma@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 <bhupesh.sharma@nxp.com> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > .../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&0xFFFF8000); > 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 SubregionDisableMask > ); > > #endif > -- > 1.9.1 > Regards, Bhupesh ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH 1/1] ArmPlatformPkg/ArmTrustZone: Add support for specifying Subregions to be disabled 2016-10-17 10:18 ` Bhupesh Sharma @ 2016-10-17 13:25 ` Leif Lindholm 2016-10-17 14:15 ` Ard Biesheuvel 2016-10-27 5:21 ` Bhupesh Sharma 0 siblings, 2 replies; 6+ messages in thread From: Leif Lindholm @ 2016-10-17 13:25 UTC (permalink / raw) To: Bhupesh Sharma Cc: edk2-devel@ml01.01.org, Ard Biesheuvel, linaro-uefi@lists.linaro.org, evan.lloyd, ryan.harkin On Mon, Oct 17, 2016 at 10:18:01AM +0000, 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.sharma@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 <bhupesh.sharma@nxp.com> > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > --- > > .../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 > > --- 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&0xFFFF8000); > > 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)); I think these additions tip the code over to where a temporary variable should be used for the register value. > > > > 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 SubregionDisableMask > > ); This modifies a prototype of a function implemented in: ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9x4Sec.c ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibRTSM/RTSMSec.c ArmPlatformPkg/Library/ArmPlatformSecLibNull/ArmPlatformLibNullSec.c But only modifies the implementation in one of them. Arguably, the RTSM one could simply be deleted (probably), but if we're keeping this library, then the LibNullSec should at least be kept up to date. Ard: comments on deleting the RTSM one? Regards, Leif > > > > #endif > > -- > > 1.9.1 > > > > Regards, > Bhupesh > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH 1/1] ArmPlatformPkg/ArmTrustZone: Add support for specifying Subregions to be disabled 2016-10-17 13:25 ` Leif Lindholm @ 2016-10-17 14:15 ` Ard Biesheuvel 2016-10-27 5:23 ` Bhupesh Sharma 2016-10-27 5:21 ` Bhupesh Sharma 1 sibling, 1 reply; 6+ messages in thread From: Ard Biesheuvel @ 2016-10-17 14:15 UTC (permalink / raw) To: Leif Lindholm Cc: Bhupesh Sharma, edk2-devel@ml01.01.org, linaro-uefi@lists.linaro.org, Evan Lloyd, Ryan Harkin On 17 October 2016 at 14:25, Leif Lindholm <leif.lindholm@linaro.org> wrote: > On Mon, Oct 17, 2016 at 10:18:01AM +0000, 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.sharma@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 <bhupesh.sharma@nxp.com> >> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> > --- >> > .../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 >> > --- 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&0xFFFF8000); >> > 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)); > > I think these additions tip the code over to where a temporary > variable should be used for the register value. > >> > >> > 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 SubregionDisableMask >> > ); > > This modifies a prototype of a function implemented in: > ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9x4Sec.c > ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibRTSM/RTSMSec.c > ArmPlatformPkg/Library/ArmPlatformSecLibNull/ArmPlatformLibNullSec.c > > But only modifies the implementation in one of them. > Arguably, the RTSM one could simply be deleted (probably), but if > we're keeping this library, then the LibNullSec should at least be > kept up to date. > > Ard: comments on deleting the RTSM one? > Nope. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH 1/1] ArmPlatformPkg/ArmTrustZone: Add support for specifying Subregions to be disabled 2016-10-17 14:15 ` Ard Biesheuvel @ 2016-10-27 5:23 ` Bhupesh Sharma 0 siblings, 0 replies; 6+ messages in thread From: Bhupesh Sharma @ 2016-10-27 5:23 UTC (permalink / raw) To: Ard Biesheuvel, Leif Lindholm, Bhupesh Sharma Cc: edk2-devel@ml01.01.org, linaro-uefi@lists.linaro.org, Evan Lloyd, Ryan Harkin Hi Ard, > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > Sent: Monday, October 17, 2016 7:46 PM > > On 17 October 2016 at 14:25, Leif Lindholm <leif.lindholm@linaro.org> > wrote: > > On Mon, Oct 17, 2016 at 10:18:01AM +0000, 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.sharma@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 <bhupesh.sharma@nxp.com> > >> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> > >> > --- > >> > .../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); > >> > + 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&0xFFFF8000); > >> > 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)); > > > > I think these additions tip the code over to where a temporary > > variable should be used for the register value. > > > >> > > >> > 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 SubregionDisableMask > >> > ); > > > > This modifies a prototype of a function implemented in: > > > ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9x4Se > > c.c > > ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibRTSM/RTSMSec.c > > ArmPlatformPkg/Library/ArmPlatformSecLibNull/ArmPlatformLibNullSec.c > > > > But only modifies the implementation in one of them. > > Arguably, the RTSM one could simply be deleted (probably), but if > > we're keeping this library, then the LibNullSec should at least be > > kept up to date. > > > > Ard: comments on deleting the RTSM one? > > > > Nope. Ok, so should I send these changes for RTSM model also or should I update the LibNullSec. Please let me know. Thanks, Bhupesh ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH 1/1] ArmPlatformPkg/ArmTrustZone: Add support for specifying Subregions to be disabled 2016-10-17 13:25 ` Leif Lindholm 2016-10-17 14:15 ` Ard Biesheuvel @ 2016-10-27 5:21 ` Bhupesh Sharma 1 sibling, 0 replies; 6+ messages in thread From: Bhupesh Sharma @ 2016-10-27 5:21 UTC (permalink / raw) To: Leif Lindholm, Bhupesh Sharma Cc: edk2-devel@ml01.01.org, Ard Biesheuvel, linaro-uefi@lists.linaro.org, evan.lloyd@arm.com, ryan.harkin@linaro.org Hi Leif, Thanks for the review. Please see my replies in-line. > From: Leif Lindholm [mailto:leif.lindholm@linaro.org] > Sent: Monday, October 17, 2016 6:55 PM > > On Mon, Oct 17, 2016 at 10:18:01AM +0000, 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.sharma@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 <bhupesh.sharma@nxp.com> > > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > > --- > > > .../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, > > > + 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&0xFFFF8000); > > > 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)); > > I think these additions tip the code over to where a temporary variable > should be used for the register value. Ok. Will change this in V2. > > > > > > 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 SubregionDisableMask > > > ); > > This modifies a prototype of a function implemented in: > ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9x4Sec > .c > ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibRTSM/RTSMSec.c > ArmPlatformPkg/Library/ArmPlatformSecLibNull/ArmPlatformLibNullSec.c > > But only modifies the implementation in one of them. > Arguably, the RTSM one could simply be deleted (probably), but if we're > keeping this library, then the LibNullSec should at least be kept up to > date. > > Ard: comments on deleting the RTSM one? > > Regards, > > Leif > > > > > > > #endif > > > -- > > > 1.9.1 > > > > > > > Regards, > > Bhupesh > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.01.org > > https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-10-27 5:23 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-10-14 11:09 [RESEND PATCH 1/1] ArmPlatformPkg/ArmTrustZone: Add support for specifying Subregions to be disabled Bhupesh Sharma 2016-10-17 10:18 ` Bhupesh Sharma 2016-10-17 13:25 ` Leif Lindholm 2016-10-17 14:15 ` Ard Biesheuvel 2016-10-27 5:23 ` Bhupesh Sharma 2016-10-27 5:21 ` Bhupesh Sharma
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox