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. Changes since V2: - remove the unnecessary and operation for the devie_features after validating it is a subset of what device supports - provison the features via vdpasim_create() to benefit all type of simulators - fix the reference leak when fail to provision feature for vp_vdpa - add a warn when failing to provision vp_vdpa device features Changes since V1: - Add vdpa tool command output Please review. 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.c | 12 +++++++++++- drivers/vdpa/vdpa_sim/vdpa_sim.h | 3 ++- drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 2 +- drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 5 +++-- drivers/vdpa/virtio_pci/vp_vdpa.c | 22 ++++++++++++++++++++-- include/linux/vdpa.h | 1 + include/uapi/linux/vdpa.h | 2 ++ 8 files changed, 45 insertions(+), 7 deletions(-) -- 2.25.1
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. Reviewed-by: Eli Cohen <elic at nvidia.com> Signed-off-by: Jason Wang <jasowang at 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
Jason Wang
2022-Sep-27 07:48 UTC
[PATCH V3 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 For example: # vdpa mgmtdev show vdpasim_net: supported_classes net max_supported_vqs 3 dev_features MTU MAC CTRL_VQ CTRL_MAC_ADDR ANY_LAYOUT VERSION_1 ACCESS_PLATFORM 1) provision vDPA device with all features that are supported by the net simulator # vdpa dev add name dev1 mgmtdev vdpasim_net # vdpa dev config show dev1: mac 00:00:00:00:00:00 link up link_announce false mtu 1500 negotiated_features MTU MAC CTRL_VQ CTRL_MAC_ADDR VERSION_1 ACCESS_PLATFORM 2) provision vDPA device with a subset of the features # vdpa dev add name dev1 mgmtdev vdpasim_net device_features 0x300020000 # vdpa dev config show dev1: mac 00:00:00:00:00:00 link up link_announce false mtu 1500 negotiated_features CTRL_VQ VERSION_1 ACCESS_PLATFORM Reviewed-by: Eli Cohen <elic at nvidia.com> Signed-off-by: Jason Wang <jasowang at redhat.com> --- drivers/vdpa/vdpa_sim/vdpa_sim.c | 12 +++++++++++- drivers/vdpa/vdpa_sim/vdpa_sim.h | 3 ++- drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 2 +- drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 5 +++-- 4 files changed, 17 insertions(+), 5 deletions(-) diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c index 225b7f5d8be3..b071f0d842fb 100644 --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c @@ -18,6 +18,7 @@ #include <linux/vdpa.h> #include <linux/vhost_iotlb.h> #include <linux/iova.h> +#include <uapi/linux/vdpa.h> #include "vdpa_sim.h" @@ -245,13 +246,22 @@ static const struct dma_map_ops vdpasim_dma_ops = { static const struct vdpa_config_ops vdpasim_config_ops; static const struct vdpa_config_ops vdpasim_batch_config_ops; -struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr) +struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr, + const struct vdpa_dev_set_config *config) { const struct vdpa_config_ops *ops; struct vdpasim *vdpasim; struct device *dev; int i, ret = -ENOMEM; + if (config->mask & BIT_ULL(VDPA_ATTR_DEV_FEATURES)) { + if (config->device_features & + ~dev_attr->supported_features) + return ERR_PTR(-EINVAL); + dev_attr->supported_features + config->device_features; + } + if (batch_mapping) ops = &vdpasim_batch_config_ops; else diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.h b/drivers/vdpa/vdpa_sim/vdpa_sim.h index 061986f30911..0e78737dcc16 100644 --- a/drivers/vdpa/vdpa_sim/vdpa_sim.h +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.h @@ -71,7 +71,8 @@ struct vdpasim { spinlock_t iommu_lock; }; -struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *attr); +struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *attr, + const struct vdpa_dev_set_config *config); /* TODO: cross-endian support */ static inline bool vdpasim_is_little_endian(struct vdpasim *vdpasim) diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c index c8bfea3b7db2..c6db1a1baf76 100644 --- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c @@ -383,7 +383,7 @@ static int vdpasim_blk_dev_add(struct vdpa_mgmt_dev *mdev, const char *name, dev_attr.work_fn = vdpasim_blk_work; dev_attr.buffer_size = VDPASIM_BLK_CAPACITY << SECTOR_SHIFT; - simdev = vdpasim_create(&dev_attr); + simdev = vdpasim_create(&dev_attr, config); if (IS_ERR(simdev)) return PTR_ERR(simdev); diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c index 886449e88502..c3cb225ea469 100644 --- a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c @@ -254,7 +254,7 @@ 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; - simdev = vdpasim_create(&dev_attr); + simdev = vdpasim_create(&dev_attr, config); if (IS_ERR(simdev)) return PTR_ERR(simdev); @@ -294,7 +294,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
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 For example: # vdpa mgmtdev show pci/0000:02:00.0: supported_classes net max_supported_vqs 3 dev_features CSUM GUEST_CSUM CTRL_GUEST_OFFLOADS MAC GUEST_TSO4 GUEST_TSO6 GUEST_ECN GUEST_UFO HOST_TSO4 HOST_TSO6 HOST_ECN HOST_UFO MRG_RXBUF STATUS CTRL_VQ CTRL_RX CTRL_VLAN CTRL_RX_EXTRA GUEST_ANNOUNCE CTRL_MAC_ADDR RING_INDIRECT_DESC RING_EVENT_IDX VERSION_1 ACCESS_PLATFORM 1) provision vDPA device with all features that are supported by the virtio-pci # vdpa dev add name dev1 mgmtdev pci/0000:02:00.0 # vdpa dev config show dev1: mac 52:54:00:12:34:56 link up link_announce false mtu 65535 negotiated_features CSUM GUEST_CSUM CTRL_GUEST_OFFLOADS MAC GUEST_TSO4 GUEST_TSO6 GUEST_ECN GUEST_UFO HOST_TSO4 HOST_TSO6 HOST_ECN HOST_UFO MRG_RXBUF STATUS CTRL_VQ CTRL_RX CTRL_VLAN GUEST_ANNOUNCE CTRL_MAC_ADDR RING_INDIRECT_DESC RING_EVENT_IDX VERSION_1 ACCESS_PLATFORM 2) provision vDPA device with a subset of the features # vdpa dev add name dev1 mgmtdev pci/0000:02:00.0 device_features 0x300020000 # dev1: mac 52:54:00:12:34:56 link up link_announce false mtu 65535 negotiated_features CTRL_VQ VERSION_1 ACCESS_PLATFORM Reviewed-by: Eli Cohen <elic at nvidia.com> Signed-off-by: Jason Wang <jasowang at redhat.com> --- drivers/vdpa/virtio_pci/vp_vdpa.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/drivers/vdpa/virtio_pci/vp_vdpa.c b/drivers/vdpa/virtio_pci/vp_vdpa.c index 04522077735b..d448db0c4de3 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,20 @@ 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) { + ret = -EINVAL; + dev_err(&pdev->dev, "Try to provision features " + "that are not supported by the device: " + "device_features 0x%llx provisioned 0x%llx\n", + device_features, add_config->device_features); + goto err; + } + 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 +616,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
Stefano Garzarella
2022-Oct-04 12:46 UTC
[PATCH V3 2/3] vdpa_sim_net: support feature provisioning
On Tue, Sep 27, 2022 at 03:48:09PM +0800, Jason Wang wrote:>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 > >For example: > ># vdpa mgmtdev show >vdpasim_net: > supported_classes net > max_supported_vqs 3 > dev_features MTU MAC CTRL_VQ CTRL_MAC_ADDR ANY_LAYOUT VERSION_1 ACCESS_PLATFORM > >1) provision vDPA device with all features that are supported by the > net simulator > ># vdpa dev add name dev1 mgmtdev vdpasim_net ># vdpa dev config show >dev1: mac 00:00:00:00:00:00 link up link_announce false mtu 1500 > negotiated_features MTU MAC CTRL_VQ CTRL_MAC_ADDR VERSION_1 ACCESS_PLATFORM > >2) provision vDPA device with a subset of the features > ># vdpa dev add name dev1 mgmtdev vdpasim_net device_features 0x300020000 ># vdpa dev config show >dev1: mac 00:00:00:00:00:00 link up link_announce false mtu 1500 > negotiated_features CTRL_VQ VERSION_1 ACCESS_PLATFORM > >Reviewed-by: Eli Cohen <elic at nvidia.com> >Signed-off-by: Jason Wang <jasowang at redhat.com> >--- > drivers/vdpa/vdpa_sim/vdpa_sim.c | 12 +++++++++++- > drivers/vdpa/vdpa_sim/vdpa_sim.h | 3 ++- > drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 2 +- > drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 5 +++-- > 4 files changed, 17 insertions(+), 5 deletions(-) > >diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c >index 225b7f5d8be3..b071f0d842fb 100644 >--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c >+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c >@@ -18,6 +18,7 @@ > #include <linux/vdpa.h> > #include <linux/vhost_iotlb.h> > #include <linux/iova.h> >+#include <uapi/linux/vdpa.h> > > #include "vdpa_sim.h" > >@@ -245,13 +246,22 @@ static const struct dma_map_ops vdpasim_dma_ops = { > static const struct vdpa_config_ops vdpasim_config_ops; > static const struct vdpa_config_ops vdpasim_batch_config_ops; > >-struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr) >+struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr, >+ const struct vdpa_dev_set_config *config) > { > const struct vdpa_config_ops *ops; > struct vdpasim *vdpasim; > struct device *dev; > int i, ret = -ENOMEM; > >+ if (config->mask & BIT_ULL(VDPA_ATTR_DEV_FEATURES)) { >+ if (config->device_features & >+ ~dev_attr->supported_features) >+ return ERR_PTR(-EINVAL); >+ dev_attr->supported_features >+ config->device_features; >+ } >+ > if (batch_mapping) > ops = &vdpasim_batch_config_ops; > else >diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.h b/drivers/vdpa/vdpa_sim/vdpa_sim.h >index 061986f30911..0e78737dcc16 100644 >--- a/drivers/vdpa/vdpa_sim/vdpa_sim.h >+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.h >@@ -71,7 +71,8 @@ struct vdpasim { > spinlock_t iommu_lock; > }; > >-struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *attr); >+struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *attr, >+ const struct vdpa_dev_set_config *config); > > /* TODO: cross-endian support */ > static inline bool vdpasim_is_little_endian(struct vdpasim *vdpasim) >diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c >index c8bfea3b7db2..c6db1a1baf76 100644 >--- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c >+++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c >@@ -383,7 +383,7 @@ static int vdpasim_blk_dev_add(struct vdpa_mgmt_dev *mdev, const char *name, > dev_attr.work_fn = vdpasim_blk_work; > dev_attr.buffer_size = VDPASIM_BLK_CAPACITY << SECTOR_SHIFT; > >- simdev = vdpasim_create(&dev_attr); >+ simdev = vdpasim_create(&dev_attr, config); > if (IS_ERR(simdev)) > return PTR_ERR(simdev); > >diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c >index 886449e88502..c3cb225ea469 100644 >--- a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c >+++ b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c >@@ -254,7 +254,7 @@ 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; > >- simdev = vdpasim_create(&dev_attr); >+ simdev = vdpasim_create(&dev_attr, config); > if (IS_ERR(simdev)) > return PTR_ERR(simdev); > >@@ -294,7 +294,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 >Fine for me: Reviewed-by: Stefano Garzarella <sgarzare at redhat.com> I'll send a followup to enable VDPA_ATTR_DEV_FEATURES in vdpa_sim_blk and test it. Thanks, Stefano