[U-Boot] [PATCH] armv8: sec_firmware: Add support for multiple loadables

[Sumit Garg]

> From: York Sun
> Sent: Monday, January 15, 2018 10:16 PM
> 
> On 01/14/2018 08:55 PM, Sumit Garg wrote:
> > Enable support for multiple loadable images in SEC firmware FIT image.
> >
> > Signed-off-by: Sumit Garg <sumit.garg at nxp.com>
> > ---
> >  arch/arm/cpu/armv8/sec_firmware.c | 51
> > +++++++++++++++++++++++++++++++--------
> >  1 file changed, 41 insertions(+), 10 deletions(-)
> >
> > diff --git a/arch/arm/cpu/armv8/sec_firmware.c
> > b/arch/arm/cpu/armv8/sec_firmware.c
> > index 927eae4..28de81c 100644
> > --- a/arch/arm/cpu/armv8/sec_firmware.c
> > +++ b/arch/arm/cpu/armv8/sec_firmware.c
> > @@ -116,11 +116,13 @@ static int sec_firmware_check_copy_loadable(const
> void *sec_firmware_img,
> >  					    u32 *loadable_l, u32 *loadable_h)  {
> >  	phys_addr_t sec_firmware_loadable_addr = 0;
> > -	int conf_node_off, ld_node_off;
> > +	int conf_node_off, ld_node_off, images;
> >  	char *conf_node_name = NULL;
> >  	const void *data;
> >  	size_t size;
> >  	ulong load;
> > +	const char *name, *str, *type;
> > +	int len;
> >
> >  	conf_node_name = SEC_FIRMEWARE_FIT_CNF_NAME;
> >
> > @@ -130,11 +132,32 @@ static int sec_firmware_check_copy_loadable(const
> void *sec_firmware_img,
> >  	return -ENOENT;
> >  	}
> >
> > -	ld_node_off = fit_conf_get_prop_node(sec_firmware_img,
> conf_node_off,
> > -					     FIT_LOADABLE_PROP);
> > -	if (ld_node_off >= 0) {
> > -		printf("SEC Firmware: '%s' present in config\n",
> > -		       FIT_LOADABLE_PROP);
> > +	/* find the node holding the images information */
> > +	images = fdt_path_offset(sec_firmware_img, FIT_IMAGES_PATH);
> > +	if (images < 0) {
> > +		debug("%s: Cannot find /images node: %d\n", __func__,
> images);
> 
> Do you expect this often? If it should happen, change debug to printf.

No this isn't expected as fit image should at least have ppa monitor image.

> 
> > +		return -1;
> > +	}
> > +
> > +	type = FIT_LOADABLE_PROP;
> > +
> > +	name = fdt_getprop(sec_firmware_img, conf_node_off, type, &len);
> > +	if (!name) {
> > +		/* Loadables not present */
> > +		return 0;
> > +	}
> > +
> > +	printf("SEC Firmware: '%s' present in config\n", type);
> 
> How about replace printf() with debug()?

Actually I wanted to differentiate from u-boot logs whether there were any "loadables" in ppa fit image or not.

> 
> > +
> > +	for (str = name; str && ((str - name) < len);
> > +	     str = strchr(str, '\0') + 1) {
> > +		printf("%s: '%s'\n", type, str);
> > +		ld_node_off = fdt_subnode_offset(sec_firmware_img,
> images, str);
> > +		if (ld_node_off < 0) {
> > +			printf("cannot find image node '%s': %d\n", str,
> > +			       ld_node_off);
> > +			return -EINVAL;
> > +		}
> >
> >  		/* Verify secure firmware image */
> >  		if (!(fit_image_verify(sec_firmware_img, ld_node_off))) { @@
> > -164,11 +187,19 @@ static int sec_firmware_check_copy_loadable(const void
> *sec_firmware_img,
> >  		memcpy((void *)sec_firmware_loadable_addr, data, size);
> >  		flush_dcache_range(sec_firmware_loadable_addr,
> >  				   sec_firmware_loadable_addr + size);
> > -	}
> >
> > -	/* Populate address ptrs for loadable image with loadbale addr */
> > -	out_le32(loadable_l, (sec_firmware_loadable_addr & WORD_MASK));
> > -	out_le32(loadable_h, (sec_firmware_loadable_addr >>
> WORD_SHIFT));
> > +		/* Populate loadable address only for Trusted OS */
> > +		if (!strcmp(str, "trustedOS at 1")) {
> > +			/*
> > +			 * Populate address ptrs for loadable image with
> > +			 * loadbale addr
> > +			 */
> > +			out_le32(loadable_l, (sec_firmware_loadable_addr &
> > +					      WORD_MASK));
> > +			out_le32(loadable_h, (sec_firmware_loadable_addr >>
> > +					      WORD_SHIFT));
> > +		}
> > +	}
> >
> >  	return 0;
> >  }
> >
> 
> It is unclear that you are looking for "trustedOS at 1" to set the address.

Actually this patch is meant to load "fuse provisioning script" used by PPA to provision fuses on our SoC's in PPA DDR space (2MB). And this allocation of PPA DDR space for this script will be fixed and hardcoded in PPA.

But in case of trusted OS with own DDR space (64MB), we have to communicate load address of trusted OS to PPA to initialize it.

> Please add some document to describe the FIT image you are expecting.

Sure I will add a document to describe the FIT image.

-Sumit