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

[Simon Glass]

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.

>
> I totally agree with all the rest of your comments on the patchset.

Regards,
Simon