On Wed, Jul 20, 2022 at 2:54 AM Ilias Apalodimas <ilias.apalodi...@linaro.org> wrote: > > Hi Jassi, > > On Tue, 19 Jul 2022 at 18:27, Jassi Brar <jaswinder.si...@linaro.org> wrote: > > > > On Mon, 18 Jul 2022 at 16:00, Tom Rini <tr...@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? > > The metadata is defined by a spec and they don't have a field for > bootcounting. Once Sughosh resends his patches he'll include a > bootcount backend that reuses EFI variables. Can't we just use that? > Yes, I am aware metadata spec has no provision of vendor data. But there is nothing illegal in appending vendor-data to metadata and that is trivial to implement ... basically use sizeof(struct fwu_mdata) + sizeof(struct sni_vendor_mdata) while read/write meta-data. That will also be zero extra-overhead.
fwu-mdata { compatible = "u-boot,fwu-mdata-mtd"; fwu-mdata-store = <&spi_flash>; mdata-offsets = <0x500000 0x530000>; vendor-data-size = <0x100>; // optional }; Sure we can use an efi variable, but I see more uses of vendor-data :- shared among BL1/BL2/BL3x/OS so we can emulate reset-syndrome, crash-logging, per-image bootcount etc when the h/w doesn't support these features. Ofcourse, please feel free to implement efi-variables still. thanks.