Re: [U-Boot] [PATCH v2 11/12] tegra124: Reserve secure RAM using MC_SECURITY_CFG{0, 1}_0

2015-02-17 Thread Jan Kiszka
On 2015-02-17 22:06, Stephen Warren wrote:
> On 02/16/2015 05:54 AM, Jan Kiszka wrote:
>> From: Ian Campbell 
>>
>> These registers can be used to prevent non-secure world from accessing a
>> megabyte aligned region of RAM, use them to protect the u-boot secure
>> monitor
>> code.
>>
>> At first I tried to do this from s_init(), however this inexplicably
>> causes
>> u-boot's networking (e.g. DHCP) to fail, while networking under Linux
>> was fine.
>>
>> So instead I have added a new weak arch function protect_secure_section()
>> called from relocate_secure_section() and reserved the region there.
>> This is
>> better overall since it defers the reservation until after the sec vs.
>> non-sec
>> decision (which can be influenced by an envvar) has been made when
>> booting the
>> os.
> 
>> diff --git a/arch/arm/cpu/tegra-common/ap.c
>> b/arch/arm/cpu/tegra-common/ap.c
> 
>> +void protect_secure_section(void)
> 
>> +writel(CONFIG_ARMV7_SECURE_BASE, &mc->mc_security_cfg0);
>> +writel(CONFIG_ARMV7_SECURE_RESERVE_SIZE>>20, &mc->mc_security_cfg1);
> 
> Spaces around the >> ?

Fixed for v3.

Thanks,
Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 11/12] tegra124: Reserve secure RAM using MC_SECURITY_CFG{0, 1}_0

2015-02-17 Thread Stephen Warren

On 02/16/2015 05:54 AM, Jan Kiszka wrote:

From: Ian Campbell 

These registers can be used to prevent non-secure world from accessing a
megabyte aligned region of RAM, use them to protect the u-boot secure monitor
code.

At first I tried to do this from s_init(), however this inexplicably causes
u-boot's networking (e.g. DHCP) to fail, while networking under Linux was fine.

So instead I have added a new weak arch function protect_secure_section()
called from relocate_secure_section() and reserved the region there. This is
better overall since it defers the reservation until after the sec vs. non-sec
decision (which can be influenced by an envvar) has been made when booting the
os.



diff --git a/arch/arm/cpu/tegra-common/ap.c b/arch/arm/cpu/tegra-common/ap.c



+void protect_secure_section(void)



+   writel(CONFIG_ARMV7_SECURE_BASE, &mc->mc_security_cfg0);
+   writel(CONFIG_ARMV7_SECURE_RESERVE_SIZE>>20, &mc->mc_security_cfg1);


Spaces around the >> ?
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 11/12] tegra124: Reserve secure RAM using MC_SECURITY_CFG{0, 1}_0

2015-02-16 Thread Jan Kiszka
On 2015-02-16 14:49, Mark Rutland wrote:
> On Mon, Feb 16, 2015 at 12:54:48PM +, Jan Kiszka wrote:
>> From: Ian Campbell 
>>
>> These registers can be used to prevent non-secure world from accessing a
>> megabyte aligned region of RAM, use them to protect the u-boot secure monitor
>> code.
> 
> What happens if the CPU tried to read this memory from the non-secure
> world? If the OS has it mapped then the CPU could perform speculative
> reads at any point in time.
> 
> If that can raise an abort then the OS needs to not map the region.
> 
> I take it U-Boot uses a secure mapping for the region (which I believe
> should avoid the mismatched attributes issue I mentioned in my other
> reply).

What I can contribute to this are kernel messages due to a
misconfiguration of our hypervisor Jailhouse (while Linux was still
trying to boot it):

[   61.896860] tegra-mc 70019000.memory-controller: mpcorew: write 
@0xfff00040: Security violation (TrustZone violation)
[   61.896888] tegra-mc 70019000.memory-controller: mpcorew: write 
@0xfff2d340: Security violation (TrustZone violation)

So it seems that Linux is receiving a violation report here when trying
to access the memory.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 11/12] tegra124: Reserve secure RAM using MC_SECURITY_CFG{0, 1}_0

