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

Ramesh Thomas
2024-12-03T18:01:54Z

Hi Gerd, Jason and Alex,

I got delayed in sending the below mentioned patches as I was assigned
to other projects. I will shortly send the patch series that extends the
8-byte VFIO PCI read/write support to the x86 architecture.

Thanks,
Ramesh

On 6/21/2024 2:50 PM, Ramesh Thomas wrote:
> 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(-)
>>