[PATCH 0/3] vdpa: device feature provisioning

[PATCH 0/3] vdpa: device feature provisioning

[Jason Wang]

Hi All:

Virtio features are neogiated between the device and the drivers. This
allows the mediation layer like vDPA to hide some features from the
driver to faciliate the cross vendor live migration:

vDPA on the source supports feature set X
vDPA on the destination supports feature set Y

Management can simply provision the vDPA instance with features X&Y on
both source and destination to let the vDPA can be migrate-able
between the two vDPA devies with different features support.

This series tries to allow the device features to be provisioned via
netlink to achieve this.

Please review.

Thanks

Jason Wang (3):
  vdpa: device feature provisioning
  vdpa_sim_net: support feature provisioning
  vp_vdpa: support feature provisioning

 drivers/vdpa/vdpa.c                  |  5 +++++
 drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 11 ++++++++++-
 drivers/vdpa/virtio_pci/vp_vdpa.c    | 16 ++++++++++++++--
 include/linux/vdpa.h                 |  1 +
 include/uapi/linux/vdpa.h            |  2 ++
 5 files changed, 32 insertions(+), 3 deletions(-)

-- 
2.25.1

[PATCH 1/3] vdpa: device feature provisioning

[Jason Wang]

This patch allows the device features to be provisioned through
netlink. A new attribute is introduced to allow the userspace to pass
a 64bit device features during device adding.

This provides several advantages:

- Allow to provision a subset of the features to ease the cross vendor
  live migration.
- Better debug-ability for vDPA framework and parent.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/vdpa/vdpa.c       | 5 +++++
 include/linux/vdpa.h      | 1 +
 include/uapi/linux/vdpa.h | 2 ++
 3 files changed, 8 insertions(+)

diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
index c06c02704461..278e26bfa492 100644
--- a/drivers/vdpa/vdpa.c
+++ b/drivers/vdpa/vdpa.c
@@ -600,6 +600,11 @@ static int vdpa_nl_cmd_dev_add_set_doit(struct sk_buff *skb, struct genl_info *i
 		}
 		config.mask |= BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MAX_VQP);
 	}
+	if (nl_attrs[VDPA_ATTR_DEV_FEATURES]) {
+		config.device_features =
+			nla_get_u64(nl_attrs[VDPA_ATTR_DEV_FEATURES]);
+		config.mask |= BIT_ULL(VDPA_ATTR_DEV_FEATURES);
+	}
 
 	/* Skip checking capability if user didn't prefer to configure any
 	 * device networking attributes. It is likely that user might have used
diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
index d282f464d2f1..6d0f5e4e82c2 100644
--- a/include/linux/vdpa.h
+++ b/include/linux/vdpa.h
@@ -104,6 +104,7 @@ struct vdpa_iova_range {
 };
 
 struct vdpa_dev_set_config {
+	u64 device_features;
 	struct {
 		u8 mac[ETH_ALEN];
 		u16 mtu;
diff --git a/include/uapi/linux/vdpa.h b/include/uapi/linux/vdpa.h
index 25c55cab3d7c..9dc855f37c59 100644
--- a/include/uapi/linux/vdpa.h
+++ b/include/uapi/linux/vdpa.h
@@ -52,6 +52,8 @@ enum vdpa_attr {
 	VDPA_ATTR_DEV_VENDOR_ATTR_NAME,		/* string */
 	VDPA_ATTR_DEV_VENDOR_ATTR_VALUE,        /* u64 */
 
+	VDPA_ATTR_DEV_FEATURES,                 /* u64 */
+
 	/* new attributes must be added above here */
 	VDPA_ATTR_MAX,
 };
-- 
2.25.1

[PATCH 2/3] vdpa_sim_net: support feature provisioning

[Jason Wang]

This patch implements features provisioning for vdpa_sim_net.

1) validating the provisioned features to be a subset of the parent
   features.
2) clearing the features that is not wanted by the  userspace

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
index 886449e88502..a9ba02be378b 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
@@ -254,6 +254,14 @@ static int vdpasim_net_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
 	dev_attr.work_fn = vdpasim_net_work;
 	dev_attr.buffer_size = PAGE_SIZE;
 
+	if (config->mask & BIT_ULL(VDPA_ATTR_DEV_FEATURES)) {
+		if (config->device_features &
+		    ~dev_attr.supported_features)
+			return -EINVAL;
+		dev_attr.supported_features &=
+			 config->device_features;
+	}
+
 	simdev = vdpasim_create(&dev_attr);
 	if (IS_ERR(simdev))
 		return PTR_ERR(simdev);