2015-02-16 Thread Mark Rutland
On Mon, Feb 16, 2015 at 12:54:48PM +, Jan Kiszka wrote:
> From: Ian Campbell 
> 
> These registers can be used to prevent non-secure world from accessing a
> megabyte aligned region of RAM, use them to protect the u-boot secure monitor
> code.

What happens if the CPU tried to read this memory from the non-secure
world? If the OS has it mapped then the CPU could perform speculative
reads at any point in time.

If that can raise an abort then the OS needs to not map the region.

I take it U-Boot uses a secure mapping for the region (which I believe
should avoid the mismatched attributes issue I mentioned in my other
reply).

Thanks,
Mark.

> At first I tried to do this from s_init(), however this inexplicably causes
> u-boot's networking (e.g. DHCP) to fail, while networking under Linux was 
> fine.
> 
> So instead I have added a new weak arch function protect_secure_section()
> called from relocate_secure_section() and reserved the region there. This is
> better overall since it defers the reservation until after the sec vs. non-sec
> decision (which can be influenced by an envvar) has been made when booting the
> os.
> 
> Signed-off-by: Ian Campbell 
> Signed-off-by: Jan Kiszka 
> ---
>  arch/arm/cpu/armv7/virt-v7.c   |  5 +
>  arch/arm/cpu/tegra-common/ap.c | 15 +++
>  arch/arm/include/asm/system.h  |  1 +
>  3 files changed, 21 insertions(+)
> 
> diff --git a/arch/arm/cpu/armv7/virt-v7.c b/arch/arm/cpu/armv7/virt-v7.c
> index b69fd37..eb6195c 100644
> --- a/arch/arm/cpu/armv7/virt-v7.c
> +++ b/arch/arm/cpu/armv7/virt-v7.c
> @@ -46,6 +46,10 @@ static unsigned long get_gicd_base_address(void)
>  #endif
>  }
>  
> +/* Define a specific version of this function to enable any available
> + * hardware protections for the reserved region */
> +void __weak protect_secure_section(void) {}
> +
>  static void relocate_secure_section(void)
>  {
>  #ifdef CONFIG_ARMV7_SECURE_BASE
> @@ -54,6 +58,7 @@ static void relocate_secure_section(void)
>   memcpy((void *)CONFIG_ARMV7_SECURE_BASE, __secure_start, sz);
>   flush_dcache_range(CONFIG_ARMV7_SECURE_BASE,
>  CONFIG_ARMV7_SECURE_BASE + sz + 1);
> + protect_secure_section();
>   invalidate_icache_all();
>  #endif
>  }
> diff --git a/arch/arm/cpu/tegra-common/ap.c b/arch/arm/cpu/tegra-common/ap.c
> index a17dfd1..f1d3070 100644
> --- a/arch/arm/cpu/tegra-common/ap.c
> +++ b/arch/arm/cpu/tegra-common/ap.c
> @@ -10,6 +10,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -154,6 +155,20 @@ static void init_pmc_scratch(void)
>   writel(odmdata, &pmc->pmc_scratch20);
>  }
>  
> +#ifdef CONFIG_ARMV7_SECURE_RESERVE_SIZE
> +void protect_secure_section(void)
> +{
> + struct mc_ctlr *mc = (struct mc_ctlr *)NV_PA_MC_BASE;
> +
> + /* Must be MB aligned */
> + BUILD_BUG_ON(CONFIG_ARMV7_SECURE_BASE & 0xF);
> + BUILD_BUG_ON(CONFIG_ARMV7_SECURE_RESERVE_SIZE & 0xF);
> +
> + writel(CONFIG_ARMV7_SECURE_BASE, &mc->mc_security_cfg0);
> + writel(CONFIG_ARMV7_SECURE_RESERVE_SIZE>>20, &mc->mc_security_cfg1);
> +}
> +#endif
> +
>  void s_init(void)
>  {
>   /* Init PMC scratch memory */
> diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
> index 89f2294..21be69d 100644
> --- a/arch/arm/include/asm/system.h
> +++ b/arch/arm/include/asm/system.h
> @@ -76,6 +76,7 @@ void armv8_switch_to_el1(void);
>  void gic_init(void);
>  void gic_send_sgi(unsigned long sgino);
>  void wait_for_wakeup(void);
> +void protect_secure_region(void);
>  void smp_kick_all_cpus(void);
>  
>  void flush_l3_cache(void);
> -- 
> 2.1.4
> 
> 
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH v2 11/12] tegra124: Reserve secure RAM using MC_SECURITY_CFG{0, 1}_0

