On Mon, Nov 02, 2020 at 11:56:07AM +1100, David Gibson wrote:
> On Fri, Oct 30, 2020 at 02:56:58PM -0400, Tom Rini wrote:
> > Start with commit 6dcb8ba4 "libfdt: Add helpers for accessing unaligned
> > words" which introduced changes to support unaligned reads for ARM
> > platforms and 11738cf01f15 "libfdt: Don't use memcpy to handle unaligned
> > reads on ARM" to improve the performance of these helpers, libfdt has
> > defaulted to assuming that unaligned memory access could be a fatal
> > problem.
> >
> > Upon further discussion on the mailing list, we leave the improved
> > unaligned-safe memory load functions available if needed, but go back to
> > using fdt{32,64}_to_cpu() for access as generally platforms handle
> > unaligned access safely and there is still a sizable performance and
> > size impact of using the always-safe helpers in all cases.
>
> I think the basic change is ok, but there's some details I'd like to
> see changed.
>
> First, I'd like to see an alignment check added to fdt_probe_ro_().
OK, that's not hard. Do you want a new error code or FDT_ERR_INTERNAL
or something else?
> > Signed-off-by: Tom Rini <trini@xxxxxxxxxxxx>
> > ---
> > fdtget.c | 2 +-
> > libfdt/fdt_ro.c | 20 ++++++++++----------
> > libfdt/libfdt.h | 2 +-
> > 3 files changed, 12 insertions(+), 12 deletions(-)
> >
> > diff --git a/fdtget.c b/fdtget.c
> > index 777582e2d45f..7cee28718cbc 100644
> > --- a/fdtget.c
> > +++ b/fdtget.c
> > @@ -62,7 +62,7 @@ static int show_cell_list(struct display_info *disp, const char *data, int len,
> > for (i = 0; i < len; i += size, p += size) {
> > if (i)
> > printf(" ");
> > - value = size == 4 ? fdt32_ld((const fdt32_t *)p) :
> > + value = size == 4 ? fdt32_to_cpu(*(const fdt32_t *)p) :
>
> Second, getting rid of these ugly open-coded constructions was an
> additional reason I went to using fdt32_ld() everywhere. So I'd like
> to see both an assume-aligned and an unaligned-safe helper, rather
> than open-coding the load-and-byteswap everywhere.
OK, I think I've got an idea.
--
Tom
Attachment:
signature.asc
Description: PGP signature