@@ -294,7 +302,8 @@ static struct vdpa_mgmt_dev mgmt_dev = {
 	.id_table = id_table,
 	.ops = &vdpasim_net_mgmtdev_ops,
 	.config_attr_mask = (1 << VDPA_ATTR_DEV_NET_CFG_MACADDR |
-			     1 << VDPA_ATTR_DEV_NET_CFG_MTU),
+			     1 << VDPA_ATTR_DEV_NET_CFG_MTU |
+		             1 << VDPA_ATTR_DEV_FEATURES),
 	.max_supported_vqs = VDPASIM_NET_VQ_NUM,
 	.supported_features = VDPASIM_NET_FEATURES,
 };
-- 
2.25.1

[PATCH 3/3] vp_vdpa: support feature provisioning

[Jason Wang]

This patch allows the device features to be provisioned via
netlink. This is done by:

1) validating the provisioned features to be a subset of the parent
   features.
2) clearing the features that is not wanted by the userspace

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/vdpa/virtio_pci/vp_vdpa.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/vdpa/virtio_pci/vp_vdpa.c b/drivers/vdpa/virtio_pci/vp_vdpa.c
index 04522077735b..4b28e0c95ba2 100644
--- a/drivers/vdpa/virtio_pci/vp_vdpa.c
+++ b/drivers/vdpa/virtio_pci/vp_vdpa.c
@@ -17,6 +17,7 @@
 #include <linux/virtio_ring.h>
 #include <linux/virtio_pci.h>
 #include <linux/virtio_pci_modern.h>
+#include <uapi/linux/vdpa.h>
 
 #define VP_VDPA_QUEUE_MAX 256
 #define VP_VDPA_DRIVER_NAME "vp_vdpa"
