Re: [PATCH v7 00/10] Consolidate IO memcpy functions

[Niklas Schnelle <schnelle@linux.ibm.com>]

On Mon, 2024-09-30 at 15:23 +0200, Julian Vetter wrote:
> Thank you all for your remarks. I have addressed your feedback. I have
> also added the full history of the patchset, because it now targets
> additional architectures.
> 
> Arnd: Unfortunately when adding the prototypes as 'extern ..' into
> asm-generic/io.h they conflict with some individual implementations
> (namely m68k, alpha, parisc, and sh). So, I have aligned these functions
> to match the prototypes in asm-generic/io.h.
> For the um problem, unfortunately there are A LOT of drivers that use
> these IO memcpy functions, so I went a bit the lazy route and added
> dummy functions to um's io.h.
> 
> David: Thank you for your remarks. I have replaced the mix of long,
> uintptr_t, etc. all by long + sizeof(long). I have also split the
> read/write operation from the put/get_unaligned into two lines for
> better readability.
> 
> Signed-off-by: Julian Vetter <jvetter@kalrayinc.com>
> ---
> 

Hi Julian,

It seems you missed the memcpy_toio()/memcpy_fromio() in
arch/s390/include/asm/io.h, probably because these are macros
and minimal configs might not build s390x with PCI enabled.

One snippet from the error output is:

In file included from arch/s390/kernel/asm-offsets.c:11:
In file included from ./include/linux/kvm_host.h:19:
In file included from ./include/linux/msi.h:24:
In file included from ./include/linux/irq.h:20:
In file included from ./include/linux/io.h:14:
In file included from ./arch/s390/include/asm/io.h:93:
./include/asm-generic/io.h:105:13: error: conflicting types for 'zpci_memcpy_fromio'
  105 | extern void memcpy_fromio(void *to, const volatile void __iomem *from,
      |             ^
./arch/s390/include/asm/io.h:61:40: note: expanded from macro 'memcpy_fromio'
   61 | #define memcpy_fromio(dst, src, count)  zpci_memcpy_fromio(dst, src, count)
      |                                         ^
./arch/s390/include/asm/pci_io.h:144:19: note: previous definition is here
  144 | static inline int zpci_memcpy_fromio(void *dst,
      |                   ^

I think the best course of action might be to change the
zpci_memcpy_…() functions to match the generic signatures. While the
generic implementation might work it would be very inefficient for us
as we really need to use the PCI Store Block instructions.

Thanks,
Niklas