Re: [PATCH v2 1/2] vfio/pci: Enable iowrite64 and ioread64 for vfio pci

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

On Tue, 2024-12-03 at 10:41 -0800, Ramesh Thomas wrote:
> Definitions of ioread64 and iowrite64 macros in asm/io.h called by vfio
> pci implementations are enclosed inside check for CONFIG_GENERIC_IOMAP.
> They don't get defined if CONFIG_GENERIC_IOMAP is defined. Include
> linux/io-64-nonatomic-lo-hi.h to define iowrite64 and ioread64 macros
> when they are not defined. io-64-nonatomic-lo-hi.h maps the macros to
> generic implementation in lib/iomap.c. The generic implementation does
> 64 bit rw if readq/writeq is defined for the architecture, otherwise it
> would do 32 bit back to back rw.
> 
> Note that there are two versions of the generic implementation that
> differs in the order the 32 bit words are written if 64 bit support is
> not present. This is not the little/big endian ordering, which is
> handled separately. This patch uses the lo followed by hi word ordering
> which is consistent with current back to back implementation in the
> vfio/pci code.
> 
> Signed-off-by: Ramesh Thomas <ramesh.thomas@intel.com>
> ---
>  drivers/vfio/pci/vfio_pci_rdwr.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/vfio/pci/vfio_pci_rdwr.c b/drivers/vfio/pci/vfio_pci_rdwr.c
> index 66b72c289284..a0595c745732 100644
> --- a/drivers/vfio/pci/vfio_pci_rdwr.c
> +++ b/drivers/vfio/pci/vfio_pci_rdwr.c
> @@ -16,6 +16,7 @@
>  #include <linux/io.h>
>  #include <linux/vfio.h>
>  #include <linux/vgaarb.h>
> +#include <linux/io-64-nonatomic-lo-hi.h>>
>  
>  #include "vfio_pci_priv.h"
>  

Getting from linux/io-64-nonatomic-lo-hi.h to the lib/iomap.c
implementations is a bit of a wild goose chase but I convinced myself
that it happens as you describe when GENERIC_IOMAP is set. Also makes
sense to me to use the linux/io-64-nonatomic-lo-hi.h header for a
generic fallback where the access order matches what vfio already does.

I do wonder if this actually improves things for more than just x86. As
far as I can see powerpc also uses GENERIC_IOMAP with POWERNV which is
also 64 bit and also has users of vfio-pci. So adding Michael Ellerman
for awareness.

Thanks,
Niklas