diff mbox series

[v3,6/6] IB/mlx5: Use __iowrite64_copy() for write combining stores

Message ID 6-v3-1893cd8b9369+1925-mlx5_arm_wc_jgg@nvidia.com (mailing list archive)
State Not Applicable
Headers show
Series Fix mlx5 write combining support on new ARM64 cores | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Guessed tree name to be net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 942 this patch: 942
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 9 of 9 maintainers
netdev/build_clang success Errors and warnings before: 953 this patch: 953
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 953 this patch: 953
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 26 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-04-11--21-00 (tests: 959)

Commit Message

Jason Gunthorpe April 11, 2024, 4:46 p.m. UTC
mlx5 has a built in self-test at driver startup to evaluate if the
platform supports write combining to generate a 64 byte PCIe TLP or
not. This has proven necessary because a lot of common scenarios end up
with broken write combining (especially inside virtual machines) and there
is other way to learn this information.

This self test has been consistently failing on new ARM64 CPU
designs (specifically with NVIDIA Grace's implementation of Neoverse
V2). The C loop around writeq() generates some pretty terrible ARM64
assembly, but historically this has worked on a lot of existing ARM64 CPUs
till now.

We see it succeed about 1 time in 10,000 on the worst effected
systems. The CPU architects speculate that the load instructions
interspersed with the stores makes the WC buffers statistically flush too
often and thus the generation of large TLPs becomes infrequent. This makes
the boot up test unreliable in that it indicates no write-combining,
however userspace would be fine since it uses a ST4 instruction.

Further, S390 has similar issues where only the special zpci_memcpy_toio()
will actually generate large TLPs, and the open coded loop does not
trigger it at all.

Fix both ARM64 and S390 by switching to __iowrite64_copy() which now
provides architecture specific variants that have a high change of
generating a large TLP with write combining. x86 continues to use a
similar writeq loop in the generate __iowrite64_copy().

Fixes: 11f552e21755 ("IB/mlx5: Test write combining support")
Tested-by: Niklas Schnelle <schnelle@linux.ibm.com>
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
 drivers/infiniband/hw/mlx5/mem.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

Comments

Leon Romanovsky April 16, 2024, 8:29 a.m. UTC | #1
On Thu, Apr 11, 2024 at 01:46:19PM -0300, Jason Gunthorpe wrote:
> mlx5 has a built in self-test at driver startup to evaluate if the
> platform supports write combining to generate a 64 byte PCIe TLP or
> not. This has proven necessary because a lot of common scenarios end up
> with broken write combining (especially inside virtual machines) and there
> is other way to learn this information.
> 
> This self test has been consistently failing on new ARM64 CPU
> designs (specifically with NVIDIA Grace's implementation of Neoverse
> V2). The C loop around writeq() generates some pretty terrible ARM64
> assembly, but historically this has worked on a lot of existing ARM64 CPUs
> till now.
> 
> We see it succeed about 1 time in 10,000 on the worst effected
> systems. The CPU architects speculate that the load instructions
> interspersed with the stores makes the WC buffers statistically flush too
> often and thus the generation of large TLPs becomes infrequent. This makes
> the boot up test unreliable in that it indicates no write-combining,
> however userspace would be fine since it uses a ST4 instruction.
> 
> Further, S390 has similar issues where only the special zpci_memcpy_toio()
> will actually generate large TLPs, and the open coded loop does not
> trigger it at all.
> 
> Fix both ARM64 and S390 by switching to __iowrite64_copy() which now
> provides architecture specific variants that have a high change of
> generating a large TLP with write combining. x86 continues to use a
> similar writeq loop in the generate __iowrite64_copy().
> 
> Fixes: 11f552e21755 ("IB/mlx5: Test write combining support")
> Tested-by: Niklas Schnelle <schnelle@linux.ibm.com>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  drivers/infiniband/hw/mlx5/mem.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 

Thanks,
Acked-by: Leon Romanovsky <leonro@nvidia.com>
diff mbox series

Patch

diff --git a/drivers/infiniband/hw/mlx5/mem.c b/drivers/infiniband/hw/mlx5/mem.c
index 96ffbbaf0a73d1..5a22be14d958f2 100644
--- a/drivers/infiniband/hw/mlx5/mem.c
+++ b/drivers/infiniband/hw/mlx5/mem.c
@@ -30,6 +30,7 @@ 
  * SOFTWARE.
  */
 
+#include <linux/io.h>
 #include <rdma/ib_umem_odp.h>
 #include "mlx5_ib.h"
 #include <linux/jiffies.h>
@@ -108,7 +109,6 @@  static int post_send_nop(struct mlx5_ib_dev *dev, struct ib_qp *ibqp, u64 wr_id,
 	__be32 mmio_wqe[16] = {};
 	unsigned long flags;
 	unsigned int idx;
-	int i;
 
 	if (unlikely(dev->mdev->state == MLX5_DEVICE_STATE_INTERNAL_ERROR))
 		return -EIO;
@@ -148,10 +148,8 @@  static int post_send_nop(struct mlx5_ib_dev *dev, struct ib_qp *ibqp, u64 wr_id,
 	 * we hit doorbell
 	 */
 	wmb();
-	for (i = 0; i < 8; i++)
-		mlx5_write64(&mmio_wqe[i * 2],
-			     bf->bfreg->map + bf->offset + i * 8);
-	io_stop_wc();
+	__iowrite64_copy(bf->bfreg->map + bf->offset, mmio_wqe,
+			 sizeof(mmio_wqe) / 8);
 
 	bf->offset ^= bf->buf_size;