@@ -35,6 +36,7 @@ struct vp_vdpa {
 	struct virtio_pci_modern_device *mdev;
 	struct vp_vring *vring;
 	struct vdpa_callback config_cb;
+	u64 device_features;
 	char msix_name[VP_VDPA_NAME_SIZE];
 	int config_irq;
 	int queues;
@@ -66,9 +68,9 @@ static struct virtio_pci_modern_device *vp_vdpa_to_mdev(struct vp_vdpa *vp_vdpa)
 
 static u64 vp_vdpa_get_device_features(struct vdpa_device *vdpa)
 {
-	struct virtio_pci_modern_device *mdev = vdpa_to_mdev(vdpa);
+	struct vp_vdpa *vp_vdpa = vdpa_to_vp(vdpa);
 
-	return vp_modern_get_features(mdev);
+	return vp_vdpa->device_features;
 }
 
 static int vp_vdpa_set_driver_features(struct vdpa_device *vdpa, u64 features)
@@ -475,6 +477,7 @@ static int vp_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name,
 	struct pci_dev *pdev = mdev->pci_dev;
 	struct device *dev = &pdev->dev;
 	struct vp_vdpa *vp_vdpa = NULL;
+	u64 device_features;
 	int ret, i;
 
 	vp_vdpa = vdpa_alloc_device(struct vp_vdpa, vdpa,
@@ -491,6 +494,14 @@ static int vp_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name,
 	vp_vdpa->queues = vp_modern_get_num_queues(mdev);
 	vp_vdpa->mdev = mdev;
 
+	device_features = vp_modern_get_features(mdev);
+	if (add_config->mask & BIT_ULL(VDPA_ATTR_DEV_FEATURES)) {
+		if (add_config->device_features & ~device_features)
+			return -EINVAL;
+		device_features &= add_config->device_features;
+	}
+	vp_vdpa->device_features = device_features;
+
 	ret = devm_add_action_or_reset(dev, vp_vdpa_free_irq_vectors, pdev);
 	if (ret) {
 		dev_err(&pdev->dev,
@@ -599,6 +610,7 @@ static int vp_vdpa_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	mgtdev->id_table = mdev_id;
 	mgtdev->max_supported_vqs = vp_modern_get_num_queues(mdev);
 	mgtdev->supported_features = vp_modern_get_features(mdev);
+	mgtdev->config_attr_mask = (1 << VDPA_ATTR_DEV_FEATURES);
 	pci_set_master(pdev);
 	pci_set_drvdata(pdev, vp_vdpa_mgtdev);
 
-- 
2.25.1

RE: [PATCH 1/3] vdpa: device feature provisioning

[Eli Cohen]

> From: Jason Wang <jasowang@redhat.com>
> Sent: Thursday, 15 September 2022 11:51
> To: mst@redhat.com; jasowang@redhat.com
> Cc: Eli Cohen <elic@nvidia.com>; si-wei.liu@oracle.com; Parav Pandit
> <parav@nvidia.com>; wuzongyong@linux.alibaba.com;
> virtualization@lists.linux-foundation.org; linux-kernel@vger.kernel.org;
> eperezma@redhat.com; lingshan.zhu@intel.com; gdawar@xilinx.com;
> lulu@redhat.com; xieyongji@bytedance.com
> Subject: [PATCH 1/3] vdpa: device feature provisioning
> 
> This patch allows the device features to be provisioned through
> netlink. A new attribute is introduced to allow the userspace to pass
> a 64bit device features during device adding.
> 
> This provides several advantages:
> 
> - Allow to provision a subset of the features to ease the cross vendor
>   live migration.
> - Better debug-ability for vDPA framework and parent.
> 
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  drivers/vdpa/vdpa.c       | 5 +++++
>  include/linux/vdpa.h      | 1 +
>  include/uapi/linux/vdpa.h | 2 ++
>  3 files changed, 8 insertions(+)
> 
> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
> index c06c02704461..278e26bfa492 100644
> --- a/drivers/vdpa/vdpa.c
> +++ b/drivers/vdpa/vdpa.c
> @@ -600,6 +600,11 @@ static int vdpa_nl_cmd_dev_add_set_doit(struct
> sk_buff *skb, struct genl_info *i
>  		}
>  		config.mask |=
> BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MAX_VQP);
>  	}
> +	if (nl_attrs[VDPA_ATTR_DEV_FEATURES]) {
> +		config.device_features =
> +			nla_get_u64(nl_attrs[VDPA_ATTR_DEV_FEATURES]);
> +		config.mask |= BIT_ULL(VDPA_ATTR_DEV_FEATURES);
> +	}
> 
>  	/* Skip checking capability if user didn't prefer to configure any
>  	 * device networking attributes. It is likely that user might have used
> diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
> index d282f464d2f1..6d0f5e4e82c2 100644
> --- a/include/linux/vdpa.h
> +++ b/include/linux/vdpa.h
> @@ -104,6 +104,7 @@ struct vdpa_iova_range {
>  };
> 
>  struct vdpa_dev_set_config {
> +	u64 device_features;
>  	struct {
>  		u8 mac[ETH_ALEN];
>  		u16 mtu;
> diff --git a/include/uapi/linux/vdpa.h b/include/uapi/linux/vdpa.h
> index 25c55cab3d7c..9dc855f37c59 100644
> --- a/include/uapi/linux/vdpa.h
> +++ b/include/uapi/linux/vdpa.h
> @@ -52,6 +52,8 @@ enum vdpa_attr {
>  	VDPA_ATTR_DEV_VENDOR_ATTR_NAME,		/* string */
>  	VDPA_ATTR_DEV_VENDOR_ATTR_VALUE,        /* u64 */
> 
> +	VDPA_ATTR_DEV_FEATURES,                 /* u64 */

Maybe VDPA_ATTR_DEV_FEATURES_MASK would be more informative. Other than that:
Reviewed-by: Eli Cohen <elic@nvidia.com>

> +
>  	/* new attributes must be added above here */
>  	VDPA_ATTR_MAX,
>  };
> --
> 2.25.1

RE: [PATCH 2/3] vdpa_sim_net: support feature provisioning

[Eli Cohen]

> From: Jason Wang <jasowang@redhat.com>
> Sent: Thursday, 15 September 2022 11:51
> To: mst@redhat.com; jasowang@redhat.com
> Cc: Eli Cohen <elic@nvidia.com>; si-wei.liu@oracle.com; Parav Pandit
> <parav@nvidia.com>; wuzongyong@linux.alibaba.com;
> virtualization@lists.linux-foundation.org; linux-kernel@vger.kernel.org;
> eperezma@redhat.com; lingshan.zhu@intel.com; gdawar@xilinx.com;
> lulu@redhat.com; xieyongji@bytedance.com
> Subject: [PATCH 2/3] vdpa_sim_net: support feature provisioning
> 
> This patch implements features provisioning for vdpa_sim_net.
> 
> 1) validating the provisioned features to be a subset of the parent
>    features.
> 2) clearing the features that is not wanted by the  userspace
> 

