mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Arnd Bergmann" <arnd@arndb.de>
To: "Julian Vetter" <jvetter@outer-limits.org>,
	"Russell King" <linux@armlinux.org.uk>
Cc: linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org,
	"Julian Vetter" <julian@outer-limits.org>,
	"Linus Walleij" <linus.walleij@linaro.org>
Subject: Re: [PATCH] arm: Remove IO memcpy for Big-Endian
Date: Tue, 03 Dec 2024 11:08:17 +0100	[thread overview]
Message-ID: <609b026d-d54c-4a11-b7df-6ef0ac315f25@app.fastmail.com> (raw)
In-Reply-To: <20241203083838.3260036-1-jvetter@outer-limits.org>

On Tue, Dec 3, 2024, at 09:38, Julian Vetter wrote:
> From: Julian Vetter <julian@outer-limits.org>
>
> Recently a new IO memcpy was added in libs/iomem_copy.c. So, remove the
> byte-wise IO memcpy operations used in ARM big endian builds and fall
> back to the new generic implementation. It will be slightly faster,
> because it uses machine word accesses if the memory is aligned and falls
> back to byte-wise accesses if its not.
>
> Signed-off-by: Julian Vetter <julian@outer-limits.org>
> ---
>  arch/arm/include/asm/io.h | 11 ----------
>  arch/arm/kernel/io.c      | 46 ---------------------------------------
>  2 files changed, 57 deletions(-)

I'm not sure if this is safe on all platforms. Big-endian arm
is extremely rare in practice, and in comes in multiple variants
that behave slightly differently:

- On modern ARMv7 the byte-invariant big-endian "BE8" mode
  generally well-behaved and works as one would expect it to.

- There is one ARMv5 "BE32" based platform, the ixp4xx, which
  works differently, and this in turn allows multiple configurations
  for its buses where a byte-swap is performed in the PCI
  controller.

When the little-endian I/O string operations got optimized to
calling the word-based helpers in commit 7ddfe625cbc1 ("ARM:
optimize memset_io()/memcpy_fromio()/memcpy_toio()"), Russell
intentionally left the big-endian versions alone, which I think
was done for the case of PCI on ixp4xx, but could have been
out of general caution.

Before we apply your patch, I think the minimum would be to
have Linus Walleij try it out on an an ixp4xx with a driver
that uses these functions. Maybe Russell remembers the exact
constraints that led to using byte access for big-endian
mmio string operations, and whether the new lib/iomem_copy.c
version causes problems.

I also looked at the little-endian arm32 version, and
it seems that here the generic code would work fine, but
the custom variant is likely much faster when both the
source and destination buffers are aligned, as it can
do larger MMIO transactions using ldm/stm instructions,
though the generic version would be a bit better if the
in-memory buffer is unaligned.
We could get the best of both by implementing optimized
arm32 versions __iowrite32_copy()/__ioread32_copy and
using those in the generic memcpy_{from,to}io.

       Arnd

  reply	other threads:[~2024-12-03 10:08 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-03  8:38 Julian Vetter
2024-12-03 10:08 ` Arnd Bergmann [this message]
2024-12-04  7:27   ` Julian Vetter
2024-12-04 14:59   ` Linus Walleij

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=609b026d-d54c-4a11-b7df-6ef0ac315f25@app.fastmail.com \
    --to=arnd@arndb.de \
    --cc=julian@outer-limits.org \
    --cc=jvetter@outer-limits.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®