[PATCHv4 2/5] fwu: move meta-data management in core

[Jassi Brar]

Hi Ilias,

On Thu, Feb 23, 2023 at 2:36 AM Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
> > +int fwu_get_verified_mdata(struct fwu_mdata *mdata)
> > +{
> > +     int err;
> > +     bool pri_ok, sec_ok;
> > +     struct fwu_mdata s, *p_mdata, *s_mdata;
> > +
> > +     p_mdata = &g_mdata;
> > +     s_mdata = &s;
> > +
> > +     /* if mdata already read and ready */
> > +     err = mdata_crc_check(p_mdata);
> > +     if (!err)
> > +             goto ret_mdata;
> > +     /* else read, verify and, if needed, fix mdata */
> > +
> > +     pri_ok = false;
> > +     err = fwu_read_mdata(g_dev, p_mdata, true);
> > +     if (!err) {
> > +             err = mdata_crc_check(p_mdata);
> > +             if (!err)
> > +                     pri_ok = true;
> > +             else
> > +                     log_debug("primary mdata: crc32 failed\n");
> > +     }
> > +
> > +     sec_ok = false;
> > +     err = fwu_read_mdata(g_dev, s_mdata, false);
> > +     if (!err) {
> > +             err = mdata_crc_check(s_mdata);
> > +             if (!err)
> > +                     sec_ok = true;
> > +             else
> > +                     log_debug("secondary mdata: crc32 failed\n");
> > +     }
>
> Isn't it better to define pri_ok, sec_ok and their equivalent mdata as
> arrays ? IOW something along the lines of
>
> bool parts_ok[2] = { false };
> struct fwu_mdata parts_mdata[2];
>
> parts_mdata[0] = &g_mdata;
> parts_mdata[1] = .....
> for (i = 0; i < 2; i++) {
>     err = fwu_read_mdata(g_dev, parts_mdata[i], !(i % 2) ? true : false);
>     if (!err)
>         err = mdata_crc_check(parts_mdata[i]);
>         etc....
> }
>
> > +
> > +     if (pri_ok && sec_ok) {
>
> And then also adjust this part?
>
> > +             /*
> > +              * Before returning, check that both the
> > +              * FWU metadata copies are the same.
> > +              */
> > +             err = memcmp(p_mdata, s_mdata, sizeof(struct fwu_mdata));
> > +             if (!err)
> > +                     goto ret_mdata;
> > +
> > +             /*
> > +              * If not, populate the secondary partition from the
> > +              * primary partition copy.
> > +              */
> > +             log_info("Both FWU metadata copies are valid but do not match.");
> > +             log_info(" Restoring the secondary partition from the primary\n");
> > +             sec_ok = false;
> > +     }
> > +
> > +     if (!pri_ok) {
> > +             memcpy(p_mdata, s_mdata, sizeof(struct fwu_mdata));
> > +             err = fwu_sync_mdata(p_mdata, PRIMARY_PART);
> > +             if (err) {
> > +                     log_debug("mdata : primary write failed\n");
> > +                     return err;
> > +             }
> > +     }
> > +
> > +     if (!sec_ok) {
> > +             memcpy(s_mdata, p_mdata, sizeof(struct fwu_mdata));
> > +             err = fwu_sync_mdata(s_mdata, SECONDARY_PART);
> > +             if (err) {
> > +                     log_debug("mdata : secondary write failed\n");
> > +                     return err;
> > +             }
> > +     }
>
> And this could also be folded into a for loop
>
I have done these modifications and submitted v5.

Thanks.