Reviewed-by: Eli Cohen <elic@nvidia.com>

> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
> b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
> index 886449e88502..a9ba02be378b 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
> @@ -254,6 +254,14 @@ static int vdpasim_net_dev_add(struct
> vdpa_mgmt_dev *mdev, const char *name,
>  	dev_attr.work_fn = vdpasim_net_work;
>  	dev_attr.buffer_size = PAGE_SIZE;
> 
> +	if (config->mask & BIT_ULL(VDPA_ATTR_DEV_FEATURES)) {
> +		if (config->device_features &
> +		    ~dev_attr.supported_features)
> +			return -EINVAL;
> +		dev_attr.supported_features &=
> +			 config->device_features;
> +	}
> +
>  	simdev = vdpasim_create(&dev_attr);
>  	if (IS_ERR(simdev))
>  		return PTR_ERR(simdev);
> @@ -294,7 +302,8 @@ static struct vdpa_mgmt_dev mgmt_dev = {
>  	.id_table = id_table,
>  	.ops = &vdpasim_net_mgmtdev_ops,
>  	.config_attr_mask = (1 << VDPA_ATTR_DEV_NET_CFG_MACADDR |
> -			     1 << VDPA_ATTR_DEV_NET_CFG_MTU),
> +			     1 << VDPA_ATTR_DEV_NET_CFG_MTU |
> +		             1 << VDPA_ATTR_DEV_FEATURES),
>  	.max_supported_vqs = VDPASIM_NET_VQ_NUM,
>  	.supported_features = VDPASIM_NET_FEATURES,
>  };
> --
> 2.25.1

RE: [PATCH 3/3] vp_vdpa: support feature provisioning

[Eli Cohen]

