On 4/11/2023 7:01 AM, Jason Gunthorpe wrote: > On Mon, Apr 10, 2023 at 04:07:49PM +0300, Leon Romanovsky wrote: >> From: Leon Romanovsky <leonro@xxxxxxxxxx> >> >> From Avihai, >> >> Currently, Relaxed Ordering (RO) can't be used in VFs directly and in >> VFs assigned to QEMU, even if the PF supports RO. This is due to issues >> in reporting/emulation of PCI config space RO bit and due to current >> HCA capability behavior. >> >> This series fixes it by using a new HCA capability and by relying on FW >> to do the "right thing" according to the PF's PCI config space RO value. >> >> Allowing RO in VFs and VMs is valuable since it can greatly improve >> performance on some setups. For example, testing throughput of a VF on >> an AMD EPYC 7763 and ConnectX-6 Dx setup showed roughly 60% performance >> improvement. >> >> Thanks >> >> Avihai Horon (4): >> RDMA/mlx5: Remove pcie_relaxed_ordering_enabled() check for RO write >> RDMA/mlx5: Check pcie_relaxed_ordering_enabled() in UMR >> net/mlx5: Update relaxed ordering read HCA capabilities >> RDMA/mlx5: Allow relaxed ordering read in VFs and VMs > > This looks OK, but the patch structure is pretty confusing. > > It seems to me there are really only two patches here, the first is to > add some static inline > > 'mlx5 supports read ro' > > which supports both the cap bits described in > the PRM, with a little comment to explain that old devices only set > the old cap. > > And a second patch to call it in all the places we need to check before > setting the mkc ro read bit. > > Maybe a final third patch to sort out that mistake in the write side. > > But this really doesn't have anything to do with VFs and VMs, this is > adjusting the code to follow the current PRM because the old one was > mis-desgined. > > Jason FWIW I think Jason's outline here makes sense too and might be slightly better. However, reading through the series I was reasonably able to understand things enough that I think its fine as-is. In some sense its not about VF or VM, but fixing this has the result that it fixes a setup with VF and VM, so I think thats an ok thing to call out as the goal.
