Re: [PATCH 6/6] vdpa/mlx5: support device features provisioning

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 1/31/2023 4:56 AM, Eli Cohen wrote:

On 30/01/2023 22:30, Si-Wei Liu wrote:
This patch implements features provisioning for mlx5_vdpa.

1) Validate the provisioned features are a subset of the parent
    features.
2) Clearing features that are not wanted by userspace.
3) Set config space field only when the corresponding feature is
    provisioned.

For example:

    # vdpa mgmtdev show
    pci/0000:41:04.2:
      supported_classes net
      max_supported_vqs 65
      dev_features CSUM GUEST_CSUM MTU MAC HOST_TSO4 HOST_TSO6 STATUS CTRL_VQ CTRL_VLAN MQ CTRL_MAC_ADDR VERSION_1 ACCESS_PLATFORM

1) Provision vDPA device with all features derived from the parent

    # vdpa dev add name vdpa1 mgmtdev pci/0000:41:04.2
    # vdpa dev config show
    vdpa1: mac e4:11:c6:d3:45:f0 link up link_announce false max_vq_pairs 1 mtu 1500       negotiated_features CSUM GUEST_CSUM MTU HOST_TSO4 HOST_TSO6 STATUS CTRL_VQ CTRL_VLAN MQ CTRL_MAC_ADDR VERSION_1 ACCESS_PLATFORM

2) Provision vDPA device with a subset of parent features

    # vdpa dev add name vdpa1 mgmtdev pci/0000:41:04.2 device_features 0x300020000
    # vdpa dev config show
    vdpa1:
      negotiated_features CTRL_VQ VERSION_1 ACCESS_PLATFORM

Signed-off-by: Si-Wei Liu <si-wei.liu@xxxxxxxxxx>
---
  drivers/vdpa/mlx5/net/mlx5_vnet.c | 72 +++++++++++++++++++++++++++++++--------
  1 file changed, 58 insertions(+), 14 deletions(-)

diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index 3a6dbbc6..5d6dfd2 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -2183,6 +2183,7 @@ static u64 get_supported_features(struct mlx5_core_dev *mdev)
      mlx_vdpa_features |= BIT_ULL(VIRTIO_NET_F_STATUS);
      mlx_vdpa_features |= BIT_ULL(VIRTIO_NET_F_MTU);
      mlx_vdpa_features |= BIT_ULL(VIRTIO_NET_F_CTRL_VLAN);
+    mlx_vdpa_features |= BIT_ULL(VIRTIO_NET_F_MAC);
        return mlx_vdpa_features;
  }
