[PATCH v4 7/7] smbios: Require the caller to align the SMBIOS table

[Simon Glass]

Hi Ilias,

On Wed, Dec 27, 2023 at 11:12 AM Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> Hi Simon,
>
> I commented on v3 as well, but in case you miss that
>
> On Wed, 27 Dec 2023 at 09:40, Simon Glass <sjg at chromium.org> wrote:
> >
> > All callers handle this alignment, so drop the unnecessary code. This
> > simplifies things a little.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > Reviewed-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> > ---
> >
> > (no changes since v1)
> >
> >  include/smbios.h | 5 +----
> >  lib/smbios.c     | 2 --
> >  2 files changed, 1 insertion(+), 6 deletions(-)
> >
> > diff --git a/include/smbios.h b/include/smbios.h
> > index 77be58887a2..879b8a814b8 100644
> > --- a/include/smbios.h
> > +++ b/include/smbios.h
> > @@ -258,12 +258,9 @@ static inline void fill_smbios_header(void *table, int type,
> >   *
> >   * This writes SMBIOS table at a given address.
> >   *
> > - * @addr:      start address to write SMBIOS table. If this is not
> > - *             16-byte-aligned then it will be aligned before the table is
> > - *             written.
> > + * @addr:      start address to write SMBIOS table (must be 16-byte-aligned)
> >   * Return:     end address of SMBIOS table (and start address for next entry)
> >   *             or NULL in case of an error
> > - *
> >   */
> >  ulong write_smbios_table(ulong addr);
> >
> > diff --git a/lib/smbios.c b/lib/smbios.c
> > index 7f79d969c92..cfd451e4088 100644
> > --- a/lib/smbios.c
> > +++ b/lib/smbios.c
> > @@ -563,8 +563,6 @@ ulong write_smbios_table(ulong addr)
> >                 ctx.dev = NULL;
> >         }
> >
> > -       /* 16 byte align the table address */
> > -       addr = ALIGN(addr, 16);
>
> I think this is wrong.  It will break SMBIOS on a user error. I am
> fine replacing that with a check instead and error out if the address
> is not 16b aligned

Well this is why the comment says it must be aligned. This function is
not called willy nilly, from code we control. Checking for alignment
at runtime creates confusion and adds to code size.

>
> Thanks
> /Ilias
> >         start_addr = addr;
> >
> >         /*
> > --
> > 2.34.1
> >

Regards,
Simon