[U-Boot] [U-Boot,V3,1/2] spl: Fix redundant image of uboot

[Tom Rini]

On Fri, Jul 20, 2018 at 10:27:26PM +0200, Michael Nazzareno Trimarchi wrote:

> Hi
> 
> On Fri, Jul 20, 2018 at 10:11 PM, Tom Rini <trini at konsulko.com> wrote:
> > On Fri, Jul 20, 2018 at 10:09:00PM +0200, Michael Nazzareno Trimarchi wrote:
> >> Hi Tom
> >>
> >> On Fri, Jul 20, 2018 at 9:54 PM, Tom Rini <trini at konsulko.com> wrote:
> >> > On Fri, Jul 06, 2018 at 05:09:22PM +0200, Michael Trimarchi wrote:
> >> >
> >> >> We need to address the redundat image case and undestand if the
> >> >> image is corrupted or not. In error case we need to try the fallback copy.
> >> >> The function used before was always return 0 without any evaluation of the
> >> >> error. We try to make it work properly
> >> >>
> >> >> Signed-off-by: Michael Trimarchi <michael at amarulasolutions.com>
> >> >> ---
> >> >> Changes V2->V3:
> >> >>         Fix patch mistake due the a wrong edit of it
> >> >> Changes V1->V2:
> >> >>         Address the comments on using the err variable
> >> >> ---
> >> >>  common/spl/spl_nand.c | 34 +++++++++++++++++++++++++---------
> >> >>  1 file changed, 25 insertions(+), 9 deletions(-)
> >> >
> >> > I see two problems here.  First, this is a generic issue (any
> >> > legacy-style U-Boot image that we load should be verified).  Second, we
> >> > need to make this behavior configurable as as-is this overflows one
> >> > board (omapl138_lcdk) and I expect would be problematic for many more
> >> > boards when we make it done more commonly.
> >>
> >> This patch fix a no-working uboot feature and this was the address problem on
> >> the specific case. We can call ->verify image every ->load, anyway can you
> >> explain better why you need a configurable behavior?
> >
> > It fixes the legitimate case of having read in bad data and the
> > controller didn't return up an error to us, and it wasn't in the header,
> > yes.  And it needs to be configurable as adding in these checks
> > increases the binary size and various targets will fail to build, such
> > as omapl138_lcdk does with your patch as-is.  Thanks!
> >
> 
> +#if defined(CONFIG_VERIFY_IMAGE)

CONFIG_SPL_VERIFY_IMAGE, for namespace.

> +int verify_image(spl_image)
> +{
> +...
> +}
> +#else
> +int verify_image(spl_image) { return 0; }
> +#endif
> +
>  static int spl_load_image(struct spl_image_info *spl_image,
>                           struct spl_image_loader *loader)
>  {
>         struct spl_boot_device bootdev;
> +       int ret;
> 
>         bootdev.boot_device = loader->boot_device;
>         bootdev.boot_device_name = NULL;
> 
> -       return loader->load_image(spl_image, &bootdev);
> +       ret = loader->load_image(spl_image, &bootdev);
> +       if (ret)
> +               return -EINVAL;
> +
> +       return verify_image(spl_image);
>  }
> 
> Somenthing like this?
> 
> or return loader->verify_image()

Yes, something like that.  And please pass it through travis-ci for the
world build and seeing if anything fails to fit into size constraints,
thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180720/083718d0/attachment.sig>