[U-Boot,RFC] omap: Invalidate first page to avoid speculation

Hello u-boot list,

Here is a "request for comments" on the best way to solve a little
"speculation" issue on recent OMAPs. Any guidance/feedback on the way to go
would be greatly appreciated, please.

I am using u-boot on an OMAP5 HS device (with security, that is), and I am
experiencing "security violations" due to speculative accesses done by the
Cortex-A15 processor to the region near address zero. This region is a secure
region, where non-secure accesses are forbidden and reported by the security
firmware on an OMAP HS device. On an OMAP GP device, those accesses may very
well exist, but are silently ignored by the firmware. Note that the speculative
accesses are not actual functional accesses, so their being aborted does not
harm the functionality of u-boot as it is.
  A quick (and dirty) solution is to mark the region near address zero as being
invalid, which prevents the processor from doing speculative accesses there
(see patch).
  This patch as it is has a number of issues: it impacts all ARM devices and it
unmaps too large a region. I am not sure how to cleanly rework the patch so
that it would be made OMAP-only cleanly. Also, unmapping a smaller region to
better fit the hardware characteristics would require using second level
descriptors, and I do not know if this is recommended. To make this worse,
chips in the OMAP family have differences in their secure rom boundaries.

Does the u-boot community feels this issue needs to be addressed? What would be
the best way to solve this?

Best regards,

V.


Signed-off-by: Vincent Stehlé <v-stehle@ti.com>
---
 arch/arm/lib/cache-cp15.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Hi Vincent,

On Fri, 16 Nov 2012 14:36:29 +0100, Vincent Stehlé <v-stehle@ti.com>
wrote:

> 
> Hello u-boot list,
> 
> Here is a "request for comments" on the best way to solve a little
> "speculation" issue on recent OMAPs. Any guidance/feedback on the way to go
> would be greatly appreciated, please.
> 
> I am using u-boot on an OMAP5 HS device (with security, that is), and I am
> experiencing "security violations" due to speculative accesses done by the
> Cortex-A15 processor to the region near address zero. This region is a secure
> region, where non-secure accesses are forbidden and reported by the security
> firmware on an OMAP HS device. On an OMAP GP device, those accesses may very
> well exist, but are silently ignored by the firmware. Note that the speculative
> accesses are not actual functional accesses, so their being aborted does not
> harm the functionality of u-boot as it is.
>   A quick (and dirty) solution is to mark the region near address zero as being
> invalid, which prevents the processor from doing speculative accesses there
> (see patch).
>   This patch as it is has a number of issues: it impacts all ARM devices and it
> unmaps too large a region. I am not sure how to cleanly rework the patch so
> that it would be made OMAP-only cleanly. Also, unmapping a smaller region to
> better fit the hardware characteristics would require using second level
> descriptors, and I do not know if this is recommended. To make this worse,
> chips in the OMAP family have differences in their secure rom boundaries.
> 
> Does the u-boot community feels this issue needs to be addressed? What would be
> the best way to solve this?
> 
> Best regards,
> 
> V.
> 
> 
> Signed-off-by: Vincent Stehlé <v-stehle@ti.com>
> ---
>  arch/arm/lib/cache-cp15.c |    7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/lib/cache-cp15.c b/arch/arm/lib/cache-cp15.c
> index 939de10..57e1974 100644
> --- a/arch/arm/lib/cache-cp15.c
> +++ b/arch/arm/lib/cache-cp15.c
> @@ -72,8 +72,13 @@ static inline void mmu_setup(void)
>  	u32 reg;
>  
>  	arm_init_before_mmu();
> +
> +	/* First page (starting at 0x0) is made invalid to avoid
> +	 * speculative accesses in secure rom. */
> +	page_table[0] = 0;
> +
>  	/* Set up an identity-mapping for all 4GB, rw for everyone */
> -	for (i = 0; i < 4096; i++)
> +	for (i = 1; i < 4096; i++)
>  		page_table[i] = i << 20 | (3 << 10) | 0x12;
>  
>  	for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {

To make this affect only some CPUs or even boards, you can define and
use a weak function which would handle filling the page-table; the weak,
default, function would fill table[0] like others, while OMAP5 would
have a strong version which would clear table[0].

Amicalement,

Albert Aribaud:
> To make this affect only some CPUs or even boards, you can define and use a
> weak function which would handle filling the page-table; the weak, default,
> function would fill table[0] like others, while OMAP5 would have a strong
> version which would clear table[0].

Hi Albert,

Thank you very much for your comments.

Here is "v2" of the patch, reworked to follow your advice. Comments are
welcome. I think it is now cleaner to split it in two patches:

  [PATCH 1/2] ARM: cache: introduce weak arm_setup_identity_mapping
  [PATCH 2/2] ARM: OMAP5: redefine arm_setup_identity_mapping

I picked the 'arm_setup_identity_mapping' name more or less arbitrarily, please
advise if not.

I verified this patch series on an OMAP5 HS device (with security) and on a GP
device (without security), and both work for me.

Best regards,

V.