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

[Pierre Morel]

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