[PATCH v3 0/2] Extend 8-byte PCI load/store support to x86 arch

[PATCH v3 0/2] Extend 8-byte PCI load/store support to x86 arch

[Ramesh Thomas]

This patch series extends the recently added 8-byte PCI load/store
support to the x86 architecture. 

Refer patch series adding above support:
https://lore.kernel.org/all/20240522150651.1999584-1-gbayer@linux.ibm.com/

The 8-byte implementations are enclosed inside #ifdef checks of the
macros "ioread64" and "iowrite64". These macros don't get defined if
CONFIG_GENERIC_IOMAP is defined. CONFIG_GENERIC_IOMAP gets defined for
x86 and hence the macros are undefined. Due to this the 8-byte support
was not enabled for x86 architecture.

To resolve this, include the header file io-64-nonatomic-lo-hi.h that
maps the ioread64 and iowrite64 macros to a generic implementation in
lib/iomap.c. This was the intention of defining CONFIG_GENERIC_IOMAP.

Tested using a pass-through PCI device bound to vfio-pci driver and
doing BAR reads and writes that trigger calls to
vfio_pci_core_do_io_rw() that does the 8-byte reads and writes.

Patch history:
v3: Do not add the check for CONFIG_64BIT and only remove the checks for
ioread64 and iowrite64.

v2: Based on Jason's feedback moved #include io-64-nonatomic-lo-hi.h
to vfio_pci_rdwr.c and replaced #ifdef checks of iowrite64 and ioread64
macros with checks for CONFIG_64BIT.

https://lore.kernel.org/all/20240522232125.548643-1-ramesh.thomas@intel.com/
https://lore.kernel.org/all/20240524140013.GM69273@ziepe.ca/
https://lore.kernel.org/all/bfb273b2-fc5e-4a8b-a40d-56996fc9e0af@intel.com/

Ramesh Thomas (2):
  vfio/pci: Enable iowrite64 and ioread64 for vfio pci
  vfio/pci: Remove #ifdef iowrite64 and #ifdef ioread64

 drivers/vfio/pci/vfio_pci_rdwr.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

-- 
2.34.1

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

[Ramesh Thomas]

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"
 
-- 
2.34.1

[PATCH v3 2/2] vfio/pci: Remove #ifdef iowrite64 and #ifdef ioread64

[Ramesh Thomas]

Remove the #ifdef iowrite64 and #ifdef ioread64 checks around calls to
64 bit IO access. Since default implementations have been enabled, the
checks are not required.

Signed-off-by: Ramesh Thomas <ramesh.thomas@intel.com>
---
 drivers/vfio/pci/vfio_pci_rdwr.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/drivers/vfio/pci/vfio_pci_rdwr.c b/drivers/vfio/pci/vfio_pci_rdwr.c
