Re: [PATCH] libfdt: Default to assuming aligned reads are OK

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]



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


[Index of Archives]     [Device Tree]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux