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

[Alex Williamson <alex.williamson@redhat.com>]

On Fri, 19 Apr 2024 13:11:35 -0300
Jason Gunthorpe <jgg@ziepe.ca> wrote:

> On Fri, Apr 19, 2024 at 05:57:52PM +0200, Niklas Schnelle wrote:
> > On Fri, 2024-04-19 at 10:58 -0300, Jason Gunthorpe wrote:  
> > > On Fri, Apr 19, 2024 at 03:53:23PM +0200, Gerd Bayer wrote:  
> > > > From: Ben Segal <bpsegal@us.ibm.com>
> > > > 
> > > > Many PCI adapters can benefit or even require full 64bit read
> > > > and write access to their registers. In order to enable work on
> > > > user-space drivers for these devices add two new variations
> > > > vfio_pci_core_io{read|write}64 of the existing access methods
> > > > when the architecture supports 64-bit ioreads and iowrites.
> > > > 
> > > > Signed-off-by: Ben Segal <bpsegal@us.ibm.com>
> > > > Co-developed-by: Gerd Bayer <gbayer@linux.ibm.com>
> > > > Signed-off-by: Gerd Bayer <gbayer@linux.ibm.com>
> > > > ---
> > > > 
> > > > Hi all,
> > > > 
> > > > we've successfully used this patch with a user-mode driver for a PCI
> > > > device that requires 64bit register read/writes on s390.  
> > > 
> > > But why? S390 already has a system call for userspace to do the 64 bit
> > > write, and newer S390 has a userspace instruction to do it.
> > > 
> > > Why would you want to use a VFIO system call on the mmio emulation
> > > path?
> > > 
> > > mmap the registers and access them normally?  
> > 
> > It's a very good point and digging into why this wasn't used by
> > Benjamin. It turns out VFIO_PCI_MMAP is disabled for S390 which it
> > really shouldn't be especially now that we have the user-space
> > instructions. Before that though Benjamin turned to this interface
> > which then lead him to this limitation. So yeah we'll definitely verify
> > that it also works via VFIO_PCI_MMAP and send a patch to enable that.  
> 
> Make sense to me!
> 
> > That said I still think it's odd not to have the 8 byte case working
> > here even if it isn't the right approach. Could still be useful for
> > debug/testing without having to add the MIO instructions or the our
> > special syscall.  
> 
> Yes, this also makes sense, but this patch needs some adjusting

Yes, I think so too, falling back to 4-byte accesses of course if
8-byte is not available.  Thanks,

Alex