Re: Linux 6.11-rc1

[Arnd Bergmann]

On Mon, Jul 29, 2024, at 21:50, Linus Torvalds wrote:
> On Mon, 29 Jul 2024 at 12:23, Linus Torvalds
> <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>>
>> And that fix (if it fixes it - I think it will) still leaves the alpha
>> allmodconfig build and all the failed tests.
>>
>> I'll take a look.
>
> Well, the alpha allmodconfig case is apparently
>
>   ERROR: modpost: "iowrite64be" [drivers/crypto/caam/caam_jr.ko] undefined!
>
> which I suspect it just a result of commit beba3771d9e0 ("crypto:
> caam: Make CRYPTO_DEV_FSL_CAAM dependent of COMPILE_TEST").
>
> IOW, that is almost certainly simply due to better build test
> coverage, not a new bug.
>
> But I didn't look into *why* it would fail. We have a comment about
> iowrite64be saying
>
>  * These get provided from <asm-generic/iomap.h> since alpha does not
>  * select GENERIC_IOMAP.
>
> and I'm not sure why that isn't correct.
>
> I get a feeling that lib/iomap.c is missing a couple of functions, but
> didn't look into it a lot.
>
> I suspect Arnd may be the right person to ask. Arnd?

Yes, I've noticed this problem a few weeks ago with another
driver as we tried to fix the usage of iowrite64() on 32-bit
architectures. We actually have two old bugs here and still
need to make a decision about how to fix that properly:

- ioread64()/iowrite64() and their variants are defined
  differently on architectures depending on whether they use
  CONFIG_GENERIC_IOMAP (x86, um, and a few rare configs
  elsewhere) or not. On GENERIC_IOMAP architectures, there
  is no 64-bit PIO, so lib/iomap.c only provides the
  iowrite64_hi_lo()/iowrite64_lo_hi() etc wrappers that do
  a pair of 32-bit accessors for PIO but native 64-bit
  MMIO. On other 64-bit architectures, iowrite64() is the
  same as writeq() and it can operate on PCI I/O space as
  well. Drivers with big-endian registers tend to use
  iowriteXXbe() in order to the correct byteswap in the
  absence of writeX_be().

- Alpha (and I think parisc) uses the asm-generic/iomap.h
  header that is meant for GENERIC_IOMAP but then provides
  its own functions. It never had iowrite64be() and we
  didn't notice this in the absence of users. The caam driver
  includes include/linux/io-64-nonatomic-lo-hi.h, which
  then redirects iowrite64be() to iowrite64be_lo_hi()
  on x86 (since it does not define iowrite64be()) and
  on 32-bit architectures, but uses iowrite64be() from
  include/asm-generic/io.h on most other 64-bit
  architectures. On alpha it uses the incorrect
  prototype.

I suspect we can fix the alpha issue with the trivial
change below (haven't tested yet), but the way we are
inconsistent about these will likely keep biting us
unless we come up with a better way to handle them
across architectures.

      Arnd

diff --git a/arch/alpha/include/asm/io.h b/arch/alpha/include/asm/io.h
index 2bb8cbeedf91..52212e47e917 100644
--- a/arch/alpha/include/asm/io.h
+++ b/arch/alpha/include/asm/io.h
@@ -534,8 +534,10 @@ extern inline void writeq(u64 b, volatile void __iomem *addr)
 
 #define ioread16be(p) swab16(ioread16(p))
 #define ioread32be(p) swab32(ioread32(p))
+#define ioread64be(p) swab64(ioread64(p))
 #define iowrite16be(v,p) iowrite16(swab16(v), (p))
 #define iowrite32be(v,p) iowrite32(swab32(v), (p))
+#define iowrite64be(v,p) iowrite64(swab64(v), (p))
 
 #define inb_p          inb
 #define inw_p          inw
@@ -634,8 +637,6 @@ extern void outsl (unsigned long port, const void *src, unsigned long count);
  */
 #define ioread64 ioread64
 #define iowrite64 iowrite64
-#define ioread64be ioread64be
-#define iowrite64be iowrite64be
 #define ioread8_rep ioread8_rep
 #define ioread16_rep ioread16_rep
 #define ioread32_rep ioread32_rep