2015-02-16 Thread Jan Kiszka
From: Ian Campbell 

These registers can be used to prevent non-secure world from accessing a
megabyte aligned region of RAM, use them to protect the u-boot secure monitor
code.

At first I tried to do this from s_init(), however this inexplicably causes
u-boot's networking (e.g. DHCP) to fail, while networking under Linux was fine.

So instead I have added a new weak arch function protect_secure_section()
called from relocate_secure_section() and reserved the region there. This is
better overall since it defers the reservation until after the sec vs. non-sec
decision (which can be influenced by an envvar) has been made when booting the
os.

Signed-off-by: Ian Campbell 
Signed-off-by: Jan Kiszka 
---
 arch/arm/cpu/armv7/virt-v7.c   |  5 +
 arch/arm/cpu/tegra-common/ap.c | 15 +++
 arch/arm/include/asm/system.h  |  1 +
 3 files changed, 21 insertions(+)

diff --git a/arch/arm/cpu/armv7/virt-v7.c b/arch/arm/cpu/armv7/virt-v7.c
index b69fd37..eb6195c 100644
--- a/arch/arm/cpu/armv7/virt-v7.c
+++ b/arch/arm/cpu/armv7/virt-v7.c
@@ -46,6 +46,10 @@ static unsigned long get_gicd_base_address(void)
 #endif
 }
 
+/* Define a specific version of this function to enable any available
+ * hardware protections for the reserved region */
+void __weak protect_secure_section(void) {}
+
 static void relocate_secure_section(void)
 {
 #ifdef CONFIG_ARMV7_SECURE_BASE
@@ -54,6 +58,7 @@ static void relocate_secure_section(void)
memcpy((void *)CONFIG_ARMV7_SECURE_BASE, __secure_start, sz);
flush_dcache_range(CONFIG_ARMV7_SECURE_BASE,
   CONFIG_ARMV7_SECURE_BASE + sz + 1);
+   protect_secure_section();
invalidate_icache_all();
 #endif
 }
diff --git a/arch/arm/cpu/tegra-common/ap.c b/arch/arm/cpu/tegra-common/ap.c
index a17dfd1..f1d3070 100644
--- a/arch/arm/cpu/tegra-common/ap.c
+++ b/arch/arm/cpu/tegra-common/ap.c
@@ -10,6 +10,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -154,6 +155,20 @@ static void init_pmc_scratch(void)
writel(odmdata, &pmc->pmc_scratch20);
 }
 
+#ifdef CONFIG_ARMV7_SECURE_RESERVE_SIZE
+void protect_secure_section(void)
+{
+   struct mc_ctlr *mc = (struct mc_ctlr *)NV_PA_MC_BASE;
+
+   /* Must be MB aligned */
+   BUILD_BUG_ON(CONFIG_ARMV7_SECURE_BASE & 0xF);
+   BUILD_BUG_ON(CONFIG_ARMV7_SECURE_RESERVE_SIZE & 0xF);
+
+   writel(CONFIG_ARMV7_SECURE_BASE, &mc->mc_security_cfg0);
+   writel(CONFIG_ARMV7_SECURE_RESERVE_SIZE>>20, &mc->mc_security_cfg1);
+}
+#endif
+
 void s_init(void)
 {
/* Init PMC scratch memory */
diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
index 89f2294..21be69d 100644
--- a/arch/arm/include/asm/system.h
+++ b/arch/arm/include/asm/system.h
@@ -76,6 +76,7 @@ void armv8_switch_to_el1(void);
 void gic_init(void);
 void gic_send_sgi(unsigned long sgino);
 void wait_for_wakeup(void);
+void protect_secure_region(void);
 void smp_kick_all_cpus(void);
 
 void flush_l3_cache(void);
-- 
2.1.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot