-----Original Message-----
From: Eran Ben Elisha <redacted>
Sent: Thursday, August 15, 2019 7:35 AM
To: Mark Bloch <redacted>; Haiyang Zhang
[off-list ref]; sashal@kernel.org; davem@davemloft.net;
Saeed Mahameed [off-list ref]; leon@kernel.org;
lorenzo.pieralisi@arm.com; bhelgaas@google.com; linux-
pci@vger.kernel.org; linux-hyperv@vger.kernel.org; netdev@vger.kernel.org
Cc: KY Srinivasan <kys@microsoft.com>; Stephen Hemminger
[off-list ref]; linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next, 3/6] net/mlx5: Add wrappers for HyperV PCIe
operations



On 8/14/2019 11:41 PM, Mark Bloch wrote:

quoted

On 8/14/19 12:08 PM, Haiyang Zhang wrote:

quoted

From: Eran Ben Elisha <redacted>

Add wrapper functions for HyperV PCIe read / write /
block_invalidate_register operations.  This will be used as an
infrastructure in the downstream patch for software communication.

This will be enabled by default if CONFIG_PCI_HYPERV_MINI is set.

Signed-off-by: Eran Ben Elisha <redacted>
Signed-off-by: Saeed Mahameed <redacted>
---
  drivers/net/ethernet/mellanox/mlx5/core/Makefile |  1 +
  drivers/net/ethernet/mellanox/mlx5/core/lib/hv.c | 64

++++++++++++++++++++++++

quoted

quoted

  drivers/net/ethernet/mellanox/mlx5/core/lib/hv.h | 22 ++++++++
  3 files changed, 87 insertions(+)
  create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/lib/hv.c
  create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/lib/hv.h

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/Makefile

b/drivers/net/ethernet/mellanox/mlx5/core/Makefile

quoted

quoted

index 8b7edaa..a8950b1 100644

--- a/drivers/net/ethernet/mellanox/mlx5/core/Makefile
+++ b/drivers/net/ethernet/mellanox/mlx5/core/Makefile

@@ -45,6 +45,7 @@ mlx5_core-$(CONFIG_MLX5_ESWITCH)   +=

eswitch.o eswitch_offloads.o eswitch_offlo

quoted

quoted

  mlx5_core-$(CONFIG_MLX5_MPFS)      += lib/mpfs.o
  mlx5_core-$(CONFIG_VXLAN)          += lib/vxlan.o
  mlx5_core-$(CONFIG_PTP_1588_CLOCK) += lib/clock.o
+mlx5_core-$(CONFIG_PCI_HYPERV_MINI)     += lib/hv.o

  #
  # Ipoib netdev

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/hv.c

b/drivers/net/ethernet/mellanox/mlx5/core/lib/hv.c

quoted

quoted

new file mode 100644
index 0000000..cf08d02

--- /dev/null
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/hv.c

@@ -0,0 +1,64 @@
+// SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB
+// Copyright (c) 2018 Mellanox Technologies
+
+#include <linux/hyperv.h>
+#include "mlx5_core.h"
+#include "lib/hv.h"
+
+static int mlx5_hv_config_common(struct mlx5_core_dev *dev, void

*buf, int len,

quoted

quoted

+				 int offset, bool read)
+{
+	int rc = -EOPNOTSUPP;
+	int bytes_returned;
+	int block_id;
+
+	if (offset % HV_CONFIG_BLOCK_SIZE_MAX || len %

HV_CONFIG_BLOCK_SIZE_MAX)

quoted

quoted

+		return -EINVAL;
+
+	block_id = offset / HV_CONFIG_BLOCK_SIZE_MAX;
+
+	rc = read ?
+	     hyperv_read_cfg_blk(dev->pdev, buf,
+				 HV_CONFIG_BLOCK_SIZE_MAX, block_id,
+				 &bytes_returned) :
+	     hyperv_write_cfg_blk(dev->pdev, buf,
+				  HV_CONFIG_BLOCK_SIZE_MAX, block_id);
+
+	/* Make sure len bytes were read successfully  */
+	if (read)
+		rc |= !(len == bytes_returned);
+
+	if (rc) {
+		mlx5_core_err(dev, "Failed to %s hv config, err = %d, len

= %d, offset = %d\n",

quoted

quoted

+			      read ? "read" : "write", rc, len,
+			      offset);
+		return rc;
+	}
+
+	return 0;
+}

This seems out of place why not expose this function as part of hyperv and

mlx5

quoted

will just pass the pdev.

The HV driver works with block chunks. I found it less convenience to do
so directly, so I add a small wrapper for mlx5 core.

Haiyangz,
Do you see a reason to export this callback style from the HYPERV level
instead?

I don’t think the wrapper has to be in the hv interface. 
One function for read, another for write, are pretty straight forward. 
Users (other drivers) may use or wrap them in the way they like to.

Thanks,
- Haiyang