Hi Heinrich, On Thu, Dec 28, 2023 at 5:36 PM Heinrich Schuchardt <xypron.g...@gmx.de> wrote: > > > > Am 28. Dezember 2023 14:37:19 MEZ schrieb Simon Glass <s...@chromium.org>: > >Hi Ilias, > > > >On Wed, Dec 27, 2023 at 11:12 AM Ilias Apalodimas > ><ilias.apalodi...@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 <s...@chromium.org> wrote: > >> > > >> > All callers handle this alignment, so drop the unnecessary code. This > >> > simplifies things a little. > >> > > >> > Signed-off-by: Simon Glass <s...@chromium.org> > >> > Reviewed-by: Heinrich Schuchardt <xypron.g...@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. > > The SMBIOS tables themself have no alignment requirement. Only on non UEFI > x86 system presenting a UEFI entry point in the range 000F0000h to 000FFFFFh > this copy of the entry point has to be 16 byte aligned.
OK. So perhaps I should reword the comment to say that any arch-specific alignment must be respected by the caller? Regards, Simon