[U-Boot] [PATCH 0/4] ARM: tegra: GPU WPR region support

This series makes U-boot program the write-protected (WPR) region of T210 chips, allowing the kernel to perform GPU secure firmware loading.
Tegra 210's GPU requires its firmware to be loaded though a write-protected region. An area of physical memory is carved-out, programmed into the corresponding memory controller registers, and locked such as only the GPU can write into it. This area needs to be set up by the bootloader since it cannot be re-claimed for normal use after being locked.
The first 3 patches of this series are cleanup patches. Patch 2 implements a suggestion made by Stephen, patch 3 renames GPU-related functions to sound less generic.
The last patch adds support for the GPU WPR region. The top 256KB of memory are removed from the available memory, and the corresponding MC registers are programmed to point to it, which allows the kernel to initiate secure firmware loading.
Alexandre Courbot (4): ARM: tegra: remove vpr_configured() function ARM: tegra: simplify GPU setup ARM: tegra: rename GPU functions ARM: tegra210: gpu: configure WPR region
arch/arm/include/asm/arch-tegra/gpu.h | 14 +++------ arch/arm/include/asm/arch-tegra210/mc.h | 12 ++++++++ arch/arm/mach-tegra/board.c | 4 +++ arch/arm/mach-tegra/board2.c | 22 +++++++++++++- arch/arm/mach-tegra/gpu.c | 52 +++++++++++++++++++++++++++++---- board/nvidia/jetson-tk1/jetson-tk1.c | 8 ----- board/nvidia/p2571/p2571.c | 7 ----- board/nvidia/venice2/venice2.c | 8 ----- include/configs/jetson-tk1.h | 2 -- include/configs/p2571.h | 2 -- include/configs/tegra-common.h | 2 ++ include/configs/venice2.h | 2 -- 12 files changed, 89 insertions(+), 46 deletions(-)

