[PATCH v2 0/5] Initial support of Trusted Foundations on Tegra30

[PATCH v2 0/5] Initial support of Trusted Foundations on Tegra30

[Dmitry Osipenko]

Hello,

This series of patches brings initial support of Trusted Foundations to
Tegra30, that is to the consumer-grade Tegra30 devices which do not allow
to easily replace the proprietary bootloader. Support is initial because
this series implements only a proper CPU boot-up (main + secondary cores)
and a basic L2 cache maintenance that is done using the TF firmware.
Suspend-resume support is missing yet as I couldn't get it to work
(CPU hangs on resume from suspend after awhile and seems that is related
to inappropriately done cache maintenance during of suspend-resume using
the firmware), it is work-in-progress for now.

This patchset is partially based on the work done by Michał Mirosław [0].

[0] https://www.spinics.net/lists/linux-tegra/msg30368.html

Changelog:

v2:
	- The "Don't apply CPU erratas in insecure mode" patch got some
	  cleanup, in particular resolved the messiness in
	  __tegra_cpu_reset_handler_data.

	- Added a comment to tf_cache_write_sec(), justifying the warning
	  message.

Dmitry Osipenko (5):
  ARM: trusted_foundations: Implement L2 cache initialization callback
  ARM: trusted_foundations: Provide information about whether firmware
    is registered
  ARM: tegra: Setup L2 cache using Trusted Foundations firmware
  ARM: tegra: Don't apply CPU erratas in insecure mode
  ARM: tegra: Always boot CPU in ARM-mode

 arch/arm/firmware/trusted_foundations.c    | 32 ++++++++++++++++++++++
 arch/arm/include/asm/trusted_foundations.h |  7 +++++
 arch/arm/mach-tegra/reset-handler.S        | 25 +++++++++--------
 arch/arm/mach-tegra/reset.c                |  5 +++-
 arch/arm/mach-tegra/reset.h                |  9 ++++--
 arch/arm/mach-tegra/sleep-tegra20.S        |  4 +++
 arch/arm/mach-tegra/tegra.c                | 15 ++++++++++
 7 files changed, 82 insertions(+), 15 deletions(-)

-- 
2.17.1

[PATCH v2 1/5] ARM: trusted_foundations: Implement L2 cache initialization callback

[Dmitry Osipenko]

Implement L2 cache initialization firmware callback that should be invoked
early in boot to enable cache HW.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 arch/arm/firmware/trusted_foundations.c | 27 +++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/arch/arm/firmware/trusted_foundations.c b/arch/arm/firmware/trusted_foundations.c
index 3fb1b5a1dce9..30df6547020f 100644
--- a/arch/arm/firmware/trusted_foundations.c
+++ b/arch/arm/firmware/trusted_foundations.c
@@ -18,8 +18,13 @@
 #include <linux/init.h>
 #include <linux/of.h>
 #include <asm/firmware.h>
+#include <asm/outercache.h>
 #include <asm/trusted_foundations.h>
 
+#define TF_CACHE_MAINT		0xfffff100
+
+#define TF_CACHE_INIT		1
+
 #define TF_SET_CPU_BOOT_ADDR_SMC 0xfffff200
 
 #define TF_CPU_PM		0xfffffffc
@@ -63,9 +68,31 @@ static int tf_prepare_idle(void)
 	return 0;
 }
 
+#ifdef CONFIG_CACHE_L2X0
+static void tf_cache_write_sec(unsigned long val, unsigned int reg)
+{
+	/*
+	 * The L2X0 cache driver shouldn't invoke a write to a secure registers,
+	 * though it's better to reinsure by printing a warning message.
+	 */
+	pr_warn("%s: Ignoring write [0x%x]: 0x%08lx\n", __func__, reg, val);
+}
+
+static int tf_init_cache(void)
+{
+	outer_cache.write_sec = tf_cache_write_sec;
+	tf_generic_smc(TF_CACHE_MAINT, TF_CACHE_INIT, 0);
+
+	return 0;
+}
+#endif /* CONFIG_CACHE_L2X0 */
+
 static const struct firmware_ops trusted_foundations_ops = {
 	.set_cpu_boot_addr = tf_set_cpu_boot_addr,
 	.prepare_idle = tf_prepare_idle,
+#ifdef CONFIG_CACHE_L2X0
+	.l2x0_init = tf_init_cache,
+#endif
 };
 
 void register_trusted_foundations(struct trusted_foundations_platform_data *pd)
