On Tue, Oct 5, 2021 at 4:28 PM Jason Gunthorpe <jgg@xxxxxxxx> wrote:
>
> On Tue, Oct 05, 2021 at 04:09:54PM -0700, Si-Wei Liu wrote:
> > On Fri, Oct 1, 2021 at 6:02 AM Jason Gunthorpe <jgg@xxxxxxxx> wrote:
> > >
> > > On Fri, Oct 01, 2021 at 11:59:15AM +0000, Haakon Bugge wrote:
> > > >
> > > >
> > > > > On 1 Oct 2021, at 13:54, Jason Gunthorpe <jgg@xxxxxxxx> wrote:
> > > > >
> > > > > On Fri, Oct 01, 2021 at 11:05:15AM +0000, Haakon Bugge wrote:
> > > > >> Hey,
> > > > >>
> > > > >>
> > > > >> Commit 1477d44ce47d ("RDMA/mlx5: Enable Relaxed Ordering by default
> > > > >> for kernel ULPs") uses pcie_relaxed_ordering_enabled() to check if
> > > > >> RO can be enabled. This function checks if the Enable Relaxed
> > > > >> Ordering bit in the Device Control register is set. However, on a
> > > > >> VF, this bit is RsvdP (Reserved for future RW
> > > > >> implementations. Register bits are read-only and must return zero
> > > > >> when read. Software must preserve the value read for writes to
> > > > >> bits.).
> > > > >>
> > > > >> Hence, AFAICT, RO will not be enabled when using a VF.
> > > > >>
> > > > >> How can that be fixed?
> > > > >
> > > > > When qemu takes a VF and turns it into a PF in a VM it must emulate
> > > > > the RO bit and return one
> > > >
> > > > I have a pass-through VF:
> > > >
> > > > # lspci -s ff:00.0 -vvv
> > > > ff:00.0 Ethernet controller: Mellanox Technologies MT28800 Family [ConnectX-5 Ex Virtual Function]
> > > > []
> > > > DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported-
> > > > RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop- FLReset-
> > >
> > > Like I said, it is a problem in the qemu area..
> > >
> > > Jason
> > Can you clarify why this is a problem in the QEMU area?
> >
> > Even though Mellanox device might well support it (on VF), there's no
> > way for QEMU to really know if an arbitrary passthrough device may
> > support RO.
>
> That isn't what the cap bit means
>
> The cap bit on the PF completely disables generation of RO at the
> device at all.
>
> If the PF's cap bit is disabled then no VF can generate RO, and qemu
> should expose a wired to zero RO bit in the emulated PF.
>
> If the cap bit is enabled then the VFs could generate RO, depending on
> their drivers, and qemu should generate defaulted to 1 bit in the
> emulated PF.
Set the broken root port and the P2P DMA cases aside, let's say we
have a RO enabled PF where there's a working root port upstream that
well supports RO. As VF mostly inherits PF's state/config, no matter
what value the DevCtl RlxdOrd bit presents in the host it doesn't mean
anything, although we know getting RO disabled on the PF implies
prohibiting RO TLP being sent by all its child VFs. There's no
question for this part. The real problem though, is if the RlxdOrd cap
bit for the VF can be controlled individually similar to the way the
toggling on PF is done? For e.g, suppose the RO cap bit for the VF
emulated by QEMU defaults to enabled where the backing PF and all
child VFs have RO enabled. Will a PCI write of zero to the bit be able
to prevent RO ULP initiated by that specific VF from being sent out,
which is to resemble PF's behaviour? This being the Mellanox VF's
specifics? More broadly, should the VFs for arbitrary PCIe devices
have that kind of control on an individual VF's level? I don't find it
anywhere in the PCIe SR-IOV spec that this should be the case.
-Siwei
>
> > PCIe device functions up to the root port throughout the PCIe fabric,
> > or it may follow PF's enabling status if it is at all capable. I don't
> > see what QEMU can do by just forcefully emulating the bit?
>
> IMHO Kernel/BIOS should be responsible to clear the RO bit at the PF
> if RO is not supportable in the environment. It is proper to prevent
> the device from using RO completely if it is broken.
>
> > Not to mention the current implementation only takes care of broken
> > root port but not the intermediate switches.
> > https://lore.kernel.org/linux-arm-kernel/MWHPR12MB1600255ACFCD3FB3C80EB8B6C88B0@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/
>
> Which is what this message suggests doing
>
> Jason