There is no justification for this function, especially in exported form.
Signed-off-by: Alexandre Courbot acourbot@nvidia.com --- arch/arm/include/asm/arch-tegra/gpu.h | 6 ------ arch/arm/mach-tegra/gpu.c | 7 +------ 2 files changed, 1 insertion(+), 12 deletions(-)
diff --git a/arch/arm/include/asm/arch-tegra/gpu.h b/arch/arm/include/asm/arch-tegra/gpu.h index 52280f40ce31..2fdb2c5049e6 100644 --- a/arch/arm/include/asm/arch-tegra/gpu.h +++ b/arch/arm/include/asm/arch-tegra/gpu.h @@ -11,7 +11,6 @@ #if defined(CONFIG_TEGRA_GPU)
void config_gpu(void); -bool gpu_configured(void);
#else /* CONFIG_TEGRA_GPU */
@@ -19,11 +18,6 @@ static inline void config_gpu(void) { }
-static inline bool gpu_configured(void) -{ - return false; -} - #endif /* CONFIG_TEGRA_GPU */
#if defined(CONFIG_OF_LIBFDT) diff --git a/arch/arm/mach-tegra/gpu.c b/arch/arm/mach-tegra/gpu.c index 4ea046d3e5b6..dc29b79e0126 100644 --- a/arch/arm/mach-tegra/gpu.c +++ b/arch/arm/mach-tegra/gpu.c @@ -41,18 +41,13 @@ void config_gpu(void) _configured = true; }
-bool vpr_configured(void) -{ - return _configured; -} - #if defined(CONFIG_OF_LIBFDT)
int gpu_enable_node(void *blob, const char *gpupath) { int offset;
- if (vpr_configured()) { + if (_configured) { offset = fdt_path_offset(blob, gpupath); if (offset > 0) { fdt_status_okay(blob, offset);

Enable the GPU node in the system-wide ft_system_setup() hook instead of the board-specific ft_board_hook(). This allows us to enable GPU per SoC generation instead of per-board as we did initially.
Reported-by: Stephen Warren swarren@nvidia.com Signed-off-by: Alexandre Courbot acourbot@nvidia.com --- arch/arm/mach-tegra/board2.c | 20 ++++++++++++++++++++ board/nvidia/jetson-tk1/jetson-tk1.c | 8 -------- board/nvidia/p2571/p2571.c | 7 ------- board/nvidia/venice2/venice2.c | 8 -------- include/configs/jetson-tk1.h | 2 -- include/configs/p2571.h | 2 -- include/configs/tegra-common.h | 2 ++ include/configs/venice2.h | 2 -- 8 files changed, 22 insertions(+), 29 deletions(-)
diff --git a/arch/arm/mach-tegra/board2.c b/arch/arm/mach-tegra/board2.c index 8ecc67459a10..ff9e77cfa3a3 100644 --- a/arch/arm/mach-tegra/board2.c +++ b/arch/arm/mach-tegra/board2.c @@ -403,3 +403,23 @@ ulong board_get_usable_ram_top(ulong total_size) { return CONFIG_SYS_SDRAM_BASE + usable_ram_size_below_4g(); } + +/* + * This function is called right before the kernel is booted. "blob" is the + * device tree that will be passed to the kernel. + */ +int ft_system_setup(void *blob, bd_t *bd) +{ + const char *gpu_path = +#if defined(CONFIG_TEGRA124) || defined(CONFIG_TEGRA210) + "/gpu@0,57000000"; +#else + NULL; +#endif + + /* Enable GPU node if GPU setup has been performed */ + if (gpu_path != NULL) + return gpu_enable_node(blob, gpu_path); + + return 0; +} diff --git a/board/nvidia/jetson-tk1/jetson-tk1.c b/board/nvidia/jetson-tk1/jetson-tk1.c index 3c21767ce4da..52425a8f6dea 100644 --- a/board/nvidia/jetson-tk1/jetson-tk1.c +++ b/board/nvidia/jetson-tk1/jetson-tk1.c @@ -11,7 +11,6 @@
#include <asm/arch/gpio.h> #include <asm/arch/pinmux.h> -#include <asm/arch-tegra/gpu.h>
#include "pinmux-config-jetson-tk1.h"
@@ -80,10 +79,3 @@ int board_eth_init(bd_t *bis) return pci_eth_init(bis); } #endif /* PCI */ - -int ft_board_setup(void *blob, bd_t *bd) -{ - gpu_enable_node(blob, "/gpu@0,57000000"); - - return 0; -} diff --git a/board/nvidia/p2571/p2571.c b/board/nvidia/p2571/p2571.c index d33e4d12b2fa..d80a7d0d3e31 100644 --- a/board/nvidia/p2571/p2571.c +++ b/board/nvidia/p2571/p2571.c @@ -11,7 +11,6 @@ #include <asm/arch/pinmux.h> #include <asm/gpio.h> #include "max77620_init.h" -#include <asm/arch-tegra/gpu.h> #include "pinmux-config-p2571.h"
void pin_mux_mmc(void) @@ -62,9 +61,3 @@ void start_cpu_fan(void) gpio_request(GPIO_PE4, "FAN_VDD"); gpio_direction_output(GPIO_PE4, 1); } - -int ft_board_setup(void *blob, bd_t *bd) -{ - gpu_enable_node(blob, "/gpu@0,57000000"); - return 0; -} diff --git a/board/nvidia/venice2/venice2.c b/board/nvidia/venice2/venice2.c index 3e2b9a7745e9..c56ef129d6c7 100644 --- a/board/nvidia/venice2/venice2.c +++ b/board/nvidia/venice2/venice2.c @@ -8,7 +8,6 @@ #include <common.h> #include <asm/arch/gpio.h> #include <asm/arch/pinmux.h> -#include <asm/arch-tegra/gpu.h> #include "pinmux-config-venice2.h"
/* @@ -28,10 +27,3 @@ void pinmux_init(void) pinmux_config_drvgrp_table(venice2_drvgrps, ARRAY_SIZE(venice2_drvgrps)); } - -int ft_board_setup(void *blob, bd_t *bd) -{ - gpu_enable_node(blob, "/gpu@0,57000000"); - - return 0; -} diff --git a/include/configs/jetson-tk1.h b/include/configs/jetson-tk1.h index e87a01047d30..f63957ab92fd 100644 --- a/include/configs/jetson-tk1.h +++ b/include/configs/jetson-tk1.h @@ -78,6 +78,4 @@ #define CONFIG_ARMV7_SECURE_BASE 0xfff00000 #define CONFIG_ARMV7_SECURE_RESERVE_SIZE 0x00100000
-#define CONFIG_OF_BOARD_SETUP - #endif /* __CONFIG_H */ diff --git a/include/configs/p2571.h b/include/configs/p2571.h index c65d3e5fcbc1..a5de411121b0 100644 --- a/include/configs/p2571.h +++ b/include/configs/p2571.h @@ -60,6 +60,4 @@ #include "tegra-common-usb-gadget.h" #include "tegra-common-post.h"
-#define CONFIG_OF_BOARD_SETUP - #endif /* _P2571_H */ diff --git a/include/configs/tegra-common.h b/include/configs/tegra-common.h index 1c469d092e8c..9f0d4644431a 100644 --- a/include/configs/tegra-common.h +++ b/include/configs/tegra-common.h @@ -144,4 +144,6 @@ #define CONFIG_FAT_WRITE #endif
+#define CONFIG_OF_SYSTEM_SETUP + #endif /* _TEGRA_COMMON_H_ */ diff --git a/include/configs/venice2.h b/include/configs/venice2.h index 0fc8cf7674d1..a374cd948849 100644 --- a/include/configs/venice2.h +++ b/include/configs/venice2.h @@ -60,6 +60,4 @@ #include "tegra-common-usb-gadget.h" #include "tegra-common-post.h"
-#define CONFIG_OF_BOARD_SETUP - #endif /* __CONFIG_H */

Rename GPU functions to less generic names to avoid potential name collisions.
Signed-off-by: Alexandre Courbot acourbot@nvidia.com --- arch/arm/include/asm/arch-tegra/gpu.h | 8 ++++---- arch/arm/mach-tegra/board2.c | 4 ++-- arch/arm/mach-tegra/gpu.c | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/arch/arm/include/asm/arch-tegra/gpu.h b/arch/arm/include/asm/arch-tegra/gpu.h index 2fdb2c5049e6..4423386f2805 100644 --- a/arch/arm/include/asm/arch-tegra/gpu.h +++ b/arch/arm/include/asm/arch-tegra/gpu.h @@ -10,11 +10,11 @@
#if defined(CONFIG_TEGRA_GPU)
-void config_gpu(void); +void tegra_gpu_config(void);
#else /* CONFIG_TEGRA_GPU */
-static inline void config_gpu(void) +static inline void tegra_gpu_config(void) { }
@@ -22,11 +22,11 @@ static inline void config_gpu(void)
#if defined(CONFIG_OF_LIBFDT)
-int gpu_enable_node(void *blob, const char *gpupath); +int tegra_gpu_enable_node(void *blob, const char *gpupath);
#else /* CONFIG_OF_LIBFDT */
-static inline int gpu_enable_node(void *blob, const char *gpupath) +static inline int tegra_gpu_enable_node(void *blob, const char *gpupath) { return 0; } diff --git a/arch/arm/mach-tegra/board2.c b/arch/arm/mach-tegra/board2.c index ff9e77cfa3a3..8ba143d996ca 100644 --- a/arch/arm/mach-tegra/board2.c +++ b/arch/arm/mach-tegra/board2.c @@ -128,7 +128,7 @@ int board_init(void) clock_init(); clock_verify();
- config_gpu(); + tegra_gpu_config();
#ifdef CONFIG_TEGRA_SPI pin_mux_spi(); @@ -419,7 +419,7 @@ int ft_system_setup(void *blob, bd_t *bd)
/* Enable GPU node if GPU setup has been performed */ if (gpu_path != NULL) - return gpu_enable_node(blob, gpu_path); + return tegra_gpu_enable_node(blob, gpu_path);
return 0; } diff --git a/arch/arm/mach-tegra/gpu.c b/arch/arm/mach-tegra/gpu.c index dc29b79e0126..c7d705d8efe9 100644 --- a/arch/arm/mach-tegra/gpu.c +++ b/arch/arm/mach-tegra/gpu.c @@ -25,7 +25,7 @@
static bool _configured;
-void config_gpu(void) +void tegra_gpu_config(void) { struct mc_ctlr *mc = (struct mc_ctlr *)NV_PA_MC_BASE;
@@ -43,7 +43,7 @@ void config_gpu(void)
#if defined(CONFIG_OF_LIBFDT)
-int gpu_enable_node(void *blob, const char *gpupath) +int tegra_gpu_enable_node(void *blob, const char *gpupath) { int offset;

On 10/18/2015 10:57 PM, Alexandre Courbot wrote:
Rename GPU functions to less generic names to avoid potential name collisions.
Patches 1-3, Acked-by: Stephen Warren swarren@nvidia.com

T210's GPU secure firmware loading requires a write-protected region to be set up.
This patch reserves the upper 256KB of RAM as the WPR region and locks it so the kernel can initiate secure firmware loading.
Signed-off-by: Alexandre Courbot acourbot@nvidia.com --- arch/arm/include/asm/arch-tegra210/mc.h | 12 +++++++++ arch/arm/mach-tegra/board.c | 4 +++ arch/arm/mach-tegra/gpu.c | 47 ++++++++++++++++++++++++++++++++- 3 files changed, 62 insertions(+), 1 deletion(-)
diff --git a/arch/arm/include/asm/arch-tegra210/mc.h b/arch/arm/include/asm/arch-tegra210/mc.h index 77e9aa51f60f..e6d1758d372f 100644 --- a/arch/arm/include/asm/arch-tegra210/mc.h +++ b/arch/arm/include/asm/arch-tegra210/mc.h @@ -62,6 +62,16 @@ struct mc_ctlr { u32 mc_video_protect_bom; /* offset 0x648 */ u32 mc_video_protect_size_mb; /* offset 0x64c */ u32 mc_video_protect_reg_ctrl; /* offset 0x650 */ + u32 reserved11[385]; /* offset 0x654 - 0xc54 */ + u32 mc_security_carveout2_cfg0; /* offset 0xc58 */ + u32 mc_security_carveout2_bom; /* offset 0xc5c */ + u32 mc_security_carveout2_bom_hi; /* offset 0xc60 */ + u32 mc_security_carveout2_size_128k; /* offset 0xc64 */ + u32 reserved12[16]; /* offset 0xc68 - 0xca4 */ + u32 mc_security_carveout3_cfg0; /* offset 0xca8 */ + u32 mc_security_carveout3_bom; /* offset 0xcac */ + u32 mc_security_carveout3_bom_hi; /* offset 0xcb0 */ + u32 mc_security_carveout3_size_128k; /* offset 0xcb4 */ };
#define TEGRA_MC_SMMU_CONFIG_ENABLE (1 << 0) @@ -69,4 +79,6 @@ struct mc_ctlr { #define TEGRA_MC_VIDEO_PROTECT_REG_WRITE_ACCESS_ENABLED (0 << 0) #define TEGRA_MC_VIDEO_PROTECT_REG_WRITE_ACCESS_DISABLED (1 << 0)
+#define TEGRA_MC_SECURITY_CARVEOUT_CFG_LOCKED (1 << 1) + #endif /* _TEGRA210_MC_H_ */ diff --git a/arch/arm/mach-tegra/board.c b/arch/arm/mach-tegra/board.c index b00e4b5c1e25..0bff063b00f4 100644 --- a/arch/arm/mach-tegra/board.c +++ b/arch/arm/mach-tegra/board.c @@ -111,6 +111,10 @@ static phys_size_t query_sdram_size(void) if (size_bytes == SZ_2G) size_bytes -= SZ_1M; #endif +#if defined(CONFIG_TEGRA210) + /* Reserve GPU WPR area, 2 * 128KB */ + size_bytes = round_down(size_bytes - (SZ_128K * 2), SZ_128K); +#endif
return size_bytes; } diff --git a/arch/arm/mach-tegra/gpu.c b/arch/arm/mach-tegra/gpu.c index c7d705d8efe9..61d734fd5767 100644 --- a/arch/arm/mach-tegra/gpu.c +++ b/arch/arm/mach-tegra/gpu.c @@ -23,9 +23,11 @@
#include <fdt_support.h>
+DECLARE_GLOBAL_DATA_PTR; + static bool _configured;
-void tegra_gpu_config(void) +static void config_vpr(void) { struct mc_ctlr *mc = (struct mc_ctlr *)NV_PA_MC_BASE;
@@ -37,6 +39,49 @@ void tegra_gpu_config(void) readl(&mc->mc_video_protect_reg_ctrl);
debug("configured VPR\n"); +} + +#if defined(CONFIG_TEGRA210) +static void config_wpr(void) +{ + struct mc_ctlr *mc = (struct mc_ctlr *)NV_PA_MC_BASE; + u64 wpr_start = NV_PA_SDRAM_BASE + gd->ram_size; + u32 reg; + + /* + * Carveout2 uses the upper 256KB of upper memory that we reserved as + * WPR region for secure firmware loading + */ + writel(lower_32_bits(wpr_start), &mc->mc_security_carveout2_bom); + writel(upper_32_bits(wpr_start), &mc->mc_security_carveout2_bom_hi); + writel(0x2, &mc->mc_security_carveout2_size_128k); + reg = readl(&mc->mc_security_carveout2_cfg0); + reg |= TEGRA_MC_SECURITY_CARVEOUT_CFG_LOCKED; + writel(reg, &mc->mc_security_carveout2_cfg0); + + /* Carveout3 is left empty */ + writel(0x0, &mc->mc_security_carveout3_bom); + writel(0x0, &mc->mc_security_carveout3_bom_hi); + writel(0x0, &mc->mc_security_carveout3_size_128k); + reg = readl(&mc->mc_security_carveout3_cfg0); + reg |= TEGRA_MC_SECURITY_CARVEOUT_CFG_LOCKED; + writel(reg, &mc->mc_security_carveout3_cfg0); + + /* read back to ensure the write went through */ + readl(&mc->mc_security_carveout3_cfg0); + + debug("configured WPR\n"); +} +#else +static inline void config_wpr(void) +{ +} +#endif + +void tegra_gpu_config(void) +{ + config_vpr(); + config_wpr();
_configured = true; }

On 10/18/2015 10:57 PM, Alexandre Courbot wrote:
T210's GPU secure firmware loading requires a write-protected region to be set up.
This patch reserves the upper 256KB of RAM as the WPR region and locks it so the kernel can initiate secure firmware loading.
On T210, it's the responsibility of nvtboot (which runs before U-Boot) to set up any and all carve-outs. This code should not be necessary, and indeed I expect the registers it touches can't actually be programmed from U-Boot, which runs in non-secure mode after WPR is already locked.

On 10/29/2015 02:59 AM, Stephen Warren wrote:
On 10/18/2015 10:57 PM, Alexandre Courbot wrote:
T210's GPU secure firmware loading requires a write-protected region to be set up.
This patch reserves the upper 256KB of RAM as the WPR region and locks it so the kernel can initiate secure firmware loading.
On T210, it's the responsibility of nvtboot (which runs before U-Boot) to set up any and all carve-outs. This code should not be necessary, and indeed I expect the registers it touches can't actually be programmed from U-Boot, which runs in non-secure mode after WPR is already locked.
Ok, I was running from Thierry's miniloader which did not program or lock these registers.
The question then is: do we have an official nvtboot binary available for upstream support? AFAICT Thierry's miniloader solution is the only solution available to the public, and without this setup one cannot use the GPU on T210.

On 10/28/2015 05:55 PM, Alexandre Courbot wrote:
On 10/29/2015 02:59 AM, Stephen Warren wrote:
On 10/18/2015 10:57 PM, Alexandre Courbot wrote:
T210's GPU secure firmware loading requires a write-protected region to be set up.
This patch reserves the upper 256KB of RAM as the WPR region and locks it so the kernel can initiate secure firmware loading.
On T210, it's the responsibility of nvtboot (which runs before U-Boot) to set up any and all carve-outs. This code should not be necessary, and indeed I expect the registers it touches can't actually be programmed from U-Boot, which runs in non-secure mode after WPR is already locked.
Ok, I was running from Thierry's miniloader which did not program or lock these registers.
The question then is: do we have an official nvtboot binary available for upstream support? AFAICT Thierry's miniloader solution is the only solution available to the public, and without this setup one cannot use the GPU on T210.
Once L4T for T210 has been released, it'll contain a full set of nvtboot binaries and flashing support. You an easily acquire this internally already; send me an email if you need a link. I'd expect a public SW release by the time any T210 boards capable of running upstream are available.

On Wed, Oct 28, 2015 at 11:59:04AM -0600, Stephen Warren wrote:
On 10/18/2015 10:57 PM, Alexandre Courbot wrote:
T210's GPU secure firmware loading requires a write-protected region to be set up.
This patch reserves the upper 256KB of RAM as the WPR region and locks it so the kernel can initiate secure firmware loading.
On T210, it's the responsibility of nvtboot (which runs before U-Boot) to set up any and all carve-outs. This code should not be necessary, and indeed I expect the registers it touches can't actually be programmed from U-Boot, which runs in non-secure mode after WPR is already locked.
Can we document this assumption somewhere? It's entirely possible that someone might want to run U-Boot without nvtboot, in which case nvtboot would still need to be responsible for setting this up. Or if it isn't we could still point at some location where the interactions between a first stage bootloader and U-Boot are documented.
Do we have a document of this kind already?
Thierry

On 11/09/2015 07:36 AM, Thierry Reding wrote:
On Wed, Oct 28, 2015 at 11:59:04AM -0600, Stephen Warren wrote:
On 10/18/2015 10:57 PM, Alexandre Courbot wrote:
T210's GPU secure firmware loading requires a write-protected region to be set up.
This patch reserves the upper 256KB of RAM as the WPR region and locks it so the kernel can initiate secure firmware loading.
On T210, it's the responsibility of nvtboot (which runs before U-Boot) to set up any and all carve-outs. This code should not be necessary, and indeed I expect the registers it touches can't actually be programmed from U-Boot, which runs in non-secure mode after WPR is already locked.
Can we document this assumption somewhere? It's entirely possible that someone might want to run U-Boot without nvtboot
That's not currently a supported use-case.
, in which case nvtboot would still need to be responsible for setting this up.
I assume s/nvtboot/whatever other bootloader is in use/?
Or if it isn't we could still point at some location where the interactions between a first stage bootloader and U-Boot are documented.
Do we have a document of this kind already?
The L4T U-Boot source code:-)

On Mon, Nov 09, 2015 at 08:19:48AM -0700, Stephen Warren wrote:
On 11/09/2015 07:36 AM, Thierry Reding wrote:
On Wed, Oct 28, 2015 at 11:59:04AM -0600, Stephen Warren wrote:
On 10/18/2015 10:57 PM, Alexandre Courbot wrote:
T210's GPU secure firmware loading requires a write-protected region to be set up.
This patch reserves the upper 256KB of RAM as the WPR region and locks it so the kernel can initiate secure firmware loading.
On T210, it's the responsibility of nvtboot (which runs before U-Boot) to set up any and all carve-outs. This code should not be necessary, and indeed I expect the registers it touches can't actually be programmed from U-Boot, which runs in non-secure mode after WPR is already locked.
Can we document this assumption somewhere? It's entirely possible that someone might want to run U-Boot without nvtboot
That's not currently a supported use-case.
Isn't the whole point of open-source for people to be able to do all the things that aren't supported?
, in which case nvtboot would still need to be responsible for setting this up.
I assume s/nvtboot/whatever other bootloader is in use/?
Right, I was going to write U-Boot here, but that's obviously only true if U-Boot is still running in secure mode. Otherwise, yes, it'd be what- ever is the other bootloader being used.
Or if it isn't we could still point at some location where the interactions between a first stage bootloader and U-Boot are documented.
Do we have a document of this kind already?
The L4T U-Boot source code:-)
There's a bunch of things that even the L4T U-Boot doesn't document. The fact that it doesn't initialize the SMMU or WPR doesn't indicate that it is something that a first stage bootloader needs to do.
Thierry