-- 
2.17.1

Re: [PATCH v2 1/5] ARM: trusted_foundations: Implement L2 cache initialization callback

[Dmitry Osipenko]

On Tuesday, 19 June 2018 14:00:23 MSK Dmitry Osipenko wrote:
> Implement L2 cache initialization firmware callback that should be invoked
> early in boot to enable cache HW.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  arch/arm/firmware/trusted_foundations.c | 27 +++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)

Russell, could you please take a look at the arm/firmware/ patches and ACK 
them if they are good to you?

[PATCH v2 2/5] ARM: trusted_foundations: Provide information about whether firmware is registered

[Dmitry Osipenko]

Add a helper that provides information about whether Trusted Foundations
firmware operations have been registered.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 arch/arm/firmware/trusted_foundations.c    | 5 +++++
 arch/arm/include/asm/trusted_foundations.h | 7 +++++++
 2 files changed, 12 insertions(+)

diff --git a/arch/arm/firmware/trusted_foundations.c b/arch/arm/firmware/trusted_foundations.c
index 30df6547020f..f6eeb67de217 100644
--- a/arch/arm/firmware/trusted_foundations.c
+++ b/arch/arm/firmware/trusted_foundations.c
@@ -124,3 +124,8 @@ void of_register_trusted_foundations(void)
 		panic("Trusted Foundation: missing version-minor property\n");
 	register_trusted_foundations(&pdata);
 }
+
+bool trusted_foundations_registered(void)
+{
+	return firmware_ops == &trusted_foundations_ops;
+}
diff --git a/arch/arm/include/asm/trusted_foundations.h b/arch/arm/include/asm/trusted_foundations.h
index 00748350cf72..bfd0d780824b 100644
--- a/arch/arm/include/asm/trusted_foundations.h
+++ b/arch/arm/include/asm/trusted_foundations.h
@@ -31,6 +31,7 @@
 #include <linux/of.h>
 #include <linux/cpu.h>
 #include <linux/smp.h>
+#include <linux/types.h>
 
 struct trusted_foundations_platform_data {
 	unsigned int version_major;
@@ -41,6 +42,7 @@ struct trusted_foundations_platform_data {
 
 void register_trusted_foundations(struct trusted_foundations_platform_data *pd);
 void of_register_trusted_foundations(void);
+bool trusted_foundations_registered(void);
 
 #else /* CONFIG_TRUSTED_FOUNDATIONS */
 
@@ -68,6 +70,11 @@ static inline void of_register_trusted_foundations(void)
 	if (of_find_compatible_node(NULL, NULL, "tlm,trusted-foundations"))
 		register_trusted_foundations(NULL);
 }
+
+static inline bool trusted_foundations_registered(void)
+{
+	return false;
+}
 #endif /* CONFIG_TRUSTED_FOUNDATIONS */
 
 #endif
-- 
2.17.1

[PATCH v2 3/5] ARM: tegra: Setup L2 cache using Trusted Foundations firmware

[Dmitry Osipenko]

On Tegra20/30 L2 cache must be initialized using firmware call if CPU
is running in insecure mode. Initialize L2 cache and setup the outer-cache
callbacks in early boot using the firmware API.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 arch/arm/mach-tegra/tegra.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c
index f9587be48235..590b1cf1a8c4 100644
--- a/arch/arm/mach-tegra/tegra.c
+++ b/arch/arm/mach-tegra/tegra.c
@@ -38,6 +38,7 @@
 #include <soc/tegra/fuse.h>
 #include <soc/tegra/pmc.h>
 
+#include <asm/firmware.h>
 #include <asm/hardware/cache-l2x0.h>
 #include <asm/mach/arch.h>
 #include <asm/mach/time.h>
@@ -70,9 +71,23 @@ u32 tegra_uart_config[3] = {
 	0,
 };
 
+static void __init tegra_trusted_foundations_l2x0_cache_init(void)
+{
+	if (IS_ENABLED(CONFIG_CACHE_L2X0) &&
+	    IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC) &&
+	    of_machine_is_compatible("nvidia,tegra20"))
+		call_firmware_op(l2x0_init);
+
+	if (IS_ENABLED(CONFIG_CACHE_L2X0) &&
+	    IS_ENABLED(CONFIG_ARCH_TEGRA_3x_SOC) &&
+	    of_machine_is_compatible("nvidia,tegra30"))
+		call_firmware_op(l2x0_init);
+}
+
 static void __init tegra_init_early(void)
 {
 	of_register_trusted_foundations();
+	tegra_trusted_foundations_l2x0_cache_init();
 	tegra_cpu_reset_handler_init();
 }
 
