Re: [PATCH 0/8] s390x/pci: Fixing s390 vfio-pci ISM support

[Matthew Rosato <mjrosato@linux.ibm.com>]

On 1/20/21 4:12 AM, Pierre Morel wrote:
> 
> 
> On 1/19/21 9:44 PM, Matthew Rosato wrote:
>> Today, ISM devices are completely disallowed for vfio-pci passthrough as
>> QEMU rejects the device due to an (inappropriate) MSI-X check.  Removing
>> this fence, however, reveals additional deficiencies in the s390x PCI
>> interception layer that prevent ISM devices from working correctly.
>> Namely, ISM block write operations have particular requirements in 
>> regards
>> to the alignment, size and order of writes performed that cannot be
>> guaranteed when breaking up write operations through the typical
>> vfio_pci_bar_rw paths. Furthermore, ISM requires that legacy/non-MIO
>> s390 PCI instructions are used, which is also not guaranteed when the I/O
>> is passed through the typical userspace channels.
>>
>> This patchset provides a set of fixes related to enabling ISM device
>> passthrough and includes patches to enable use of a new vfio region that
>> will allow s390x PCI pass-through devices to perform s390 PCI 
>> instructions
>> in such a way that the same instruction issued on the guest is re-issued
>> on the host.
>>
>> Associated kernel patchset:
>> https://lkml.org/lkml/2021/1/19/874
>>
>> Changes from RFC -> v1:
>> - Refresh the header sync (built using Eric's 'update-linux-headers:
>> Include const.h' + manually removed pvrdma_ring.h again)
>> - Remove s390x/pci: fix pcistb length (already merged)
>> - Remove s390x/pci: Fix memory_region_access_valid call (already merged)
>> - Fix bug: s390_pci_vfio_pcistb should use the pre-allocated PCISTB
>> buffer pcistb_buf rather than allocating/freeing its own.
>> - New patch: track the PFT (PCI Function Type) separately from guest CLP
>> response data -- we tell the guest '0' for now due to limitations in
>> measurement block support, but we can still use the real value 
>> provided via
>> the vfio CLP capabilities to make decisions.
>> - Use the PFT (pci function type) to determine when to use the region
>> for PCISTB/PCILG (only for ISM), rather than using the relaxed alignment
>> bit.
>> - As a result, the pcistb_default is now updated to also handle the
>> possibility of relaxed alignment via 2 new functions, 
>> pcistb_validate_write
>> and pcistb_write, which serve as wrappers to the memory_region calls.
>> - New patch, which partially restores the MSI-X fence for passthrough
>> devices...  Could potentially be squashed with 's390x/pci: MSI-X isn't
>> strictly required for passthrough' but left separately for now as I 
>> felt it
>> needed a clear commit description of why we should still fence this case.
>>
> Hi,
> 
> The choice of using the new VFIO region is made on the ISM PCI function 
> type (PFT), which makes the patch ISM specific, why don't we use here 
> the MIO bit common to any zPCI function and present in kernel to make 
> the choice?
> 

As discussed during the RFC (and see my reply also to the kernel set), 
the use of this region only works for devices that do not rely on MSI-X 
interrupts.  If we did as you suggest, other device types like mlx would 
not receive MSI-X interrupts in the guest (And I did indeed try 
variations where I used the special VFIO region for all 
PCISTG/PCILG/PCISTB for various device types)

So the idea for now was to solve the specific problem at hand (getting 
ISM devices working).