Hi Jassi
On Wed, 20 Jul 2022 at 17:30, Jassi Brar jassisinghbrar@gmail.com wrote:
On Wed, Jul 20, 2022 at 2:54 AM Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hi Jassi,
On Tue, 19 Jul 2022 at 18:27, Jassi Brar jaswinder.singh@linaro.org wrote:
On Mon, 18 Jul 2022 at 16:00, 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?
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.
Ok, in that case, you'll still have to implement this as a 'special' bootcount method since the A/B updates code will use that API to get/set the values.
Thanks /Ilias
thanks.