[U-Boot] [PATCH 1/2] spl: atf: add SPL_ATF_NO_PLATFORM_PARAM option
While we expect to call a pointer to a valid FDT (or NULL) as the platform parameter to an ATF, some ATF versions are not U-Boot aware and have an insufficiently robust (or an overzealour) parameter validation: either way, this may cause a hard-stop with uncooperative ATF versions.
This change adds the option to suppress passing a platform parameter and will always pass NULL.
Debug output from ATF w/ this option disabled (i.e. default): INFO: plat_param_from_bl2: 0x291450 Debug output from ATF w/ this option enabled: INFO: plat_param_from_bl2: 0
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com Tested-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com ---
common/spl/Kconfig | 18 ++++++++++++++++-- common/spl/spl_atf.c | 12 +++++++++++- 2 files changed, 27 insertions(+), 3 deletions(-)
diff --git a/common/spl/Kconfig b/common/spl/Kconfig index aef0034..9d35f41 100644 --- a/common/spl/Kconfig +++ b/common/spl/Kconfig @@ -721,10 +721,24 @@ config SPL_ATF bool "Support ARM Trusted Firmware" depends on ARM64 help - ATF(ARM Trusted Firmware) is a component for ARM arch64 which - is loaded by SPL(which is considered as BL2 in ATF terminology). + ATF(ARM Trusted Firmware) is a component for ARM AArch64 which + is loaded by SPL (which is considered as BL2 in ATF terminology). More detail at: https://github.com/ARM-software/arm-trusted-firmware
+config SPL_ATF_NO_PLATFORM_PARAM + bool "Pass no platform parameter" + depends on SPL_ATF + help + While we expect to call a pointer to a valid FDT (or NULL) + as the platform parameter to an ATF, some ATF versions are + not U-Boot aware and have an insufficiently robust parameter + validation to gracefully reject a FDT being passed. + + If this option is enabled, the spl_atf os-type handler will + always pass NULL for the platform parameter. + + If your ATF is affected, say Y. + config TPL bool depends on SUPPORT_TPL diff --git a/common/spl/spl_atf.c b/common/spl/spl_atf.c index 63557c0..a942de9 100644 --- a/common/spl/spl_atf.c +++ b/common/spl/spl_atf.c @@ -144,6 +144,7 @@ void spl_invoke_atf(struct spl_image_info *spl_image) { uintptr_t bl33_entry = CONFIG_SYS_TEXT_BASE; void *blob = spl_image->fdt_addr; + uintptr_t platform_param = (uintptr_t)blob; int node;
/* @@ -158,8 +159,17 @@ void spl_invoke_atf(struct spl_image_info *spl_image) bl33_entry = spl_fit_images_get_entry(blob, node);
/* + * If ATF_NO_PLATFORM_PARAM is set, we override the platform + * parameter and always pass 0. This is a workaround for + * older ATF versions that have insufficiently robust (or + * overzealous) argument validation. + */ + if (CONFIG_IS_ENABLED(ATF_NO_PLATFORM_PARAM)) + platform_param = 0; + + /* * We don't provide a BL3-2 entry yet, but this will be possible * using similar logic. */ - bl31_entry(spl_image->entry_point, bl33_entry, (uintptr_t)blob); + bl31_entry(spl_image->entry_point, bl33_entry, platform_param); }
The Rockchip-released ATF for the Firefly apparently (i.e. Kever reported this) does not tolerate a FDT being passed as the platform parameter and will run into a hard stop.
To work around this limitation in the ATF parameter handling, we enable SPL_ATF_NO_PLATFORM_PARAM (which will force passing NULL for the platform parameters).
Note that this only affects this platform, as the ATF releases for the RK3368 and RK3399 have always either ignored the platform parameter (i.e. before the FDT-based parameters were supported) or support receiving a pointer to a FDT.
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com ---
configs/firefly-rk3399_defconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/configs/firefly-rk3399_defconfig b/configs/firefly-rk3399_defconfig index ab25015..4071fea 100644 --- a/configs/firefly-rk3399_defconfig +++ b/configs/firefly-rk3399_defconfig @@ -15,6 +15,7 @@ CONFIG_SPL_FIT_GENERATOR="arch/arm/mach-rockchip/make_fit_atf.py" CONFIG_SPL_STACK_R=y CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x4000 CONFIG_SPL_ATF=y +CONFIG_SPL_ATF_NO_PLATFORM_PARAM=y CONFIG_CMD_BOOTZ=y CONFIG_CMD_GPT=y CONFIG_CMD_MMC=y
Hi Philipp,
On 01/03/2018 04:16 AM, Philipp Tomsich wrote:
The Rockchip-released ATF for the Firefly apparently (i.e. Kever reported this) does not tolerate a FDT being passed as the platform parameter and will run into a hard stop.
To work around this limitation in the ATF parameter handling, we enable SPL_ATF_NO_PLATFORM_PARAM (which will force passing NULL for the platform parameters).
Note that this only affects this platform, as the ATF releases for the RK3368 and RK3399 have always either ignored the platform parameter (i.e. before the FDT-based parameters were supported) or support receiving a pointer to a FDT.
Upstream ATF of Rockchip platform support a vendor defined " structbl31_plat_param *bl2_param", will add support for FDT later, see params_early_setup() in https://github.com/ARM-software/arm-trusted-firmware/blob/master/plat/rockch...
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
Reviewed-by: Kever Yang <kever.yang＠rock-chips.com>
Thanks, - Kever
configs/firefly-rk3399_defconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/configs/firefly-rk3399_defconfig b/configs/firefly-rk3399_defconfig index ab25015..4071fea 100644 --- a/configs/firefly-rk3399_defconfig +++ b/configs/firefly-rk3399_defconfig @@ -15,6 +15,7 @@ CONFIG_SPL_FIT_GENERATOR="arch/arm/mach-rockchip/make_fit_atf.py" CONFIG_SPL_STACK_R=y CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x4000 CONFIG_SPL_ATF=y +CONFIG_SPL_ATF_NO_PLATFORM_PARAM=y CONFIG_CMD_BOOTZ=y CONFIG_CMD_GPT=y CONFIG_CMD_MMC=y
Kever,
On 3 Jan 2018, at 03:34, Kever Yang kever.yang@rock-chips.com wrote:
Hi Philipp,
On 01/03/2018 04:16 AM, Philipp Tomsich wrote:
The Rockchip-released ATF for the Firefly apparently (i.e. Kever reported this) does not tolerate a FDT being passed as the platform parameter and will run into a hard stop.
To work around this limitation in the ATF parameter handling, we enable SPL_ATF_NO_PLATFORM_PARAM (which will force passing NULL for the platform parameters).
Note that this only affects this platform, as the ATF releases for the RK3368 and RK3399 have always either ignored the platform parameter (i.e. before the FDT-based parameters were supported) or support receiving a pointer to a FDT.
Upstream ATF of Rockchip platform support a vendor defined " struct bl31_plat_param *bl2_param", will add support for FDT later, see params_early_setup() in https://github.com/ARM-software/arm-trusted-firmware/blob/master/plat/rockch...
I have cleaning up and submitting (in the meantime, they are publicly available from our public GIT server anyway) the ATF patches on my to-do list, so let me know once this becomes urgent for you.
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
Reviewed-by: Kever Yang <kever.yang＠rock-chips.com>
Thanks,
- Kever
configs/firefly-rk3399_defconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/configs/firefly-rk3399_defconfig b/configs/firefly-rk3399_defconfig index ab25015..4071fea 100644 --- a/configs/firefly-rk3399_defconfig +++ b/configs/firefly-rk3399_defconfig @@ -15,6 +15,7 @@ CONFIG_SPL_FIT_GENERATOR="arch/arm/mach-rockchip/make_fit_atf.py" CONFIG_SPL_STACK_R=y CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x4000 CONFIG_SPL_ATF=y +CONFIG_SPL_ATF_NO_PLATFORM_PARAM=y CONFIG_CMD_BOOTZ=y CONFIG_CMD_GPT=y CONFIG_CMD_MMC=y
Hi Kever,
On 3.1.2018 03:34, Kever Yang wrote:
Hi Philipp,
On 01/03/2018 04:16 AM, Philipp Tomsich wrote:
The Rockchip-released ATF for the Firefly apparently (i.e. Kever reported this) does not tolerate a FDT being passed as the platform parameter and will run into a hard stop.
To work around this limitation in the ATF parameter handling, we enable SPL_ATF_NO_PLATFORM_PARAM (which will force passing NULL for the platform parameters).
Note that this only affects this platform, as the ATF releases for the RK3368 and RK3399 have always either ignored the platform parameter (i.e. before the FDT-based parameters were supported) or support receiving a pointer to a FDT.
Upstream ATF of Rockchip platform support a vendor defined " structbl31_plat_param *bl2_param", will add support for FDT later, see params_early_setup() in https://github.com/ARM-software/arm-trusted-firmware/blob/master/plat/rockch...
What exactly do you want to configure in ATF from FDT? Do you have any code somewhere to take a look?
Thanks, Michal
Michal,
On 4 Jan 2018, at 08:56, Michal Simek monstr@monstr.eu wrote:
Hi Kever,
On 3.1.2018 03:34, Kever Yang wrote:
Hi Philipp,
On 01/03/2018 04:16 AM, Philipp Tomsich wrote:
The Rockchip-released ATF for the Firefly apparently (i.e. Kever reported this) does not tolerate a FDT being passed as the platform parameter and will run into a hard stop.
To work around this limitation in the ATF parameter handling, we enable SPL_ATF_NO_PLATFORM_PARAM (which will force passing NULL for the platform parameters).
Note that this only affects this platform, as the ATF releases for the RK3368 and RK3399 have always either ignored the platform parameter (i.e. before the FDT-based parameters were supported) or support receiving a pointer to a FDT.
Upstream ATF of Rockchip platform support a vendor defined " structbl31_plat_param *bl2_param", will add support for FDT later, see params_early_setup() in https://github.com/ARM-software/arm-trusted-firmware/blob/master/plat/rockch...
What exactly do you want to configure in ATF from FDT? Do you have any code somewhere to take a look?
On the RK3399-Q7, we configure both what our sysreset-gpio is and signal the location from where the PMU firmware should be relocated to its final location.
For an older versions of these changes (these are currently being reworked for upstreaming), see: https://git.theobroma-systems.com/arm-trusted-firmware.git/commit/?id=07b6f3...
Regards, Philipp.
Thanks, Michal
-- Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91 w: www.monstr.eu http://www.monstr.eu/ p: +42-0-721842854 Maintainer of Linux kernel - Xilinx Microblaze Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP SoCs
Michal,
On 01/04/2018 03:56 PM, Michal Simek wrote:
Hi Kever,
On 3.1.2018 03:34, Kever Yang wrote:
Hi Philipp,
On 01/03/2018 04:16 AM, Philipp Tomsich wrote:
The Rockchip-released ATF for the Firefly apparently (i.e. Kever reported this) does not tolerate a FDT being passed as the platform parameter and will run into a hard stop.
To work around this limitation in the ATF parameter handling, we enable SPL_ATF_NO_PLATFORM_PARAM (which will force passing NULL for the platform parameters).
Note that this only affects this platform, as the ATF releases for the RK3368 and RK3399 have always either ignored the platform parameter (i.e. before the FDT-based parameters were supported) or support receiving a pointer to a FDT.
Upstream ATF of Rockchip platform support a vendor defined " structbl31_plat_param *bl2_param", will add support for FDT later, see params_early_setup() in https://github.com/ARM-software/arm-trusted-firmware/blob/master/plat/rockch...
What exactly do you want to configure in ATF from FDT? Do you have any code somewhere to take a look?
Sorry, no code for it now. I think we will use and append reserved memory for it. SPL set available memory region, and ATF reserved for its TA so that U-Boot won't use it, we are using the hard code for this feature now.
Thanks, - Kever
Thanks, Michal
The Rockchip-released ATF for the Firefly apparently (i.e. Kever reported this) does not tolerate a FDT being passed as the platform parameter and will run into a hard stop.
To work around this limitation in the ATF parameter handling, we enable SPL_ATF_NO_PLATFORM_PARAM (which will force passing NULL for the platform parameters).
Note that this only affects this platform, as the ATF releases for the RK3368 and RK3399 have always either ignored the platform parameter (i.e. before the FDT-based parameters were supported) or support receiving a pointer to a FDT.
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com Reviewed-by: Kever Yang <kever.yang＠rock-chips.com>
configs/firefly-rk3399_defconfig | 1 + 1 file changed, 1 insertion(+)
Applied to u-boot-rockchip, thanks!
On 01/03/2018 04:16 AM, Philipp Tomsich wrote:
While we expect to call a pointer to a valid FDT (or NULL) as the platform parameter to an ATF, some ATF versions are not U-Boot aware and have an insufficiently robust (or an overzealour) parameter validation: either way, this may cause a hard-stop with uncooperative ATF versions.
This change adds the option to suppress passing a platform parameter and will always pass NULL.
Debug output from ATF w/ this option disabled (i.e. default): INFO: plat_param_from_bl2: 0x291450 Debug output from ATF w/ this option enabled: INFO: plat_param_from_bl2: 0
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com Tested-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
common/spl/Kconfig | 18 ++++++++++++++++-- common/spl/spl_atf.c | 12 +++++++++++- 2 files changed, 27 insertions(+), 3 deletions(-)
diff --git a/common/spl/Kconfig b/common/spl/Kconfig index aef0034..9d35f41 100644 --- a/common/spl/Kconfig +++ b/common/spl/Kconfig @@ -721,10 +721,24 @@ config SPL_ATF bool "Support ARM Trusted Firmware" depends on ARM64 help
ATF(ARM Trusted Firmware) is a component for ARM arch64 whichis loaded by SPL(which is considered as BL2 in ATF terminology).
ATF(ARM Trusted Firmware) is a component for ARM AArch64 whichis loaded by SPL (which is considered as BL2 in ATF terminology).+config SPL_ATF_NO_PLATFORM_PARAM
bool "Pass no platform parameter"- depends on SPL_ATF
- help
While we expect to call a pointer to a valid FDT (or NULL)as the platform parameter to an ATF, some ATF versions arenot U-Boot aware and have an insufficiently robust parametervalidation to gracefully reject a FDT being passed.If this option is enabled, the spl_atf os-type handler willalways pass NULL for the platform parameter.If your ATF is affected, say Y.- config TPL bool depends on SUPPORT_TPL
diff --git a/common/spl/spl_atf.c b/common/spl/spl_atf.c index 63557c0..a942de9 100644 --- a/common/spl/spl_atf.c +++ b/common/spl/spl_atf.c @@ -144,6 +144,7 @@ void spl_invoke_atf(struct spl_image_info *spl_image) { uintptr_t bl33_entry = CONFIG_SYS_TEXT_BASE; void *blob = spl_image->fdt_addr;
uintptr_t platform_param = (uintptr_t)blob; int node;
/*
@@ -158,8 +159,17 @@ void spl_invoke_atf(struct spl_image_info *spl_image) bl33_entry = spl_fit_images_get_entry(blob, node);
/*
* If ATF_NO_PLATFORM_PARAM is set, we override the platform* parameter and always pass 0. This is a workaround for* older ATF versions that have insufficiently robust (or* overzealous) argument validation.*/- if (CONFIG_IS_ENABLED(ATF_NO_PLATFORM_PARAM))
platform_param = 0;- /*
*/
- We don't provide a BL3-2 entry yet, but this will be possible
- using similar logic.
- bl31_entry(spl_image->entry_point, bl33_entry, (uintptr_t)blob);
- bl31_entry(spl_image->entry_point, bl33_entry, platform_param); }
Reviewed-by: Kever Yang kever.yang@rock-chips.com
Thanks, - Kever
While we expect to call a pointer to a valid FDT (or NULL) as the platform parameter to an ATF, some ATF versions are not U-Boot aware and have an insufficiently robust (or an overzealour) parameter validation: either way, this may cause a hard-stop with uncooperative ATF versions.
This change adds the option to suppress passing a platform parameter and will always pass NULL.
Debug output from ATF w/ this option disabled (i.e. default): INFO: plat_param_from_bl2: 0x291450 Debug output from ATF w/ this option enabled: INFO: plat_param_from_bl2: 0
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com Tested-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com Reviewed-by: Kever Yang kever.yang@rock-chips.com
common/spl/Kconfig | 18 ++++++++++++++++-- common/spl/spl_atf.c | 12 +++++++++++- 2 files changed, 27 insertions(+), 3 deletions(-)
Applied to u-boot-rockchip, thanks!
participants (4)
-
Dr. Philipp Tomsich -
Kever Yang -
Michal Simek -
Philipp Tomsich