-- 
2.17.1

[PATCH v2 4/5] ARM: tegra: Don't apply CPU erratas in insecure mode

[Dmitry Osipenko]

CPU isn't allowed to touch secure registers while running under secure
monitor. Hence skip applying CPU erratas in the reset handler if Trusted
Foundations firmware presents.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 arch/arm/mach-tegra/reset-handler.S | 24 ++++++++++++------------
 arch/arm/mach-tegra/reset.c         |  3 +++
 arch/arm/mach-tegra/reset.h         |  9 +++++++--
 arch/arm/mach-tegra/sleep-tegra20.S |  4 ++++
 4 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/arch/arm/mach-tegra/reset-handler.S b/arch/arm/mach-tegra/reset-handler.S
index 805f306fa6f7..e9f2a7775998 100644
--- a/arch/arm/mach-tegra/reset-handler.S
+++ b/arch/arm/mach-tegra/reset-handler.S
@@ -29,8 +29,6 @@
 
 #define PMC_SCRATCH41	0x140
 
-#define RESET_DATA(x)	((TEGRA_RESET_##x)*4)
-
 #ifdef CONFIG_PM_SLEEP
 /*
  *	tegra_resume
@@ -121,6 +119,12 @@ ENTRY(__tegra_cpu_reset_handler)
 	cpsid	aif, 0x13			@ SVC mode, interrupts disabled
 
 	tegra_get_soc_id TEGRA_APB_MISC_BASE, r6
+
+	adr	r12, __tegra_cpu_reset_handler_data
+	ldr	r0, [r12, #RESET_DATA(TF_PRESENT)]
+	cmp	r0, #0
+	bne	after_errata
+
 #ifdef CONFIG_ARCH_TEGRA_2x_SOC
 t20_check:
 	cmp	r6, #TEGRA20
@@ -155,7 +159,6 @@ after_errata:
 	and	r10, r10, #0x3			@ R10 = CPU number
 	mov	r11, #1
 	mov	r11, r11, lsl r10  		@ R11 = CPU mask
-	adr	r12, __tegra_cpu_reset_handler_data
 
 #ifdef CONFIG_SMP
 	/* Does the OS know about this CPU? */
@@ -169,10 +172,9 @@ after_errata:
 	cmp	r6, #TEGRA20
 	bne	1f
 	/* If not CPU0, don't let CPU0 reset CPU1 now that CPU1 is coming up. */
-	mov32	r5, TEGRA_IRAM_BASE + TEGRA_IRAM_RESET_HANDLER_OFFSET
 	mov	r0, #CPU_NOT_RESETTABLE
 	cmp	r10, #0
