Tzc update #1994
Tzc update #1994
Conversation
MrVan
commented
Dec 8, 2017
MrVan
commented
- Do not touch reserved bits for region 0.
- Export more API for flexible configuration.
core/drivers/tzc380.c
Outdated
| @@ -117,8 +117,13 @@ void tzc_configure_region(uint8_t region, vaddr_t region_base, uint32_t attr) | |||
|
|
|||
| assert(region < tzc.num_regions); | |||
|
|
|||
| tzc_write_region_base_low(tzc.base, region, addr_low(region_base)); | |||
| tzc_write_region_base_high(tzc.base, region, addr_high(region_base)); | |||
| if (!region) { | |||
There was a problem hiding this comment.
It wouldn't hurt with a comment here with something about region0
core/drivers/tzc380.c
Outdated
| write32(1, base + SECURITY_INV_EN_OFF); | ||
| } | ||
|
|
||
| void tzc_enable_region(uint8_t region) |
There was a problem hiding this comment.
tzc_region_enable() would be more consistent with for instance tzc_security_inversion_en() and tzc_int_clear()
There was a problem hiding this comment.
It wouldn't hurt with a comment with a short description of some of the more important functions
|
Updated with more comments added. And tzc_enable_region -> tzc_region_enable |
|
|
core/drivers/tzc380.c
Outdated
| @@ -59,6 +61,11 @@ static void tzc_write_action(vaddr_t base, enum tzc_action action) | |||
| write32(action, base + ACTION_OFF); | |||
| } | |||
|
|
|||
| static uint32_t tzc_read_region_attributes(vaddr_t base, uint32_t region) | |||
There was a problem hiding this comment.
minor: maybe move right above tzc_write_region_attributes
core/drivers/tzc380.c
Outdated
| EMSG("Fail address Low %x\n", read32(base + 0x20)); | ||
| EMSG("Fail address High %x\n", read32(base + 0x24)); | ||
| EMSG("Fail Control %x\n", read32(base + 0x28)); | ||
| EMSG("Fail Id %x\n", read32(base + 0x2C)); |
There was a problem hiding this comment.
Could use define macros for the offsets?
#define TZC_FAIL_ADDRESS_LOW 0x20
#define TZC_FAIL_ADDRESS_HIGH 0x24
#define TZC_FAIL_CONTROL 0x28
#define TZC_FAIL_ID 0x2C
core/drivers/tzc380.c
Outdated
| * For region 0, this high/low/size/en field is Read Only (RO). | ||
| * So should not configure those field for region 0. | ||
| */ | ||
| if (!region) { |
|
Updated. Use macros for tzc_fail_dump. "s/!region/region/", in my local, I use "region != 0", my bad that not use up to date patch, in last pull request. |
|
@etienne-lms ping |
core/drivers/tzc380.c
Outdated
| vaddr_t base __maybe_unused = core_mmu_get_va(tzc.base, | ||
| MEM_AREA_IO_SEC); | ||
|
|
||
| EMSG("Fail address Low %x\n", read32(base + FAIL_ADDRESS_LOW_OFF)); |
There was a problem hiding this comment.
Replace %x with %" PRIx32 ", here and below.
|
Updated with "Replace %x with 0x%" PRIx32 ", 2nd commit log refined. |
|
Thanks @MrVan . I think we can consider @etienne-lms's "looks ok to me" as an |
For region0, only SP is configurable, so should not configure region low/high. Signed-off-by: Peng Fan <peng.fan@nxp.com> Acked-by: Jens Wiklander <jens.wiklander@linaro.org> Acked-by: Etienne Carriere <etienne.carriere@linaro.org>
Introduce tzc_security_inversion_en tzc_enable_region tzc_fail_dump and tzc_int_clear. When we want to block secure access to region configured TZC_ATTR_SP_NS_RW, need to use tzc_security_inversion_en. Sometimes we need to configure the regions first, then enable the region. tzasc380 interrupt could be enabled to catch some illegal access with tzc_fail_dump and tzc_int_clear. Signed-off-by: Peng Fan <peng.fan@nxp.com> Acked-by: Jens Wiklander <jens.wiklander@linaro.org> Acked-by: Etienne Carriere <etienne.carriere@linaro.org>
|
@jforissier Updated. Thanks. |