@@ -3009,6 +3010,8 @@ static int event_handler(struct notifier_block *nb, unsigned long event, void *p
      struct mlx5_vdpa_wq_ent *wqent;
        if (event == MLX5_EVENT_TYPE_PORT_CHANGE) {
+        if (!(ndev->mvdev.actual_features & BIT_ULL(VIRTIO_NET_F_STATUS)))
+            return NOTIFY_DONE;
Does not belong in this patch
Yep, I can split the patch. Though feature provisioning on mlx5_vdpa has to depend on that patch.

          switch (eqe->sub_type) {
          case MLX5_PORT_CHANGE_SUBTYPE_DOWN:
          case MLX5_PORT_CHANGE_SUBTYPE_ACTIVE:
@@ -3060,6 +3063,7 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name,
      struct mlx5_vdpa_dev *mvdev;
      struct mlx5_vdpa_net *ndev;
      struct mlx5_core_dev *mdev;
+    u64 device_features;
      u32 max_vqs;
      u16 mtu;
      int err;
@@ -3068,6 +3072,25 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name,
          return -ENOSPC;
        mdev = mgtdev->madev->mdev;
+    device_features = mgtdev->mgtdev.supported_features;
+    if (add_config->mask & BIT_ULL(VDPA_ATTR_DEV_FEATURES)) {
+        if (add_config->device_features & ~device_features) {
+            dev_warn(mdev->device,
+                 "The provisioned features 0x%llx are not supported by this device with features 0x%llx\n",
+                 add_config->device_features, device_features);
+            return -EINVAL;
+        }
+        device_features &= add_config->device_features;
+    }
+    if (add_config->mask & BIT_ULL(VDPA_ATTR_DEV_FEATURES) &&
This looks redundant.
This will be handy to rewrite the conditional when legacy device (non VERSION_1) is going to be provisioned which may support in future.. But I can remove the check for now.

+        !(device_features & BIT_ULL(VIRTIO_F_VERSION_1) &&
+          device_features & BIT_ULL(VIRTIO_F_ACCESS_PLATFORM))) {
+        dev_warn(mdev->device,
+             "Must provision minimum features 0x%llx for this device",
+             BIT_ULL(VIRTIO_F_VERSION_1) | BIT_ULL(VIRTIO_F_ACCESS_PLATFORM));
+        return -EOPNOTSUPP;
+    }
+
      if (!(MLX5_CAP_DEV_VDPA_EMULATION(mdev, virtio_queue_type) &
          MLX5_VIRTIO_EMULATION_CAP_VIRTIO_QUEUE_TYPE_SPLIT)) {
          dev_warn(mdev->device, "missing support for split virtqueues\n"); @@ -3096,7 +3119,6 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name,
      if (IS_ERR(ndev))
          return PTR_ERR(ndev);
  -    ndev->mvdev.mlx_features = mgtdev->mgtdev.supported_features;
      ndev->mvdev.max_vqs = max_vqs;
      mvdev = &ndev->mvdev;
      mvdev->mdev = mdev;
@@ -3118,20 +3140,26 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name,
              goto err_alloc;
      }
  -    err = query_mtu(mdev, &mtu);
-    if (err)
-        goto err_alloc;
+    if (device_features & BIT_ULL(VIRTIO_NET_F_MTU)) {
+        err = query_mtu(mdev, &mtu);
+        if (err)
+            goto err_alloc;
  -    ndev->config.mtu = cpu_to_mlx5vdpa16(mvdev, mtu);
+        ndev->config.mtu = cpu_to_mlx5vdpa16(mvdev, mtu);
+    }
  -    if (get_link_state(mvdev))
-        ndev->config.status |= cpu_to_mlx5vdpa16(mvdev, VIRTIO_NET_S_LINK_UP);
-    else
-        ndev->config.status &= cpu_to_mlx5vdpa16(mvdev, ~VIRTIO_NET_S_LINK_UP);
+    if (device_features & BIT_ULL(VIRTIO_NET_F_STATUS)) {
+        if (get_link_state(mvdev))
+            ndev->config.status |= cpu_to_mlx5vdpa16(mvdev, VIRTIO_NET_S_LINK_UP);
+        else
+            ndev->config.status &= cpu_to_mlx5vdpa16(mvdev, ~VIRTIO_NET_S_LINK_UP);
+    }
Doesn't belong in this patch
Will split patch.

-Siwei
        if (add_config->mask & (1 << VDPA_ATTR_DEV_NET_CFG_MACADDR)) {
          memcpy(ndev->config.mac, add_config->net.mac, ETH_ALEN);
-    } else {
+    /* No bother setting mac address in config if not going to provision _F_MAC */ +    } else if ((add_config->mask & BIT_ULL(VDPA_ATTR_DEV_FEATURES)) == 0 ||
+           device_features & BIT_ULL(VIRTIO_NET_F_MAC)) {
          err = mlx5_query_nic_vport_mac_address(mdev, 0, 0, config->mac);
          if (err)
              goto err_alloc;
@@ -3142,11 +3170,26 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name,
          err = mlx5_mpfs_add_mac(pfmdev, config->mac);
          if (err)
              goto err_alloc;
-
-        ndev->mvdev.mlx_features |= BIT_ULL(VIRTIO_NET_F_MAC);
+    } else if ((add_config->mask & BIT_ULL(VDPA_ATTR_DEV_FEATURES)) == 0) {
+        /*
+         * We used to clear _F_MAC feature bit if seeing
+         * zero mac address when device features are not
+         * specifically provisioned. Keep the behaviour
+         * so old scripts do not break.
+         */
+        device_features &= ~BIT_ULL(VIRTIO_NET_F_MAC);
+    } else if (device_features & BIT_ULL(VIRTIO_NET_F_MAC)) {
+        /* Don't provision zero mac address for _F_MAC */
+        mlx5_vdpa_warn(&ndev->mvdev,
+                   "No mac address provisioned?\n");
+        err = -EINVAL;
+        goto err_alloc;
      }
  -    config->max_virtqueue_pairs = cpu_to_mlx5vdpa16(mvdev, max_vqs / 2);
+    if (device_features & BIT_ULL(VIRTIO_NET_F_MQ))
+        config->max_virtqueue_pairs = cpu_to_mlx5vdpa16(mvdev, max_vqs / 2);
+
+    ndev->mvdev.mlx_features = device_features;
      mvdev->vdev.dma_dev = &mdev->pdev->dev;
      err = mlx5_vdpa_alloc_resources(&ndev->mvdev);
      if (err)
@@ -3243,7 +3286,8 @@ static int mlx5v_probe(struct auxiliary_device *adev,
      mgtdev->mgtdev.id_table = id_table;
      mgtdev->mgtdev.config_attr_mask = BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MACADDR) |
                        BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MAX_VQP) |
-                      BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MTU);
+                      BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MTU) |
+                      BIT_ULL(VDPA_ATTR_DEV_FEATURES);
      mgtdev->mgtdev.max_supported_vqs =
          MLX5_CAP_DEV_VDPA_EMULATION(mdev, max_num_virtio_queues) + 1;
      mgtdev->mgtdev.supported_features = get_supported_features(mdev);

_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization




[Index of Archives]     [KVM Development]     [Libvirt Development]     [Libvirt Users]     [CentOS Virtualization]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux