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@xxxxxxxxxxxxx/ > > > > > > Signed-off-by: Ramesh Thomas <ramesh.thomas@xxxxxxxxx> > > > --- > > > 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
