Re: [PATCH v5 19/23] FWU: synquacer: Add FWU Multi bank update support for DeveloperBox

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 2454 bytes --]

On Tue, Jul 19, 2022 at 10:23:08AM -0500, Jassi Brar wrote:
> On Mon, Jul 18, 2022 at 4:00 PM Tom Rini <trini@konsulko.com> wrote:
> > On Mon, Jul 18, 2022 at 10:31:56AM -0500, Jassi Brar wrote:
> 
> > > > > > > > > +
> > > > > > > > > +#define PLAT_METADATA_OFFSET 0x510000
> > > > > > > > > +#define PLAT_METADATA_SIZE   (sizeof(struct devbox_metadata))
> > > > > > > > > +
> > > > > > > > > +struct __packed devbox_metadata {
> > > > > > > > > +     u32 boot_index;
> > > > > > > > > +     u32 boot_count;
> > > > > > > >
> > > > > > > > There is the whole bootcount infrastructure for this. I think it would be much
> > > > > > > > better to use that framework instead of creating parallel one.
> > > > > > > >
> > > > > > > Yes, this goes too.
> > > > > >
> > > > > > Is bootcount really suited for this case?
> > > > > > AFAIK bootcount either requires device specific registers (which won't
> > > > > > reset on reboots), or an environment you can write data to.
> > > > > > But what if a user wants to disable writing the env variables and the
> > > > > > device doesn't have a set of registers we can use?
> > > > > >
> > > > > Maybe it should be moved in 'struct fwu_mdata' ?
> > > >
> > > > I was mostly thinking on moving this count as another 'bootcount'
> > > > method.  So in case the user has disabled writing evn variables but he
> > > > is booting with EFI he can use that.
> > >
> > > Sorry, not sure I understand.... IIUIC there has to be some persistent storage.
> >
> > No, there just has to be "somewhere" to do the counting.  We've got a
> > DDR backed driver, for example.  So yes, I think we should try and use
> > the bootcount framework here.
> >
> OK, for platforms that can preserve ram across reboot, using
> non-persistent storage can work.
> My platform neither preserves ram, nor has any warmreset-proof
> registers. So I have to choose between saving the bootcount in efi-env
> or in vendor specific structure next to the metadata. I prefer
> metadata because it is common to all stages of boot. Any corrections
> to this approach?

What I'm trying to say is that we have an abstraction for counting the
number of times the system has booted since something reset the counter
to zero, to signal the system is up and functional.  I'll leave the
details of how it's used here, and how / what backend is used or created
for it up to everyone else on the thread.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]