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

[Michael Nazzareno Trimarchi]

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)
+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()

Michael

> --
> Tom



-- 
| Michael Nazzareno Trimarchi                     Amarula Solutions BV |
| COO  -  Founder                                      Cruquiuskade 47 |
| +31(0)851119172                                 Amsterdam 1018 AM NL |
|                  [`as] http://www.amarulasolutions.com               |