[PATCH 0/4] ifcvf/vDPA implement features provisioning

[PATCH 0/4] ifcvf/vDPA implement features provisioning

[Zhu Lingshan]

This series implements features provisioning for ifcvf.
By applying this series, we allow userspace to create
a vDPA device with selected (management device supported)
feature bits and mask out others.

Please help review

Thanks

Zhu Lingshan (4):
  vDPA/ifcvf: ifcvf base layer interfaces work on struct ifcvf_hw
  vDPA/ifcvf: IRQ interfaces work on ifcvf_hw
  vDPA/ifcvf: allocate ifcvf_adapter in dev_add()
  vDPA/ifcvf: implement features provisioning

 drivers/vdpa/ifcvf/ifcvf_base.c |  32 ++-----
 drivers/vdpa/ifcvf/ifcvf_base.h |  10 +-
 drivers/vdpa/ifcvf/ifcvf_main.c | 156 +++++++++++++++-----------------
 3 files changed, 89 insertions(+), 109 deletions(-)

-- 
2.31.1

[PATCH 1/4] vDPA/ifcvf: ifcvf base layer interfaces work on struct ifcvf_hw

[Zhu Lingshan]

To be more rubust and low coupling in ifcvf base layer,
this commit gets rid of struct ifcvf_adapter in ifcvf_base
which is introduced in ifcvf_main.

Now ifcvf base layer interfaces work on ifcvf_hw only, so that
the base interfaces can be safely invoked since probe.

Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
---
 drivers/vdpa/ifcvf/ifcvf_base.c | 30 +++++++-----------------------
 drivers/vdpa/ifcvf/ifcvf_base.h |  2 ++
 2 files changed, 9 insertions(+), 23 deletions(-)

diff --git a/drivers/vdpa/ifcvf/ifcvf_base.c b/drivers/vdpa/ifcvf/ifcvf_base.c
index 3e4486bfa0b7..3ec5ca3aefe1 100644
--- a/drivers/vdpa/ifcvf/ifcvf_base.c
+++ b/drivers/vdpa/ifcvf/ifcvf_base.c
@@ -10,11 +10,6 @@
 
 #include "ifcvf_base.h"
 
