Re: [PATCH v6 0/3] vfio/pci: Support 8-byte PCI loads and stores

[Ramesh Thomas <ramesh.thomas@intel.com>]

On 6/19/2024 4:58 AM, Gerd Bayer wrote:
> Hi all,
> 
> this all started with a single patch by Ben to enable writing a user-mode
> driver for a PCI device that requires 64bit register read/writes on s390.
> A quick grep showed that there are several other drivers for PCI devices
> in the kernel that use readq/writeq and eventually could use this, too.
> So we decided to propose this for general inclusion.
> 
> A couple of suggestions for refactorizations by Jason Gunthorpe and Alex
> Williamson later [1], I arrived at this little series that avoids some
> code duplication in vfio_pci_core_do_io_rw().
> Also, I've added a small patch to correct the spelling in one of the
> declaration macros that was suggested by Ramesh Thomas [2]. However,
> after some discussions about making 8-byte accesses available for x86,
> Ramesh and I decided to do this in a separate patch [3].

The patchset looks good. I will post the x86 8-byte access enabling 
patch as soon as I get enough testing done. Thanks.

Reviewed-by: Ramesh Thomas <ramesh.thomas@intel.com>

> 
> This version was tested with a pass-through PCI device in a KVM guest
> and with explicit test reads of size 8, 16, 32, and 64 bit on s390.
> For 32bit architectures this has only been compile tested for the
> 32bit ARM architecture.
> 
> Thank you,
> Gerd Bayer
> 
> 
> [1] https://lore.kernel.org/all/20240422153508.2355844-1-gbayer@linux.ibm.com/
> [2] https://lore.kernel.org/kvm/20240425165604.899447-1-gbayer@linux.ibm.com/T/#m1b51fe155c60d04313695fbee11a2ccea856a98c
> [3] https://lore.kernel.org/all/20240522232125.548643-1-ramesh.thomas@intel.com/
> 
> Changes v5 -> v6:
> - restrict patch 3/3 to just the typo fix - no move of semicolons
> 
> Changes v4 -> v5:
> - Make 8-byte accessors depend on the definitions of ioread64 and
>    iowrite64, again. Ramesh agreed to sort these out for x86 separately.
> 
> Changes v3 -> v4:
> - Make 64-bit accessors depend on CONFIG_64BIT (for x86, too).
> - Drop conversion of if-else if chain to switch-case.
> - Add patch to fix spelling of declaration macro.
> 
> Changes v2 -> v3:
> - Introduce macro to generate body of different-size accesses in
>    vfio_pci_core_do_io_rw (courtesy Alex Williamson).
> - Convert if-else if chain to a switch-case construct to better
>    accommodate conditional compiles.
> 
> Changes v1 -> v2:
> - On non 64bit architecture use at most 32bit accesses in
>    vfio_pci_core_do_io_rw and describe that in the commit message.
> - Drop the run-time error on 32bit architectures.
> - The #endif splitting the "else if" is not really fortunate, but I'm
>    open to suggestions.
> 
> 
> Ben Segal (1):
>    vfio/pci: Support 8-byte PCI loads and stores
> 
> Gerd Bayer (2):
>    vfio/pci: Extract duplicated code into macro
>    vfio/pci: Fix typo in macro to declare accessors
> 
>   drivers/vfio/pci/vfio_pci_rdwr.c | 122 ++++++++++++++++---------------
>   include/linux/vfio_pci_core.h    |  21 +++---
>   2 files changed, 74 insertions(+), 69 deletions(-)
>