-	strneb	r0, [r5, #__tegra20_cpu1_resettable_status_offset]
+	strneb	r0, [r12, #RESET_DATA(RESETTABLE_STATUS)]
 1:
 #endif
 
@@ -277,14 +279,12 @@ ENDPROC(__tegra_cpu_reset_handler)
 	.align L1_CACHE_SHIFT
 	.type	__tegra_cpu_reset_handler_data, %object
 	.globl	__tegra_cpu_reset_handler_data
+	.globl	__tegra_cpu_reset_handler_data_offset
+	.equ	__tegra_cpu_reset_handler_data_offset, \
+					. - __tegra_cpu_reset_handler_start
 __tegra_cpu_reset_handler_data:
-	.rept	TEGRA_RESET_DATA_SIZE
-	.long	0
+	.rept   TEGRA_RESET_DATA_SIZE
+	.long   0
 	.endr
-	.globl	__tegra20_cpu1_resettable_status_offset
-	.equ	__tegra20_cpu1_resettable_status_offset, \
-					. - __tegra_cpu_reset_handler_start
-	.byte	0
 	.align L1_CACHE_SHIFT
-
 ENTRY(__tegra_cpu_reset_handler_end)
diff --git a/arch/arm/mach-tegra/reset.c b/arch/arm/mach-tegra/reset.c
index dc558892753c..b02ae7699842 100644
--- a/arch/arm/mach-tegra/reset.c
+++ b/arch/arm/mach-tegra/reset.c
@@ -24,6 +24,7 @@
 #include <asm/cacheflush.h>
 #include <asm/firmware.h>
 #include <asm/hardware/cache-l2x0.h>
+#include <asm/trusted_foundations.h>
 
 #include "iomap.h"
 #include "irammap.h"
@@ -89,6 +90,8 @@ static void __init tegra_cpu_reset_handler_enable(void)
 
 void __init tegra_cpu_reset_handler_init(void)
 {
+	__tegra_cpu_reset_handler_data[TEGRA_RESET_TF_PRESENT] =
+		trusted_foundations_registered();
 
 #ifdef CONFIG_SMP
 	__tegra_cpu_reset_handler_data[TEGRA_RESET_MASK_PRESENT] =
diff --git a/arch/arm/mach-tegra/reset.h b/arch/arm/mach-tegra/reset.h
index 9c479c7925b8..db0e6b3097ab 100644
--- a/arch/arm/mach-tegra/reset.h
+++ b/arch/arm/mach-tegra/reset.h
@@ -25,7 +25,11 @@
 #define TEGRA_RESET_STARTUP_SECONDARY	3
 #define TEGRA_RESET_STARTUP_LP2		4
 #define TEGRA_RESET_STARTUP_LP1		5
-#define TEGRA_RESET_DATA_SIZE		6
+#define TEGRA_RESET_RESETTABLE_STATUS	6
+#define TEGRA_RESET_TF_PRESENT		7
+#define TEGRA_RESET_DATA_SIZE		8
+
+#define RESET_DATA(x)	((TEGRA_RESET_##x)*4)
 
 #ifndef __ASSEMBLY__
 
@@ -49,7 +53,8 @@ void __tegra_cpu_reset_handler_end(void);
 	 (u32)__tegra_cpu_reset_handler_start)))
 #define tegra20_cpu1_resettable_status \
 	(IO_ADDRESS(TEGRA_IRAM_BASE + TEGRA_IRAM_RESET_HANDLER_OFFSET + \
-	 (u32)__tegra20_cpu1_resettable_status_offset))
+	((u32)&__tegra_cpu_reset_handler_data[TEGRA_RESET_RESETTABLE_STATUS] - \
+	 (u32)__tegra_cpu_reset_handler_start)))
 #endif
 
 #define tegra_cpu_reset_handler_offset \
diff --git a/arch/arm/mach-tegra/sleep-tegra20.S b/arch/arm/mach-tegra/sleep-tegra20.S
index 5c8e638ee51a..11f423e4a263 100644
--- a/arch/arm/mach-tegra/sleep-tegra20.S
+++ b/arch/arm/mach-tegra/sleep-tegra20.S
@@ -28,6 +28,7 @@
 #include <asm/cache.h>
 
 #include "irammap.h"
+#include "reset.h"
 #include "sleep.h"
 
 #define EMC_CFG				0xc
