Hello Sascha,
On 20.11.23 15:44, Sascha Hauer wrote:
> With dba1c26f70 we replaced request_sdram_region() for the PPA with
> reserve_sdram_region(). The effect is that the region is marked as
> reserved and mapped non executable. While this is desired for EL2, it
> also has the effect that we can't start the PPA anymore from EL3.
>
> Map the region cached/executable to start the PPA, then map it
> uncached/non executable once we are in EL2.
>
> Fixes: dba1c26f70 ("arm: layerscape: ppa: reserve SDRAM region for PPA")
> Signed-off-by: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>
> ---
> arch/arm/mach-layerscape/ppa.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/arch/arm/mach-layerscape/ppa.c b/arch/arm/mach-layerscape/ppa.c
> index 521e6b89da..43145a7ece 100644
> --- a/arch/arm/mach-layerscape/ppa.c
> +++ b/arch/arm/mach-layerscape/ppa.c
> @@ -4,6 +4,7 @@
>
> #include <common.h>
> #include <init.h>
> +#include <mmu.h>
> #include <firmware.h>
> #include <memory.h>
> #include <linux/sizes.h>
> @@ -54,17 +55,11 @@ static int ppa_init(void *ppa, size_t ppa_size, void *sec_firmware_addr)
> int ret;
> u32 *boot_loc_ptr_l, *boot_loc_ptr_h;
> struct ccsr_scfg __iomem *scfg = (void *)(LSCH2_SCFG_ADDR);
> - int el = current_el();
> struct fit_handle *fit;
> void *conf;
> const void *buf;
> unsigned long firmware_size;
>
> - if (el < 3) {
> - printf("EL%d, skip ppa init\n", el);
> - return 0;
> - }
> -
> boot_loc_ptr_l = &scfg->scratchrw[1];
> boot_loc_ptr_h = &scfg->scratchrw[0];
>
> @@ -115,6 +110,7 @@ int ls1046a_ppa_init(resource_size_t ppa_start, resource_size_t ppa_size)
> struct resource *res;
> void *ppa_fw;
> size_t ppa_fw_size;
> + int el = current_el();
> int ret;
>
> res = reserve_sdram_region("ppa", ppa_start, ppa_size);
> @@ -126,9 +122,13 @@ int ls1046a_ppa_init(resource_size_t ppa_start, resource_size_t ppa_size)
>
> get_builtin_firmware(ppa_ls1046a_bin, &ppa_fw, &ppa_fw_size);
>
> - ret = ppa_init(ppa_fw, ppa_fw_size, (void *)ppa_start);
> - if (ret)
> - return ret;
> + if (el == 3) {
> + remap_range((void *)ppa_start, ppa_size, MAP_CACHED);
> + ret = ppa_init(ppa_fw, ppa_fw_size, (void *)ppa_start);
> + remap_range((void *)ppa_start, ppa_size, MAP_UNCACHED);
> + if (ret)
> + return ret;
Isn't this still racy? I think the EL2 mapping should already have the
PPA area uncached _before_ jumping into PPA.
Cheers,
Ahmad
> + }
>
> of_register_fixup(of_fixup_reserved_memory, res);
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
