From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-x22a.google.com (mail-wm0-x22a.google.com [IPv6:2a00:1450:400c:c09::22a]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id CF0D281DC2 for ; Mon, 16 Jan 2017 09:20:15 -0800 (PST) Received: by mail-wm0-x22a.google.com with SMTP id c85so167117280wmi.1 for ; Mon, 16 Jan 2017 09:20:16 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=fk+Puqb5dyLVoYOY1auK+l7rIqTnwXl3Izb+B2ClbLg=; b=Vpln891mkmnTgQmGBkE8yFn6v3G4aMKdkbNvbO8FnnVJrG3O6cktoGn6Kf9huza1Fc 4twrL/BGDE8OzeCYAxPi6yGKVWE4Hz3bN4GTRDzJ3sjdexspipiq0zuK8RCVnwc3QwMW ylFWniYIdfJ4d+KHv0WxptY6mAIfChV1qETuI= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=fk+Puqb5dyLVoYOY1auK+l7rIqTnwXl3Izb+B2ClbLg=; b=FQKermCWDJLgLLTwVjHN/X0ecv6x253stHn3nV+/HKUlNjpzHs8NIi3L/ZE77XbCO8 x4IFhH2urmT2v8ifLJlb5zNFO1H5NAERgDJfluJz9riOZ3mSY0v7uqTIv34fq2vG4owr 2+6M+iA7FU9DgZf8Oh15quRybP8QS4qys2fWrANubZlI8PQTSxfMVfAmM8qELGzQtuTf jB0n5xo1/mxL1HVDrTw70/RNQGjHb98E5fk4gpxVpPOPaBmY7BrScmZ2xe+U7Ox5JDe4 cRBcNj0sL05nDKz5iSNnJp7NHn4ola5/gvbrGke1C26OxCr7oOZSbB/fYYWvBinPL8sQ OQNw== X-Gm-Message-State: AIkVDXJo3Q6k3kSqg6Ovb10k0paSFEk7oEMN1NhFbhcSiJ6/MXrFT/FnILjwiDR/XjOlJWJJ X-Received: by 10.223.150.238 with SMTP id u101mr19468995wrb.175.1484587214808; Mon, 16 Jan 2017 09:20:14 -0800 (PST) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id m145sm30098437wma.3.2017.01.16.09.20.13 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 16 Jan 2017 09:20:14 -0800 (PST) Date: Mon, 16 Jan 2017 17:20:11 +0000 From: Leif Lindholm To: Bhupesh Sharma Cc: linaro-uefi@lists.linaro.org, edk2-devel@lists.01.org, Ard Biesheuvel Message-ID: <20170116172011.GM25883@bivouac.eciton.net> References: <1482786942-9600-1-git-send-email-bhupesh.linux@gmail.com> MIME-Version: 1.0 In-Reply-To: <1482786942-9600-1-git-send-email-bhupesh.linux@gmail.com> User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [PATCH V2 1/1] ArmPlatformPkg/TZASC: Allow specifying subregions to be disabled X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 16 Jan 2017 17:20:16 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 > Cc: Ard Biesheuvel > Signed-off-by: Bhupesh Sharma Please add Signed-off-by: Bhupesh Sharma 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 >