On 11/09/2015 08:48 AM, Thierry Reding wrote:
On Mon, Nov 09, 2015 at 08:19:48AM -0700, Stephen Warren wrote:
On 11/09/2015 07:36 AM, Thierry Reding wrote:
On Wed, Oct 28, 2015 at 11:59:04AM -0600, Stephen Warren wrote:
On 10/18/2015 10:57 PM, Alexandre Courbot wrote:
T210's GPU secure firmware loading requires a write-protected region to be set up.
This patch reserves the upper 256KB of RAM as the WPR region and locks it so the kernel can initiate secure firmware loading.
On T210, it's the responsibility of nvtboot (which runs before U-Boot) to set up any and all carve-outs. This code should not be necessary, and indeed I expect the registers it touches can't actually be programmed from U-Boot, which runs in non-secure mode after WPR is already locked.
Can we document this assumption somewhere? It's entirely possible that someone might want to run U-Boot without nvtboot
That's not currently a supported use-case.
Isn't the whole point of open-source for people to be able to do all the things that aren't supported?
Sure, but the default case that we support should "just work", and be represented by the code we ship, and be supported by us. Any non-default case is going to require some research or additional coding/fixing/... effort, and we haven't necessarily published the information require to make those cases work.
, in which case nvtboot would still need to be responsible for setting this up.
I assume s/nvtboot/whatever other bootloader is in use/?
Right, I was going to write U-Boot here, but that's obviously only true if U-Boot is still running in secure mode. Otherwise, yes, it'd be what- ever is the other bootloader being used.
Or if it isn't we could still point at some location where the interactions between a first stage bootloader and U-Boot are documented.
Do we have a document of this kind already?
The L4T U-Boot source code:-)
There's a bunch of things that even the L4T U-Boot doesn't document. The fact that it doesn't initialize the SMMU or WPR doesn't indicate that it is something that a first stage bootloader needs to do.
I meant this more along the lines of "if there's no code to do something in L4T U-Boot, upstream U-Boot wouldn't need that code either". That should be true at least w.r.t. nvtboot integration; certainly there are features that only exist in upstream U-Boot and haven't been back-ported downstream. I wasn't really implying that there's actual documentation (comments) in the L4T U-Boot source code.