@@ -54,6 +55,9 @@
 #define APB_MISC_XM2CFGCPADCTRL2	0x8e4
 #define APB_MISC_XM2CFGDPADCTRL2	0x8e8
 
+#define __tegra20_cpu1_resettable_status_offset \
+	(__tegra_cpu_reset_handler_data_offset + RESET_DATA(RESETTABLE_STATUS))
+
 .macro pll_enable, rd, r_car_base, pll_base
 	ldr	\rd, [\r_car_base, #\pll_base]
 	tst	\rd, #(1 << 30)
-- 
2.17.1

[PATCH v2 5/5] ARM: tegra: Always boot CPU in ARM-mode

[Dmitry Osipenko]

CPU always jumps into the reset handler in ARM-mode from the Trusted
Foundations firmware, hence make CPU to always jump into kernel in
ARM-mode regardless of the firmware presence to support Thumb2 kernel + TF
case.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 arch/arm/mach-tegra/reset-handler.S | 1 +
 arch/arm/mach-tegra/reset.c         | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-tegra/reset-handler.S b/arch/arm/mach-tegra/reset-handler.S
index e9f2a7775998..8eda5c786f50 100644
--- a/arch/arm/mach-tegra/reset-handler.S
+++ b/arch/arm/mach-tegra/reset-handler.S
@@ -113,6 +113,7 @@ ENTRY(__tegra_cpu_reset_handler_start)
  *       must be position-independent.
  */
 
+	.arm
 	.align L1_CACHE_SHIFT
 ENTRY(__tegra_cpu_reset_handler)
 
diff --git a/arch/arm/mach-tegra/reset.c b/arch/arm/mach-tegra/reset.c
index b02ae7699842..3f1ef4561298 100644
--- a/arch/arm/mach-tegra/reset.c
+++ b/arch/arm/mach-tegra/reset.c
@@ -97,7 +97,7 @@ void __init tegra_cpu_reset_handler_init(void)
 	__tegra_cpu_reset_handler_data[TEGRA_RESET_MASK_PRESENT] =
 		*((u32 *)cpu_possible_mask);
 	__tegra_cpu_reset_handler_data[TEGRA_RESET_STARTUP_SECONDARY] =
-		__pa_symbol((void *)secondary_startup);
+		__pa_symbol((void *)secondary_startup_arm);
 #endif
 
 #ifdef CONFIG_PM_SLEEP
-- 
2.17.1

Re: [PATCH v2 0/5] Initial support of Trusted Foundations on Tegra30

[Peter Geis]

Good Afternoon,

I have tested these patches on the Ouya T3 device.
They work great to enable the L2 cache controller, however they do not 
respect explicitly disabling the L2 cache controller via the kernel 
config nor device tree.

With CONFIG_CACHE_L2X0 disabled, but CONFIG_TRUSTED_FOUNDATIONS enabled, 
the L2 cache controller is silently enabled and allows all four cores to 
boot.

One must also disable CONFIG_TRUSTED_FOUNDATIONS to stop the L2 cache 
controller from spinning up.

Tested-by: Peter Geis <pgwipeout@gmail.com>

On 06/19/2018 07:00 AM, Dmitry Osipenko wrote:
> Hello,
> 
> This series of patches brings initial support of Trusted Foundations to
> Tegra30, that is to the consumer-grade Tegra30 devices which do not allow
> to easily replace the proprietary bootloader. Support is initial because
> this series implements only a proper CPU boot-up (main + secondary cores)
> and a basic L2 cache maintenance that is done using the TF firmware.
> Suspend-resume support is missing yet as I couldn't get it to work
> (CPU hangs on resume from suspend after awhile and seems that is related
> to inappropriately done cache maintenance during of suspend-resume using
> the firmware), it is work-in-progress for now.
> 
> This patchset is partially based on the work done by Michał Mirosław [0].
> 
> [0] https://www.spinics.net/lists/linux-tegra/msg30368.html
> 
> Changelog:
> 
> v2:
> 	- The "Don't apply CPU erratas in insecure mode" patch got some
> 	  cleanup, in particular resolved the messiness in
> 	  __tegra_cpu_reset_handler_data.
> 
> 	- Added a comment to tf_cache_write_sec(), justifying the warning
> 	  message.
> 
> Dmitry Osipenko (5):
>    ARM: trusted_foundations: Implement L2 cache initialization callback
>    ARM: trusted_foundations: Provide information about whether firmware
>      is registered
>    ARM: tegra: Setup L2 cache using Trusted Foundations firmware
>    ARM: tegra: Don't apply CPU erratas in insecure mode
>    ARM: tegra: Always boot CPU in ARM-mode
> 
>   arch/arm/firmware/trusted_foundations.c    | 32 ++++++++++++++++++++++
>   arch/arm/include/asm/trusted_foundations.h |  7 +++++
>   arch/arm/mach-tegra/reset-handler.S        | 25 +++++++++--------
>   arch/arm/mach-tegra/reset.c                |  5 +++-
>   arch/arm/mach-tegra/reset.h                |  9 ++++--
>   arch/arm/mach-tegra/sleep-tegra20.S        |  4 +++
>   arch/arm/mach-tegra/tegra.c                | 15 ++++++++++
>   7 files changed, 82 insertions(+), 15 deletions(-)
> 

Re: [PATCH v2 0/5] Initial support of Trusted Foundations on Tegra30

[Dmitry Osipenko]

On Friday, 29 June 2018 22:37:02 MSK Peter Geis wrote:
> Good Afternoon,
> 
> I have tested these patches on the Ouya T3 device.
> They work great to enable the L2 cache controller, however they do not
> respect explicitly disabling the L2 cache controller via the kernel
> config nor device tree.
> 
> With CONFIG_CACHE_L2X0 disabled, but CONFIG_TRUSTED_FOUNDATIONS enabled,
> the L2 cache controller is silently enabled and allows all four cores to
> boot.
> 

I don't see how cache could be enabled with CONFIG_CACHE_L2X0 disabled, there 
is no code to do that. Could you elaborate please?

Secondary cores do not depend on the cache state, disabled cache shouldn't 
prevent them to boot.

> One must also disable CONFIG_TRUSTED_FOUNDATIONS to stop the L2 cache
> controller from spinning up.
> 
> Tested-by: Peter Geis <pgwipeout@gmail.com>
> 

Re: [PATCH v2 0/5] Initial support of Trusted Foundations on Tegra30

[Peter Geis]

On 07/02/2018 10:48 AM, Dmitry Osipenko wrote:
> On Friday, 29 June 2018 22:37:02 MSK Peter Geis wrote:
>> Good Afternoon,
>>
>> I have tested these patches on the Ouya T3 device.
>> They work great to enable the L2 cache controller, however they do not
>> respect explicitly disabling the L2 cache controller via the kernel
>> config nor device tree.
>>
>> With CONFIG_CACHE_L2X0 disabled, but CONFIG_TRUSTED_FOUNDATIONS enabled,
>> the L2 cache controller is silently enabled and allows all four cores to
>> boot.
>>
> 
> I don't see how cache could be enabled with CONFIG_CACHE_L2X0 disabled, there
> is no code to do that. Could you elaborate please?
> 
> Secondary cores do not depend on the cache state, disabled cache shouldn't
> prevent them to boot.

On the untouched mainline kernel running on a Trusted Foundations T3 
device, I observed the following indications:
With the L2 cache controller enabled, all four processor cores were 
enabled, but it would immediately panic for writing to a secure register 
from insecure mode.
With the L2 cache controller disabled, only the boot core is detected, 
but it successfully boots.
This is the issue that I inquired originally to you about.

With your patch running on the same device, the following is observed:
With the L2 controller enabled, all four cores are active, and the cache 
controller appears to function.
With the L2 controller disabled, but trusted foundations enabled, the L2 
controller enabled kernel message is missing, however all four cores 
still enable.

After looking through the code a little more deeply, I see you modified 
the reset handler for handling offline cores.
I am wondering if you fixed an additional issue inadvertently.

The reason I discovered this is I am working with kexec as a bootloader.
On a device without trusted foundations, kexec works without issue.
On a device with trusted foundations and your patch, I found that 
disabling the l2 cache controller and trusted foundations allow kexec to 
occur.
I haven't tried an either/or scenario, though now you have me thinking I 
should.
> 
>> One must also disable CONFIG_TRUSTED_FOUNDATIONS to stop the L2 cache
>> controller from spinning up.
>>
>> Tested-by: Peter Geis <pgwipeout@gmail.com>
>>
> 
> 
> 
> 

Re: [PATCH v2 0/5] Initial support of Trusted Foundations on Tegra30

[Dmitry Osipenko]

On Monday, 2 July 2018 21:53:08 MSK Peter Geis wrote:
> On 07/02/2018 10:48 AM, Dmitry Osipenko wrote:
> > On Friday, 29 June 2018 22:37:02 MSK Peter Geis wrote:
> >> Good Afternoon,
> >> 
> >> I have tested these patches on the Ouya T3 device.
> >> They work great to enable the L2 cache controller, however they do not
> >> respect explicitly disabling the L2 cache controller via the kernel
> >> config nor device tree.
> >> 
> >> With CONFIG_CACHE_L2X0 disabled, but CONFIG_TRUSTED_FOUNDATIONS enabled,
> >> the L2 cache controller is silently enabled and allows all four cores to
> >> boot.
> > 
> > I don't see how cache could be enabled with CONFIG_CACHE_L2X0 disabled,
> > there is no code to do that. Could you elaborate please?
> > 
> > Secondary cores do not depend on the cache state, disabled cache shouldn't
> > prevent them to boot.
> 
> On the untouched mainline kernel running on a Trusted Foundations T3
> device, I observed the following indications:
> With the L2 cache controller enabled, all four processor cores were
> enabled, but it would immediately panic for writing to a secure register
> from insecure mode.
> With the L2 cache controller disabled, only the boot core is detected,
> but it successfully boots.
> This is the issue that I inquired originally to you about.
> 
> With your patch running on the same device, the following is observed:
> With the L2 controller enabled, all four cores are active, and the cache
> controller appears to function.
> With the L2 controller disabled, but trusted foundations enabled, the L2
> controller enabled kernel message is missing, however all four cores
> still enable.
> 

This is the correct behaviour.

> After looking through the code a little more deeply, I see you modified
> the reset handler for handling offline cores.
> I am wondering if you fixed an additional issue inadvertently.
> 

CPU will fail to boot if it tries to apply erratas via accessing the secure 
registers. I've changed the reset handler to skip erratas on T20/30 if trusted 
foundations present, see "Don't apply CPU erratas in insecure mode" patch. 
This is an intentional change.

> The reason I discovered this is I am working with kexec as a bootloader.
> On a device without trusted foundations, kexec works without issue.
> On a device with trusted foundations and your patch, I found that
> disabling the l2 cache controller and trusted foundations allow kexec to
> occur.
> I haven't tried an either/or scenario, though now you have me thinking I
> should.
> 

Secondary cores are dying in the reset handler in a case of disabled L2 + 
disabled TF. Looks like there is something wrong in regards to stopping 
secondary CPU cores / flushing CPU caches, the suspend-resume isn't working 
right now because of it and likely that kexec is suffering from the same 
issue.

> >> One must also disable CONFIG_TRUSTED_FOUNDATIONS to stop the L2 cache
> >> controller from spinning up.
> >> 
> >> Tested-by: Peter Geis <pgwipeout@gmail.com>

Thanks for testing!

Re: [PATCH v2 0/5] Initial support of Trusted Foundations on Tegra30

[Dmitry Osipenko]

Kexec works for me on T30 with the following kernel config options on top of the
tegra_defconfig:

CONFIG_SMP=n
CONFIG_CPU_IDLE=n
CONFIG_TEGRA_IOMMU_SMMU=n

CPU stopping isn't working correctly with the trusted foundations and it's not
obvious what is wrong. I hope we'll fix it at some point.