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

[Pierre Morel <pmorel@linux.ibm.com>]

On 1/20/21 3:03 PM, Matthew Rosato wrote:
> 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).
> 
> 

Sorry, if I missed or forgot some discussions, but I understood that we 
are using this region to handle PCISTB instructions when the device do 
not support MIO.
Don't we?

I do not understand the relation between MSI-X and MIO.
Can you please explain?

Thanks,
Pierre

-- 
Pierre Morel
IBM Lab Boeblingen