-struct ifcvf_adapter *vf_to_adapter(struct ifcvf_hw *hw)
-{
-	return container_of(hw, struct ifcvf_adapter, vf);
-}
-
 u16 ifcvf_set_vq_vector(struct ifcvf_hw *hw, u16 qid, int vector)
 {
 	struct virtio_pci_common_cfg __iomem *cfg = hw->common_cfg;
@@ -37,8 +32,6 @@ u16 ifcvf_set_config_vector(struct ifcvf_hw *hw, int vector)
 static void __iomem *get_cap_addr(struct ifcvf_hw *hw,
 				  struct virtio_pci_cap *cap)
 {
-	struct ifcvf_adapter *ifcvf;
-	struct pci_dev *pdev;
 	u32 length, offset;
 	u8 bar;
 
@@ -46,17 +39,14 @@ static void __iomem *get_cap_addr(struct ifcvf_hw *hw,
 	offset = le32_to_cpu(cap->offset);
 	bar = cap->bar;
 
-	ifcvf= vf_to_adapter(hw);
-	pdev = ifcvf->pdev;
-
 	if (bar >= IFCVF_PCI_MAX_RESOURCE) {
-		IFCVF_DBG(pdev,
+		IFCVF_DBG(hw->pdev,
 			  "Invalid bar number %u to get capabilities\n", bar);
 		return NULL;
 	}
 
-	if (offset + length > pci_resource_len(pdev, bar)) {
-		IFCVF_DBG(pdev,
+	if (offset + length > pci_resource_len(hw->pdev, bar)) {
+		IFCVF_DBG(hw->pdev,
 			  "offset(%u) + len(%u) overflows bar%u's capability\n",
 			  offset, length, bar);
 		return NULL;
@@ -92,6 +82,7 @@ int ifcvf_init_hw(struct ifcvf_hw *hw, struct pci_dev *pdev)
 		IFCVF_ERR(pdev, "Failed to read PCI capability list\n");
 		return -EIO;
 	}
+	hw->pdev = pdev;
 
 	while (pos) {
 		ret = ifcvf_read_config_range(pdev, (u32 *)&cap,
@@ -220,10 +211,8 @@ u64 ifcvf_get_features(struct ifcvf_hw *hw)
 
 int ifcvf_verify_min_features(struct ifcvf_hw *hw, u64 features)
 {
-	struct ifcvf_adapter *ifcvf = vf_to_adapter(hw);
-
 	if (!(features & BIT_ULL(VIRTIO_F_ACCESS_PLATFORM)) && features) {
-		IFCVF_ERR(ifcvf->pdev, "VIRTIO_F_ACCESS_PLATFORM is not negotiated\n");
+		IFCVF_ERR(hw->pdev, "VIRTIO_F_ACCESS_PLATFORM is not negotiated\n");
 		return -EINVAL;
 	}
 
@@ -232,13 +221,11 @@ int ifcvf_verify_min_features(struct ifcvf_hw *hw, u64 features)
 
 u32 ifcvf_get_config_size(struct ifcvf_hw *hw)
 {
-	struct ifcvf_adapter *adapter;
 	u32 net_config_size = sizeof(struct virtio_net_config);
 	u32 blk_config_size = sizeof(struct virtio_blk_config);
 	u32 cap_size = hw->cap_dev_config_size;
 	u32 config_size;
 
-	adapter = vf_to_adapter(hw);
 	/* If the onboard device config space size is greater than
 	 * the size of struct virtio_net/blk_config, only the spec
 	 * implementing contents size is returned, this is very
@@ -253,7 +240,7 @@ u32 ifcvf_get_config_size(struct ifcvf_hw *hw)
 		break;
 	default:
 		config_size = 0;
-		IFCVF_ERR(adapter->pdev, "VIRTIO ID %u not supported\n", hw->dev_type);
+		IFCVF_ERR(hw->pdev, "VIRTIO ID %u not supported\n", hw->dev_type);
 	}
 
 	return config_size;
@@ -301,14 +288,11 @@ static void ifcvf_set_features(struct ifcvf_hw *hw, u64 features)
 
 static int ifcvf_config_features(struct ifcvf_hw *hw)
 {
-	struct ifcvf_adapter *ifcvf;
-
-	ifcvf = vf_to_adapter(hw);
 	ifcvf_set_features(hw, hw->req_features);
 	ifcvf_add_status(hw, VIRTIO_CONFIG_S_FEATURES_OK);
 
 	if (!(ifcvf_get_status(hw) & VIRTIO_CONFIG_S_FEATURES_OK)) {
-		IFCVF_ERR(ifcvf->pdev, "Failed to set FEATURES_OK status\n");
+		IFCVF_ERR(hw->pdev, "Failed to set FEATURES_OK status\n");
 		return -EIO;
 	}
 
diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h
index f5563f665cc6..63ce1f7f6841 100644
--- a/drivers/vdpa/ifcvf/ifcvf_base.h
+++ b/drivers/vdpa/ifcvf/ifcvf_base.h
@@ -89,6 +89,7 @@ struct ifcvf_hw {
 	u16 nr_vring;
 	/* VIRTIO_PCI_CAP_DEVICE_CFG size */
 	u32 cap_dev_config_size;
+	struct pci_dev *pdev;
 };
 
 struct ifcvf_adapter {
@@ -110,6 +111,7 @@ struct ifcvf_lm_cfg {
 struct ifcvf_vdpa_mgmt_dev {
 	struct vdpa_mgmt_dev mdev;
 	struct ifcvf_adapter *adapter;
+	struct ifcvf_hw vf;
 	struct pci_dev *pdev;
 };
 
-- 
2.31.1

[PATCH 2/4] vDPA/ifcvf: IRQ interfaces work on ifcvf_hw

[Zhu Lingshan]

In this commit, ifcvf IRQ interfaces work on ifcvf_hw,
so these functions can be safely invoked before
the adapter struct is allocated since probe.

Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
---
 drivers/vdpa/ifcvf/ifcvf_main.c | 85 ++++++++++++++-------------------
 1 file changed, 37 insertions(+), 48 deletions(-)

diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c
index f9c0044c6442..bae518ff6234 100644
--- a/drivers/vdpa/ifcvf/ifcvf_main.c
+++ b/drivers/vdpa/ifcvf/ifcvf_main.c
@@ -69,10 +69,9 @@ static void ifcvf_free_irq_vectors(void *data)
 	pci_free_irq_vectors(data);
 }
 
-static void ifcvf_free_per_vq_irq(struct ifcvf_adapter *adapter)
+static void ifcvf_free_per_vq_irq(struct ifcvf_hw *vf)
 {
-	struct pci_dev *pdev = adapter->pdev;
-	struct ifcvf_hw *vf = &adapter->vf;
+	struct pci_dev *pdev = vf->pdev;
 	int i;
 
 	for (i = 0; i < vf->nr_vring; i++) {
@@ -83,10 +82,9 @@ static void ifcvf_free_per_vq_irq(struct ifcvf_adapter *adapter)
 	}
 }
 
-static void ifcvf_free_vqs_reused_irq(struct ifcvf_adapter *adapter)
+static void ifcvf_free_vqs_reused_irq(struct ifcvf_hw *vf)
 {
-	struct pci_dev *pdev = adapter->pdev;
-	struct ifcvf_hw *vf = &adapter->vf;
+	struct pci_dev *pdev = vf->pdev;
 
 	if (vf->vqs_reused_irq != -EINVAL) {
 		devm_free_irq(&pdev->dev, vf->vqs_reused_irq, vf);
@@ -95,20 +93,18 @@ static void ifcvf_free_vqs_reused_irq(struct ifcvf_adapter *adapter)
 
 }
 
-static void ifcvf_free_vq_irq(struct ifcvf_adapter *adapter)
+static void ifcvf_free_vq_irq(struct ifcvf_hw *vf)
 {
-	struct ifcvf_hw *vf = &adapter->vf;
 
 	if (vf->msix_vector_status == MSIX_VECTOR_PER_VQ_AND_CONFIG)
-		ifcvf_free_per_vq_irq(adapter);
+		ifcvf_free_per_vq_irq(vf);
 	else
-		ifcvf_free_vqs_reused_irq(adapter);
+		ifcvf_free_vqs_reused_irq(vf);
 }
 
-static void ifcvf_free_config_irq(struct ifcvf_adapter *adapter)
+static void ifcvf_free_config_irq(struct ifcvf_hw *vf)
 {
-	struct pci_dev *pdev = adapter->pdev;
-	struct ifcvf_hw *vf = &adapter->vf;
+	struct pci_dev *pdev = vf->pdev;
 
 	if (vf->config_irq == -EINVAL)
 		return;
@@ -123,12 +119,12 @@ static void ifcvf_free_config_irq(struct ifcvf_adapter *adapter)
 	}
 }
 
-static void ifcvf_free_irq(struct ifcvf_adapter *adapter)
+static void ifcvf_free_irq(struct ifcvf_hw *vf)
 {
-	struct pci_dev *pdev = adapter->pdev;
+	struct pci_dev *pdev = vf->pdev;
 
-	ifcvf_free_vq_irq(adapter);
-	ifcvf_free_config_irq(adapter);
+	ifcvf_free_vq_irq(vf);
+	ifcvf_free_config_irq(vf);
 	ifcvf_free_irq_vectors(pdev);
 }
 
@@ -137,10 +133,9 @@ static void ifcvf_free_irq(struct ifcvf_adapter *adapter)
  * It returns the number of allocated vectors, negative
  * return value when fails.
  */
-static int ifcvf_alloc_vectors(struct ifcvf_adapter *adapter)
+static int ifcvf_alloc_vectors(struct ifcvf_hw *vf)
 {
-	struct pci_dev *pdev = adapter->pdev;
-	struct ifcvf_hw *vf = &adapter->vf;
+	struct pci_dev *pdev = vf->pdev;
 	int max_intr, ret;
 
 	/* all queues and config interrupt  */
@@ -160,10 +155,9 @@ static int ifcvf_alloc_vectors(struct ifcvf_adapter *adapter)
 	return ret;
 }
 
-static int ifcvf_request_per_vq_irq(struct ifcvf_adapter *adapter)
+static int ifcvf_request_per_vq_irq(struct ifcvf_hw *vf)
 {
-	struct pci_dev *pdev = adapter->pdev;
-	struct ifcvf_hw *vf = &adapter->vf;
+	struct pci_dev *pdev = vf->pdev;
 	int i, vector, ret, irq;
 
 	vf->vqs_reused_irq = -EINVAL;
@@ -190,15 +184,14 @@ static int ifcvf_request_per_vq_irq(struct ifcvf_adapter *adapter)
 
 	return 0;
 err:
-	ifcvf_free_irq(adapter);
+	ifcvf_free_irq(vf);
 
 	return -EFAULT;
 }
 
-static int ifcvf_request_vqs_reused_irq(struct ifcvf_adapter *adapter)
+static int ifcvf_request_vqs_reused_irq(struct ifcvf_hw *vf)
 {
-	struct pci_dev *pdev = adapter->pdev;
-	struct ifcvf_hw *vf = &adapter->vf;
+	struct pci_dev *pdev = vf->pdev;
 	int i, vector, ret, irq;
 
 	vector = 0;
@@ -224,15 +217,14 @@ static int ifcvf_request_vqs_reused_irq(struct ifcvf_adapter *adapter)
 
 	return 0;
 err:
-	ifcvf_free_irq(adapter);
+	ifcvf_free_irq(vf);
 
 	return -EFAULT;
 }
 
-static int ifcvf_request_dev_irq(struct ifcvf_adapter *adapter)
+static int ifcvf_request_dev_irq(struct ifcvf_hw *vf)
 {
-	struct pci_dev *pdev = adapter->pdev;
-	struct ifcvf_hw *vf = &adapter->vf;
+	struct pci_dev *pdev = vf->pdev;
 	int i, vector, ret, irq;
 
 	vector = 0;
@@ -265,29 +257,27 @@ static int ifcvf_request_dev_irq(struct ifcvf_adapter *adapter)
 
 	return 0;
 err:
-	ifcvf_free_irq(adapter);
+	ifcvf_free_irq(vf);
 
 	return -EFAULT;
 
 }
 
-static int ifcvf_request_vq_irq(struct ifcvf_adapter *adapter)
+static int ifcvf_request_vq_irq(struct ifcvf_hw *vf)
 {
-	struct ifcvf_hw *vf = &adapter->vf;
 	int ret;
 
 	if (vf->msix_vector_status == MSIX_VECTOR_PER_VQ_AND_CONFIG)
-		ret = ifcvf_request_per_vq_irq(adapter);
+		ret = ifcvf_request_per_vq_irq(vf);
 	else
-		ret = ifcvf_request_vqs_reused_irq(adapter);
+		ret = ifcvf_request_vqs_reused_irq(vf);
 
 	return ret;
 }
 
-static int ifcvf_request_config_irq(struct ifcvf_adapter *adapter)
+static int ifcvf_request_config_irq(struct ifcvf_hw *vf)
 {
-	struct pci_dev *pdev = adapter->pdev;
-	struct ifcvf_hw *vf = &adapter->vf;
+	struct pci_dev *pdev = vf->pdev;
 	int config_vector, ret;
 
 	if (vf->msix_vector_status == MSIX_VECTOR_PER_VQ_AND_CONFIG)
@@ -320,17 +310,16 @@ static int ifcvf_request_config_irq(struct ifcvf_adapter *adapter)
 
 	return 0;
 err:
-	ifcvf_free_irq(adapter);
+	ifcvf_free_irq(vf);
 
 	return -EFAULT;
 }
 
-static int ifcvf_request_irq(struct ifcvf_adapter *adapter)
+static int ifcvf_request_irq(struct ifcvf_hw *vf)
 {
-	struct ifcvf_hw *vf = &adapter->vf;
 	int nvectors, ret, max_intr;
 
-	nvectors = ifcvf_alloc_vectors(adapter);
+	nvectors = ifcvf_alloc_vectors(vf);
 	if (nvectors <= 0)
 		return -EFAULT;
 
@@ -341,16 +330,16 @@ static int ifcvf_request_irq(struct ifcvf_adapter *adapter)
 
 	if (nvectors == 1) {
 		vf->msix_vector_status = MSIX_VECTOR_DEV_SHARED;
-		ret = ifcvf_request_dev_irq(adapter);
+		ret = ifcvf_request_dev_irq(vf);
 
 		return ret;
 	}
 
-	ret = ifcvf_request_vq_irq(adapter);
+	ret = ifcvf_request_vq_irq(vf);
 	if (ret)
 		return ret;
 
-	ret = ifcvf_request_config_irq(adapter);
+	ret = ifcvf_request_config_irq(vf);
 
 	if (ret)
 		return ret;
@@ -479,7 +468,7 @@ static void ifcvf_vdpa_set_status(struct vdpa_device *vdpa_dev, u8 status)
 
 	if ((status & VIRTIO_CONFIG_S_DRIVER_OK) &&
 	    !(status_old & VIRTIO_CONFIG_S_DRIVER_OK)) {
-		ret = ifcvf_request_irq(adapter);
+		ret = ifcvf_request_irq(vf);
 		if (ret) {
 			status = ifcvf_get_status(vf);
 			status |= VIRTIO_CONFIG_S_FAILED;
@@ -511,7 +500,7 @@ static int ifcvf_vdpa_reset(struct vdpa_device *vdpa_dev)
 
 	if (status_old & VIRTIO_CONFIG_S_DRIVER_OK) {
 		ifcvf_stop_datapath(adapter);
-		ifcvf_free_irq(adapter);
+		ifcvf_free_irq(vf);
 	}
 
 	ifcvf_reset_vring(adapter);
-- 
2.31.1

[PATCH 3/4] vDPA/ifcvf: allocate ifcvf_adapter in dev_add()

[Zhu Lingshan]

This commits allocates ifcvf_adapter which is
a container of struct vdpa_device in dev_add()
interface than in probe()

Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
---
 drivers/vdpa/ifcvf/ifcvf_base.h |  5 +--
 drivers/vdpa/ifcvf/ifcvf_main.c | 58 ++++++++++++++-------------------
 2 files changed, 26 insertions(+), 37 deletions(-)

diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h
index 63ce1f7f6841..15d5badc7dbd 100644
--- a/drivers/vdpa/ifcvf/ifcvf_base.h
+++ b/drivers/vdpa/ifcvf/ifcvf_base.h
@@ -38,9 +38,6 @@
 #define IFCVF_DBG(pdev, fmt, ...)	dev_dbg(&pdev->dev, fmt, ##__VA_ARGS__)
 #define IFCVF_INFO(pdev, fmt, ...)	dev_info(&pdev->dev, fmt, ##__VA_ARGS__)
 
-#define ifcvf_private_to_vf(adapter) \
-	(&((struct ifcvf_adapter *)adapter)->vf)
-
 /* all vqs and config interrupt has its own vector */
 #define MSIX_VECTOR_PER_VQ_AND_CONFIG		1
 /* all vqs share a vector, and config interrupt has a separate vector */
@@ -95,7 +92,7 @@ struct ifcvf_hw {
 struct ifcvf_adapter {
 	struct vdpa_device vdpa;
 	struct pci_dev *pdev;
-	struct ifcvf_hw vf;
+	struct ifcvf_hw *vf;
 };
 
 struct ifcvf_vring_lm_cfg {
diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c
index bae518ff6234..76ac324c271b 100644
--- a/drivers/vdpa/ifcvf/ifcvf_main.c
+++ b/drivers/vdpa/ifcvf/ifcvf_main.c
@@ -347,9 +347,9 @@ static int ifcvf_request_irq(struct ifcvf_hw *vf)
 	return 0;
 }
 
-static int ifcvf_start_datapath(void *private)
+static int ifcvf_start_datapath(struct ifcvf_adapter *adapter)
 {
-	struct ifcvf_hw *vf = ifcvf_private_to_vf(private);
+	struct ifcvf_hw *vf = adapter->vf;
 	u8 status;
 	int ret;
 
@@ -363,9 +363,10 @@ static int ifcvf_start_datapath(void *private)
 	return ret;
 }
 
-static int ifcvf_stop_datapath(void *private)
+static int ifcvf_stop_datapath(struct ifcvf_adapter *adapter)
 {
-	struct ifcvf_hw *vf = ifcvf_private_to_vf(private);
+
+	struct ifcvf_hw *vf = adapter->vf;
 	int i;
 
 	for (i = 0; i < vf->nr_vring; i++)
@@ -378,7 +379,7 @@ static int ifcvf_stop_datapath(void *private)
 
 static void ifcvf_reset_vring(struct ifcvf_adapter *adapter)
 {
-	struct ifcvf_hw *vf = ifcvf_private_to_vf(adapter);
+	struct ifcvf_hw *vf = adapter->vf;
 	int i;
 
 	for (i = 0; i < vf->nr_vring; i++) {
@@ -403,7 +404,7 @@ static struct ifcvf_hw *vdpa_to_vf(struct vdpa_device *vdpa_dev)
 {
 	struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev);
 
-	return &adapter->vf;
+	return adapter->vf;
 }
 
 static u64 ifcvf_vdpa_get_device_features(struct vdpa_device *vdpa_dev)
@@ -747,12 +748,20 @@ static int ifcvf_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
 	int ret;
 
 	ifcvf_mgmt_dev = container_of(mdev, struct ifcvf_vdpa_mgmt_dev, mdev);
-	if (!ifcvf_mgmt_dev->adapter)
-		return -EOPNOTSUPP;
+	vf = &ifcvf_mgmt_dev->vf;
+	pdev = vf->pdev;
+	adapter = vdpa_alloc_device(struct ifcvf_adapter, vdpa,
+				    &pdev->dev, &ifc_vdpa_ops, 1, 1, NULL, false);
+	if (IS_ERR(adapter)) {
+		IFCVF_ERR(pdev, "Failed to allocate vDPA structure");
+		return PTR_ERR(adapter);
+	}
 
-	adapter = ifcvf_mgmt_dev->adapter;
-	vf = &adapter->vf;
-	pdev = adapter->pdev;
+	ifcvf_mgmt_dev->adapter = adapter;
+	adapter->pdev = pdev;
+	adapter->vdpa.dma_dev = &pdev->dev;
+	adapter->vdpa.mdev = mdev;
+	adapter->vf = vf;
 	vdpa_dev = &adapter->vdpa;
 
 	if (name)
@@ -770,7 +779,6 @@ static int ifcvf_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
 	return 0;
 }
 
-
 static void ifcvf_vdpa_dev_del(struct vdpa_mgmt_dev *mdev, struct vdpa_device *dev)
 {
 	struct ifcvf_vdpa_mgmt_dev *ifcvf_mgmt_dev;
@@ -789,7 +797,6 @@ static int ifcvf_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 {
 	struct ifcvf_vdpa_mgmt_dev *ifcvf_mgmt_dev;
 	struct device *dev = &pdev->dev;
-	struct ifcvf_adapter *adapter;
 	struct ifcvf_hw *vf;
 	u32 dev_type;
 	int ret, i;
@@ -820,21 +827,16 @@ static int ifcvf_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	}
 
 	pci_set_master(pdev);
-
-	adapter = vdpa_alloc_device(struct ifcvf_adapter, vdpa,
-				    dev, &ifc_vdpa_ops, 1, 1, NULL, false);
-	if (IS_ERR(adapter)) {
-		IFCVF_ERR(pdev, "Failed to allocate vDPA structure");
-		return PTR_ERR(adapter);
+	ifcvf_mgmt_dev = kzalloc(sizeof(struct ifcvf_vdpa_mgmt_dev), GFP_KERNEL);
+	if (!ifcvf_mgmt_dev) {
+		IFCVF_ERR(pdev, "Failed to alloc memory for the vDPA management device\n");
+		return -ENOMEM;
 	}
 
-	vf = &adapter->vf;
+	vf = &ifcvf_mgmt_dev->vf;
 	vf->dev_type = get_dev_type(pdev);
 	vf->base = pcim_iomap_table(pdev);
 
-	adapter->pdev = pdev;
-	adapter->vdpa.dma_dev = &pdev->dev;
-
 	ret = ifcvf_init_hw(vf, pdev);
 	if (ret) {
 		IFCVF_ERR(pdev, "Failed to init IFCVF hw\n");
@@ -847,15 +849,8 @@ static int ifcvf_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	vf->hw_features = ifcvf_get_hw_features(vf);
 	vf->config_size = ifcvf_get_config_size(vf);
 
-	ifcvf_mgmt_dev = kzalloc(sizeof(struct ifcvf_vdpa_mgmt_dev), GFP_KERNEL);
-	if (!ifcvf_mgmt_dev) {
-		IFCVF_ERR(pdev, "Failed to alloc memory for the vDPA management device\n");
-		return -ENOMEM;
-	}
-
 	ifcvf_mgmt_dev->mdev.ops = &ifcvf_vdpa_mgmt_dev_ops;
 	ifcvf_mgmt_dev->mdev.device = dev;
-	ifcvf_mgmt_dev->adapter = adapter;
 
 	dev_type = get_dev_type(pdev);
 	switch (dev_type) {
@@ -874,9 +869,6 @@ static int ifcvf_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	ifcvf_mgmt_dev->mdev.max_supported_vqs = vf->nr_vring;
 	ifcvf_mgmt_dev->mdev.supported_features = vf->hw_features;
 
-	adapter->vdpa.mdev = &ifcvf_mgmt_dev->mdev;
-
-
 	ret = vdpa_mgmtdev_register(&ifcvf_mgmt_dev->mdev);
 	if (ret) {
 		IFCVF_ERR(pdev,
-- 
2.31.1

[PATCH 4/4] vDPA/ifcvf: implement features provisioning

[Zhu Lingshan]

This commit implements features provisioning for ifcvf, that means:
1)checkk whether the provisioned features are supported by
the management device
2)vDPA device only presents selected feature bits

Examples:
a)The management device supported features:
$ vdpa mgmtdev show pci/0000:01:00.5
pci/0000:01:00.5:
  supported_classes net
  max_supported_vqs 9
  dev_features MTU MAC MRG_RXBUF CTRL_VQ MQ ANY_LAYOUT VERSION_1 ACCESS_PLATFORM

b)Provision a vDPA device with all supported features:
$ vdpa dev add name vdpa0 mgmtdev pci/0000:01:00.5
$ vdpa/vdpa dev config show vdpa0
vdpa0: mac 00:e8:ca:11:be:05 link up link_announce false max_vq_pairs 4 mtu 1500
  negotiated_features MRG_RXBUF CTRL_VQ MQ VERSION_1 ACCESS_PLATFORM

c)Provision a vDPA device with a subset of the supported features:
$ vdpa dev add name vdpa0 mgmtdev pci/0000:01:00.5 device_features 0x300020020
$ vdpa dev config show vdpa0
mac 00:e8:ca:11:be:05 link up link_announce false
  negotiated_features CTRL_VQ VERSION_1 ACCESS_PLATFORM

Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
---
 drivers/vdpa/ifcvf/ifcvf_base.c |  2 +-
 drivers/vdpa/ifcvf/ifcvf_base.h |  3 +++
 drivers/vdpa/ifcvf/ifcvf_main.c | 13 +++++++++++++
 3 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/vdpa/ifcvf/ifcvf_base.c b/drivers/vdpa/ifcvf/ifcvf_base.c
index 3ec5ca3aefe1..5563b3a773c7 100644
--- a/drivers/vdpa/ifcvf/ifcvf_base.c
+++ b/drivers/vdpa/ifcvf/ifcvf_base.c
@@ -206,7 +206,7 @@ u64 ifcvf_get_hw_features(struct ifcvf_hw *hw)
 
 u64 ifcvf_get_features(struct ifcvf_hw *hw)
 {
-	return hw->hw_features;
+	return hw->dev_features;
 }
 
 int ifcvf_verify_min_features(struct ifcvf_hw *hw, u64 features)
diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h
index 15d5badc7dbd..e2dff9a46388 100644
--- a/drivers/vdpa/ifcvf/ifcvf_base.h
+++ b/drivers/vdpa/ifcvf/ifcvf_base.h
@@ -19,6 +19,7 @@
 #include <uapi/linux/virtio_blk.h>
 #include <uapi/linux/virtio_config.h>
 #include <uapi/linux/virtio_pci.h>
+#include <uapi/linux/vdpa.h>
 
 #define N3000_DEVICE_ID		0x1041
 #define N3000_SUBSYS_DEVICE_ID	0x001A
@@ -75,6 +76,8 @@ struct ifcvf_hw {
 	u32 dev_type;
 	u64 req_features;
 	u64 hw_features;
+	/* provisioned device features */
+	u64 dev_features;
 	struct virtio_pci_common_cfg __iomem *common_cfg;
 	void __iomem *dev_cfg;
 	struct vring_info vring[IFCVF_MAX_QUEUES];
diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c
index 76ac324c271b..22bf7029399e 100644
--- a/drivers/vdpa/ifcvf/ifcvf_main.c
+++ b/drivers/vdpa/ifcvf/ifcvf_main.c
@@ -745,6 +745,7 @@ static int ifcvf_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
 	struct vdpa_device *vdpa_dev;
 	struct pci_dev *pdev;
 	struct ifcvf_hw *vf;
+	u64 device_features;
 	int ret;
 
 	ifcvf_mgmt_dev = container_of(mdev, struct ifcvf_vdpa_mgmt_dev, mdev);
@@ -764,6 +765,17 @@ static int ifcvf_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
 	adapter->vf = vf;
 	vdpa_dev = &adapter->vdpa;
 
+	device_features = vf->hw_features;
+	if (config->mask & BIT_ULL(VDPA_ATTR_DEV_FEATURES)) {
+		if (config->device_features & ~device_features) {
+			IFCVF_ERR(pdev, "The provisioned features 0x%llx are not supported by this device with features 0x%llx\n",
+				  config->device_features, device_features);
+			return -EINVAL;
+		}
+		device_features &= config->device_features;
+	}
+	vf->dev_features = device_features;
+
 	if (name)
 		ret = dev_set_name(&vdpa_dev->dev, "%s", name);
 	else
@@ -868,6 +880,7 @@ static int ifcvf_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 
 	ifcvf_mgmt_dev->mdev.max_supported_vqs = vf->nr_vring;
 	ifcvf_mgmt_dev->mdev.supported_features = vf->hw_features;
+	ifcvf_mgmt_dev->mdev.config_attr_mask = (1 << VDPA_ATTR_DEV_FEATURES);
 
 	ret = vdpa_mgmtdev_register(&ifcvf_mgmt_dev->mdev);
 	if (ret) {
-- 
2.31.1

Re: [PATCH 0/4] ifcvf/vDPA implement features provisioning

[Jason Wang]

On Mon, Nov 7, 2022 at 5:42 PM Zhu Lingshan <lingshan.zhu@intel.com> wrote:
>
> This series implements features provisioning for ifcvf.
> By applying this series, we allow userspace to create
> a vDPA device with selected (management device supported)
> feature bits and mask out others.

I don't see a direct relationship between the first 3 and the last.
Maybe you can state the reason why the restructure is a must for the
feature provisioning. Otherwise, we'd better split the series.

Thanks

>
> Please help review
>
> Thanks
>
> Zhu Lingshan (4):
>   vDPA/ifcvf: ifcvf base layer interfaces work on struct ifcvf_hw
>   vDPA/ifcvf: IRQ interfaces work on ifcvf_hw
>   vDPA/ifcvf: allocate ifcvf_adapter in dev_add()
>   vDPA/ifcvf: implement features provisioning
>
>  drivers/vdpa/ifcvf/ifcvf_base.c |  32 ++-----
>  drivers/vdpa/ifcvf/ifcvf_base.h |  10 +-
>  drivers/vdpa/ifcvf/ifcvf_main.c | 156 +++++++++++++++-----------------
>  3 files changed, 89 insertions(+), 109 deletions(-)
>
> --
> 2.31.1
>

Re: [PATCH 0/4] ifcvf/vDPA implement features provisioning

[Zhu, Lingshan]

On 11/9/2022 2:51 PM, Jason Wang wrote:
> On Mon, Nov 7, 2022 at 5:42 PM Zhu Lingshan <lingshan.zhu@intel.com> wrote:
>> This series implements features provisioning for ifcvf.
>> By applying this series, we allow userspace to create
>> a vDPA device with selected (management device supported)
>> feature bits and mask out others.
> I don't see a direct relationship between the first 3 and the last.
> Maybe you can state the reason why the restructure is a must for the
> feature provisioning. Otherwise, we'd better split the series.
When introducing features provisioning ability to ifcvf, there is a need 
to re-create vDPA devices
on a VF with different feature bits.

When remove a vDPA device, the container of struct vdpa_device (here is 
ifcvf_adapter) is free-ed in
dev_del() interface, so we need to allocate ifcvf_adapter in dev_add() 
than in probe(). That's
why I have re-factored the adapter/mgmt_dev code.

For re-factoring the irq related code and ifcvf_base, let them work on 
struct ifcvf_hw, the
reason is that the adapter is allocated in dev_add(), if we want theses 
functions to work
before dev_add(), like in probe, we need them work on ifcvf_hw than the 
adapter.

Thanks
Zhu Lingshan
>
> Thanks
>
>> Please help review
>>
>> Thanks
>>
>> Zhu Lingshan (4):
>>    vDPA/ifcvf: ifcvf base layer interfaces work on struct ifcvf_hw
>>    vDPA/ifcvf: IRQ interfaces work on ifcvf_hw
>>    vDPA/ifcvf: allocate ifcvf_adapter in dev_add()
>>    vDPA/ifcvf: implement features provisioning
>>
>>   drivers/vdpa/ifcvf/ifcvf_base.c |  32 ++-----
>>   drivers/vdpa/ifcvf/ifcvf_base.h |  10 +-
>>   drivers/vdpa/ifcvf/ifcvf_main.c | 156 +++++++++++++++-----------------
>>   3 files changed, 89 insertions(+), 109 deletions(-)
>>
>> --
>> 2.31.1
>>

Re: [PATCH 0/4] ifcvf/vDPA implement features provisioning

[Jason Wang]

On Wed, Nov 9, 2022 at 4:14 PM Zhu, Lingshan <lingshan.zhu@intel.com> wrote:
>
>
>
> On 11/9/2022 2:51 PM, Jason Wang wrote:
> > On Mon, Nov 7, 2022 at 5:42 PM Zhu Lingshan <lingshan.zhu@intel.com> wrote:
> >> This series implements features provisioning for ifcvf.
> >> By applying this series, we allow userspace to create
> >> a vDPA device with selected (management device supported)
> >> feature bits and mask out others.
> > I don't see a direct relationship between the first 3 and the last.
> > Maybe you can state the reason why the restructure is a must for the
> > feature provisioning. Otherwise, we'd better split the series.
> When introducing features provisioning ability to ifcvf, there is a need
> to re-create vDPA devices
> on a VF with different feature bits.

This seems a requirement even without feature provisioning? Device
could be deleted from the management device anyhow.

Thakns

>
> When remove a vDPA device, the container of struct vdpa_device (here is
> ifcvf_adapter) is free-ed in
> dev_del() interface, so we need to allocate ifcvf_adapter in dev_add()
> than in probe(). That's
> why I have re-factored the adapter/mgmt_dev code.
>
> For re-factoring the irq related code and ifcvf_base, let them work on
> struct ifcvf_hw, the
> reason is that the adapter is allocated in dev_add(), if we want theses
> functions to work
> before dev_add(), like in probe, we need them work on ifcvf_hw than the
> adapter.
>
> Thanks
> Zhu Lingshan
> >
> > Thanks
> >
> >> Please help review
> >>
> >> Thanks
> >>
> >> Zhu Lingshan (4):
> >>    vDPA/ifcvf: ifcvf base layer interfaces work on struct ifcvf_hw
> >>    vDPA/ifcvf: IRQ interfaces work on ifcvf_hw
> >>    vDPA/ifcvf: allocate ifcvf_adapter in dev_add()
> >>    vDPA/ifcvf: implement features provisioning
> >>
> >>   drivers/vdpa/ifcvf/ifcvf_base.c |  32 ++-----
> >>   drivers/vdpa/ifcvf/ifcvf_base.h |  10 +-
> >>   drivers/vdpa/ifcvf/ifcvf_main.c | 156 +++++++++++++++-----------------
> >>   3 files changed, 89 insertions(+), 109 deletions(-)
> >>
> >> --
> >> 2.31.1
> >>
>

Re: [PATCH 0/4] ifcvf/vDPA implement features provisioning

[Zhu, Lingshan]

On 11/9/2022 4:59 PM, Jason Wang wrote:
> On Wed, Nov 9, 2022 at 4:14 PM Zhu, Lingshan <lingshan.zhu@intel.com> wrote:
>>
>>
>> On 11/9/2022 2:51 PM, Jason Wang wrote:
>>> On Mon, Nov 7, 2022 at 5:42 PM Zhu Lingshan <lingshan.zhu@intel.com> wrote:
>>>> This series implements features provisioning for ifcvf.
>>>> By applying this series, we allow userspace to create
>>>> a vDPA device with selected (management device supported)
>>>> feature bits and mask out others.
>>> I don't see a direct relationship between the first 3 and the last.
>>> Maybe you can state the reason why the restructure is a must for the
>>> feature provisioning. Otherwise, we'd better split the series.
>> When introducing features provisioning ability to ifcvf, there is a need
>> to re-create vDPA devices
>> on a VF with different feature bits.
> This seems a requirement even without feature provisioning? Device
> could be deleted from the management device anyhow.
Yes, we need this to delete and re-create a vDPA device.

We create vDPA device from a VF, so without features provisioning 
requirements,
we don't need to re-create the vDPA device. But with features provisioning,
it is a must now.

Thanks


>
> Thakns
>
>> When remove a vDPA device, the container of struct vdpa_device (here is
>> ifcvf_adapter) is free-ed in
>> dev_del() interface, so we need to allocate ifcvf_adapter in dev_add()
>> than in probe(). That's
>> why I have re-factored the adapter/mgmt_dev code.
>>
>> For re-factoring the irq related code and ifcvf_base, let them work on
>> struct ifcvf_hw, the
>> reason is that the adapter is allocated in dev_add(), if we want theses
>> functions to work
>> before dev_add(), like in probe, we need them work on ifcvf_hw than the
>> adapter.
>>
>> Thanks
>> Zhu Lingshan
>>> Thanks
>>>
>>>> Please help review
>>>>
>>>> Thanks
>>>>
>>>> Zhu Lingshan (4):
>>>>     vDPA/ifcvf: ifcvf base layer interfaces work on struct ifcvf_hw
>>>>     vDPA/ifcvf: IRQ interfaces work on ifcvf_hw
>>>>     vDPA/ifcvf: allocate ifcvf_adapter in dev_add()
>>>>     vDPA/ifcvf: implement features provisioning
>>>>
>>>>    drivers/vdpa/ifcvf/ifcvf_base.c |  32 ++-----
>>>>    drivers/vdpa/ifcvf/ifcvf_base.h |  10 +-
>>>>    drivers/vdpa/ifcvf/ifcvf_main.c | 156 +++++++++++++++-----------------
>>>>    3 files changed, 89 insertions(+), 109 deletions(-)
>>>>
>>>> --
>>>> 2.31.1
>>>>

Re: [PATCH 0/4] ifcvf/vDPA implement features provisioning

[Jason Wang]

On Wed, Nov 9, 2022 at 5:06 PM Zhu, Lingshan <lingshan.zhu@intel.com> wrote:
>
>
>
> On 11/9/2022 4:59 PM, Jason Wang wrote:
> > On Wed, Nov 9, 2022 at 4:14 PM Zhu, Lingshan <lingshan.zhu@intel.com> wrote:
> >>
> >>
> >> On 11/9/2022 2:51 PM, Jason Wang wrote:
> >>> On Mon, Nov 7, 2022 at 5:42 PM Zhu Lingshan <lingshan.zhu@intel.com> wrote:
> >>>> This series implements features provisioning for ifcvf.
> >>>> By applying this series, we allow userspace to create
> >>>> a vDPA device with selected (management device supported)
> >>>> feature bits and mask out others.
> >>> I don't see a direct relationship between the first 3 and the last.
> >>> Maybe you can state the reason why the restructure is a must for the
> >>> feature provisioning. Otherwise, we'd better split the series.
> >> When introducing features provisioning ability to ifcvf, there is a need
> >> to re-create vDPA devices
> >> on a VF with different feature bits.
> > This seems a requirement even without feature provisioning? Device
> > could be deleted from the management device anyhow.
> Yes, we need this to delete and re-create a vDPA device.

I wonder if we need something that works for -stable.

AFAIK, we can move the vdpa_alloc_device() from probe() to dev_add()
and it seems to work?

Thanks

>
> We create vDPA device from a VF, so without features provisioning
> requirements,
> we don't need to re-create the vDPA device. But with features provisioning,
> it is a must now.
>
> Thanks
>
>
> >
> > Thakns
> >
> >> When remove a vDPA device, the container of struct vdpa_device (here is
> >> ifcvf_adapter) is free-ed in
> >> dev_del() interface, so we need to allocate ifcvf_adapter in dev_add()
> >> than in probe(). That's
> >> why I have re-factored the adapter/mgmt_dev code.
> >>
> >> For re-factoring the irq related code and ifcvf_base, let them work on
> >> struct ifcvf_hw, the
> >> reason is that the adapter is allocated in dev_add(), if we want theses
> >> functions to work
> >> before dev_add(), like in probe, we need them work on ifcvf_hw than the
> >> adapter.
> >>
> >> Thanks
> >> Zhu Lingshan
> >>> Thanks
> >>>
> >>>> Please help review
> >>>>
> >>>> Thanks
> >>>>
> >>>> Zhu Lingshan (4):
> >>>>     vDPA/ifcvf: ifcvf base layer interfaces work on struct ifcvf_hw
> >>>>     vDPA/ifcvf: IRQ interfaces work on ifcvf_hw
> >>>>     vDPA/ifcvf: allocate ifcvf_adapter in dev_add()
> >>>>     vDPA/ifcvf: implement features provisioning
> >>>>
> >>>>    drivers/vdpa/ifcvf/ifcvf_base.c |  32 ++-----
> >>>>    drivers/vdpa/ifcvf/ifcvf_base.h |  10 +-
> >>>>    drivers/vdpa/ifcvf/ifcvf_main.c | 156 +++++++++++++++-----------------
> >>>>    3 files changed, 89 insertions(+), 109 deletions(-)
> >>>>
> >>>> --
> >>>> 2.31.1
> >>>>
>

Re: [PATCH 0/4] ifcvf/vDPA implement features provisioning

[Zhu, Lingshan]

On 11/10/2022 11:49 AM, Jason Wang wrote:
> On Wed, Nov 9, 2022 at 5:06 PM Zhu, Lingshan <lingshan.zhu@intel.com> wrote:
>>
>>
>> On 11/9/2022 4:59 PM, Jason Wang wrote:
>>> On Wed, Nov 9, 2022 at 4:14 PM Zhu, Lingshan <lingshan.zhu@intel.com> wrote:
>>>>
>>>> On 11/9/2022 2:51 PM, Jason Wang wrote:
>>>>> On Mon, Nov 7, 2022 at 5:42 PM Zhu Lingshan <lingshan.zhu@intel.com> wrote:
>>>>>> This series implements features provisioning for ifcvf.
>>>>>> By applying this series, we allow userspace to create
>>>>>> a vDPA device with selected (management device supported)
>>>>>> feature bits and mask out others.
>>>>> I don't see a direct relationship between the first 3 and the last.
>>>>> Maybe you can state the reason why the restructure is a must for the
>>>>> feature provisioning. Otherwise, we'd better split the series.
>>>> When introducing features provisioning ability to ifcvf, there is a need
>>>> to re-create vDPA devices
>>>> on a VF with different feature bits.
>>> This seems a requirement even without feature provisioning? Device
>>> could be deleted from the management device anyhow.
>> Yes, we need this to delete and re-create a vDPA device.
> I wonder if we need something that works for -stable.
I can add a fix tag, so these three patches could apply to stable
>
> AFAIK, we can move the vdpa_alloc_device() from probe() to dev_add()
> and it seems to work?
Yes and this is done in this series and that's why we need these
refactoring code.

By the way, do you have any comments to the patches?

Thanks,
Zhu Lingshan
>
> Thanks
>
>> We create vDPA device from a VF, so without features provisioning
>> requirements,
>> we don't need to re-create the vDPA device. But with features provisioning,
>> it is a must now.
>>
>> Thanks
>>
>>
>>> Thakns
>>>
>>>> When remove a vDPA device, the container of struct vdpa_device (here is
>>>> ifcvf_adapter) is free-ed in
>>>> dev_del() interface, so we need to allocate ifcvf_adapter in dev_add()
>>>> than in probe(). That's
>>>> why I have re-factored the adapter/mgmt_dev code.
>>>>
>>>> For re-factoring the irq related code and ifcvf_base, let them work on
>>>> struct ifcvf_hw, the
>>>> reason is that the adapter is allocated in dev_add(), if we want theses
>>>> functions to work
>>>> before dev_add(), like in probe, we need them work on ifcvf_hw than the
>>>> adapter.
>>>>
>>>> Thanks
>>>> Zhu Lingshan
>>>>> Thanks
>>>>>
>>>>>> Please help review
>>>>>>
>>>>>> Thanks
>>>>>>
>>>>>> Zhu Lingshan (4):
>>>>>>      vDPA/ifcvf: ifcvf base layer interfaces work on struct ifcvf_hw
>>>>>>      vDPA/ifcvf: IRQ interfaces work on ifcvf_hw
>>>>>>      vDPA/ifcvf: allocate ifcvf_adapter in dev_add()
>>>>>>      vDPA/ifcvf: implement features provisioning
>>>>>>
>>>>>>     drivers/vdpa/ifcvf/ifcvf_base.c |  32 ++-----
>>>>>>     drivers/vdpa/ifcvf/ifcvf_base.h |  10 +-
>>>>>>     drivers/vdpa/ifcvf/ifcvf_main.c | 156 +++++++++++++++-----------------
>>>>>>     3 files changed, 89 insertions(+), 109 deletions(-)
>>>>>>
>>>>>> --
>>>>>> 2.31.1
>>>>>>

Re: [PATCH 0/4] ifcvf/vDPA implement features provisioning

[Jason Wang]

在 2022/11/10 14:20, Zhu, Lingshan 写道:
>
>
> On 11/10/2022 11:49 AM, Jason Wang wrote:
>> On Wed, Nov 9, 2022 at 5:06 PM Zhu, Lingshan <lingshan.zhu@intel.com> 
>> wrote:
>>>
>>>
>>> On 11/9/2022 4:59 PM, Jason Wang wrote:
>>>> On Wed, Nov 9, 2022 at 4:14 PM Zhu, Lingshan 
>>>> <lingshan.zhu@intel.com> wrote:
>>>>>
>>>>> On 11/9/2022 2:51 PM, Jason Wang wrote:
>>>>>> On Mon, Nov 7, 2022 at 5:42 PM Zhu Lingshan 
>>>>>> <lingshan.zhu@intel.com> wrote:
>>>>>>> This series implements features provisioning for ifcvf.
>>>>>>> By applying this series, we allow userspace to create
>>>>>>> a vDPA device with selected (management device supported)
>>>>>>> feature bits and mask out others.
>>>>>> I don't see a direct relationship between the first 3 and the last.
>>>>>> Maybe you can state the reason why the restructure is a must for the
>>>>>> feature provisioning. Otherwise, we'd better split the series.
>>>>> When introducing features provisioning ability to ifcvf, there is 
>>>>> a need
>>>>> to re-create vDPA devices
>>>>> on a VF with different feature bits.
>>>> This seems a requirement even without feature provisioning? Device
>>>> could be deleted from the management device anyhow.
>>> Yes, we need this to delete and re-create a vDPA device.
>> I wonder if we need something that works for -stable.
> I can add a fix tag, so these three patches could apply to stable


It's too huge for -stable.


>>
>> AFAIK, we can move the vdpa_alloc_device() from probe() to dev_add()
>> and it seems to work?
> Yes and this is done in this series and that's why we need these
> refactoring code.


I meant there's probably no need to change the association of existing 
structure but just do the allocation in dev_add(), then we will have a 
patch with much more small changeset that fit for -stable.

Thanks


>
> By the way, do you have any comments to the patches?
>
> Thanks,
> Zhu Lingshan
>>
>> Thanks
>>
>>> We create vDPA device from a VF, so without features provisioning
>>> requirements,
>>> we don't need to re-create the vDPA device. But with features 
>>> provisioning,
>>> it is a must now.
>>>
>>> Thanks
>>>
>>>
>>>> Thakns
>>>>
>>>>> When remove a vDPA device, the container of struct vdpa_device 
>>>>> (here is
>>>>> ifcvf_adapter) is free-ed in
>>>>> dev_del() interface, so we need to allocate ifcvf_adapter in 
>>>>> dev_add()
>>>>> than in probe(). That's
>>>>> why I have re-factored the adapter/mgmt_dev code.
>>>>>
>>>>> For re-factoring the irq related code and ifcvf_base, let them 
>>>>> work on
>>>>> struct ifcvf_hw, the
>>>>> reason is that the adapter is allocated in dev_add(), if we want 
>>>>> theses
>>>>> functions to work
>>>>> before dev_add(), like in probe, we need them work on ifcvf_hw 
>>>>> than the
>>>>> adapter.
>>>>>
>>>>> Thanks
>>>>> Zhu Lingshan
>>>>>> Thanks
>>>>>>
>>>>>>> Please help review
>>>>>>>
>>>>>>> Thanks
>>>>>>>
>>>>>>> Zhu Lingshan (4):
>>>>>>>      vDPA/ifcvf: ifcvf base layer interfaces work on struct 
>>>>>>> ifcvf_hw
>>>>>>>      vDPA/ifcvf: IRQ interfaces work on ifcvf_hw
>>>>>>>      vDPA/ifcvf: allocate ifcvf_adapter in dev_add()
>>>>>>>      vDPA/ifcvf: implement features provisioning
>>>>>>>
>>>>>>>     drivers/vdpa/ifcvf/ifcvf_base.c |  32 ++-----
>>>>>>>     drivers/vdpa/ifcvf/ifcvf_base.h |  10 +-
>>>>>>>     drivers/vdpa/ifcvf/ifcvf_main.c | 156 
>>>>>>> +++++++++++++++-----------------
>>>>>>>     3 files changed, 89 insertions(+), 109 deletions(-)
>>>>>>>
>>>>>>> -- 
>>>>>>> 2.31.1
>>>>>>>
>

Re: [PATCH 0/4] ifcvf/vDPA implement features provisioning

[Zhu, Lingshan]

On 11/10/2022 2:29 PM, Jason Wang wrote:
>
> 在 2022/11/10 14:20, Zhu, Lingshan 写道:
>>
>>
>> On 11/10/2022 11:49 AM, Jason Wang wrote:
>>> On Wed, Nov 9, 2022 at 5:06 PM Zhu, Lingshan 
>>> <lingshan.zhu@intel.com> wrote:
>>>>
>>>>
>>>> On 11/9/2022 4:59 PM, Jason Wang wrote:
>>>>> On Wed, Nov 9, 2022 at 4:14 PM Zhu, Lingshan 
>>>>> <lingshan.zhu@intel.com> wrote:
>>>>>>
>>>>>> On 11/9/2022 2:51 PM, Jason Wang wrote:
>>>>>>> On Mon, Nov 7, 2022 at 5:42 PM Zhu Lingshan 
>>>>>>> <lingshan.zhu@intel.com> wrote:
>>>>>>>> This series implements features provisioning for ifcvf.
>>>>>>>> By applying this series, we allow userspace to create
>>>>>>>> a vDPA device with selected (management device supported)
>>>>>>>> feature bits and mask out others.
>>>>>>> I don't see a direct relationship between the first 3 and the last.
>>>>>>> Maybe you can state the reason why the restructure is a must for 
>>>>>>> the
>>>>>>> feature provisioning. Otherwise, we'd better split the series.
>>>>>> When introducing features provisioning ability to ifcvf, there is 
>>>>>> a need
>>>>>> to re-create vDPA devices
>>>>>> on a VF with different feature bits.
>>>>> This seems a requirement even without feature provisioning? Device
>>>>> could be deleted from the management device anyhow.
>>>> Yes, we need this to delete and re-create a vDPA device.
>>> I wonder if we need something that works for -stable.
>> I can add a fix tag, so these three patches could apply to stable
>
>
> It's too huge for -stable.
>
>
>>>
>>> AFAIK, we can move the vdpa_alloc_device() from probe() to dev_add()
>>> and it seems to work?
>> Yes and this is done in this series and that's why we need these
>> refactoring code.
>
>
> I meant there's probably no need to change the association of existing 
> structure but just do the allocation in dev_add(), then we will have a 
> patch with much more small changeset that fit for -stable.
Patch 1(ifcvf_base only work on ifcvf_hw) and patch 2(irq functions only 
work on ifcvf_hw) are not needed for stable.
I have already done this allocation of ifcvf_adapter which is the 
container of struct vdpa_device in dev_add() in Patch 3, this should be 
merged to stable.
Patch 3 is huge but necessary, not only allocate ifcvf_adapter in 
dev_add(), it also refactors the structures of ifcvf_mgmt_dev and 
ifcvf_adapter,
because we need to initialize the VF's hw structure ifcvf_hw(which was a 
member of ifcvf_adapter but now should be a member of ifcvf_mgmt_dev) in 
probe.

Is it still huge?

Thanks
>
> Thanks
>
>
>>
>> By the way, do you have any comments to the patches?
>>
>> Thanks,
>> Zhu Lingshan
>>>
>>> Thanks
>>>
>>>> We create vDPA device from a VF, so without features provisioning
>>>> requirements,
>>>> we don't need to re-create the vDPA device. But with features 
>>>> provisioning,
>>>> it is a must now.
>>>>
>>>> Thanks
>>>>
>>>>
>>>>> Thakns
>>>>>
>>>>>> When remove a vDPA device, the container of struct vdpa_device 
>>>>>> (here is
>>>>>> ifcvf_adapter) is free-ed in
>>>>>> dev_del() interface, so we need to allocate ifcvf_adapter in 
>>>>>> dev_add()
>>>>>> than in probe(). That's
>>>>>> why I have re-factored the adapter/mgmt_dev code.
>>>>>>
>>>>>> For re-factoring the irq related code and ifcvf_base, let them 
>>>>>> work on
>>>>>> struct ifcvf_hw, the
>>>>>> reason is that the adapter is allocated in dev_add(), if we want 
>>>>>> theses
>>>>>> functions to work
>>>>>> before dev_add(), like in probe, we need them work on ifcvf_hw 
>>>>>> than the
>>>>>> adapter.
>>>>>>
>>>>>> Thanks
>>>>>> Zhu Lingshan
>>>>>>> Thanks
>>>>>>>
>>>>>>>> Please help review
>>>>>>>>
>>>>>>>> Thanks
>>>>>>>>
>>>>>>>> Zhu Lingshan (4):
>>>>>>>>      vDPA/ifcvf: ifcvf base layer interfaces work on struct 
>>>>>>>> ifcvf_hw
>>>>>>>>      vDPA/ifcvf: IRQ interfaces work on ifcvf_hw
>>>>>>>>      vDPA/ifcvf: allocate ifcvf_adapter in dev_add()
>>>>>>>>      vDPA/ifcvf: implement features provisioning
>>>>>>>>
>>>>>>>>     drivers/vdpa/ifcvf/ifcvf_base.c |  32 ++-----
>>>>>>>>     drivers/vdpa/ifcvf/ifcvf_base.h |  10 +-
>>>>>>>>     drivers/vdpa/ifcvf/ifcvf_main.c | 156 
>>>>>>>> +++++++++++++++-----------------
>>>>>>>>     3 files changed, 89 insertions(+), 109 deletions(-)
>>>>>>>>
>>>>>>>> -- 
>>>>>>>> 2.31.1
>>>>>>>>
>>
>

Re: [PATCH 0/4] ifcvf/vDPA implement features provisioning

[Jason Wang]

On Thu, Nov 10, 2022 at 4:59 PM Zhu, Lingshan <lingshan.zhu@intel.com> wrote:
>
>
>
> On 11/10/2022 2:29 PM, Jason Wang wrote:
> >
> > 在 2022/11/10 14:20, Zhu, Lingshan 写道:
> >>
> >>
> >> On 11/10/2022 11:49 AM, Jason Wang wrote:
> >>> On Wed, Nov 9, 2022 at 5:06 PM Zhu, Lingshan
> >>> <lingshan.zhu@intel.com> wrote:
> >>>>
> >>>>
> >>>> On 11/9/2022 4:59 PM, Jason Wang wrote:
> >>>>> On Wed, Nov 9, 2022 at 4:14 PM Zhu, Lingshan
> >>>>> <lingshan.zhu@intel.com> wrote:
> >>>>>>
> >>>>>> On 11/9/2022 2:51 PM, Jason Wang wrote:
> >>>>>>> On Mon, Nov 7, 2022 at 5:42 PM Zhu Lingshan
> >>>>>>> <lingshan.zhu@intel.com> wrote:
> >>>>>>>> This series implements features provisioning for ifcvf.
> >>>>>>>> By applying this series, we allow userspace to create
> >>>>>>>> a vDPA device with selected (management device supported)
> >>>>>>>> feature bits and mask out others.
> >>>>>>> I don't see a direct relationship between the first 3 and the last.
> >>>>>>> Maybe you can state the reason why the restructure is a must for
> >>>>>>> the
> >>>>>>> feature provisioning. Otherwise, we'd better split the series.
> >>>>>> When introducing features provisioning ability to ifcvf, there is
> >>>>>> a need
> >>>>>> to re-create vDPA devices
> >>>>>> on a VF with different feature bits.
> >>>>> This seems a requirement even without feature provisioning? Device
> >>>>> could be deleted from the management device anyhow.
> >>>> Yes, we need this to delete and re-create a vDPA device.
> >>> I wonder if we need something that works for -stable.
> >> I can add a fix tag, so these three patches could apply to stable
> >
> >
> > It's too huge for -stable.
> >
> >
> >>>
> >>> AFAIK, we can move the vdpa_alloc_device() from probe() to dev_add()
> >>> and it seems to work?
> >> Yes and this is done in this series and that's why we need these
> >> refactoring code.
> >
> >
> > I meant there's probably no need to change the association of existing
> > structure but just do the allocation in dev_add(), then we will have a
> > patch with much more small changeset that fit for -stable.
> Patch 1(ifcvf_base only work on ifcvf_hw) and patch 2(irq functions only
> work on ifcvf_hw) are not needed for stable.
> I have already done this allocation of ifcvf_adapter which is the
> container of struct vdpa_device in dev_add() in Patch 3, this should be
> merged to stable.
> Patch 3 is huge but necessary, not only allocate ifcvf_adapter in
> dev_add(), it also refactors the structures of ifcvf_mgmt_dev and
> ifcvf_adapter,
> because we need to initialize the VF's hw structure ifcvf_hw(which was a
> member of ifcvf_adapter but now should be a member of ifcvf_mgmt_dev) in
> probe.
>
> Is it still huge?

Then please reorder the patches, stable-kernel-rules.rst said:

 - It cannot be bigger than 100 lines, with context.

Let's see.

Thanks

>
> Thanks
> >
> > Thanks
> >
> >
> >>
> >> By the way, do you have any comments to the patches?
> >>
> >> Thanks,
> >> Zhu Lingshan
> >>>
> >>> Thanks
> >>>
> >>>> We create vDPA device from a VF, so without features provisioning
> >>>> requirements,
> >>>> we don't need to re-create the vDPA device. But with features
> >>>> provisioning,
> >>>> it is a must now.
> >>>>
> >>>> Thanks
> >>>>
> >>>>
> >>>>> Thakns
> >>>>>
> >>>>>> When remove a vDPA device, the container of struct vdpa_device
> >>>>>> (here is
> >>>>>> ifcvf_adapter) is free-ed in
> >>>>>> dev_del() interface, so we need to allocate ifcvf_adapter in
> >>>>>> dev_add()
> >>>>>> than in probe(). That's
> >>>>>> why I have re-factored the adapter/mgmt_dev code.
> >>>>>>
> >>>>>> For re-factoring the irq related code and ifcvf_base, let them
> >>>>>> work on
> >>>>>> struct ifcvf_hw, the
> >>>>>> reason is that the adapter is allocated in dev_add(), if we want
> >>>>>> theses
> >>>>>> functions to work
> >>>>>> before dev_add(), like in probe, we need them work on ifcvf_hw
> >>>>>> than the
> >>>>>> adapter.
> >>>>>>
> >>>>>> Thanks
> >>>>>> Zhu Lingshan
> >>>>>>> Thanks
> >>>>>>>
> >>>>>>>> Please help review
> >>>>>>>>
> >>>>>>>> Thanks
> >>>>>>>>
> >>>>>>>> Zhu Lingshan (4):
> >>>>>>>>      vDPA/ifcvf: ifcvf base layer interfaces work on struct
> >>>>>>>> ifcvf_hw
> >>>>>>>>      vDPA/ifcvf: IRQ interfaces work on ifcvf_hw
> >>>>>>>>      vDPA/ifcvf: allocate ifcvf_adapter in dev_add()
> >>>>>>>>      vDPA/ifcvf: implement features provisioning
> >>>>>>>>
> >>>>>>>>     drivers/vdpa/ifcvf/ifcvf_base.c |  32 ++-----
> >>>>>>>>     drivers/vdpa/ifcvf/ifcvf_base.h |  10 +-
> >>>>>>>>     drivers/vdpa/ifcvf/ifcvf_main.c | 156
> >>>>>>>> +++++++++++++++-----------------
> >>>>>>>>     3 files changed, 89 insertions(+), 109 deletions(-)
> >>>>>>>>
> >>>>>>>> --
> >>>>>>>> 2.31.1
> >>>>>>>>
> >>
> >
>

Re: [PATCH 0/4] ifcvf/vDPA implement features provisioning

[Zhu, Lingshan]

On 11/10/2022 5:13 PM, Jason Wang wrote:
> On Thu, Nov 10, 2022 at 4:59 PM Zhu, Lingshan <lingshan.zhu@intel.com> wrote:
>>
>>
>> On 11/10/2022 2:29 PM, Jason Wang wrote:
>>> 在 2022/11/10 14:20, Zhu, Lingshan 写道:
>>>>
>>>> On 11/10/2022 11:49 AM, Jason Wang wrote:
>>>>> On Wed, Nov 9, 2022 at 5:06 PM Zhu, Lingshan
>>>>> <lingshan.zhu@intel.com> wrote:
>>>>>>
>>>>>> On 11/9/2022 4:59 PM, Jason Wang wrote:
>>>>>>> On Wed, Nov 9, 2022 at 4:14 PM Zhu, Lingshan
>>>>>>> <lingshan.zhu@intel.com> wrote:
>>>>>>>> On 11/9/2022 2:51 PM, Jason Wang wrote:
>>>>>>>>> On Mon, Nov 7, 2022 at 5:42 PM Zhu Lingshan
>>>>>>>>> <lingshan.zhu@intel.com> wrote:
>>>>>>>>>> This series implements features provisioning for ifcvf.
>>>>>>>>>> By applying this series, we allow userspace to create
>>>>>>>>>> a vDPA device with selected (management device supported)
>>>>>>>>>> feature bits and mask out others.
>>>>>>>>> I don't see a direct relationship between the first 3 and the last.
>>>>>>>>> Maybe you can state the reason why the restructure is a must for
>>>>>>>>> the
>>>>>>>>> feature provisioning. Otherwise, we'd better split the series.
>>>>>>>> When introducing features provisioning ability to ifcvf, there is
>>>>>>>> a need
>>>>>>>> to re-create vDPA devices
>>>>>>>> on a VF with different feature bits.
>>>>>>> This seems a requirement even without feature provisioning? Device
>>>>>>> could be deleted from the management device anyhow.
>>>>>> Yes, we need this to delete and re-create a vDPA device.
>>>>> I wonder if we need something that works for -stable.
>>>> I can add a fix tag, so these three patches could apply to stable
>>>
>>> It's too huge for -stable.
>>>
>>>
>>>>> AFAIK, we can move the vdpa_alloc_device() from probe() to dev_add()
>>>>> and it seems to work?
>>>> Yes and this is done in this series and that's why we need these
>>>> refactoring code.
>>>
>>> I meant there's probably no need to change the association of existing
>>> structure but just do the allocation in dev_add(), then we will have a
>>> patch with much more small changeset that fit for -stable.
>> Patch 1(ifcvf_base only work on ifcvf_hw) and patch 2(irq functions only
>> work on ifcvf_hw) are not needed for stable.
>> I have already done this allocation of ifcvf_adapter which is the
>> container of struct vdpa_device in dev_add() in Patch 3, this should be
>> merged to stable.
>> Patch 3 is huge but necessary, not only allocate ifcvf_adapter in
>> dev_add(), it also refactors the structures of ifcvf_mgmt_dev and
>> ifcvf_adapter,
>> because we need to initialize the VF's hw structure ifcvf_hw(which was a
>> member of ifcvf_adapter but now should be a member of ifcvf_mgmt_dev) in
>> probe.
>>
>> Is it still huge?
> Then please reorder the patches, stable-kernel-rules.rst said:
>
>   - It cannot be bigger than 100 lines, with context.
>
> Let's see.
It is over 180 lines, so maybe re-ordering can not help here, I will try 
to split patch 3.

Thanks,
Zhu Lingshan
>
> Thanks
>
>> Thanks
>>> Thanks
>>>
>>>
>>>> By the way, do you have any comments to the patches?
>>>>
>>>> Thanks,
>>>> Zhu Lingshan
>>>>> Thanks
>>>>>
>>>>>> We create vDPA device from a VF, so without features provisioning
>>>>>> requirements,
>>>>>> we don't need to re-create the vDPA device. But with features
>>>>>> provisioning,
>>>>>> it is a must now.
>>>>>>
>>>>>> Thanks
>>>>>>
>>>>>>
>>>>>>> Thakns
>>>>>>>
>>>>>>>> When remove a vDPA device, the container of struct vdpa_device
>>>>>>>> (here is
>>>>>>>> ifcvf_adapter) is free-ed in
>>>>>>>> dev_del() interface, so we need to allocate ifcvf_adapter in
>>>>>>>> dev_add()
>>>>>>>> than in probe(). That's
>>>>>>>> why I have re-factored the adapter/mgmt_dev code.
>>>>>>>>
>>>>>>>> For re-factoring the irq related code and ifcvf_base, let them
>>>>>>>> work on
>>>>>>>> struct ifcvf_hw, the
>>>>>>>> reason is that the adapter is allocated in dev_add(), if we want
>>>>>>>> theses
>>>>>>>> functions to work
>>>>>>>> before dev_add(), like in probe, we need them work on ifcvf_hw
>>>>>>>> than the
>>>>>>>> adapter.
>>>>>>>>
>>>>>>>> Thanks
>>>>>>>> Zhu Lingshan
>>>>>>>>> Thanks
>>>>>>>>>
>>>>>>>>>> Please help review
>>>>>>>>>>
>>>>>>>>>> Thanks
>>>>>>>>>>
>>>>>>>>>> Zhu Lingshan (4):
>>>>>>>>>>       vDPA/ifcvf: ifcvf base layer interfaces work on struct
>>>>>>>>>> ifcvf_hw
>>>>>>>>>>       vDPA/ifcvf: IRQ interfaces work on ifcvf_hw
>>>>>>>>>>       vDPA/ifcvf: allocate ifcvf_adapter in dev_add()
>>>>>>>>>>       vDPA/ifcvf: implement features provisioning
>>>>>>>>>>
>>>>>>>>>>      drivers/vdpa/ifcvf/ifcvf_base.c |  32 ++-----
>>>>>>>>>>      drivers/vdpa/ifcvf/ifcvf_base.h |  10 +-
>>>>>>>>>>      drivers/vdpa/ifcvf/ifcvf_main.c | 156
>>>>>>>>>> +++++++++++++++-----------------
>>>>>>>>>>      3 files changed, 89 insertions(+), 109 deletions(-)
>>>>>>>>>>
>>>>>>>>>> --
>>>>>>>>>> 2.31.1
>>>>>>>>>>