Re: [PATCH] vfio/pci: Add iowrite64 and ioread64 support for vfio pci

[Gerd Bayer <gbayer@linux.ibm.com>]

Hi Ramesh, hi Jason,

being back from a short vacation, I think I'm sold on enabling x86 for
the 64bit accessors in vfio/pci.

On Tue, 2024-05-28 at 15:48 -0700, Ramesh Thomas wrote:
> Hi Jason,
> 
> 
> On 5/24/2024 7:00 AM, Jason Gunthorpe wrote:
> > On Wed, May 22, 2024 at 04:21:25PM -0700, Ramesh Thomas wrote:
> > > ioread64 and iowrite64 macros called by vfio pci implementations
> > > are
> > > defined in asm/io.h if CONFIG_GENERIC_IOMAP is not 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.
> > > 
> > > Refer patch series the requirement originated from:
> > > https://lore.kernel.org/all/20240522150651.1999584-1-gbayer@linux.ibm.com/
> > > 
> > > Signed-off-by: Ramesh Thomas <ramesh.thomas@intel.com>
> > > ---
> > >   drivers/vfio/pci/vfio_pci_priv.h | 1 +
> > >   1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/drivers/vfio/pci/vfio_pci_priv.h
> > > b/drivers/vfio/pci/vfio_pci_priv.h
> > > index 5e4fa69aee16..5eab5abf2ff2 100644
> > > --- a/drivers/vfio/pci/vfio_pci_priv.h
> > > +++ b/drivers/vfio/pci/vfio_pci_priv.h
> > > @@ -3,6 +3,7 @@
> > >   #define VFIO_PCI_PRIV_H
> > >   
> > >   #include <linux/vfio_pci_core.h>
> > > +#include <linux/io-64-nonatomic-lo-hi.h>
> > 
> > Why include it here though?
> > 
> > It should go in vfio_pci_rdwr.c and this patch should remove all
> > the "#ifdef iowrite64"'s from that file too.
> 
> I was trying to make it future proof, but I agree it should be
> included only where iowrite64/ioread64 is getting called. I will make
> both the changes.
> 
> Thanks,
> Ramesh
> 
> > 
> > But the idea looks right to me
> > 
> > Thanks,
> > Jason

So how should we go about this? 
To keep the scope that I can test manageable, my proposal would be:

I'll post a v5 of my series with the conditional compiles for
"ioread64"/"iowrite64" (effectively still excluding x86) - and you
Ramesh run this patch (add the include + change #ifdef's) as an
explicit patch on top?

Thanks,
Gerd