> From: Jason Wang <jasowang@redhat.com>
> Sent: Thursday, 15 September 2022 11:51
> To: mst@redhat.com; jasowang@redhat.com
> Cc: Eli Cohen <elic@nvidia.com>; si-wei.liu@oracle.com; Parav Pandit
> <parav@nvidia.com>; wuzongyong@linux.alibaba.com;
> virtualization@lists.linux-foundation.org; linux-kernel@vger.kernel.org;
> eperezma@redhat.com; lingshan.zhu@intel.com; gdawar@xilinx.com;
> lulu@redhat.com; xieyongji@bytedance.com
> Subject: [PATCH 3/3] vp_vdpa: support feature provisioning
> 
> This patch allows the device features to be provisioned via
> netlink. This is done by:
> 
> 1) validating the provisioned features to be a subset of the parent
>    features.
> 2) clearing the features that is not wanted by the userspace
> 
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  drivers/vdpa/virtio_pci/vp_vdpa.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/vdpa/virtio_pci/vp_vdpa.c
> b/drivers/vdpa/virtio_pci/vp_vdpa.c
> index 04522077735b..4b28e0c95ba2 100644
> --- a/drivers/vdpa/virtio_pci/vp_vdpa.c
> +++ b/drivers/vdpa/virtio_pci/vp_vdpa.c
> @@ -17,6 +17,7 @@
>  #include <linux/virtio_ring.h>
>  #include <linux/virtio_pci.h>
>  #include <linux/virtio_pci_modern.h>
> +#include <uapi/linux/vdpa.h>
> 
>  #define VP_VDPA_QUEUE_MAX 256
>  #define VP_VDPA_DRIVER_NAME "vp_vdpa"
> @@ -35,6 +36,7 @@ struct vp_vdpa {
>  	struct virtio_pci_modern_device *mdev;
>  	struct vp_vring *vring;
>  	struct vdpa_callback config_cb;
> +	u64 device_features;
>  	char msix_name[VP_VDPA_NAME_SIZE];
>  	int config_irq;
>  	int queues;
> @@ -66,9 +68,9 @@ static struct virtio_pci_modern_device
> *vp_vdpa_to_mdev(struct vp_vdpa *vp_vdpa)
> 
>  static u64 vp_vdpa_get_device_features(struct vdpa_device *vdpa)
>  {
> -	struct virtio_pci_modern_device *mdev = vdpa_to_mdev(vdpa);
> +	struct vp_vdpa *vp_vdpa = vdpa_to_vp(vdpa);
> 
> -	return vp_modern_get_features(mdev);
> +	return vp_vdpa->device_features;
>  }

Unrelated to this patch. Maybe put in another patch?

> 
>  static int vp_vdpa_set_driver_features(struct vdpa_device *vdpa, u64
> features)
> @@ -475,6 +477,7 @@ static int vp_vdpa_dev_add(struct vdpa_mgmt_dev
> *v_mdev, const char *name,
>  	struct pci_dev *pdev = mdev->pci_dev;
>  	struct device *dev = &pdev->dev;
>  	struct vp_vdpa *vp_vdpa = NULL;
> +	u64 device_features;
>  	int ret, i;
> 
>  	vp_vdpa = vdpa_alloc_device(struct vp_vdpa, vdpa,
> @@ -491,6 +494,14 @@ static int vp_vdpa_dev_add(struct vdpa_mgmt_dev
> *v_mdev, const char *name,
>  	vp_vdpa->queues = vp_modern_get_num_queues(mdev);
>  	vp_vdpa->mdev = mdev;
> 
> +	device_features = vp_modern_get_features(mdev);
> +	if (add_config->mask & BIT_ULL(VDPA_ATTR_DEV_FEATURES)) {
> +		if (add_config->device_features & ~device_features)
> +			return -EINVAL;
> +		device_features &= add_config->device_features;
> +	}
> +	vp_vdpa->device_features = device_features;
> +
>  	ret = devm_add_action_or_reset(dev, vp_vdpa_free_irq_vectors,
> pdev);
>  	if (ret) {
>  		dev_err(&pdev->dev,
> @@ -599,6 +610,7 @@ static int vp_vdpa_probe(struct pci_dev *pdev, const
> struct pci_device_id *id)
>  	mgtdev->id_table = mdev_id;
>  	mgtdev->max_supported_vqs =
> vp_modern_get_num_queues(mdev);
>  	mgtdev->supported_features = vp_modern_get_features(mdev);
> +	mgtdev->config_attr_mask = (1 << VDPA_ATTR_DEV_FEATURES);
>  	pci_set_master(pdev);
>  	pci_set_drvdata(pdev, vp_vdpa_mgtdev);
> 
> --
> 2.25.1

Re: [PATCH 3/3] vp_vdpa: support feature provisioning

[Jason Wang]

On Mon, Sep 19, 2022 at 3:05 PM Eli Cohen <elic@nvidia.com> wrote:
>
> > From: Jason Wang <jasowang@redhat.com>
> > Sent: Thursday, 15 September 2022 11:51
> > To: mst@redhat.com; jasowang@redhat.com
> > Cc: Eli Cohen <elic@nvidia.com>; si-wei.liu@oracle.com; Parav Pandit
> > <parav@nvidia.com>; wuzongyong@linux.alibaba.com;
> > virtualization@lists.linux-foundation.org; linux-kernel@vger.kernel.org;
> > eperezma@redhat.com; lingshan.zhu@intel.com; gdawar@xilinx.com;
> > lulu@redhat.com; xieyongji@bytedance.com
> > Subject: [PATCH 3/3] vp_vdpa: support feature provisioning
> >
> > This patch allows the device features to be provisioned via
> > netlink. This is done by:
> >
> > 1) validating the provisioned features to be a subset of the parent
> >    features.
> > 2) clearing the features that is not wanted by the userspace
> >
> > Signed-off-by: Jason Wang <jasowang@redhat.com>
> > ---
> >  drivers/vdpa/virtio_pci/vp_vdpa.c | 16 ++++++++++++++--
> >  1 file changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/vdpa/virtio_pci/vp_vdpa.c
> > b/drivers/vdpa/virtio_pci/vp_vdpa.c
> > index 04522077735b..4b28e0c95ba2 100644
> > --- a/drivers/vdpa/virtio_pci/vp_vdpa.c
> > +++ b/drivers/vdpa/virtio_pci/vp_vdpa.c
> > @@ -17,6 +17,7 @@
> >  #include <linux/virtio_ring.h>
> >  #include <linux/virtio_pci.h>
> >  #include <linux/virtio_pci_modern.h>
> > +#include <uapi/linux/vdpa.h>
> >
> >  #define VP_VDPA_QUEUE_MAX 256
> >  #define VP_VDPA_DRIVER_NAME "vp_vdpa"
> > @@ -35,6 +36,7 @@ struct vp_vdpa {
> >       struct virtio_pci_modern_device *mdev;
> >       struct vp_vring *vring;
> >       struct vdpa_callback config_cb;
> > +     u64 device_features;
> >       char msix_name[VP_VDPA_NAME_SIZE];
> >       int config_irq;
> >       int queues;
> > @@ -66,9 +68,9 @@ static struct virtio_pci_modern_device
> > *vp_vdpa_to_mdev(struct vp_vdpa *vp_vdpa)
> >
> >  static u64 vp_vdpa_get_device_features(struct vdpa_device *vdpa)
> >  {
> > -     struct virtio_pci_modern_device *mdev = vdpa_to_mdev(vdpa);
> > +     struct vp_vdpa *vp_vdpa = vdpa_to_vp(vdpa);
> >
> > -     return vp_modern_get_features(mdev);
> > +     return vp_vdpa->device_features;
> >  }
>
> Unrelated to this patch. Maybe put in another patch?

Actually not, since we don't want to report the features that are
supported by the device, but the features that are provisioned via
netlink:

> > +     if (add_config->mask & BIT_ULL(VDPA_ATTR_DEV_FEATURES)) {
> > +             if (add_config->device_features & ~device_features)
> > +                     return -EINVAL;
> > +             device_features &= add_config->device_features;
> > +     }
> > +     vp_vdpa->device_features = device_features;

Thanks

>
> >
> >  static int vp_vdpa_set_driver_features(struct vdpa_device *vdpa, u64
> > features)
> > @@ -475,6 +477,7 @@ static int vp_vdpa_dev_add(struct vdpa_mgmt_dev
> > *v_mdev, const char *name,
> >       struct pci_dev *pdev = mdev->pci_dev;
> >       struct device *dev = &pdev->dev;
> >       struct vp_vdpa *vp_vdpa = NULL;
> > +     u64 device_features;
> >       int ret, i;
> >
> >       vp_vdpa = vdpa_alloc_device(struct vp_vdpa, vdpa,
> > @@ -491,6 +494,14 @@ static int vp_vdpa_dev_add(struct vdpa_mgmt_dev
> > *v_mdev, const char *name,
> >       vp_vdpa->queues = vp_modern_get_num_queues(mdev);
> >       vp_vdpa->mdev = mdev;
> >
> > +     device_features = vp_modern_get_features(mdev);
> > +     if (add_config->mask & BIT_ULL(VDPA_ATTR_DEV_FEATURES)) {
> > +             if (add_config->device_features & ~device_features)
> > +                     return -EINVAL;
> > +             device_features &= add_config->device_features;
> > +     }
> > +     vp_vdpa->device_features = device_features;
> > +
> >       ret = devm_add_action_or_reset(dev, vp_vdpa_free_irq_vectors,
> > pdev);
> >       if (ret) {
> >               dev_err(&pdev->dev,
> > @@ -599,6 +610,7 @@ static int vp_vdpa_probe(struct pci_dev *pdev, const
> > struct pci_device_id *id)
> >       mgtdev->id_table = mdev_id;
> >       mgtdev->max_supported_vqs =
> > vp_modern_get_num_queues(mdev);
> >       mgtdev->supported_features = vp_modern_get_features(mdev);
> > +     mgtdev->config_attr_mask = (1 << VDPA_ATTR_DEV_FEATURES);
> >       pci_set_master(pdev);
> >       pci_set_drvdata(pdev, vp_vdpa_mgtdev);
> >
> > --
> > 2.25.1
>

RE: [PATCH 3/3] vp_vdpa: support feature provisioning

[Eli Cohen]

> From: Jason Wang <jasowang@redhat.com>
> Sent: Tuesday, 20 September 2022 4:46
> To: Eli Cohen <elic@nvidia.com>
> Cc: mst@redhat.com; si-wei.liu@oracle.com; Parav Pandit
> <parav@nvidia.com>; wuzongyong@linux.alibaba.com;
> virtualization@lists.linux-foundation.org; linux-kernel@vger.kernel.org;
> eperezma@redhat.com; lingshan.zhu@intel.com; gdawar@xilinx.com;
> lulu@redhat.com; xieyongji@bytedance.com
> Subject: Re: [PATCH 3/3] vp_vdpa: support feature provisioning
> 
> On Mon, Sep 19, 2022 at 3:05 PM Eli Cohen <elic@nvidia.com> wrote:
> >
> > > From: Jason Wang <jasowang@redhat.com>
> > > Sent: Thursday, 15 September 2022 11:51
> > > To: mst@redhat.com; jasowang@redhat.com
> > > Cc: Eli Cohen <elic@nvidia.com>; si-wei.liu@oracle.com; Parav Pandit
> > > <parav@nvidia.com>; wuzongyong@linux.alibaba.com;
> > > virtualization@lists.linux-foundation.org; linux-kernel@vger.kernel.org;
> > > eperezma@redhat.com; lingshan.zhu@intel.com; gdawar@xilinx.com;
> > > lulu@redhat.com; xieyongji@bytedance.com
> > > Subject: [PATCH 3/3] vp_vdpa: support feature provisioning
> > >
> > > This patch allows the device features to be provisioned via
> > > netlink. This is done by:
> > >
> > > 1) validating the provisioned features to be a subset of the parent
> > >    features.
> > > 2) clearing the features that is not wanted by the userspace
> > >
> > > Signed-off-by: Jason Wang <jasowang@redhat.com>
> > > ---
> > >  drivers/vdpa/virtio_pci/vp_vdpa.c | 16 ++++++++++++++--
> > >  1 file changed, 14 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/vdpa/virtio_pci/vp_vdpa.c
> > > b/drivers/vdpa/virtio_pci/vp_vdpa.c
> > > index 04522077735b..4b28e0c95ba2 100644
> > > --- a/drivers/vdpa/virtio_pci/vp_vdpa.c
> > > +++ b/drivers/vdpa/virtio_pci/vp_vdpa.c
> > > @@ -17,6 +17,7 @@
> > >  #include <linux/virtio_ring.h>
> > >  #include <linux/virtio_pci.h>
> > >  #include <linux/virtio_pci_modern.h>
> > > +#include <uapi/linux/vdpa.h>
> > >
> > >  #define VP_VDPA_QUEUE_MAX 256
> > >  #define VP_VDPA_DRIVER_NAME "vp_vdpa"
> > > @@ -35,6 +36,7 @@ struct vp_vdpa {
> > >       struct virtio_pci_modern_device *mdev;
> > >       struct vp_vring *vring;
> > >       struct vdpa_callback config_cb;
> > > +     u64 device_features;
> > >       char msix_name[VP_VDPA_NAME_SIZE];
> > >       int config_irq;
> > >       int queues;
> > > @@ -66,9 +68,9 @@ static struct virtio_pci_modern_device
> > > *vp_vdpa_to_mdev(struct vp_vdpa *vp_vdpa)
> > >
> > >  static u64 vp_vdpa_get_device_features(struct vdpa_device *vdpa)
> > >  {
> > > -     struct virtio_pci_modern_device *mdev = vdpa_to_mdev(vdpa);
> > > +     struct vp_vdpa *vp_vdpa = vdpa_to_vp(vdpa);
> > >
> > > -     return vp_modern_get_features(mdev);
> > > +     return vp_vdpa->device_features;
> > >  }
> >
> > Unrelated to this patch. Maybe put in another patch?
> 
> Actually not, since we don't want to report the features that are
> supported by the device, but the features that are provisioned via
> netlink:
> 

I see.

Reviewed-by: Eli Cohen <elic@nvidia.com>

> > > +     if (add_config->mask & BIT_ULL(VDPA_ATTR_DEV_FEATURES)) {
> > > +             if (add_config->device_features & ~device_features)
> > > +                     return -EINVAL;
> > > +             device_features &= add_config->device_features;
> > > +     }
> > > +     vp_vdpa->device_features = device_features;
> 
> Thanks
> 
> >
> > >
> > >  static int vp_vdpa_set_driver_features(struct vdpa_device *vdpa, u64
> > > features)
> > > @@ -475,6 +477,7 @@ static int vp_vdpa_dev_add(struct
> vdpa_mgmt_dev
> > > *v_mdev, const char *name,
> > >       struct pci_dev *pdev = mdev->pci_dev;
> > >       struct device *dev = &pdev->dev;
> > >       struct vp_vdpa *vp_vdpa = NULL;
> > > +     u64 device_features;
> > >       int ret, i;
> > >
> > >       vp_vdpa = vdpa_alloc_device(struct vp_vdpa, vdpa,
> > > @@ -491,6 +494,14 @@ static int vp_vdpa_dev_add(struct
> vdpa_mgmt_dev
> > > *v_mdev, const char *name,
> > >       vp_vdpa->queues = vp_modern_get_num_queues(mdev);
> > >       vp_vdpa->mdev = mdev;
> > >
> > > +     device_features = vp_modern_get_features(mdev);
> > > +     if (add_config->mask & BIT_ULL(VDPA_ATTR_DEV_FEATURES)) {
> > > +             if (add_config->device_features & ~device_features)
> > > +                     return -EINVAL;
> > > +             device_features &= add_config->device_features;
> > > +     }
> > > +     vp_vdpa->device_features = device_features;
> > > +
> > >       ret = devm_add_action_or_reset(dev, vp_vdpa_free_irq_vectors,
> > > pdev);
> > >       if (ret) {
> > >               dev_err(&pdev->dev,
> > > @@ -599,6 +610,7 @@ static int vp_vdpa_probe(struct pci_dev *pdev,
> const
> > > struct pci_device_id *id)
> > >       mgtdev->id_table = mdev_id;
> > >       mgtdev->max_supported_vqs =
> > > vp_modern_get_num_queues(mdev);
> > >       mgtdev->supported_features = vp_modern_get_features(mdev);
> > > +     mgtdev->config_attr_mask = (1 << VDPA_ATTR_DEV_FEATURES);
> > >       pci_set_master(pdev);
> > >       pci_set_drvdata(pdev, vp_vdpa_mgtdev);
> > >
> > > --
> > > 2.25.1
> >

RE: [PATCH 0/3] vdpa: device feature provisioning

[Parav Pandit]

Hi Jason,

> From: Jason Wang <jasowang@redhat.com>
> Sent: Thursday, September 15, 2022 4:51 AM
> To: mst@redhat.com; jasowang@redhat.com
> Cc: Eli Cohen <elic@nvidia.com>; si-wei.liu@oracle.com; Parav Pandit
> <parav@nvidia.com>; wuzongyong@linux.alibaba.com;
> virtualization@lists.linux-foundation.org; linux-kernel@vger.kernel.org;
> eperezma@redhat.com; lingshan.zhu@intel.com; gdawar@xilinx.com;
> lulu@redhat.com; xieyongji@bytedance.com
> Subject: [PATCH 0/3] vdpa: device feature provisioning
> 
> Hi All:
> 
> Virtio features are neogiated between the device and the drivers. This allows
> the mediation layer like vDPA to hide some features from the driver to
> faciliate the cross vendor live migration:
> 
> vDPA on the source supports feature set X vDPA on the destination supports
> feature set Y
> 
> Management can simply provision the vDPA instance with features X&Y on
> both source and destination to let the vDPA can be migrate-able between
> the two vDPA devies with different features support.
> 
> This series tries to allow the device features to be provisioned via netlink to
> achieve this.
Very useful series.
Can you please add vdpa example command with and without -jp option in each of the commit logs?

Thanks.

Re: [PATCH 0/3] vdpa: device feature provisioning

[Jason Wang]

On Wed, Sep 21, 2022 at 3:17 AM Parav Pandit <parav@nvidia.com> wrote:
>
> Hi Jason,
>
> > From: Jason Wang <jasowang@redhat.com>
> > Sent: Thursday, September 15, 2022 4:51 AM
> > To: mst@redhat.com; jasowang@redhat.com
> > Cc: Eli Cohen <elic@nvidia.com>; si-wei.liu@oracle.com; Parav Pandit
> > <parav@nvidia.com>; wuzongyong@linux.alibaba.com;
> > virtualization@lists.linux-foundation.org; linux-kernel@vger.kernel.org;
> > eperezma@redhat.com; lingshan.zhu@intel.com; gdawar@xilinx.com;
> > lulu@redhat.com; xieyongji@bytedance.com
> > Subject: [PATCH 0/3] vdpa: device feature provisioning
> >
> > Hi All:
> >
> > Virtio features are neogiated between the device and the drivers. This allows
> > the mediation layer like vDPA to hide some features from the driver to
> > faciliate the cross vendor live migration:
> >
> > vDPA on the source supports feature set X vDPA on the destination supports
> > feature set Y
> >
> > Management can simply provision the vDPA instance with features X&Y on
> > both source and destination to let the vDPA can be migrate-able between
> > the two vDPA devies with different features support.
> >
> > This series tries to allow the device features to be provisioned via netlink to
> > achieve this.
> Very useful series.
> Can you please add vdpa example command with and without -jp option in each of the commit logs?

Yes, I will do it in V2.

Thanks

>
> Thanks.
>