Ping Tom, how does this look to you?
On Mon, Oct 19, 2015 at 1:57 PM, Alexandre Courbot acourbot@nvidia.com wrote:
This series makes U-boot program the write-protected (WPR) region of T210 chips, allowing the kernel to perform GPU secure firmware loading.
Tegra 210's GPU requires its firmware to be loaded though a write-protected region. An area of physical memory is carved-out, programmed into the corresponding memory controller registers, and locked such as only the GPU can write into it. This area needs to be set up by the bootloader since it cannot be re-claimed for normal use after being locked.
The first 3 patches of this series are cleanup patches. Patch 2 implements a suggestion made by Stephen, patch 3 renames GPU-related functions to sound less generic.
The last patch adds support for the GPU WPR region. The top 256KB of memory are removed from the available memory, and the corresponding MC registers are programmed to point to it, which allows the kernel to initiate secure firmware loading.
Alexandre Courbot (4): ARM: tegra: remove vpr_configured() function ARM: tegra: simplify GPU setup ARM: tegra: rename GPU functions ARM: tegra210: gpu: configure WPR region
arch/arm/include/asm/arch-tegra/gpu.h | 14 +++------ arch/arm/include/asm/arch-tegra210/mc.h | 12 ++++++++ arch/arm/mach-tegra/board.c | 4 +++ arch/arm/mach-tegra/board2.c | 22 +++++++++++++- arch/arm/mach-tegra/gpu.c | 52 +++++++++++++++++++++++++++++---- board/nvidia/jetson-tk1/jetson-tk1.c | 8 ----- board/nvidia/p2571/p2571.c | 7 ----- board/nvidia/venice2/venice2.c | 8 ----- include/configs/jetson-tk1.h | 2 -- include/configs/p2571.h | 2 -- include/configs/tegra-common.h | 2 ++ include/configs/venice2.h | 2 -- 12 files changed, 89 insertions(+), 46 deletions(-)
-- 2.6.1

