| Message ID | 1472794824-6032-4-git-send-email-d-allred@ti.com |
|---|---|
| State | Accepted |
| Commit | 501f0ef304549edf41bbcec0e6da008d92cbdf0c |
| Delegated to: | Tom Rini |
| Headers | show |
On Fri, Sep 02, 2016 at 12:40:22AM -0500, Daniel Allred wrote: > After EMIF DRAM is configured, but before it is used, > calls are made on secure devices to reserve any configured > memory region needed by the secure world and then to lock the > EMIF firewall configuration. If any other firewall > configuration needs to be applied, it must happen before the > lock call. > > Signed-off-by: Daniel Allred <d-allred@ti.com> > --- > arch/arm/cpu/armv7/omap-common/emif-common.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/arch/arm/cpu/armv7/omap-common/emif-common.c b/arch/arm/cpu/armv7/omap-common/emif-common.c > index 2b79010..b26984e 100644 > --- a/arch/arm/cpu/armv7/omap-common/emif-common.c > +++ b/arch/arm/cpu/armv7/omap-common/emif-common.c > @@ -14,6 +14,7 @@ > #include <asm/arch/clock.h> > #include <asm/arch/sys_proto.h> > #include <asm/omap_common.h> > +#include <asm/omap_sec_common.h> > #include <asm/utils.h> > #include <linux/compiler.h> > > @@ -1477,6 +1478,20 @@ void sdram_init(void) > debug("get_ram_size() successful"); > } > > +#if defined(CONFIG_TI_SECURE_DEVICE) > + /* > + * On HS devices, do static EMIF firewall configuration > + * but only do it if not already running in SDRAM > + */ > + if (!in_sdram) > + if (0 != secure_emif_reserve()) > + hang(); > + > + /* On HS devices, ensure static EMIF firewall APIs are locked */ > + if (0 != secure_emif_firewall_lock()) > + hang(); Those are awkward tests (should be func() != val), and since it's just checking for function return status, we should just write that normally. Thanks!
On 09/02/2016 12:40 AM, Daniel Allred wrote: > After EMIF DRAM is configured, but before it is used, > calls are made on secure devices to reserve any configured > memory region needed by the secure world and then to lock the > EMIF firewall configuration. If any other firewall > configuration needs to be applied, it must happen before the > lock call. > > Signed-off-by: Daniel Allred <d-allred@ti.com> > --- > arch/arm/cpu/armv7/omap-common/emif-common.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/arch/arm/cpu/armv7/omap-common/emif-common.c b/arch/arm/cpu/armv7/omap-common/emif-common.c > index 2b79010..b26984e 100644 > --- a/arch/arm/cpu/armv7/omap-common/emif-common.c > +++ b/arch/arm/cpu/armv7/omap-common/emif-common.c > @@ -14,6 +14,7 @@ > #include <asm/arch/clock.h> > #include <asm/arch/sys_proto.h> > #include <asm/omap_common.h> > +#include <asm/omap_sec_common.h> > #include <asm/utils.h> > #include <linux/compiler.h> > > @@ -1477,6 +1478,20 @@ void sdram_init(void) > debug("get_ram_size() successful"); > } > > +#if defined(CONFIG_TI_SECURE_DEVICE) > + /* > + * On HS devices, do static EMIF firewall configuration > + * but only do it if not already running in SDRAM > + */ > + if (!in_sdram) Should we hang in this case? > + if (0 != secure_emif_reserve()) "0 != " could be removed here and below. > + hang(); > + > + /* On HS devices, ensure static EMIF firewall APIs are locked */ > + if (0 != secure_emif_firewall_lock()) > + hang(); > +#endif > + > if (sdram_type == EMIF_SDRAM_TYPE_DDR3 && > (!in_sdram && !warm_reset()) && (!is_dra7xx())) { > if (emif1_enabled) >
On 9/2/2016 9:54 AM, Tom Rini wrote: > On Fri, Sep 02, 2016 at 12:40:22AM -0500, Daniel Allred wrote: >> After EMIF DRAM is configured, but before it is used, >> calls are made on secure devices to reserve any configured >> memory region needed by the secure world and then to lock the >> EMIF firewall configuration. If any other firewall >> configuration needs to be applied, it must happen before the >> lock call. >> >> Signed-off-by: Daniel Allred <d-allred@ti.com> >> --- >> arch/arm/cpu/armv7/omap-common/emif-common.c | 15 +++++++++++++++ >> 1 file changed, 15 insertions(+) >> >> diff --git a/arch/arm/cpu/armv7/omap-common/emif-common.c b/arch/arm/cpu/armv7/omap-common/emif-common.c >> index 2b79010..b26984e 100644 >> --- a/arch/arm/cpu/armv7/omap-common/emif-common.c >> +++ b/arch/arm/cpu/armv7/omap-common/emif-common.c >> @@ -14,6 +14,7 @@ >> #include <asm/arch/clock.h> >> #include <asm/arch/sys_proto.h> >> #include <asm/omap_common.h> >> +#include <asm/omap_sec_common.h> >> #include <asm/utils.h> >> #include <linux/compiler.h> >> >> @@ -1477,6 +1478,20 @@ void sdram_init(void) >> debug("get_ram_size() successful"); >> } >> >> +#if defined(CONFIG_TI_SECURE_DEVICE) >> + /* >> + * On HS devices, do static EMIF firewall configuration >> + * but only do it if not already running in SDRAM >> + */ >> + if (!in_sdram) >> + if (0 != secure_emif_reserve()) >> + hang(); >> + >> + /* On HS devices, ensure static EMIF firewall APIs are locked */ >> + if (0 != secure_emif_firewall_lock()) >> + hang(); > > Those are awkward tests (should be func() != val), and since it's just > checking for function return status, we should just write that normally. > Thanks! > Fair point, will change.
On 9/6/2016 3:41 PM, Andrew F. Davis wrote: > On 09/02/2016 12:40 AM, Daniel Allred wrote: >> After EMIF DRAM is configured, but before it is used, >> calls are made on secure devices to reserve any configured >> memory region needed by the secure world and then to lock the >> EMIF firewall configuration. If any other firewall >> configuration needs to be applied, it must happen before the >> lock call. >> >> Signed-off-by: Daniel Allred <d-allred@ti.com> >> --- >> arch/arm/cpu/armv7/omap-common/emif-common.c | 15 +++++++++++++++ >> 1 file changed, 15 insertions(+) >> >> diff --git a/arch/arm/cpu/armv7/omap-common/emif-common.c b/arch/arm/cpu/armv7/omap-common/emif-common.c >> index 2b79010..b26984e 100644 >> --- a/arch/arm/cpu/armv7/omap-common/emif-common.c >> +++ b/arch/arm/cpu/armv7/omap-common/emif-common.c >> @@ -14,6 +14,7 @@ >> #include <asm/arch/clock.h> >> #include <asm/arch/sys_proto.h> >> #include <asm/omap_common.h> >> +#include <asm/omap_sec_common.h> >> #include <asm/utils.h> >> #include <linux/compiler.h> >> >> @@ -1477,6 +1478,20 @@ void sdram_init(void) >> debug("get_ram_size() successful"); >> } >> >> +#if defined(CONFIG_TI_SECURE_DEVICE) >> + /* >> + * On HS devices, do static EMIF firewall configuration >> + * but only do it if not already running in SDRAM >> + */ >> + if (!in_sdram) > > Should we hang in this case? The check is supposed to ensure that if we are already in DRAM, we don't apply the firewall config. It might make sense to still (attempt to) do the lock. I need to think a little more on it, but I don't think hanging if in sdram is the right thing. > >> + if (0 != secure_emif_reserve()) > > "0 != " could be removed here and below. > >> + hang(); >> + >> + /* On HS devices, ensure static EMIF firewall APIs are locked */ >> + if (0 != secure_emif_firewall_lock()) >> + hang(); >> +#endif >> + >> if (sdram_type == EMIF_SDRAM_TYPE_DDR3 && >> (!in_sdram && !warm_reset()) && (!is_dra7xx())) { >> if (emif1_enabled) >>
On Fri, Sep 02, 2016 at 12:40:22AM -0500, Daniel Allred wrote: > After EMIF DRAM is configured, but before it is used, > calls are made on secure devices to reserve any configured > memory region needed by the secure world and then to lock the > EMIF firewall configuration. If any other firewall > configuration needs to be applied, it must happen before the > lock call. > > Signed-off-by: Daniel Allred <d-allred@ti.com> Applied to u-boot/master, thanks!
diff --git a/arch/arm/cpu/armv7/omap-common/emif-common.c b/arch/arm/cpu/armv7/omap-common/emif-common.c index 2b79010..b26984e 100644 --- a/arch/arm/cpu/armv7/omap-common/emif-common.c +++ b/arch/arm/cpu/armv7/omap-common/emif-common.c @@ -14,6 +14,7 @@ #include <asm/arch/clock.h> #include <asm/arch/sys_proto.h> #include <asm/omap_common.h> +#include <asm/omap_sec_common.h> #include <asm/utils.h> #include <linux/compiler.h> @@ -1477,6 +1478,20 @@ void sdram_init(void) debug("get_ram_size() successful"); } +#if defined(CONFIG_TI_SECURE_DEVICE) + /* + * On HS devices, do static EMIF firewall configuration + * but only do it if not already running in SDRAM + */ + if (!in_sdram) + if (0 != secure_emif_reserve()) + hang(); + + /* On HS devices, ensure static EMIF firewall APIs are locked */ + if (0 != secure_emif_firewall_lock()) + hang(); +#endif + if (sdram_type == EMIF_SDRAM_TYPE_DDR3 && (!in_sdram && !warm_reset()) && (!is_dra7xx())) { if (emif1_enabled)
After EMIF DRAM is configured, but before it is used, calls are made on secure devices to reserve any configured memory region needed by the secure world and then to lock the EMIF firewall configuration. If any other firewall configuration needs to be applied, it must happen before the lock call. Signed-off-by: Daniel Allred <d-allred@ti.com> --- arch/arm/cpu/armv7/omap-common/emif-common.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)