index a0595c745732..78a3d0809415 100644
--- a/drivers/vfio/pci/vfio_pci_rdwr.c
+++ b/drivers/vfio/pci/vfio_pci_rdwr.c
@@ -62,9 +62,7 @@ EXPORT_SYMBOL_GPL(vfio_pci_core_iowrite##size);
 VFIO_IOWRITE(8)
 VFIO_IOWRITE(16)
 VFIO_IOWRITE(32)
-#ifdef iowrite64
 VFIO_IOWRITE(64)
-#endif
 
 #define VFIO_IOREAD(size) \
 int vfio_pci_core_ioread##size(struct vfio_pci_core_device *vdev,	\
@@ -90,9 +88,7 @@ EXPORT_SYMBOL_GPL(vfio_pci_core_ioread##size);
 VFIO_IOREAD(8)
 VFIO_IOREAD(16)
 VFIO_IOREAD(32)
-#ifdef ioread64
 VFIO_IOREAD(64)
-#endif
 
 #define VFIO_IORDWR(size)						\
 static int vfio_pci_iordwr##size(struct vfio_pci_core_device *vdev,\
@@ -128,9 +124,7 @@ static int vfio_pci_iordwr##size(struct vfio_pci_core_device *vdev,\
 VFIO_IORDWR(8)
 VFIO_IORDWR(16)
 VFIO_IORDWR(32)
-#if defined(ioread64) && defined(iowrite64)
 VFIO_IORDWR(64)
-#endif
 
 /*
  * Read or write from an __iomem region (MMIO or I/O port) with an excluded
@@ -156,7 +150,6 @@ ssize_t vfio_pci_core_do_io_rw(struct vfio_pci_core_device *vdev, bool test_mem,
 		else
 			fillable = 0;
 
-#if defined(ioread64) && defined(iowrite64)
 		if (fillable >= 8 && !(off % 8)) {
 			ret = vfio_pci_iordwr64(vdev, iswrite, test_mem,
 						io, buf, off, &filled);
@@ -164,7 +157,6 @@ ssize_t vfio_pci_core_do_io_rw(struct vfio_pci_core_device *vdev, bool test_mem,
 				return ret;
 
 		} else
-#endif
 		if (fillable >= 4 && !(off % 4)) {
 			ret = vfio_pci_iordwr32(vdev, iswrite, test_mem,
 						io, buf, off, &filled);
@@ -382,12 +374,10 @@ static void vfio_pci_ioeventfd_do_write(struct vfio_pci_ioeventfd *ioeventfd,
 		vfio_pci_core_iowrite32(ioeventfd->vdev, test_mem,
 					ioeventfd->data, ioeventfd->addr);
 		break;
-#ifdef iowrite64
 	case 8:
 		vfio_pci_core_iowrite64(ioeventfd->vdev, test_mem,
 					ioeventfd->data, ioeventfd->addr);
 		break;
-#endif
 	}
 }
 
@@ -441,10 +431,8 @@ int vfio_pci_ioeventfd(struct vfio_pci_core_device *vdev, loff_t offset,
 	      pos >= vdev->msix_offset + vdev->msix_size))
 		return -EINVAL;
 
-#ifndef iowrite64
 	if (count == 8)
 		return -EINVAL;
-#endif
 
 	ret = vfio_pci_core_setup_barmap(vdev, bar);
 	if (ret)
-- 
2.34.1

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

[Jason Gunthorpe]

On Tue, Dec 10, 2024 at 05:19:37AM -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(+)

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Jason

Re: [PATCH v3 2/2] vfio/pci: Remove #ifdef iowrite64 and #ifdef ioread64

[Jason Gunthorpe]

On Tue, Dec 10, 2024 at 05:19:38AM -0800, Ramesh Thomas wrote:
> Remove the #ifdef iowrite64 and #ifdef ioread64 checks around calls to
> 64 bit IO access. Since default implementations have been enabled, the
> checks are not required.
> 
> Signed-off-by: Ramesh Thomas <ramesh.thomas@intel.com>
> ---
>  drivers/vfio/pci/vfio_pci_rdwr.c | 12 ------------
>  1 file changed, 12 deletions(-)

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Jason

Re: [PATCH v3 0/2] Extend 8-byte PCI load/store support to x86 arch

[Alex Williamson]

On Tue, 10 Dec 2024 05:19:36 -0800
Ramesh Thomas <ramesh.thomas@intel.com> wrote:

> This patch series extends the recently added 8-byte PCI load/store
> support to the x86 architecture. 
> 
> Refer patch series adding above support:
> https://lore.kernel.org/all/20240522150651.1999584-1-gbayer@linux.ibm.com/
> 
> The 8-byte implementations are enclosed inside #ifdef checks of the
> macros "ioread64" and "iowrite64". These macros don't get defined if
> CONFIG_GENERIC_IOMAP is defined. CONFIG_GENERIC_IOMAP gets defined for
> x86 and hence the macros are undefined. Due to this the 8-byte support
> was not enabled for x86 architecture.
> 
> To resolve this, include the header file io-64-nonatomic-lo-hi.h that
> maps the ioread64 and iowrite64 macros to a generic implementation in
> lib/iomap.c. This was the intention of defining CONFIG_GENERIC_IOMAP.
> 
> Tested using a pass-through PCI device bound to vfio-pci driver and
> doing BAR reads and writes that trigger calls to
> vfio_pci_core_do_io_rw() that does the 8-byte reads and writes.
> 
> Patch history:
> v3: Do not add the check for CONFIG_64BIT and only remove the checks for
> ioread64 and iowrite64.
> 
> v2: Based on Jason's feedback moved #include io-64-nonatomic-lo-hi.h
> to vfio_pci_rdwr.c and replaced #ifdef checks of iowrite64 and ioread64
> macros with checks for CONFIG_64BIT.
> 
> https://lore.kernel.org/all/20240522232125.548643-1-ramesh.thomas@intel.com/
> https://lore.kernel.org/all/20240524140013.GM69273@ziepe.ca/
> https://lore.kernel.org/all/bfb273b2-fc5e-4a8b-a40d-56996fc9e0af@intel.com/
> 
> Ramesh Thomas (2):
>   vfio/pci: Enable iowrite64 and ioread64 for vfio pci
>   vfio/pci: Remove #ifdef iowrite64 and #ifdef ioread64
> 
>  drivers/vfio/pci/vfio_pci_rdwr.c | 13 +------------
>  1 file changed, 1 insertion(+), 12 deletions(-)
> 

Applied to vfio next branch for v6.14.  Thanks,

Alex