[PATCH v8 02/13] FWU: Add FWU metadata structure and driver for accessing metadata

[Jassi Brar]

On Fri, Aug 19, 2022 at 10:25 AM Simon Glass <sjg at chromium.org> wrote:
>
> Hi Jassi,
>
> On Fri, 19 Aug 2022 at 08:59, Jassi Brar <jassisinghbrar at gmail.com> wrote:
> >
> > Hi Simon,
> >
> > On Thu, Aug 18, 2022 at 12:50 PM Simon Glass <sjg at chromium.org> wrote:
> >
> > > > > > +/**
> > > > > > + * struct fwu_image_bank_info - firmware image information
> > > > > > + * @image_uuid: Guid value of the image in this bank
> > > > > > + * @accepted: Acceptance status of the image
> > > > > > + * @reserved: Reserved
> > > > > > + *
> > > > > > + * The structure contains image specific fields which are
> > > > > > + * used to identify the image and to specify the image's
> > > > > > + * acceptance status
> > > > > > + */
> > > > > > +struct fwu_image_bank_info {
> > > > > > +       efi_guid_t  image_uuid;
> > > > > > +       uint32_t accepted;
> > > > > > +       uint32_t reserved;
> > > > > > +} __attribute__((__packed__));
> > > > >
> > > > > Why is this packed?
> > > >
> > > > This was based on a review comment from Masami [1], as he wanted to
> > > > use the same structure in the low level bootloader as well.
> > >
> > > It doesn't actually make any sense though. The packed struct is the same as
> > > the normal struct, so far as I can tell. What am I missing?
> > >
> > I think because we want the structure to be read/written onto
> > persistent storage, and possibly manipulated by entities other than
> > uboot.
>
> But specifically, how does __packed help here? What are the other
> entities doing that changes the format and what is __packed doing to
> update that? This is actually standard C code.
>
It is just to make sure there is no padding between structure members,
and the size of structure is exactly as in the specification.
Though in this case it likely won't make any difference because the
members are already 16, 4 and 4 bytes.

Cheers.