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

Ramesh Thomas
2024-05-28T22:48:42Z

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