Sorry, Alex. Missed these.
-----Original Message----- From: Alexandre Courbot [mailto:gnurou@gmail.com] Sent: Sunday, October 25, 2015 10:50 PM To: Alex Courbot acourbot@nvidia.com Cc: Tom Warren TWarren@nvidia.com; Stephen Warren swarren@nvidia.com; Thierry Reding treding@nvidia.com; u- boot@lists.denx.de; linux-tegra@vger.kernel.org Subject: Re: [PATCH 0/4] ARM: tegra: GPU WPR region support
Ping Tom, how does this look to you?
Looks pretty good, but what about saving the security_carveout reg settings back to the BCT or scratch regs so those settings will be restored on LP0 resume?
Tom -- nvpublic
On Mon, Oct 19, 2015 at 1:57 PM, Alexandre Courbot acourbot@nvidia.com wrote:
This series makes U-boot program the write-protected (WPR) region of T210 chips, allowing the kernel to perform GPU secure firmware loading.
Tegra 210's GPU requires its firmware to be loaded though a write-protected region. An area of physical memory is carved-out, programmed into the corresponding memory controller registers, and locked such as only the GPU can write into it. This area needs to be set up by the bootloader since it cannot be re-claimed for normal use after
being locked.
The first 3 patches of this series are cleanup patches. Patch 2 implements a suggestion made by Stephen, patch 3 renames GPU-related functions to sound less generic.
The last patch adds support for the GPU WPR region. The top 256KB of memory are removed from the available memory, and the corresponding MC registers are programmed to point to it, which allows the kernel to initiate secure firmware loading.
Alexandre Courbot (4): ARM: tegra: remove vpr_configured() function ARM: tegra: simplify GPU setup ARM: tegra: rename GPU functions ARM: tegra210: gpu: configure WPR region
arch/arm/include/asm/arch-tegra/gpu.h | 14 +++------ arch/arm/include/asm/arch-tegra210/mc.h | 12 ++++++++ arch/arm/mach-tegra/board.c | 4 +++ arch/arm/mach-tegra/board2.c | 22 +++++++++++++- arch/arm/mach-tegra/gpu.c | 52
+++++++++++++++++++++++++++++----
board/nvidia/jetson-tk1/jetson-tk1.c | 8 ----- board/nvidia/p2571/p2571.c | 7 ----- board/nvidia/venice2/venice2.c | 8 ----- include/configs/jetson-tk1.h | 2 -- include/configs/p2571.h | 2 -- include/configs/tegra-common.h | 2 ++ include/configs/venice2.h | 2 -- 12 files changed, 89 insertions(+), 46 deletions(-)
-- 2.6.1

