Re: [RFC PATCH] s390x/pci: vfio-pci breakage with disabled mem enforcement

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

On 2020-07-23 18:29, Alex Williamson wrote:
> On Thu, 23 Jul 2020 11:13:55 -0400
> Matthew Rosato <mjrosato@linux.ibm.com> wrote:
> 
>> I noticed that after kernel commit abafbc55 'vfio-pci: Invalidate mmaps
>> and block MMIO access on disabled memory' vfio-pci via qemu on s390x
>> fails spectacularly, with errors in qemu like:
>>
>> qemu-system-s390x: vfio_region_read(0001:00:00.0:region0+0x0, 4) failed: Input/output error
>>
>>  From read to bar 0 originating out of hw/s390x/s390-pci-inst.c:zpci_read_bar().
>>
>> So, I'm trying to figure out how to get vfio-pci happy again on s390x.  From
>> a bit of tracing, we seem to be triggering the new trap in
>> __vfio_pci_memory_enabled().  Sure enough, if I just force this function to
>> return 'true' as a test case, things work again.
>> The included patch attempts to enforce the setting, which restores everything
>> to working order but also triggers vfio_bar_restore() in the process....  So
>> this isn't the right answer, more of a proof-of-concept.
>>
>> @Alex: Any guidance on what needs to happen to make qemu-s390x happy with this
>> recent kernel change?
> 
> Bummer!  I won't claim to understand s390 PCI, but if we have a VF
> exposed to the "host" (ie. the first level where vfio-pci is being
> used), but we can't tell that it's a VF, how do we know whether the
> memory bit in the command register is unimplemented because it's a VF
> or unimplemented because the device doesn't support MMIO?  How are the
> device ID, vendor ID, and BAR registers virtualized to the host?  Could
> the memory enable bit also be emulated by that virtualization, much
> like vfio-pci does for userspace?  If the other registers are
> virtualized, but these command register bits are left unimplemented, do
> we need code to deduce that we have a VF based on the existence of MMIO
> BARs, but lack of memory enable bit?  Thanks,
> 
> Alex

Alex, Matt,

in s390 we have the possibility to assign a virtual function to a 
logical partition as function 0.
In this case it can not be treated as a virtual function but must be 
treated as a physical function.
This is currently working very well.
However, these functions do not set PCI_COMMAND_MEMORY as we need.

Shouldn't we fix this inside the kernel, to keep older QMEU working?

Then would it be OK to add a new bit/boolean inside the 
pci_dev/vfio_pci_device like, is_detached_vfn, that we could set during 
enumeration and test inside __vfio_pci_memory_enabled() to return true?

In the enumeration we have the possibility to know if the function is a 
HW/Firmware virtual function on devfn 0 or if it is created by SRIOV.

It seems an easy fix without side effects.

What do you think?


> 
>> @Nilkas/@Pierre: I wonder if this might be related to host device is_virtfn?
>> I note that my host device lspci output looks like:
>>
>> 0000:00:00.0 Ethernet controller: Mellanox Technologies MT27710 Family [ConnectX-4 Lx Virtual Function]
>>
>> But the device is not marked as is_virtfn..  Otherwise, Alex's fix
>> from htps://lkml.org/lkml/2020/6/25/628 should cover the case.
>>

I do not think we can fix this problem by forcing the is_virtfn bit.
AFAIU, our HW virtual function works a lot like a physical function.

>>
>>
>> Matthew Rosato (1):
>>    s390x/pci: Enforce PCI_COMMAND_MEMORY for vfio-pci
>>
>>   hw/s390x/s390-pci-inst.c | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
> 
> 

Regards,
Pierre

-- 
Pierre Morel
IBM Lab Boeblingen