On Wed, Oct 28, 2015 at 12:57 AM, Tom Warren TWarren@nvidia.com wrote:
Sorry, Alex. Missed these.
-----Original Message----- From: Alexandre Courbot [mailto:gnurou@gmail.com] Sent: Sunday, October 25, 2015 10:50 PM To: Alex Courbot acourbot@nvidia.com Cc: Tom Warren TWarren@nvidia.com; Stephen Warren swarren@nvidia.com; Thierry Reding treding@nvidia.com; u- boot@lists.denx.de; linux-tegra@vger.kernel.org Subject: Re: [PATCH 0/4] ARM: tegra: GPU WPR region support
Ping Tom, how does this look to you?
Looks pretty good, but what about saving the security_carveout reg settings back to the BCT or scratch regs so those settings will be restored on LP0 resume?
Absolutely - I am not familiar at all with BCT or scratch registers (and U-boot in general :) ) though, could you point me to some part of the code which I could use as a reference for this?
Also, how can I decide which mechanism to use over the other?
Thanks, Alex.

Alex,
-----Original Message----- From: Alexandre Courbot [mailto:gnurou@gmail.com] Sent: Wednesday, October 28, 2015 2:13 AM To: Tom Warren TWarren@nvidia.com Cc: Alex Courbot acourbot@nvidia.com; Stephen Warren swarren@nvidia.com; Thierry Reding treding@nvidia.com; u- boot@lists.denx.de; linux-tegra@vger.kernel.org Subject: Re: [PATCH 0/4] ARM: tegra: GPU WPR region support
On Wed, Oct 28, 2015 at 12:57 AM, Tom Warren TWarren@nvidia.com wrote:
Sorry, Alex. Missed these.
-----Original Message----- From: Alexandre Courbot [mailto:gnurou@gmail.com] Sent: Sunday, October 25, 2015 10:50 PM To: Alex Courbot acourbot@nvidia.com Cc: Tom Warren TWarren@nvidia.com; Stephen Warren swarren@nvidia.com; Thierry Reding treding@nvidia.com; u- boot@lists.denx.de; linux-tegra@vger.kernel.org Subject: Re: [PATCH 0/4] ARM: tegra: GPU WPR region support
Ping Tom, how does this look to you?
Looks pretty good, but what about saving the security_carveout reg settings
back to the BCT or scratch regs so those settings will be restored on LP0 resume?
Absolutely - I am not familiar at all with BCT or scratch registers (and U-boot in general :) ) though, could you point me to some part of the code which I could use as a reference for this?
Also, how can I decide which mechanism to use over the other?
I'll have to refresh my meat RAM on how U-Boot handles LP0 WRT BCT/scratch regs, but in coreboot I had to 'flush' the updated carveout regs back to the BCT copy in SDRAM so that they'd be properly restored on resume. I only pushed the modified regs (BOM and CFG0, IIRC). If you have access to Google's coreboot repo, look in src/soc/nvidia/tegra210/sdram_lp0.c, commit ID 920968258. Or just Google for sdram_lp0.c, should be the top hit.
Thanks, Alex.
participants (5)
-
Alexandre Courbot
-
Alexandre Courbot
-
Stephen Warren
-
Thierry Reding
-
Tom Warren