[PATCH linux-next v2 00/14] vdpa: Enable user to set mac address, mtu

[PATCH linux-next v2 00/14] vdpa: Enable user to set mac address, mtu

[Parav Pandit]

Currently user cannot set the mac address and mtu of the vdpa device.
This patchset enables users to set the mac address and mtu of the vdpa
device once the device is created.
If a vendor driver supports such configuration user can set it otherwise
user gets unsupported error.

vdpa mac address and mtu are device configuration layout fields.
To keep interface generic enough for multiple types of vdpa devices, mac
address and mtu setting is implemented as configuration layout config
knobs.
This enables to use similar config layout for other virtio devices.

An example of query & set of config layout fields for vdpa_sim_net
driver:

Configuration layout fields are set after device is created.
This enables user to change such fields at later point without destroying and
recreating the device for new config.

$ vdpa mgmtdev show
vdpasim_net:
  supported_classes net

Add the device:
$ vdpa dev add name bar mgmtdev vdpasim_net

Configure mac address and mtu:
$ vdpa dev config set bar mac 00:11:22:33:44:55 mtu 9000

In above command only mac address or only mtu can also be set.

View the config after setting:
$ vdpa dev config show
bar: mac 00:11:22:33:44:55 link up link_announce false mtu 9000 speed 0 duplex 0

Patch summary:
Patch-1 fix kdoc style comments
Patch-2 fix kdoc style comments
Patch-3 introduced and use helpers for get/set config and features
Patch-4 implement query device config layout
Patch-5 enanble user to set mac and mtu in config space
Patch-6 vdpa_sim_net implements get and set of config layout
Patch-7 mlx5 vdpa driver use right dma device for PCI PF,VF,SF
Patch-8 mlx5 vdpa driver uses right BAR offset for SF
Patch-9 mlx5 vdpa driver migrates to user created vdpa device
Patch-10 mlx5 vdpa driver supports user provided mac config
Patch-11 mlx5 vdpa driver uses user provided mac during rx flow steering
Patch-12 mlx5 vdpa driver excludes header length and fcs
Patch-13 mlx5 vdpa driver fixes index restoration during suspend resume
Patch-14 mlx5 vdpa driver fixes bit usage

changelog:
v1->v2:
 - new patches to fix kdoc comment to add new kdoc section
 - new patch to have synchronized access to features and config space
 - read whole net config layout instead of individual fields
 - added error extack for unmanaged vdpa device
 - fixed several endianness issues
 - introduced vdpa device ops for get config which is synchronized
   with other get/set features ops and config ops
 - fixed mtu range checking for max
 - using NLA_POLICY_ETH_ADDR
 - set config moved to device ops instead of mgmtdev ops
 - merged build and set to single routine
 - ensuring that user has NET_ADMIN capability for configuring network
   attributes
 - using updated interface and callbacks for get/set config
 - following new api for config get/set for mgmt tool in mlx5 vdpa
   driver
 - fixes for accessing right SF dma device and bar address
 - fix for mtu calculation
 - fix for bit access in features
 - fix for index restore with suspend/resume operation

Eli Cohen (7):
  vdpa/mlx5: Use the correct dma device when registering memory
  vdpa/mlx5: Retrieve BAR address suitable any function
  vdpa/mlx5: Enable user to add/delete vdpa device
  vdpa/mlx5: Support configuration of MAC
  vdpa/mlx5: Forward only packets with allowed MAC address
  vdpa/mlx5: Fix suspend/resume index restoration
  vdpa/mlx5: Fix wrong use of bit numbers

Parav Pandit (6):
  vdpa: Follow kdoc comment style
  vdpa: Follow kdoc comment style
  vdpa: Introduce and use vdpa device get,set config, features helpers
  vdpa: Introduce query of device config layout
  vdpa: Enable user to set mac and mtu of vdpa device
  vdpa_sim_net: Enable user to set mac address and mtu

Si-Wei Liu (1):
  vdpa/mlx5: should exclude header length and fcs from mtu

 drivers/vdpa/mlx5/core/mlx5_vdpa.h   |   4 +
 drivers/vdpa/mlx5/core/mr.c          |   9 +-
 drivers/vdpa/mlx5/core/resources.c   |   3 +-
 drivers/vdpa/mlx5/net/mlx5_vnet.c    | 264 ++++++++++++++----
 drivers/vdpa/vdpa.c                  | 383 ++++++++++++++++++++++++++-
 drivers/vdpa/vdpa_sim/vdpa_sim.c     |  26 ++
 drivers/vdpa/vdpa_sim/vdpa_sim.h     |   4 +
 drivers/vdpa/vdpa_sim/vdpa_sim_net.c |  39 +--
 drivers/vhost/vdpa.c                 |   6 +-
 include/linux/vdpa.h                 | 123 ++++++---
 include/uapi/linux/vdpa.h            |  12 +
 11 files changed, 752 insertions(+), 121 deletions(-)

-- 
2.26.2

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH linux-next v2 01/14] vdpa: Follow kdoc comment style

[Parav Pandit]

Follow comment style mentioned in the Writing kernel-doc document [1].

Following warnings are fixed.
$ scripts/kernel-doc -v -none include/linux/vdpa.h
include/linux/vdpa.h:11: warning: missing initial short description on line:
 * vDPA callback definition.
include/linux/vdpa.h:11: info: Scanning doc for vDPA
include/linux/vdpa.h:15: warning: cannot understand function prototype: 'struct vdpa_callback '
include/linux/vdpa.h:21: warning: missing initial short description on line:
 * vDPA notification area
include/linux/vdpa.h:21: info: Scanning doc for vDPA
include/linux/vdpa.h:25: warning: cannot understand function prototype: 'struct vdpa_notification_area '
include/linux/vdpa.h:31: warning: missing initial short description on line:
 * vDPA vq_state definition
include/linux/vdpa.h:31: info: Scanning doc for vDPA
include/linux/vdpa.h:34: warning: cannot understand function prototype: 'struct vdpa_vq_state '
include/linux/vdpa.h:41: info: Scanning doc for vDPA device
include/linux/vdpa.h:51: warning: cannot understand function prototype: 'struct vdpa_device '
include/linux/vdpa.h:62: info: Scanning doc for vDPA IOVA range
include/linux/vdpa.h:66: warning: cannot understand function prototype: 'struct vdpa_iova_range '
include/linux/vdpa.h:72: info: Scanning doc for vDPA_config_ops
include/linux/vdpa.h:203: warning: cannot understand function prototype: 'struct vdpa_config_ops '
include/linux/vdpa.h:270: info: Scanning doc for vdpa_driver
include/linux/vdpa.h:275: warning: cannot understand function prototype: 'struct vdpa_driver '
include/linux/vdpa.h:347: info: Scanning doc for vdpa_mgmtdev_ops
include/linux/vdpa.h:360: warning: cannot understand function prototype: 'struct vdpa_mgmtdev_ops '

After this fix:

scripts/kernel-doc -v -none include/linux/vdpa.h
include/linux/vdpa.h:11: info: Scanning doc for struct vdpa_calllback
include/linux/vdpa.h:21: info: Scanning doc for struct vdpa_notification_area
include/linux/vdpa.h:31: info: Scanning doc for struct vdpa_vq_state
include/linux/vdpa.h:41: info: Scanning doc for struct vdpa_device
include/linux/vdpa.h:62: info: Scanning doc for struct vdpa_iova_range
include/linux/vdpa.h:72: info: Scanning doc for struct vdpa_config_ops
include/linux/vdpa.h:270: info: Scanning doc for struct vdpa_driver
include/linux/vdpa.h:347: info: Scanning doc for struct vdpa_mgmtdev_ops

[1] https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html

Signed-off-by: Parav Pandit <parav@nvidia.com>
Reviewed-by: Eli Cohen <elic@nvidia.com>
---
 include/linux/vdpa.h | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
index 15fa085fab05..37b65ca940cf 100644
--- a/include/linux/vdpa.h
+++ b/include/linux/vdpa.h
@@ -8,7 +8,7 @@
 #include <linux/vhost_iotlb.h>
 
 /**
- * vDPA callback definition.
+ * struct vdpa_calllback - vDPA callback definition.
  * @callback: interrupt callback function
  * @private: the data passed to the callback function
  */
@@ -18,7 +18,7 @@ struct vdpa_callback {
 };
 
 /**
- * vDPA notification area
+ * struct vdpa_notification_area - vDPA notification area
  * @addr: base address of the notification area
  * @size: size of the notification area
  */
@@ -28,7 +28,7 @@ struct vdpa_notification_area {
 };
 
 /**
- * vDPA vq_state definition
+ * struct vdpa_vq_state - vDPA vq_state definition
  * @avail_index: available index
  */
 struct vdpa_vq_state {
@@ -38,7 +38,7 @@ struct vdpa_vq_state {
 struct vdpa_mgmt_dev;
 
 /**
- * vDPA device - representation of a vDPA device
+ * struct vdpa_device - representation of a vDPA device
  * @dev: underlying device
  * @dma_dev: the actual device that is performing DMA
  * @config: the configuration ops for this device.
@@ -59,7 +59,7 @@ struct vdpa_device {
 };
 
 /**
- * vDPA IOVA range - the IOVA range support by the device
+ * struct vdpa_iova_range - the IOVA range support by the device
  * @first: start of the IOVA range
  * @last: end of the IOVA range
  */
@@ -69,7 +69,7 @@ struct vdpa_iova_range {
 };
 
 /**
- * vDPA_config_ops - operations for configuring a vDPA device.
+ * struct vdpa_config_ops - operations for configuring a vDPA device.
  * Note: vDPA device drivers are required to implement all of the
  * operations unless it is mentioned to be optional in the following
  * list.
@@ -267,7 +267,7 @@ int _vdpa_register_device(struct vdpa_device *vdev, int nvqs);
 void _vdpa_unregister_device(struct vdpa_device *vdev);
 
 /**
- * vdpa_driver - operations for a vDPA driver
+ * struct vdpa_driver - operations for a vDPA driver
  * @driver: underlying device driver
  * @probe: the function to call when a device is found.  Returns 0 or -errno.
  * @remove: the function to call when a device is removed.
@@ -344,18 +344,18 @@ static inline void vdpa_get_config(struct vdpa_device *vdev, unsigned offset,
 }
 
 /**
- * vdpa_mgmtdev_ops - vdpa device ops
- * @dev_add:	Add a vdpa device using alloc and register
- *		@mdev: parent device to use for device addition
- *		@name: name of the new vdpa device
- *		Driver need to add a new device using _vdpa_register_device()
- *		after fully initializing the vdpa device. Driver must return 0
- *		on success or appropriate error code.
- * @dev_del:	Remove a vdpa device using unregister
- *		@mdev: parent device to use for device removal
- *		@dev: vdpa device to remove
- *		Driver need to remove the specified device by calling
- *		_vdpa_unregister_device().
+ * struct vdpa_mgmtdev_ops - vdpa device ops
+ * @dev_add: Add a vdpa device using alloc and register
+ *	     @mdev: parent device to use for device addition
+ *	     @name: name of the new vdpa device
+ *	     Driver need to add a new device using _vdpa_register_device()
+ *	     after fully initializing the vdpa device. Driver must return 0
+ *	     on success or appropriate error code.
+ * @dev_del: Remove a vdpa device using unregister
+ *	     @mdev: parent device to use for device removal
+ *	     @dev: vdpa device to remove
+ *	     Driver need to remove the specified device by calling
+ *	     _vdpa_unregister_device().
  */
 struct vdpa_mgmtdev_ops {
 	int (*dev_add)(struct vdpa_mgmt_dev *mdev, const char *name);
-- 
2.26.2

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH linux-next v2 01/14] vdpa: Follow kdoc comment style

[Jason Wang]

在 2021/4/7 上午1:04, Parav Pandit 写道:
> Follow comment style mentioned in the Writing kernel-doc document [1].
>
> Following warnings are fixed.
> $ scripts/kernel-doc -v -none include/linux/vdpa.h
> include/linux/vdpa.h:11: warning: missing initial short description on line:
>   * vDPA callback definition.
> include/linux/vdpa.h:11: info: Scanning doc for vDPA
> include/linux/vdpa.h:15: warning: cannot understand function prototype: 'struct vdpa_callback '
> include/linux/vdpa.h:21: warning: missing initial short description on line:
>   * vDPA notification area
> include/linux/vdpa.h:21: info: Scanning doc for vDPA
> include/linux/vdpa.h:25: warning: cannot understand function prototype: 'struct vdpa_notification_area '
> include/linux/vdpa.h:31: warning: missing initial short description on line:
>   * vDPA vq_state definition
> include/linux/vdpa.h:31: info: Scanning doc for vDPA
> include/linux/vdpa.h:34: warning: cannot understand function prototype: 'struct vdpa_vq_state '
> include/linux/vdpa.h:41: info: Scanning doc for vDPA device
> include/linux/vdpa.h:51: warning: cannot understand function prototype: 'struct vdpa_device '
> include/linux/vdpa.h:62: info: Scanning doc for vDPA IOVA range
> include/linux/vdpa.h:66: warning: cannot understand function prototype: 'struct vdpa_iova_range '
> include/linux/vdpa.h:72: info: Scanning doc for vDPA_config_ops
> include/linux/vdpa.h:203: warning: cannot understand function prototype: 'struct vdpa_config_ops '
> include/linux/vdpa.h:270: info: Scanning doc for vdpa_driver
> include/linux/vdpa.h:275: warning: cannot understand function prototype: 'struct vdpa_driver '
> include/linux/vdpa.h:347: info: Scanning doc for vdpa_mgmtdev_ops
> include/linux/vdpa.h:360: warning: cannot understand function prototype: 'struct vdpa_mgmtdev_ops '
>
> After this fix:
>
> scripts/kernel-doc -v -none include/linux/vdpa.h
> include/linux/vdpa.h:11: info: Scanning doc for struct vdpa_calllback
> include/linux/vdpa.h:21: info: Scanning doc for struct vdpa_notification_area
> include/linux/vdpa.h:31: info: Scanning doc for struct vdpa_vq_state
> include/linux/vdpa.h:41: info: Scanning doc for struct vdpa_device
> include/linux/vdpa.h:62: info: Scanning doc for struct vdpa_iova_range
> include/linux/vdpa.h:72: info: Scanning doc for struct vdpa_config_ops
> include/linux/vdpa.h:270: info: Scanning doc for struct vdpa_driver
> include/linux/vdpa.h:347: info: Scanning doc for struct vdpa_mgmtdev_ops
>
> [1] https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html
>
> Signed-off-by: Parav Pandit <parav@nvidia.com>
> Reviewed-by: Eli Cohen <elic@nvidia.com>


Acked-by: Jason Wang <jasowang@redhat.com>


> ---
>   include/linux/vdpa.h | 38 +++++++++++++++++++-------------------
>   1 file changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
> index 15fa085fab05..37b65ca940cf 100644
> --- a/include/linux/vdpa.h
> +++ b/include/linux/vdpa.h
> @@ -8,7 +8,7 @@
>   #include <linux/vhost_iotlb.h>
>   
>   /**
> - * vDPA callback definition.
> + * struct vdpa_calllback - vDPA callback definition.
>    * @callback: interrupt callback function
>    * @private: the data passed to the callback function
>    */
> @@ -18,7 +18,7 @@ struct vdpa_callback {
>   };
>   
>   /**
> - * vDPA notification area
> + * struct vdpa_notification_area - vDPA notification area
>    * @addr: base address of the notification area
>    * @size: size of the notification area
>    */
> @@ -28,7 +28,7 @@ struct vdpa_notification_area {
>   };
>   
>   /**
> - * vDPA vq_state definition
> + * struct vdpa_vq_state - vDPA vq_state definition
>    * @avail_index: available index
>    */
>   struct vdpa_vq_state {
> @@ -38,7 +38,7 @@ struct vdpa_vq_state {
>   struct vdpa_mgmt_dev;
>   
>   /**
> - * vDPA device - representation of a vDPA device
> + * struct vdpa_device - representation of a vDPA device
>    * @dev: underlying device
>    * @dma_dev: the actual device that is performing DMA
>    * @config: the configuration ops for this device.
> @@ -59,7 +59,7 @@ struct vdpa_device {
>   };
>   
>   /**
> - * vDPA IOVA range - the IOVA range support by the device
> + * struct vdpa_iova_range - the IOVA range support by the device
>    * @first: start of the IOVA range
>    * @last: end of the IOVA range
>    */
> @@ -69,7 +69,7 @@ struct vdpa_iova_range {
>   };
>   
>   /**
> - * vDPA_config_ops - operations for configuring a vDPA device.
> + * struct vdpa_config_ops - operations for configuring a vDPA device.
>    * Note: vDPA device drivers are required to implement all of the
>    * operations unless it is mentioned to be optional in the following
>    * list.
> @@ -267,7 +267,7 @@ int _vdpa_register_device(struct vdpa_device *vdev, int nvqs);
>   void _vdpa_unregister_device(struct vdpa_device *vdev);
>   
>   /**
> - * vdpa_driver - operations for a vDPA driver
> + * struct vdpa_driver - operations for a vDPA driver
>    * @driver: underlying device driver
>    * @probe: the function to call when a device is found.  Returns 0 or -errno.
>    * @remove: the function to call when a device is removed.
> @@ -344,18 +344,18 @@ static inline void vdpa_get_config(struct vdpa_device *vdev, unsigned offset,
>   }
>   
>   /**
> - * vdpa_mgmtdev_ops - vdpa device ops
> - * @dev_add:	Add a vdpa device using alloc and register
> - *		@mdev: parent device to use for device addition
> - *		@name: name of the new vdpa device
> - *		Driver need to add a new device using _vdpa_register_device()
> - *		after fully initializing the vdpa device. Driver must return 0
> - *		on success or appropriate error code.
> - * @dev_del:	Remove a vdpa device using unregister
> - *		@mdev: parent device to use for device removal
> - *		@dev: vdpa device to remove
> - *		Driver need to remove the specified device by calling
> - *		_vdpa_unregister_device().
> + * struct vdpa_mgmtdev_ops - vdpa device ops
> + * @dev_add: Add a vdpa device using alloc and register
> + *	     @mdev: parent device to use for device addition
> + *	     @name: name of the new vdpa device
> + *	     Driver need to add a new device using _vdpa_register_device()
> + *	     after fully initializing the vdpa device. Driver must return 0
> + *	     on success or appropriate error code.
> + * @dev_del: Remove a vdpa device using unregister
> + *	     @mdev: parent device to use for device removal
> + *	     @dev: vdpa device to remove
> + *	     Driver need to remove the specified device by calling
> + *	     _vdpa_unregister_device().
>    */
>   struct vdpa_mgmtdev_ops {
>   	int (*dev_add)(struct vdpa_mgmt_dev *mdev, const char *name);

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH linux-next v2 02/14] vdpa: Follow kdoc comment style

[Parav Pandit]

Follow comment style mentioned in the Writing kernel-doc document [1].

Following warnings are fixed.

$ scripts/kernel-doc -v -none drivers/vdpa/vdpa.c
drivers/vdpa/vdpa.c:67: info: Scanning doc for __vdpa_alloc_device
drivers/vdpa/vdpa.c:84: warning: No description found for return value of '__vdpa_alloc_device'
drivers/vdpa/vdpa.c:153: info: Scanning doc for _vdpa_register_device
drivers/vdpa/vdpa.c:163: warning: No description found for return value of '_vdpa_register_device'
drivers/vdpa/vdpa.c:172: info: Scanning doc for vdpa_register_device
drivers/vdpa/vdpa.c:180: warning: No description found for return value of 'vdpa_register_device'
drivers/vdpa/vdpa.c:191: info: Scanning doc for _vdpa_unregister_device
drivers/vdpa/vdpa.c:205: info: Scanning doc for vdpa_unregister_device
drivers/vdpa/vdpa.c:217: info: Scanning doc for __vdpa_register_driver
drivers/vdpa/vdpa.c:224: warning: No description found for return value of '__vdpa_register_driver'
drivers/vdpa/vdpa.c:233: info: Scanning doc for vdpa_unregister_driver
drivers/vdpa/vdpa.c:243: info: Scanning doc for vdpa_mgmtdev_register
drivers/vdpa/vdpa.c:250: warning: No description found for return value of 'vdpa_mgmtdev_register'

After the fix:

scripts/kernel-doc -v -none drivers/vdpa/vdpa.c
drivers/vdpa/vdpa.c:67: info: Scanning doc for __vdpa_alloc_device
drivers/vdpa/vdpa.c:153: info: Scanning doc for _vdpa_register_device
drivers/vdpa/vdpa.c:172: info: Scanning doc for vdpa_register_device
drivers/vdpa/vdpa.c:191: info: Scanning doc for _vdpa_unregister_device
drivers/vdpa/vdpa.c:205: info: Scanning doc for vdpa_unregister_device
drivers/vdpa/vdpa.c:217: info: Scanning doc for __vdpa_register_driver
drivers/vdpa/vdpa.c:233: info: Scanning doc for vdpa_unregister_driver
drivers/vdpa/vdpa.c:243: info: Scanning doc for vdpa_mgmtdev_register

[1] https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html

Signed-off-by: Parav Pandit <parav@nvidia.com>
Reviewed-by: Eli Cohen <elic@nvidia.com>
---
 drivers/vdpa/vdpa.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
index 5cffce67cab0..bb3f1d1f0422 100644
--- a/drivers/vdpa/vdpa.c
+++ b/drivers/vdpa/vdpa.c
@@ -75,8 +75,8 @@ static void vdpa_release_dev(struct device *d)
  * Driver should use vdpa_alloc_device() wrapper macro instead of
  * using this directly.
  *
- * Returns an error when parent/config/dma_dev is not set or fail to get
- * ida.
+ * Return: Returns an error when parent/config/dma_dev is not set or fail to get
+ *	   ida.
  */
 struct vdpa_device *__vdpa_alloc_device(struct device *parent,
 					const struct vdpa_config_ops *config,
@@ -157,7 +157,7 @@ static int __vdpa_register_device(struct vdpa_device *vdev, int nvqs)
  * @vdev: the vdpa device to be registered to vDPA bus
  * @nvqs: number of virtqueues supported by this device
  *
- * Returns an error when fail to add device to vDPA bus
+ * Return: Returns an error when fail to add device to vDPA bus
  */
 int _vdpa_register_device(struct vdpa_device *vdev, int nvqs)
 {
@@ -174,7 +174,7 @@ EXPORT_SYMBOL_GPL(_vdpa_register_device);
  * @vdev: the vdpa device to be registered to vDPA bus
  * @nvqs: number of virtqueues supported by this device
  *
- * Returns an error when fail to add to vDPA bus
+ * Return: Returns an error when fail to add to vDPA bus
  */
 int vdpa_register_device(struct vdpa_device *vdev, int nvqs)
 {
@@ -218,7 +218,7 @@ EXPORT_SYMBOL_GPL(vdpa_unregister_device);
  * @drv: the vdpa device driver to be registered
  * @owner: module owner of the driver
  *
- * Returns an err when fail to do the registration
+ * Return: Returns an err when fail to do the registration
  */
 int __vdpa_register_driver(struct vdpa_driver *drv, struct module *owner)
 {
@@ -245,6 +245,8 @@ EXPORT_SYMBOL_GPL(vdpa_unregister_driver);
  * @mdev: Pointer to vdpa management device
  * vdpa_mgmtdev_register() register a vdpa management device which supports
  * vdpa device management.
+ * Return: Returns 0 on success or failure when required callback ops are not
+ *         initialized.
  */
 int vdpa_mgmtdev_register(struct vdpa_mgmt_dev *mdev)
 {
-- 
2.26.2

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH linux-next v2 02/14] vdpa: Follow kdoc comment style

[Jason Wang]

在 2021/4/7 上午1:04, Parav Pandit 写道:
> Follow comment style mentioned in the Writing kernel-doc document [1].
>
> Following warnings are fixed.
>
> $ scripts/kernel-doc -v -none drivers/vdpa/vdpa.c
> drivers/vdpa/vdpa.c:67: info: Scanning doc for __vdpa_alloc_device
> drivers/vdpa/vdpa.c:84: warning: No description found for return value of '__vdpa_alloc_device'
> drivers/vdpa/vdpa.c:153: info: Scanning doc for _vdpa_register_device
> drivers/vdpa/vdpa.c:163: warning: No description found for return value of '_vdpa_register_device'
> drivers/vdpa/vdpa.c:172: info: Scanning doc for vdpa_register_device
> drivers/vdpa/vdpa.c:180: warning: No description found for return value of 'vdpa_register_device'
> drivers/vdpa/vdpa.c:191: info: Scanning doc for _vdpa_unregister_device
> drivers/vdpa/vdpa.c:205: info: Scanning doc for vdpa_unregister_device
> drivers/vdpa/vdpa.c:217: info: Scanning doc for __vdpa_register_driver
> drivers/vdpa/vdpa.c:224: warning: No description found for return value of '__vdpa_register_driver'
> drivers/vdpa/vdpa.c:233: info: Scanning doc for vdpa_unregister_driver
> drivers/vdpa/vdpa.c:243: info: Scanning doc for vdpa_mgmtdev_register
> drivers/vdpa/vdpa.c:250: warning: No description found for return value of 'vdpa_mgmtdev_register'
>
> After the fix:
>
> scripts/kernel-doc -v -none drivers/vdpa/vdpa.c
> drivers/vdpa/vdpa.c:67: info: Scanning doc for __vdpa_alloc_device
> drivers/vdpa/vdpa.c:153: info: Scanning doc for _vdpa_register_device
> drivers/vdpa/vdpa.c:172: info: Scanning doc for vdpa_register_device
> drivers/vdpa/vdpa.c:191: info: Scanning doc for _vdpa_unregister_device
> drivers/vdpa/vdpa.c:205: info: Scanning doc for vdpa_unregister_device
> drivers/vdpa/vdpa.c:217: info: Scanning doc for __vdpa_register_driver
> drivers/vdpa/vdpa.c:233: info: Scanning doc for vdpa_unregister_driver
> drivers/vdpa/vdpa.c:243: info: Scanning doc for vdpa_mgmtdev_register
>
> [1] https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html
>
> Signed-off-by: Parav Pandit <parav@nvidia.com>
> Reviewed-by: Eli Cohen <elic@nvidia.com>


Acked-by: Jason Wang <jasowang@redhat.com>


> ---
>   drivers/vdpa/vdpa.c | 12 +++++++-----
>   1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
> index 5cffce67cab0..bb3f1d1f0422 100644
> --- a/drivers/vdpa/vdpa.c
> +++ b/drivers/vdpa/vdpa.c
> @@ -75,8 +75,8 @@ static void vdpa_release_dev(struct device *d)
>    * Driver should use vdpa_alloc_device() wrapper macro instead of
>    * using this directly.
>    *
> - * Returns an error when parent/config/dma_dev is not set or fail to get
> - * ida.
> + * Return: Returns an error when parent/config/dma_dev is not set or fail to get
> + *	   ida.
>    */
>   struct vdpa_device *__vdpa_alloc_device(struct device *parent,
>   					const struct vdpa_config_ops *config,
> @@ -157,7 +157,7 @@ static int __vdpa_register_device(struct vdpa_device *vdev, int nvqs)
>    * @vdev: the vdpa device to be registered to vDPA bus
>    * @nvqs: number of virtqueues supported by this device
>    *
> - * Returns an error when fail to add device to vDPA bus
> + * Return: Returns an error when fail to add device to vDPA bus
>    */
>   int _vdpa_register_device(struct vdpa_device *vdev, int nvqs)
>   {
> @@ -174,7 +174,7 @@ EXPORT_SYMBOL_GPL(_vdpa_register_device);
>    * @vdev: the vdpa device to be registered to vDPA bus
>    * @nvqs: number of virtqueues supported by this device
>    *
> - * Returns an error when fail to add to vDPA bus
> + * Return: Returns an error when fail to add to vDPA bus
>    */
>   int vdpa_register_device(struct vdpa_device *vdev, int nvqs)
>   {
> @@ -218,7 +218,7 @@ EXPORT_SYMBOL_GPL(vdpa_unregister_device);
>    * @drv: the vdpa device driver to be registered
>    * @owner: module owner of the driver
>    *
> - * Returns an err when fail to do the registration
> + * Return: Returns an err when fail to do the registration
>    */
>   int __vdpa_register_driver(struct vdpa_driver *drv, struct module *owner)
>   {
> @@ -245,6 +245,8 @@ EXPORT_SYMBOL_GPL(vdpa_unregister_driver);
>    * @mdev: Pointer to vdpa management device
>    * vdpa_mgmtdev_register() register a vdpa management device which supports
>    * vdpa device management.
> + * Return: Returns 0 on success or failure when required callback ops are not
> + *         initialized.
>    */
>   int vdpa_mgmtdev_register(struct vdpa_mgmt_dev *mdev)
>   {

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH linux-next v2 03/14] vdpa: Introduce and use vdpa device get, set config, features helpers

[Parav Pandit]

Subsequent patches enable get and set configuration either
via management device or via vdpa device' config ops.

This requires synchronization between multiple callers to get and set
config callbacks. Features setting also influence the layout of the
configuration fields endianness.

To avoid exposing synchronization primitives to callers, introduce
helper for setting the configuration and use it.

Signed-off-by: Parav Pandit <parav@nvidia.com>
Reviewed-by: Eli Cohen <elic@nvidia.com>
---
changelog:
v1->v2
 - new patch to have synchronized access to features and config space
---
 drivers/vdpa/vdpa.c  | 57 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/vhost/vdpa.c |  6 ++---
 include/linux/vdpa.h | 28 +++++-----------------
 3 files changed, 65 insertions(+), 26 deletions(-)

diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
index bb3f1d1f0422..116e076c72fd 100644
--- a/drivers/vdpa/vdpa.c
+++ b/drivers/vdpa/vdpa.c
@@ -261,6 +261,63 @@ int vdpa_mgmtdev_register(struct vdpa_mgmt_dev *mdev)
 }
 EXPORT_SYMBOL_GPL(vdpa_mgmtdev_register);
 
+/**
+ * vdpa_get_features - Read vDPA device features
+ * @vdev: vdpa device whose features to read
+ *
+ * Return: Returns device features.
+ */
+u64 vdpa_get_features(struct vdpa_device *vdev)
+{
+	return vdev->config->get_features(vdev);
+}
+EXPORT_SYMBOL_GPL(vdpa_get_features);
+
+/**
+ * vdpa_set_features - Set vDPA device features
+ * @vdev: vdpa device whose features to set
+ * @features: features of the vdpa device to enable/disable
+ *
+ * Return: Returns 0 on success or error code.
+ */
+int vdpa_set_features(struct vdpa_device *vdev, u64 features)
+{
+	const struct vdpa_config_ops *ops = vdev->config;
+
+	vdev->features_valid = true;
+	return ops->set_features(vdev, features);
+}
+EXPORT_SYMBOL_GPL(vdpa_set_features);
+
+void vdpa_get_config(struct vdpa_device *vdev, unsigned int offset,
+		     void *buf, unsigned int len)
+{
+	const struct vdpa_config_ops *ops = vdev->config;
+
+	/*
+	 * Config accesses aren't supposed to trigger before features are set.
+	 * If it does happen we assume a legacy guest.
+	 */
+	if (!vdev->features_valid)
+		vdpa_set_features(vdev, 0);
+	ops->get_config(vdev, offset, buf, len);
+}
+EXPORT_SYMBOL_GPL(vdpa_get_config);
+
+/**
+ * vdpa_set_config - Set one or more device configuration fields.
+ * @dev: vdpa device to operate on
+ * @offset: starting byte offset of the field
+ * @buf: buffer pointer to read from
+ * @length: length of the configuration fields in bytes
+ */
+void vdpa_set_config(struct vdpa_device *dev, unsigned int offset,
+		     void *buf, unsigned int length)
+{
+	dev->config->set_config(dev, offset, buf, length);
+}
+EXPORT_SYMBOL_GPL(vdpa_set_config);
+
 static int vdpa_match_remove(struct device *dev, void *data)
 {
 	struct vdpa_device *vdev = container_of(dev, struct vdpa_device, dev);
diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index e0a27e336293..383f5be8ffa6 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -236,7 +236,6 @@ static long vhost_vdpa_set_config(struct vhost_vdpa *v,
 				  struct vhost_vdpa_config __user *c)
 {
 	struct vdpa_device *vdpa = v->vdpa;
-	const struct vdpa_config_ops *ops = vdpa->config;
 	struct vhost_vdpa_config config;
 	unsigned long size = offsetof(struct vhost_vdpa_config, buf);
 	u8 *buf;
@@ -250,7 +249,7 @@ static long vhost_vdpa_set_config(struct vhost_vdpa *v,
 	if (IS_ERR(buf))
 		return PTR_ERR(buf);
 
-	ops->set_config(vdpa, config.off, buf, config.len);
+	vdpa_set_config(vdpa, config.off, buf, config.len);
 
 	kvfree(buf);
 	return 0;
@@ -259,10 +258,9 @@ static long vhost_vdpa_set_config(struct vhost_vdpa *v,
 static long vhost_vdpa_get_features(struct vhost_vdpa *v, u64 __user *featurep)
 {
 	struct vdpa_device *vdpa = v->vdpa;
-	const struct vdpa_config_ops *ops = vdpa->config;
 	u64 features;
 
-	features = ops->get_features(vdpa);
+	features = vdpa_get_features(vdpa);
 
 	if (copy_to_user(featurep, &features, sizeof(features)))
 		return -EFAULT;
diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
index 37b65ca940cf..62e68ccc4c96 100644
--- a/include/linux/vdpa.h
+++ b/include/linux/vdpa.h
@@ -320,28 +320,12 @@ static inline void vdpa_reset(struct vdpa_device *vdev)
         ops->set_status(vdev, 0);
 }
 
-static inline int vdpa_set_features(struct vdpa_device *vdev, u64 features)
-{
-        const struct vdpa_config_ops *ops = vdev->config;
-
-	vdev->features_valid = true;
-        return ops->set_features(vdev, features);
-}
-
-
-static inline void vdpa_get_config(struct vdpa_device *vdev, unsigned offset,
-				   void *buf, unsigned int len)
-{
-        const struct vdpa_config_ops *ops = vdev->config;
-
-	/*
-	 * Config accesses aren't supposed to trigger before features are set.
-	 * If it does happen we assume a legacy guest.
-	 */
-	if (!vdev->features_valid)
-		vdpa_set_features(vdev, 0);
-	ops->get_config(vdev, offset, buf, len);
-}
+u64 vdpa_get_features(struct vdpa_device *vdev);
+int vdpa_set_features(struct vdpa_device *vdev, u64 features);
+void vdpa_get_config(struct vdpa_device *vdev, unsigned int offset,
+		     void *buf, unsigned int len);
+void vdpa_set_config(struct vdpa_device *dev, unsigned int offset,
+		     void *buf, unsigned int length);
 
 /**
  * struct vdpa_mgmtdev_ops - vdpa device ops
-- 
2.26.2

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH linux-next v2 03/14] vdpa: Introduce and use vdpa device get,set config, features helpers

[Jason Wang]

在 2021/4/7 上午1:04, Parav Pandit 写道:
> Subsequent patches enable get and set configuration either
> via management device or via vdpa device' config ops.
>
> This requires synchronization between multiple callers to get and set
> config callbacks. Features setting also influence the layout of the
> configuration fields endianness.
>
> To avoid exposing synchronization primitives to callers, introduce
> helper for setting the configuration and use it.
>
> Signed-off-by: Parav Pandit <parav@nvidia.com>
> Reviewed-by: Eli Cohen <elic@nvidia.com>
> ---
> changelog:
> v1->v2
>   - new patch to have synchronized access to features and config space
> ---
>   drivers/vdpa/vdpa.c  | 57 ++++++++++++++++++++++++++++++++++++++++++++
>   drivers/vhost/vdpa.c |  6 ++---
>   include/linux/vdpa.h | 28 +++++-----------------
>   3 files changed, 65 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
> index bb3f1d1f0422..116e076c72fd 100644
> --- a/drivers/vdpa/vdpa.c
> +++ b/drivers/vdpa/vdpa.c
> @@ -261,6 +261,63 @@ int vdpa_mgmtdev_register(struct vdpa_mgmt_dev *mdev)
>   }
>   EXPORT_SYMBOL_GPL(vdpa_mgmtdev_register);
>   
> +/**
> + * vdpa_get_features - Read vDPA device features
> + * @vdev: vdpa device whose features to read
> + *
> + * Return: Returns device features.
> + */
> +u64 vdpa_get_features(struct vdpa_device *vdev)
> +{
> +	return vdev->config->get_features(vdev);
> +}
> +EXPORT_SYMBOL_GPL(vdpa_get_features);
> +
> +/**
> + * vdpa_set_features - Set vDPA device features
> + * @vdev: vdpa device whose features to set
> + * @features: features of the vdpa device to enable/disable
> + *
> + * Return: Returns 0 on success or error code.
> + */
> +int vdpa_set_features(struct vdpa_device *vdev, u64 features)
> +{
> +	const struct vdpa_config_ops *ops = vdev->config;
> +
> +	vdev->features_valid = true;
> +	return ops->set_features(vdev, features);
> +}
> +EXPORT_SYMBOL_GPL(vdpa_set_features);
> +


Let's add a doc for this function?


> +void vdpa_get_config(struct vdpa_device *vdev, unsigned int offset,
> +		     void *buf, unsigned int len)
> +{
> +	const struct vdpa_config_ops *ops = vdev->config;
> +
> +	/*
> +	 * Config accesses aren't supposed to trigger before features are set.
> +	 * If it does happen we assume a legacy guest.
> +	 */
> +	if (!vdev->features_valid)
> +		vdpa_set_features(vdev, 0);
> +	ops->get_config(vdev, offset, buf, len);
> +}
> +EXPORT_SYMBOL_GPL(vdpa_get_config);
> +
> +/**
> + * vdpa_set_config - Set one or more device configuration fields.
> + * @dev: vdpa device to operate on
> + * @offset: starting byte offset of the field
> + * @buf: buffer pointer to read from
> + * @length: length of the configuration fields in bytes
> + */
> +void vdpa_set_config(struct vdpa_device *dev, unsigned int offset,
> +		     void *buf, unsigned int length)
> +{
> +	dev->config->set_config(dev, offset, buf, length);
> +}
> +EXPORT_SYMBOL_GPL(vdpa_set_config);
> +
>   static int vdpa_match_remove(struct device *dev, void *data)
>   {
>   	struct vdpa_device *vdev = container_of(dev, struct vdpa_device, dev);
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index e0a27e336293..383f5be8ffa6 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c


We probably need to convert drivers/virtio/vdpa.c as well.


> @@ -236,7 +236,6 @@ static long vhost_vdpa_set_config(struct vhost_vdpa *v,
>   				  struct vhost_vdpa_config __user *c)
>   {
>   	struct vdpa_device *vdpa = v->vdpa;
> -	const struct vdpa_config_ops *ops = vdpa->config;
>   	struct vhost_vdpa_config config;
>   	unsigned long size = offsetof(struct vhost_vdpa_config, buf);
>   	u8 *buf;
> @@ -250,7 +249,7 @@ static long vhost_vdpa_set_config(struct vhost_vdpa *v,
>   	if (IS_ERR(buf))
>   		return PTR_ERR(buf);
>   
> -	ops->set_config(vdpa, config.off, buf, config.len);
> +	vdpa_set_config(vdpa, config.off, buf, config.len);
>   
>   	kvfree(buf);
>   	return 0;
> @@ -259,10 +258,9 @@ static long vhost_vdpa_set_config(struct vhost_vdpa *v,
>   static long vhost_vdpa_get_features(struct vhost_vdpa *v, u64 __user *featurep)
>   {
>   	struct vdpa_device *vdpa = v->vdpa;
> -	const struct vdpa_config_ops *ops = vdpa->config;
>   	u64 features;
>   
> -	features = ops->get_features(vdpa);
> +	features = vdpa_get_features(vdpa);
>   
>   	if (copy_to_user(featurep, &features, sizeof(features)))
>   		return -EFAULT;
> diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
> index 37b65ca940cf..62e68ccc4c96 100644
> --- a/include/linux/vdpa.h
> +++ b/include/linux/vdpa.h
> @@ -320,28 +320,12 @@ static inline void vdpa_reset(struct vdpa_device *vdev)
>           ops->set_status(vdev, 0);
>   }
>   
> -static inline int vdpa_set_features(struct vdpa_device *vdev, u64 features)
> -{
> -        const struct vdpa_config_ops *ops = vdev->config;
> -
> -	vdev->features_valid = true;
> -        return ops->set_features(vdev, features);
> -}
> -
> -
> -static inline void vdpa_get_config(struct vdpa_device *vdev, unsigned offset,
> -				   void *buf, unsigned int len)
> -{
> -        const struct vdpa_config_ops *ops = vdev->config;
> -
> -	/*
> -	 * Config accesses aren't supposed to trigger before features are set.
> -	 * If it does happen we assume a legacy guest.
> -	 */
> -	if (!vdev->features_valid)
> -		vdpa_set_features(vdev, 0);
> -	ops->get_config(vdev, offset, buf, len);
> -}
> +u64 vdpa_get_features(struct vdpa_device *vdev);
> +int vdpa_set_features(struct vdpa_device *vdev, u64 features);
> +void vdpa_get_config(struct vdpa_device *vdev, unsigned int offset,
> +		     void *buf, unsigned int len);
> +void vdpa_set_config(struct vdpa_device *dev, unsigned int offset,
> +		     void *buf, unsigned int length);


I think it's better to use a separate patch for this moving.

Thanks


>   
>   /**
>    * struct vdpa_mgmtdev_ops - vdpa device ops

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

RE: [PATCH linux-next v2 03/14] vdpa: Introduce and use vdpa device get,set config, features helpers

[Parav Pandit]

> From: Jason Wang <jasowang@redhat.com>
> Sent: Wednesday, April 7, 2021 8:48 AM
[..]
> > +/**
> > + * vdpa_set_features - Set vDPA device features
> > + * @vdev: vdpa device whose features to set
> > + * @features: features of the vdpa device to enable/disable
> > + *
> > + * Return: Returns 0 on success or error code.
> > + */
> > +int vdpa_set_features(struct vdpa_device *vdev, u64 features) {
> > +	const struct vdpa_config_ops *ops = vdev->config;
> > +
> > +	vdev->features_valid = true;
> > +	return ops->set_features(vdev, features); }
> > +EXPORT_SYMBOL_GPL(vdpa_set_features);
> > +
> 
> 
> Let's add a doc for this function?
Kdoc comment is added above the function in this patch.

> > --- a/drivers/vhost/vdpa.c
> > +++ b/drivers/vhost/vdpa.c
> 
> 
> We probably need to convert drivers/virtio/vdpa.c as well.
Yes, will do.

> 
> 
> > @@ -236,7 +236,6 @@ static long vhost_vdpa_set_config(struct
> vhost_vdpa *v,
> >   				  struct vhost_vdpa_config __user *c)
> >   {
> >   	struct vdpa_device *vdpa = v->vdpa;
> > -	const struct vdpa_config_ops *ops = vdpa->config;
> >   	struct vhost_vdpa_config config;
> >   	unsigned long size = offsetof(struct vhost_vdpa_config, buf);
> >   	u8 *buf;
> > @@ -250,7 +249,7 @@ static long vhost_vdpa_set_config(struct
> vhost_vdpa *v,
> >   	if (IS_ERR(buf))
> >   		return PTR_ERR(buf);
> >
> > -	ops->set_config(vdpa, config.off, buf, config.len);
> > +	vdpa_set_config(vdpa, config.off, buf, config.len);
> >
> >   	kvfree(buf);
> >   	return 0;
> > @@ -259,10 +258,9 @@ static long vhost_vdpa_set_config(struct
> vhost_vdpa *v,
> >   static long vhost_vdpa_get_features(struct vhost_vdpa *v, u64 __user
> *featurep)
> >   {
> >   	struct vdpa_device *vdpa = v->vdpa;
> > -	const struct vdpa_config_ops *ops = vdpa->config;
> >   	u64 features;
> >
> > -	features = ops->get_features(vdpa);
> > +	features = vdpa_get_features(vdpa);
> >
> >   	if (copy_to_user(featurep, &features, sizeof(features)))
> >   		return -EFAULT;
> > diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h index
> > 37b65ca940cf..62e68ccc4c96 100644
> > --- a/include/linux/vdpa.h
> > +++ b/include/linux/vdpa.h
> > @@ -320,28 +320,12 @@ static inline void vdpa_reset(struct vdpa_device
> *vdev)
> >           ops->set_status(vdev, 0);
> >   }
> >
> > -static inline int vdpa_set_features(struct vdpa_device *vdev, u64
> > features) -{
> > -        const struct vdpa_config_ops *ops = vdev->config;
> > -
> > -	vdev->features_valid = true;
> > -        return ops->set_features(vdev, features);
> > -}
> > -
> > -
> > -static inline void vdpa_get_config(struct vdpa_device *vdev, unsigned
> offset,
> > -				   void *buf, unsigned int len)
> > -{
> > -        const struct vdpa_config_ops *ops = vdev->config;
> > -
> > -	/*
> > -	 * Config accesses aren't supposed to trigger before features are set.
> > -	 * If it does happen we assume a legacy guest.
> > -	 */
> > -	if (!vdev->features_valid)
> > -		vdpa_set_features(vdev, 0);
> > -	ops->get_config(vdev, offset, buf, len);
> > -}
> > +u64 vdpa_get_features(struct vdpa_device *vdev); int
> > +vdpa_set_features(struct vdpa_device *vdev, u64 features); void
> > +vdpa_get_config(struct vdpa_device *vdev, unsigned int offset,
> > +		     void *buf, unsigned int len);
> > +void vdpa_set_config(struct vdpa_device *dev, unsigned int offset,
> > +		     void *buf, unsigned int length);
> 
> 
> I think it's better to use a separate patch for this moving.
> 
Ok. will have prepatch to this one for movement.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH linux-next v2 04/14] vdpa: Introduce query of device config layout

[Parav Pandit]

Introduce a command to query a device config layout.

An example query of network vdpa device:

$ vdpa dev add name bar mgmtdev vdpasim_net

$ vdpa dev config show
bar: mac 00:35:09:19:48:05 link up link_announce false mtu 1500 speed 0 duplex 0

$ vdpa dev config show -jp
{
    "config": {
        "bar": {
            "mac": "00:35:09:19:48:05",
            "link ": "up",
            "link_announce ": false,
            "mtu": 1500,
            "speed": 0,
            "duplex": 0
        }
    }
}

Signed-off-by: Parav Pandit <parav@nvidia.com>
Signed-off-by: Eli Cohen <elic@nvidia.com>
---
changelog:
v1->v2:
 - read whole net config layout instead of individual fields
 - added error extack for unmanaged vdpa device
 - fixed several endianness issues
 - introduced vdpa device ops for get config which is synchronized
   with other get/set features ops and config ops
---
 drivers/vdpa/vdpa.c       | 234 +++++++++++++++++++++++++++++++++++++-
 include/linux/vdpa.h      |  42 +++++++
 include/uapi/linux/vdpa.h |  11 ++
 3 files changed, 282 insertions(+), 5 deletions(-)

diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
index 116e076c72fd..9da8deb8c0f2 100644
--- a/drivers/vdpa/vdpa.c
+++ b/drivers/vdpa/vdpa.c
@@ -14,6 +14,8 @@
 #include <uapi/linux/vdpa.h>
 #include <net/genetlink.h>
 #include <linux/mod_devicetable.h>
+#include <linux/virtio_net.h>
+#include <linux/virtio_ids.h>
 
 static LIST_HEAD(mdev_head);
 /* A global mutex that protects vdpa management device and device level operations. */
@@ -60,6 +62,7 @@ static void vdpa_release_dev(struct device *d)
 		ops->free(vdev);
 
 	ida_simple_remove(&vdpa_index_ida, vdev->index);
+	mutex_destroy(&vdev->cf_mutex);
 	kfree(vdev);
 }
 
@@ -114,6 +117,7 @@ struct vdpa_device *__vdpa_alloc_device(struct device *parent,
 	if (err)
 		goto err_name;
 
+	mutex_init(&vdev->cf_mutex);
 	device_initialize(&vdev->dev);
 
 	return vdev;
@@ -269,10 +273,25 @@ EXPORT_SYMBOL_GPL(vdpa_mgmtdev_register);
  */
 u64 vdpa_get_features(struct vdpa_device *vdev)
 {
-	return vdev->config->get_features(vdev);
+	u64 features;
+
+	mutex_lock(&vdev->cf_mutex);
+	features = vdev->config->get_features(vdev);
+	mutex_unlock(&vdev->cf_mutex);
+	return features;
 }
 EXPORT_SYMBOL_GPL(vdpa_get_features);
 
+static int __vdpa_set_features(struct vdpa_device *vdev, u64 features)
+{
+	const struct vdpa_config_ops *ops = vdev->config;
+	int err;
+
+	vdev->features_valid = true;
+	err = ops->set_features(vdev, features);
+	return err;
+}
+
 /**
  * vdpa_set_features - Set vDPA device features
  * @vdev: vdpa device whose features to set
@@ -282,10 +301,12 @@ EXPORT_SYMBOL_GPL(vdpa_get_features);
  */
 int vdpa_set_features(struct vdpa_device *vdev, u64 features)
 {
-	const struct vdpa_config_ops *ops = vdev->config;
+	int err;
 
-	vdev->features_valid = true;
-	return ops->set_features(vdev, features);
+	mutex_lock(&vdev->cf_mutex);
+	err = __vdpa_set_features(vdev, features);
+	mutex_unlock(&vdev->cf_mutex);
+	return err;
 }
 EXPORT_SYMBOL_GPL(vdpa_set_features);
 
@@ -294,13 +315,15 @@ void vdpa_get_config(struct vdpa_device *vdev, unsigned int offset,
 {
 	const struct vdpa_config_ops *ops = vdev->config;
 
+	mutex_lock(&vdev->cf_mutex);
 	/*
 	 * Config accesses aren't supposed to trigger before features are set.
 	 * If it does happen we assume a legacy guest.
 	 */
 	if (!vdev->features_valid)
-		vdpa_set_features(vdev, 0);
+		__vdpa_set_features(vdev, 0);
 	ops->get_config(vdev, offset, buf, len);
+	mutex_unlock(&vdev->cf_mutex);
 }
 EXPORT_SYMBOL_GPL(vdpa_get_config);
 
@@ -314,7 +337,9 @@ EXPORT_SYMBOL_GPL(vdpa_get_config);
 void vdpa_set_config(struct vdpa_device *dev, unsigned int offset,
 		     void *buf, unsigned int length)
 {
+	mutex_lock(&dev->cf_mutex);
 	dev->config->set_config(dev, offset, buf, length);
+	mutex_unlock(&dev->cf_mutex);
 }
 EXPORT_SYMBOL_GPL(vdpa_set_config);
 
@@ -664,6 +689,198 @@ static int vdpa_nl_cmd_dev_get_dumpit(struct sk_buff *msg, struct netlink_callba
 	return msg->len;
 }
 
+static int vdpa_dev_net_mq_config_fill(struct vdpa_device *vdev,
+				       struct sk_buff *msg, u64 features,
+				       const struct vdpa_dev_config *config)
+{
+	if ((features & (1ULL << VIRTIO_NET_F_MQ)) == 0)
+		return 0;
+
+	if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MAX_VQP,
+			config->net.max_virtqueue_pairs))
+		return -EMSGSIZE;
+	return 0;
+}
+
+static int vdpa_dev_net_rss_config_fill(struct vdpa_device *vdev,
+					struct sk_buff *msg, u64 features,
+					const struct vdpa_dev_config *config)
+{
+	if ((features & (1ULL << VIRTIO_NET_F_RSS)) == 0)
+		return 0;
+
+	if (nla_put_u8(msg, VDPA_ATTR_DEV_NET_CFG_RSS_MAX_KEY_LEN,
+		       config->net.rss_max_key_size))
+		return -EMSGSIZE;
+	if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_RSS_MAX_IT_LEN,
+			config->net.rss_max_key_size))
+		return -EMSGSIZE;
+	if (nla_put_u32(msg, VDPA_ATTR_DEV_NET_CFG_RSS_HASH_TYPES,
+			config->net.supported_hash_types))
+		return -EMSGSIZE;
+	return 0;
+}
+
+static int vdpa_dev_net_config_fill(struct vdpa_device *vdev, struct sk_buff *msg)
+{
+	struct vdpa_dev_config config = {};
+	u64 features;
+	int err;
+
+	mutex_lock(&vdev->cf_mutex);
+	vdev->config->get_ce_config(vdev, &config);
+	mutex_unlock(&vdev->cf_mutex);
+
+	if (nla_put(msg, VDPA_ATTR_DEV_NET_CFG_MACADDR, sizeof(config.net.mac),
+		    config.net.mac))
+		return -EMSGSIZE;
+	if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_STATUS, config.net.status))
+		return -EMSGSIZE;
+	if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MTU, config.net.mtu))
+		return -EMSGSIZE;
+	if (nla_put_u32(msg, VDPA_ATTR_DEV_NET_CFG_SPEED, config.net.speed))
+		return -EMSGSIZE;
+	if (nla_put_u8(msg, VDPA_ATTR_DEV_NET_CFG_DUPLEX, config.net.duplex))
+		return -EMSGSIZE;
+
+	features = vdev->config->get_features(vdev);
+
+	err = vdpa_dev_net_mq_config_fill(vdev, msg, features, &config);
+	if (err)
+		return err;
+	return vdpa_dev_net_rss_config_fill(vdev, msg, features, &config);
+}
+
+static int
+vdpa_dev_config_fill(struct vdpa_device *vdev, struct sk_buff *msg, u32 portid, u32 seq,
+		     int flags, struct netlink_ext_ack *extack)
+{
+	u32 device_id;
+	void *hdr;
+	int err;
+
+	if (!vdev->config->get_ce_config) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "Configuration layout query is unsupported");
+		return -EOPNOTSUPP;
+	}
+
+	hdr = genlmsg_put(msg, portid, seq, &vdpa_nl_family, flags,
+			  VDPA_CMD_DEV_CONFIG_GET);
+	if (!hdr)
+		return -EMSGSIZE;
+
+	if (nla_put_string(msg, VDPA_ATTR_DEV_NAME, dev_name(&vdev->dev))) {
+		err = -EMSGSIZE;
+		goto msg_err;
+	}
+
+	device_id = vdev->config->get_device_id(vdev);
+	if (nla_put_u32(msg, VDPA_ATTR_DEV_ID, device_id)) {
+		err = -EMSGSIZE;
+		goto msg_err;
+	}
+
+	switch (device_id) {
+	case VIRTIO_ID_NET:
+		err = vdpa_dev_net_config_fill(vdev, msg);
+		break;
+	default:
+		err = -EOPNOTSUPP;
+		break;
+	}
+	if (err)
+		goto msg_err;
+
+	genlmsg_end(msg, hdr);
+	return 0;
+
+msg_err:
+	genlmsg_cancel(msg, hdr);
+	return err;
+}
+
+static int vdpa_nl_cmd_dev_config_get_doit(struct sk_buff *skb, struct genl_info *info)
+{
+	struct vdpa_device *vdev;
+	struct sk_buff *msg;
+	const char *devname;
+	struct device *dev;
+	int err;
+
+	if (!info->attrs[VDPA_ATTR_DEV_NAME])
+		return -EINVAL;
+	devname = nla_data(info->attrs[VDPA_ATTR_DEV_NAME]);
+	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+	if (!msg)
+		return -ENOMEM;
+
+	mutex_lock(&vdpa_dev_mutex);
+	dev = bus_find_device(&vdpa_bus, NULL, devname, vdpa_name_match);
+	if (!dev) {
+		NL_SET_ERR_MSG_MOD(info->extack, "device not found");
+		err = -ENODEV;
+		goto dev_err;
+	}
+	vdev = container_of(dev, struct vdpa_device, dev);
+	if (!vdev->mdev) {
+		NL_SET_ERR_MSG_MOD(info->extack, "unmanaged vdpa device");
+		err = -EINVAL;
+		goto mdev_err;
+	}
+	err = vdpa_dev_config_fill(vdev, msg, info->snd_portid, info->snd_seq,
+				   0, info->extack);
+	if (!err)
+		err = genlmsg_reply(msg, info);
+
+mdev_err:
+	put_device(dev);
+dev_err:
+	mutex_unlock(&vdpa_dev_mutex);
+	if (err)
+		nlmsg_free(msg);
+	return err;
+}
+
+static int vdpa_dev_config_dump(struct device *dev, void *data)
+{
+	struct vdpa_device *vdev = container_of(dev, struct vdpa_device, dev);
+	struct vdpa_dev_dump_info *info = data;
+	int err;
+
+	if (!vdev->mdev)
+		return 0;
+	if (info->idx < info->start_idx) {
+		info->idx++;
+		return 0;
+	}
+	err = vdpa_dev_config_fill(vdev, info->msg, NETLINK_CB(info->cb->skb).portid,
+				   info->cb->nlh->nlmsg_seq, NLM_F_MULTI,
+				   info->cb->extack);
+	if (err)
+		return err;
+
+	info->idx++;
+	return 0;
+}
+
+static int
+vdpa_nl_cmd_dev_config_get_dumpit(struct sk_buff *msg, struct netlink_callback *cb)
+{
+	struct vdpa_dev_dump_info info;
+
+	info.msg = msg;
+	info.cb = cb;
+	info.start_idx = cb->args[0];
+	info.idx = 0;
+
+	mutex_lock(&vdpa_dev_mutex);
+	bus_for_each_dev(&vdpa_bus, NULL, &info, vdpa_dev_config_dump);
+	mutex_unlock(&vdpa_dev_mutex);
+	cb->args[0] = info.idx;
+	return msg->len;
+}
+
 static const struct nla_policy vdpa_nl_policy[VDPA_ATTR_MAX + 1] = {
 	[VDPA_ATTR_MGMTDEV_BUS_NAME] = { .type = NLA_NUL_STRING },
 	[VDPA_ATTR_MGMTDEV_DEV_NAME] = { .type = NLA_STRING },
@@ -695,6 +912,13 @@ static const struct genl_ops vdpa_nl_ops[] = {
 		.doit = vdpa_nl_cmd_dev_get_doit,
 		.dumpit = vdpa_nl_cmd_dev_get_dumpit,
 	},
+	{
+		.cmd = VDPA_CMD_DEV_CONFIG_GET,
+		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
+		.doit = vdpa_nl_cmd_dev_config_get_doit,
+		.dumpit = vdpa_nl_cmd_dev_config_get_dumpit,
+		.flags = GENL_ADMIN_PERM,
+	},
 };
 
 static struct genl_family vdpa_nl_family __ro_after_init = {
diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
index 62e68ccc4c96..dcbbecb5dea8 100644
--- a/include/linux/vdpa.h
+++ b/include/linux/vdpa.h
@@ -6,6 +6,7 @@
 #include <linux/device.h>
 #include <linux/interrupt.h>
 #include <linux/vhost_iotlb.h>
+#include <linux/if_ether.h>
 
 /**
  * struct vdpa_calllback - vDPA callback definition.
@@ -47,6 +48,7 @@ struct vdpa_mgmt_dev;
  * @nvqs: maximum number of supported virtqueues
  * @mdev: management device pointer; caller must setup when registering device as part
  *	  of dev_add() mgmtdev ops callback before invoking _vdpa_register_device().
+ * @cf_mutex: Protects get and set access to features and configuration layout.
  */
 struct vdpa_device {
 	struct device dev;
@@ -56,6 +58,7 @@ struct vdpa_device {
 	bool features_valid;
 	int nvqs;
 	struct vdpa_mgmt_dev *mdev;
+	struct mutex cf_mutex; /* Protects get/set config and features */
 };
 
 /**
@@ -68,6 +71,39 @@ struct vdpa_iova_range {
 	u64 last;
 };
 
+/**
+ * struct vdpa_net_dev_config - vDPA net device config to get/set via
+ *                              management device.
+ * @mac: mac address of the vdpa device
+ * @status: indicates VIRTIO_NET_F_STATUS and VIRTIO_NET_S_*
+ * @max_virtqueue_pairs: Maximum number of each of transmit and receive queues.
+ *			 see VIRTIO_NET_F_MQ and VIRTIO_NET_CTRL_MQ.
+ *			 Legal values are between 1 and 0x8000.
+ * @speed: In units of 1Mb. All values 0 to INT_MAX are legal.
+ * @mtu: Default maximum transmit unit advice.
+ * @duplex: 0x00 - half duplex
+ *	    0x01 - full duplex
+ * @rss_max_key_size: Maximum size of RSS key.
+ * @supported_hash_types: Bitmask of supported VIRTIO_NET_RSS_HASH_ types
+ * @rss_max_indirection_table_length: Maximum number of indirection table
+ *				      entries.
+ */
+struct vdpa_net_dev_config {
+	u8 mac[ETH_ALEN];
+	u16 status;
+	u16 max_virtqueue_pairs;
+	u32 speed;
+	u16 mtu;
+	u8 duplex;
+	u8 rss_max_key_size;
+	u32 supported_hash_types;
+	u16 rss_max_indirection_table_length;
+};
+
+struct vdpa_dev_config {
+	struct vdpa_net_dev_config net;
+};
+
 /**
  * struct vdpa_config_ops - operations for configuring a vDPA device.
  * Note: vDPA device drivers are required to implement all of the
@@ -164,6 +200,10 @@ struct vdpa_iova_range {
  *				@buf: buffer used to write from
  *				@len: the length to write to
  *				configuration space
+ * @get_ce_config:		Read device specific configuration in
+ *				cpu endianness.
+ *				@vdev: vdpa device
+ *				@config: pointer to config buffer used to read to
  * @get_generation:		Get device config generation (optional)
  *				@vdev: vdpa device
  *				Returns u32: device generation
@@ -235,6 +275,8 @@ struct vdpa_config_ops {
 			   void *buf, unsigned int len);
 	void (*set_config)(struct vdpa_device *vdev, unsigned int offset,
 			   const void *buf, unsigned int len);
+	void (*get_ce_config)(struct vdpa_device *vdev,
+			      struct vdpa_dev_config *config);
 	u32 (*get_generation)(struct vdpa_device *vdev);
 	struct vdpa_iova_range (*get_iova_range)(struct vdpa_device *vdev);
 
diff --git a/include/uapi/linux/vdpa.h b/include/uapi/linux/vdpa.h
index 66a41e4ec163..5c31ecc3b956 100644
--- a/include/uapi/linux/vdpa.h
+++ b/include/uapi/linux/vdpa.h
@@ -17,6 +17,7 @@ enum vdpa_command {
 	VDPA_CMD_DEV_NEW,
 	VDPA_CMD_DEV_DEL,
 	VDPA_CMD_DEV_GET,		/* can dump */
+	VDPA_CMD_DEV_CONFIG_GET,	/* can dump */
 };
 
 enum vdpa_attr {
@@ -33,6 +34,16 @@ enum vdpa_attr {
 	VDPA_ATTR_DEV_MAX_VQS,			/* u32 */
 	VDPA_ATTR_DEV_MAX_VQ_SIZE,		/* u16 */
 
+	VDPA_ATTR_DEV_NET_CFG_MACADDR,		/* binary */
+	VDPA_ATTR_DEV_NET_STATUS,		/* u8 */
+	VDPA_ATTR_DEV_NET_CFG_MAX_VQP,		/* u16 */
+	VDPA_ATTR_DEV_NET_CFG_MTU,		/* u16 */
+	VDPA_ATTR_DEV_NET_CFG_SPEED,		/* u16 */
+	VDPA_ATTR_DEV_NET_CFG_DUPLEX,		/* u16 */
+	VDPA_ATTR_DEV_NET_CFG_RSS_MAX_KEY_LEN,	/* u8 */
+	VDPA_ATTR_DEV_NET_CFG_RSS_MAX_IT_LEN,	/* u16 */
+	VDPA_ATTR_DEV_NET_CFG_RSS_HASH_TYPES,	/* u32 */
+
 	/* new attributes must be added above here */
 	VDPA_ATTR_MAX,
 };
-- 
2.26.2

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH linux-next v2 04/14] vdpa: Introduce query of device config layout

[Jason Wang]

在 2021/4/7 上午1:04, Parav Pandit 写道:
> Introduce a command to query a device config layout.
>
> An example query of network vdpa device:
>
> $ vdpa dev add name bar mgmtdev vdpasim_net
>
> $ vdpa dev config show
> bar: mac 00:35:09:19:48:05 link up link_announce false mtu 1500 speed 0 duplex 0
>
> $ vdpa dev config show -jp
> {
>      "config": {
>          "bar": {
>              "mac": "00:35:09:19:48:05",
>              "link ": "up",
>              "link_announce ": false,
>              "mtu": 1500,
>              "speed": 0,
>              "duplex": 0
>          }
>      }
> }
>
> Signed-off-by: Parav Pandit <parav@nvidia.com>
> Signed-off-by: Eli Cohen <elic@nvidia.com>
> ---
> changelog:
> v1->v2:
>   - read whole net config layout instead of individual fields
>   - added error extack for unmanaged vdpa device
>   - fixed several endianness issues
>   - introduced vdpa device ops for get config which is synchronized
>     with other get/set features ops and config ops
> ---
>   drivers/vdpa/vdpa.c       | 234 +++++++++++++++++++++++++++++++++++++-
>   include/linux/vdpa.h      |  42 +++++++
>   include/uapi/linux/vdpa.h |  11 ++
>   3 files changed, 282 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
> index 116e076c72fd..9da8deb8c0f2 100644
> --- a/drivers/vdpa/vdpa.c
> +++ b/drivers/vdpa/vdpa.c
> @@ -14,6 +14,8 @@
>   #include <uapi/linux/vdpa.h>
>   #include <net/genetlink.h>
>   #include <linux/mod_devicetable.h>
> +#include <linux/virtio_net.h>
> +#include <linux/virtio_ids.h>
>   
>   static LIST_HEAD(mdev_head);
>   /* A global mutex that protects vdpa management device and device level operations. */
> @@ -60,6 +62,7 @@ static void vdpa_release_dev(struct device *d)
>   		ops->free(vdev);
>   
>   	ida_simple_remove(&vdpa_index_ida, vdev->index);
> +	mutex_destroy(&vdev->cf_mutex);
>   	kfree(vdev);
>   }
>   
> @@ -114,6 +117,7 @@ struct vdpa_device *__vdpa_alloc_device(struct device *parent,
>   	if (err)
>   		goto err_name;
>   
> +	mutex_init(&vdev->cf_mutex);
>   	device_initialize(&vdev->dev);
>   
>   	return vdev;
> @@ -269,10 +273,25 @@ EXPORT_SYMBOL_GPL(vdpa_mgmtdev_register);
>    */
>   u64 vdpa_get_features(struct vdpa_device *vdev)
>   {
> -	return vdev->config->get_features(vdev);
> +	u64 features;
> +
> +	mutex_lock(&vdev->cf_mutex);
> +	features = vdev->config->get_features(vdev);
> +	mutex_unlock(&vdev->cf_mutex);
> +	return features;
>   }
>   EXPORT_SYMBOL_GPL(vdpa_get_features);
>   
> +static int __vdpa_set_features(struct vdpa_device *vdev, u64 features)
> +{
> +	const struct vdpa_config_ops *ops = vdev->config;
> +	int err;
> +
> +	vdev->features_valid = true;
> +	err = ops->set_features(vdev, features);
> +	return err;
> +}
> +
>   /**
>    * vdpa_set_features - Set vDPA device features
>    * @vdev: vdpa device whose features to set
> @@ -282,10 +301,12 @@ EXPORT_SYMBOL_GPL(vdpa_get_features);
>    */
>   int vdpa_set_features(struct vdpa_device *vdev, u64 features)
>   {
> -	const struct vdpa_config_ops *ops = vdev->config;
> +	int err;
>   
> -	vdev->features_valid = true;
> -	return ops->set_features(vdev, features);
> +	mutex_lock(&vdev->cf_mutex);
> +	err = __vdpa_set_features(vdev, features);
> +	mutex_unlock(&vdev->cf_mutex);
> +	return err;
>   }
>   EXPORT_SYMBOL_GPL(vdpa_set_features);
>   
> @@ -294,13 +315,15 @@ void vdpa_get_config(struct vdpa_device *vdev, unsigned int offset,
>   {
>   	const struct vdpa_config_ops *ops = vdev->config;
>   
> +	mutex_lock(&vdev->cf_mutex);
>   	/*
>   	 * Config accesses aren't supposed to trigger before features are set.
>   	 * If it does happen we assume a legacy guest.
>   	 */
>   	if (!vdev->features_valid)
> -		vdpa_set_features(vdev, 0);
> +		__vdpa_set_features(vdev, 0);
>   	ops->get_config(vdev, offset, buf, len);
> +	mutex_unlock(&vdev->cf_mutex);
>   }
>   EXPORT_SYMBOL_GPL(vdpa_get_config);
>   
> @@ -314,7 +337,9 @@ EXPORT_SYMBOL_GPL(vdpa_get_config);
>   void vdpa_set_config(struct vdpa_device *dev, unsigned int offset,
>   		     void *buf, unsigned int length)
>   {
> +	mutex_lock(&dev->cf_mutex);
>   	dev->config->set_config(dev, offset, buf, length);
> +	mutex_unlock(&dev->cf_mutex);
>   }
>   EXPORT_SYMBOL_GPL(vdpa_set_config);
>   
> @@ -664,6 +689,198 @@ static int vdpa_nl_cmd_dev_get_dumpit(struct sk_buff *msg, struct netlink_callba
>   	return msg->len;
>   }
>   
> +static int vdpa_dev_net_mq_config_fill(struct vdpa_device *vdev,
> +				       struct sk_buff *msg, u64 features,
> +				       const struct vdpa_dev_config *config)
> +{
> +	if ((features & (1ULL << VIRTIO_NET_F_MQ)) == 0)
> +		return 0;
> +
> +	if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MAX_VQP,
> +			config->net.max_virtqueue_pairs))
> +		return -EMSGSIZE;
> +	return 0;
> +}
> +
> +static int vdpa_dev_net_rss_config_fill(struct vdpa_device *vdev,
> +					struct sk_buff *msg, u64 features,
> +					const struct vdpa_dev_config *config)
> +{
> +	if ((features & (1ULL << VIRTIO_NET_F_RSS)) == 0)
> +		return 0;
> +
> +	if (nla_put_u8(msg, VDPA_ATTR_DEV_NET_CFG_RSS_MAX_KEY_LEN,
> +		       config->net.rss_max_key_size))
> +		return -EMSGSIZE;
> +	if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_RSS_MAX_IT_LEN,
> +			config->net.rss_max_key_size))
> +		return -EMSGSIZE;
> +	if (nla_put_u32(msg, VDPA_ATTR_DEV_NET_CFG_RSS_HASH_TYPES,
> +			config->net.supported_hash_types))
> +		return -EMSGSIZE;
> +	return 0;
> +}
> +
> +static int vdpa_dev_net_config_fill(struct vdpa_device *vdev, struct sk_buff *msg)
> +{
> +	struct vdpa_dev_config config = {};
> +	u64 features;
> +	int err;
> +
> +	mutex_lock(&vdev->cf_mutex);
> +	vdev->config->get_ce_config(vdev, &config);
> +	mutex_unlock(&vdev->cf_mutex);
> +
> +	if (nla_put(msg, VDPA_ATTR_DEV_NET_CFG_MACADDR, sizeof(config.net.mac),
> +		    config.net.mac))
> +		return -EMSGSIZE;
> +	if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_STATUS, config.net.status))
> +		return -EMSGSIZE;
> +	if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MTU, config.net.mtu))
> +		return -EMSGSIZE;
> +	if (nla_put_u32(msg, VDPA_ATTR_DEV_NET_CFG_SPEED, config.net.speed))
> +		return -EMSGSIZE;
> +	if (nla_put_u8(msg, VDPA_ATTR_DEV_NET_CFG_DUPLEX, config.net.duplex))
> +		return -EMSGSIZE;
> +
> +	features = vdev->config->get_features(vdev);
> +
> +	err = vdpa_dev_net_mq_config_fill(vdev, msg, features, &config);
> +	if (err)
> +		return err;
> +	return vdpa_dev_net_rss_config_fill(vdev, msg, features, &config);
> +}
> +
> +static int
> +vdpa_dev_config_fill(struct vdpa_device *vdev, struct sk_buff *msg, u32 portid, u32 seq,
> +		     int flags, struct netlink_ext_ack *extack)
> +{
> +	u32 device_id;
> +	void *hdr;
> +	int err;
> +
> +	if (!vdev->config->get_ce_config) {
> +		NL_SET_ERR_MSG_MOD(extack,
> +				   "Configuration layout query is unsupported");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	hdr = genlmsg_put(msg, portid, seq, &vdpa_nl_family, flags,
> +			  VDPA_CMD_DEV_CONFIG_GET);
> +	if (!hdr)
> +		return -EMSGSIZE;
> +
> +	if (nla_put_string(msg, VDPA_ATTR_DEV_NAME, dev_name(&vdev->dev))) {
> +		err = -EMSGSIZE;
> +		goto msg_err;
> +	}
> +
> +	device_id = vdev->config->get_device_id(vdev);
> +	if (nla_put_u32(msg, VDPA_ATTR_DEV_ID, device_id)) {
> +		err = -EMSGSIZE;
> +		goto msg_err;
> +	}
> +
> +	switch (device_id) {
> +	case VIRTIO_ID_NET:
> +		err = vdpa_dev_net_config_fill(vdev, msg);
> +		break;
> +	default:
> +		err = -EOPNOTSUPP;
> +		break;
> +	}
> +	if (err)
> +		goto msg_err;
> +
> +	genlmsg_end(msg, hdr);
> +	return 0;
> +
> +msg_err:
> +	genlmsg_cancel(msg, hdr);
> +	return err;
> +}
> +
> +static int vdpa_nl_cmd_dev_config_get_doit(struct sk_buff *skb, struct genl_info *info)
> +{
> +	struct vdpa_device *vdev;
> +	struct sk_buff *msg;
> +	const char *devname;
> +	struct device *dev;
> +	int err;
> +
> +	if (!info->attrs[VDPA_ATTR_DEV_NAME])
> +		return -EINVAL;
> +	devname = nla_data(info->attrs[VDPA_ATTR_DEV_NAME]);
> +	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
> +	if (!msg)
> +		return -ENOMEM;
> +
> +	mutex_lock(&vdpa_dev_mutex);
> +	dev = bus_find_device(&vdpa_bus, NULL, devname, vdpa_name_match);
> +	if (!dev) {
> +		NL_SET_ERR_MSG_MOD(info->extack, "device not found");
> +		err = -ENODEV;
> +		goto dev_err;
> +	}
> +	vdev = container_of(dev, struct vdpa_device, dev);
> +	if (!vdev->mdev) {
> +		NL_SET_ERR_MSG_MOD(info->extack, "unmanaged vdpa device");
> +		err = -EINVAL;
> +		goto mdev_err;
> +	}
> +	err = vdpa_dev_config_fill(vdev, msg, info->snd_portid, info->snd_seq,
> +				   0, info->extack);
> +	if (!err)
> +		err = genlmsg_reply(msg, info);
> +
> +mdev_err:
> +	put_device(dev);
> +dev_err:
> +	mutex_unlock(&vdpa_dev_mutex);
> +	if (err)
> +		nlmsg_free(msg);
> +	return err;
> +}
> +
> +static int vdpa_dev_config_dump(struct device *dev, void *data)
> +{
> +	struct vdpa_device *vdev = container_of(dev, struct vdpa_device, dev);
> +	struct vdpa_dev_dump_info *info = data;
> +	int err;
> +
> +	if (!vdev->mdev)
> +		return 0;
> +	if (info->idx < info->start_idx) {
> +		info->idx++;
> +		return 0;
> +	}
> +	err = vdpa_dev_config_fill(vdev, info->msg, NETLINK_CB(info->cb->skb).portid,
> +				   info->cb->nlh->nlmsg_seq, NLM_F_MULTI,
> +				   info->cb->extack);
> +	if (err)
> +		return err;
> +
> +	info->idx++;
> +	return 0;
> +}
> +
> +static int
> +vdpa_nl_cmd_dev_config_get_dumpit(struct sk_buff *msg, struct netlink_callback *cb)
> +{
> +	struct vdpa_dev_dump_info info;
> +
> +	info.msg = msg;
> +	info.cb = cb;
> +	info.start_idx = cb->args[0];
> +	info.idx = 0;
> +
> +	mutex_lock(&vdpa_dev_mutex);
> +	bus_for_each_dev(&vdpa_bus, NULL, &info, vdpa_dev_config_dump);
> +	mutex_unlock(&vdpa_dev_mutex);
> +	cb->args[0] = info.idx;
> +	return msg->len;
> +}
> +
>   static const struct nla_policy vdpa_nl_policy[VDPA_ATTR_MAX + 1] = {
>   	[VDPA_ATTR_MGMTDEV_BUS_NAME] = { .type = NLA_NUL_STRING },
>   	[VDPA_ATTR_MGMTDEV_DEV_NAME] = { .type = NLA_STRING },
> @@ -695,6 +912,13 @@ static const struct genl_ops vdpa_nl_ops[] = {
>   		.doit = vdpa_nl_cmd_dev_get_doit,
>   		.dumpit = vdpa_nl_cmd_dev_get_dumpit,
>   	},
> +	{
> +		.cmd = VDPA_CMD_DEV_CONFIG_GET,
> +		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
> +		.doit = vdpa_nl_cmd_dev_config_get_doit,
> +		.dumpit = vdpa_nl_cmd_dev_config_get_dumpit,
> +		.flags = GENL_ADMIN_PERM,
> +	},
>   };
>   
>   static struct genl_family vdpa_nl_family __ro_after_init = {
> diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
> index 62e68ccc4c96..dcbbecb5dea8 100644
> --- a/include/linux/vdpa.h
> +++ b/include/linux/vdpa.h
> @@ -6,6 +6,7 @@
>   #include <linux/device.h>
>   #include <linux/interrupt.h>
>   #include <linux/vhost_iotlb.h>
> +#include <linux/if_ether.h>
>   
>   /**
>    * struct vdpa_calllback - vDPA callback definition.
> @@ -47,6 +48,7 @@ struct vdpa_mgmt_dev;
>    * @nvqs: maximum number of supported virtqueues
>    * @mdev: management device pointer; caller must setup when registering device as part
>    *	  of dev_add() mgmtdev ops callback before invoking _vdpa_register_device().
> + * @cf_mutex: Protects get and set access to features and configuration layout.
>    */
>   struct vdpa_device {
>   	struct device dev;
> @@ -56,6 +58,7 @@ struct vdpa_device {
>   	bool features_valid;
>   	int nvqs;
>   	struct vdpa_mgmt_dev *mdev;
> +	struct mutex cf_mutex; /* Protects get/set config and features */
>   };
>   
>   /**
> @@ -68,6 +71,39 @@ struct vdpa_iova_range {
>   	u64 last;
>   };
>   
> +/**
> + * struct vdpa_net_dev_config - vDPA net device config to get/set via
> + *                              management device.
> + * @mac: mac address of the vdpa device
> + * @status: indicates VIRTIO_NET_F_STATUS and VIRTIO_NET_S_*
> + * @max_virtqueue_pairs: Maximum number of each of transmit and receive queues.
> + *			 see VIRTIO_NET_F_MQ and VIRTIO_NET_CTRL_MQ.
> + *			 Legal values are between 1 and 0x8000.
> + * @speed: In units of 1Mb. All values 0 to INT_MAX are legal.
> + * @mtu: Default maximum transmit unit advice.
> + * @duplex: 0x00 - half duplex
> + *	    0x01 - full duplex
> + * @rss_max_key_size: Maximum size of RSS key.
> + * @supported_hash_types: Bitmask of supported VIRTIO_NET_RSS_HASH_ types
> + * @rss_max_indirection_table_length: Maximum number of indirection table
> + *				      entries.
> + */
> +struct vdpa_net_dev_config {
> +	u8 mac[ETH_ALEN];
> +	u16 status;
> +	u16 max_virtqueue_pairs;
> +	u32 speed;
> +	u16 mtu;
> +	u8 duplex;
> +	u8 rss_max_key_size;
> +	u32 supported_hash_types;
> +	u16 rss_max_indirection_table_length;
> +};
> +
> +struct vdpa_dev_config {
> +	struct vdpa_net_dev_config net;
> +};
> +
>   /**
>    * struct vdpa_config_ops - operations for configuring a vDPA device.
>    * Note: vDPA device drivers are required to implement all of the
> @@ -164,6 +200,10 @@ struct vdpa_iova_range {
>    *				@buf: buffer used to write from
>    *				@len: the length to write to
>    *				configuration space
> + * @get_ce_config:		Read device specific configuration in
> + *				cpu endianness.
> + *				@vdev: vdpa device
> + *				@config: pointer to config buffer used to read to


So I wonder what's the reason for having this? If it's only the reason 
of endian, we can just use get_config.

We don't plan to expose a legacy or transition device, so we can simply 
do the endian conversion in the device.

Then we can stick to the uapi of virtio_net_config and there's no need 
for the VDPA_ATTR_DEV_NET_CFG_XXX?

Thanks


>    * @get_generation:		Get device config generation (optional)
>    *				@vdev: vdpa device
>    *				Returns u32: device generation
> @@ -235,6 +275,8 @@ struct vdpa_config_ops {
>   			   void *buf, unsigned int len);
>   	void (*set_config)(struct vdpa_device *vdev, unsigned int offset,
>   			   const void *buf, unsigned int len);
> +	void (*get_ce_config)(struct vdpa_device *vdev,
> +			      struct vdpa_dev_config *config);
>   	u32 (*get_generation)(struct vdpa_device *vdev);
>   	struct vdpa_iova_range (*get_iova_range)(struct vdpa_device *vdev);
>   
> diff --git a/include/uapi/linux/vdpa.h b/include/uapi/linux/vdpa.h
> index 66a41e4ec163..5c31ecc3b956 100644
> --- a/include/uapi/linux/vdpa.h
> +++ b/include/uapi/linux/vdpa.h
> @@ -17,6 +17,7 @@ enum vdpa_command {
>   	VDPA_CMD_DEV_NEW,
>   	VDPA_CMD_DEV_DEL,
>   	VDPA_CMD_DEV_GET,		/* can dump */
> +	VDPA_CMD_DEV_CONFIG_GET,	/* can dump */
>   };
>   
>   enum vdpa_attr {
> @@ -33,6 +34,16 @@ enum vdpa_attr {
>   	VDPA_ATTR_DEV_MAX_VQS,			/* u32 */
>   	VDPA_ATTR_DEV_MAX_VQ_SIZE,		/* u16 */
>   
> +	VDPA_ATTR_DEV_NET_CFG_MACADDR,		/* binary */
> +	VDPA_ATTR_DEV_NET_STATUS,		/* u8 */
> +	VDPA_ATTR_DEV_NET_CFG_MAX_VQP,		/* u16 */
> +	VDPA_ATTR_DEV_NET_CFG_MTU,		/* u16 */
> +	VDPA_ATTR_DEV_NET_CFG_SPEED,		/* u16 */
> +	VDPA_ATTR_DEV_NET_CFG_DUPLEX,		/* u16 */
> +	VDPA_ATTR_DEV_NET_CFG_RSS_MAX_KEY_LEN,	/* u8 */
> +	VDPA_ATTR_DEV_NET_CFG_RSS_MAX_IT_LEN,	/* u16 */
> +	VDPA_ATTR_DEV_NET_CFG_RSS_HASH_TYPES,	/* u32 */
> +
>   	/* new attributes must be added above here */
>   	VDPA_ATTR_MAX,
>   };

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

RE: [PATCH linux-next v2 04/14] vdpa: Introduce query of device config layout

[Parav Pandit]

> From: Jason Wang <jasowang@redhat.com>
> Sent: Wednesday, April 7, 2021 9:26 AM

[..]
> >   /**
> >    * struct vdpa_config_ops - operations for configuring a vDPA device.
> >    * Note: vDPA device drivers are required to implement all of the @@
> > -164,6 +200,10 @@ struct vdpa_iova_range {
> >    *				@buf: buffer used to write from
> >    *				@len: the length to write to
> >    *				configuration space
> > + * @get_ce_config:		Read device specific configuration in
> > + *				cpu endianness.
> > + *				@vdev: vdpa device
> > + *				@config: pointer to config buffer used to
> read to
> 
> 
> So I wonder what's the reason for having this? If it's only the reason of
> endian, we can just use get_config.
> 
Didn't follow your suggestion.
How does in kernel management tool caller get_config  know how to do endianenss conversion?
Or you are proposing to send this whole virtio_config structure as binary data to user space via netlink?
If so, it is not a practice in netlink to return struct.

Also making netlink user space to understand __virtio16 doesn't sound correct.
Often orchestration is not written in C. I cannot think of returning virtio_net_config via netlink socket if it is your thought.

And decoding it requires to query features too. Querying features and config are not atomic so parsed value can be wrong.

Endianness has to be self-contained in fields return via netlink. With that baseline, lets think further.

> We don't plan to expose a legacy or transition device, so we can simply do
> the endian conversion in the device.
>
I am bit confused with Michael's suggestion in v1 [1].

VIRTIO_F_VERSION_1 is set today by mlx5 and ifcvf driver.

> Then we can stick to the uapi of virtio_net_config and there's no need for the
> VDPA_ATTR_DEV_NET_CFG_XXX?
> 
[1] https://lore.kernel.org/virtualization/20210224020336-mutt-send-email-mst@kernel.org/
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH linux-next v2 04/14] vdpa: Introduce query of device config layout

[Jason Wang]

在 2021/4/7 下午1:10, Parav Pandit 写道:
>
>> From: Jason Wang <jasowang@redhat.com>
>> Sent: Wednesday, April 7, 2021 9:26 AM
> [..]
>>>    /**
>>>     * struct vdpa_config_ops - operations for configuring a vDPA device.
>>>     * Note: vDPA device drivers are required to implement all of the @@
>>> -164,6 +200,10 @@ struct vdpa_iova_range {
>>>     *				@buf: buffer used to write from
>>>     *				@len: the length to write to
>>>     *				configuration space
>>> + * @get_ce_config:		Read device specific configuration in
>>> + *				cpu endianness.
>>> + *				@vdev: vdpa device
>>> + *				@config: pointer to config buffer used to
>> read to
>>
>>
>> So I wonder what's the reason for having this? If it's only the reason of
>> endian, we can just use get_config.
>>
> Didn't follow your suggestion.
> How does in kernel management tool caller get_config  know how to do endianenss conversion?


LE should be used, so it's the responsibility of the vDPA parent 
(driver) to do the endian conversion.


> Or you are proposing to send this whole virtio_config structure as binary data to user space via netlink?
> If so, it is not a practice in netlink to return struct.


I don't get here, it should work as mac address I think.


>
> Also making netlink user space to understand __virtio16 doesn't sound correct.
> Often orchestration is not written in C. I cannot think of returning virtio_net_config via netlink socket if it is your thought.


I'm not sure I get here. __virtio16 is part of uapi which is defined 
virtio_types.h so did the virtio_net_config. Duplicating those through 
dedicated netlink attr looks like burden. E.g you need to introduce new 
attrs for each field of the config for every virtio devices and keep it 
up-to-date with the virtio uapis.


>
> And decoding it requires to query features too. Querying features and config are not atomic so parsed value can be wrong.


So I think device should maintain a stable features that will is 
returned by get_features(), otherwise a lot of things will be broken.


>
> Endianness has to be self-contained in fields return via netlink. With that baseline, lets think further.


Then mandating LE seem self-contained.


>
>> We don't plan to expose a legacy or transition device, so we can simply do
>> the endian conversion in the device.
>>
> I am bit confused with Michael's suggestion in v1 [1].
>
> VIRTIO_F_VERSION_1 is set today by mlx5 and ifcvf driver.


I've had some disucssion with Michael, the conclusion is that we should 
only advertise (or mandate) a modern device to be exposed userspace. 
Otherwise it could be a lot of burden. Qemu can still present a 
transtitonal device by doing the endian conversion when necessary in the 
middle. I'm working on the Qemu patches to do that.

Thanks


>
>> Then we can stick to the uapi of virtio_net_config and there's no need for the
>> VDPA_ATTR_DEV_NET_CFG_XXX?
>>
> [1] https://lore.kernel.org/virtualization/20210224020336-mutt-send-email-mst@kernel.org/
>

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

RE: [PATCH linux-next v2 04/14] vdpa: Introduce query of device config layout

[Parav Pandit]

> From: Jason Wang <jasowang@redhat.com>
> Sent: Wednesday, April 7, 2021 12:42 PM
> 
> 在 2021/4/7 下午1:10, Parav Pandit 写道:
> >
> >> From: Jason Wang <jasowang@redhat.com>
> >> Sent: Wednesday, April 7, 2021 9:26 AM
> > [..]
> >>>    /**
> >>>     * struct vdpa_config_ops - operations for configuring a vDPA device.
> >>>     * Note: vDPA device drivers are required to implement all of the
> >>> @@
> >>> -164,6 +200,10 @@ struct vdpa_iova_range {
> >>>     *				@buf: buffer used to write from
> >>>     *				@len: the length to write to
> >>>     *				configuration space
> >>> + * @get_ce_config:		Read device specific configuration in
> >>> + *				cpu endianness.
> >>> + *				@vdev: vdpa device
> >>> + *				@config: pointer to config buffer used to
> >> read to
> >>
> >>
> >> So I wonder what's the reason for having this? If it's only the
> >> reason of endian, we can just use get_config.
> >>
> > Didn't follow your suggestion.
> > How does in kernel management tool caller get_config  know how to do
> endianenss conversion?
> 
> 
> LE should be used, so it's the responsibility of the vDPA parent
> (driver) to do the endian conversion.
> 
> 
> > Or you are proposing to send this whole virtio_config structure as binary
> data to user space via netlink?
> > If so, it is not a practice in netlink to return struct.
> 
> 
> I don't get here, it should work as mac address I think.
> 
> 
> >
> > Also making netlink user space to understand __virtio16 doesn't sound
> correct.
> > Often orchestration is not written in C. I cannot think of returning
> virtio_net_config via netlink socket if it is your thought.
> 
> 
> I'm not sure I get here. __virtio16 is part of uapi which is defined
> virtio_types.h so did the virtio_net_config. Duplicating those through
> dedicated netlink attr looks like burden. E.g you need to introduce new
> attrs for each field of the config for every virtio devices and keep it
> up-to-date with the virtio uapis.
> 
> 
Given that only few fields are valid from the config struct, if we pass the whole struct, we additionally need to pass the bitmask to indicate what is valid.
And so I think passing individual fields via the currently defined netlink is better.
Lets continue using individual fields.
> So I think device should maintain a stable features that will is
> returned by get_features(), otherwise a lot of things will be broken.


> I've had some disucssion with Michael, the conclusion is that we should
> only advertise (or mandate) a modern device to be exposed userspace.
> Otherwise it could be a lot of burden. Qemu can still present a
> transtitonal device by doing the endian conversion when necessary in the
> middle. I'm working on the Qemu patches to do that.

Ok. enforcing this as first thing so that netlink config callbacks can assume them LE would be nice.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH linux-next v2 05/14] vdpa: Enable user to set mac and mtu of vdpa device

[Parav Pandit]

$ vdpa dev add name bar mgmtdev vdpasim_net

$ vdpa dev config set bar mac 00:11:22:33:44:55 mtu 9000

$ vdpa dev config show
bar: mac 00:11:22:33:44:55 link up link_announce false mtu 9000 speed 0 duplex 0

$ vdpa dev config show -jp
{
    "config": {
        "bar": {
            "mac": "00:11:22:33:44:55",
            "link ": "up",
            "link_announce ": false,
            "mtu": 9000,
            "speed": 0,
            "duplex": 0
        }
    }
}

Signed-off-by: Parav Pandit <parav@nvidia.com>
Reviewed-by: Eli Cohen <elic@nvidia.com>
---
changelog:
v1->v2:
 - fixed mtu range checking for max
 - using NLA_POLICY_ETH_ADDR
 - set config moved to device ops instead of mgmtdev ops
 - merged build and set to single routine
 - ensuring that user has NET_ADMIN capability for configuring network
   attributes
---
 drivers/vdpa/vdpa.c       | 90 +++++++++++++++++++++++++++++++++++++++
 include/linux/vdpa.h      | 15 +++++++
 include/uapi/linux/vdpa.h |  1 +
 3 files changed, 106 insertions(+)

diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
index 9da8deb8c0f2..6a6ef1a085e8 100644
--- a/drivers/vdpa/vdpa.c
+++ b/drivers/vdpa/vdpa.c
@@ -881,10 +881,94 @@ vdpa_nl_cmd_dev_config_get_dumpit(struct sk_buff *msg, struct netlink_callback *
 	return msg->len;
 }
 
+static int vdpa_dev_net_config_set(struct vdpa_device *vdev,
+				   struct sk_buff *skb, struct genl_info *info)
+{
+	struct nlattr **nl_attrs = info->attrs;
+	struct vdpa_dev_set_config config = {};
+	const u8 *macaddr;
+	int err;
+
+	if (!vdev->config->set_ce_config)
+		return -EOPNOTSUPP;
+
+	if (!netlink_capable(skb, CAP_NET_ADMIN))
+		return -EPERM;
+
+	if (nl_attrs[VDPA_ATTR_DEV_NET_CFG_MACADDR]) {
+		macaddr = nla_data(nl_attrs[VDPA_ATTR_DEV_NET_CFG_MACADDR]);
+		memcpy(config.net.mac, macaddr, sizeof(config.net.mac));
+		config.net_mask.mac_valid = true;
+	}
+	if (nl_attrs[VDPA_ATTR_DEV_NET_CFG_MTU]) {
+		config.net.mtu =
+			nla_get_u16(nl_attrs[VDPA_ATTR_DEV_NET_CFG_MTU]);
+		config.net_mask.mtu_valid = true;
+	}
+
+	mutex_lock(&vdev->cf_mutex);
+	err = vdev->config->set_ce_config(vdev, &config);
+	mutex_unlock(&vdev->cf_mutex);
+	return err;
+}
+
+static int vdpa_dev_config_set(struct vdpa_device *vdev, struct sk_buff *skb,
+			       struct genl_info *info)
+{
+	int err = -EOPNOTSUPP;
+	u32 device_id;
+
+	if (!vdev->mdev)
+		return -EOPNOTSUPP;
+
+	device_id = vdev->config->get_device_id(vdev);
+	switch (device_id) {
+	case VIRTIO_ID_NET:
+		err = vdpa_dev_net_config_set(vdev, skb, info);
+		break;
+	default:
+		break;
+	}
+	return err;
+}
+
+static int vdpa_nl_cmd_dev_config_set_doit(struct sk_buff *skb, struct genl_info *info)
+{
+	struct vdpa_device *vdev;
+	const char *devname;
+	struct device *dev;
+	int err;
+
+	if (!info->attrs[VDPA_ATTR_DEV_NAME])
+		return -EINVAL;
+	devname = nla_data(info->attrs[VDPA_ATTR_DEV_NAME]);
+
+	mutex_lock(&vdpa_dev_mutex);
+	dev = bus_find_device(&vdpa_bus, NULL, devname, vdpa_name_match);
+	if (!dev) {
+		mutex_unlock(&vdpa_dev_mutex);
+		NL_SET_ERR_MSG_MOD(info->extack, "device not found");
+		return -ENODEV;
+	}
+	vdev = container_of(dev, struct vdpa_device, dev);
+	if (!vdev->mdev) {
+		mutex_unlock(&vdpa_dev_mutex);
+		put_device(dev);
+		return -EINVAL;
+	}
+	err = vdpa_dev_config_set(vdev, skb, info);
+	put_device(dev);
+	mutex_unlock(&vdpa_dev_mutex);
+	return err;
+}
+
 static const struct nla_policy vdpa_nl_policy[VDPA_ATTR_MAX + 1] = {
 	[VDPA_ATTR_MGMTDEV_BUS_NAME] = { .type = NLA_NUL_STRING },
 	[VDPA_ATTR_MGMTDEV_DEV_NAME] = { .type = NLA_STRING },
 	[VDPA_ATTR_DEV_NAME] = { .type = NLA_STRING },
+	[VDPA_ATTR_DEV_NET_CFG_MACADDR] = NLA_POLICY_ETH_ADDR,
+	/* virtio spec 1.1 section 5.1.4.1 for valid MTU range */
+	[VDPA_ATTR_DEV_NET_CFG_MTU] = NLA_POLICY_MIN(NLA_U16, 68),
 };
 
 static const struct genl_ops vdpa_nl_ops[] = {
@@ -919,6 +1003,12 @@ static const struct genl_ops vdpa_nl_ops[] = {
 		.dumpit = vdpa_nl_cmd_dev_config_get_dumpit,
 		.flags = GENL_ADMIN_PERM,
 	},
+	{
+		.cmd = VDPA_CMD_DEV_CONFIG_SET,
+		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
+		.doit = vdpa_nl_cmd_dev_config_set_doit,
+		.flags = GENL_ADMIN_PERM,
+	},
 };
 
 static struct genl_family vdpa_nl_family __ro_after_init = {
diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
index dcbbecb5dea8..b59e1a214161 100644
--- a/include/linux/vdpa.h
+++ b/include/linux/vdpa.h
@@ -104,6 +104,14 @@ struct vdpa_dev_config {
 	struct vdpa_net_dev_config net;
 };
 
+struct vdpa_dev_set_config {
+	struct vdpa_net_dev_config net;
+	struct {
+		u8 mac_valid: 1;
+		u8 mtu_valid: 1;
+	} net_mask;
+};
+
 /**
  * struct vdpa_config_ops - operations for configuring a vDPA device.
  * Note: vDPA device drivers are required to implement all of the
@@ -204,6 +212,11 @@ struct vdpa_dev_config {
  *				cpu endianness.
  *				@vdev: vdpa device
  *				@config: pointer to config buffer used to read to
+ * @set_ce_config:		Set one or more device configuration in
+ *				cpu endianness.
+ *				@vdev: vdpa device
+ *				@config: configuration to update
+ *				Returns 0 on success or error code
  * @get_generation:		Get device config generation (optional)
  *				@vdev: vdpa device
  *				Returns u32: device generation
@@ -277,6 +290,8 @@ struct vdpa_config_ops {
 			   const void *buf, unsigned int len);
 	void (*get_ce_config)(struct vdpa_device *vdev,
 			      struct vdpa_dev_config *config);
+	int (*set_ce_config)(struct vdpa_device *vdev,
+			     const struct vdpa_dev_set_config *config);
 	u32 (*get_generation)(struct vdpa_device *vdev);
 	struct vdpa_iova_range (*get_iova_range)(struct vdpa_device *vdev);
 
diff --git a/include/uapi/linux/vdpa.h b/include/uapi/linux/vdpa.h
index 5c31ecc3b956..ec349789b8d1 100644
--- a/include/uapi/linux/vdpa.h
+++ b/include/uapi/linux/vdpa.h
@@ -18,6 +18,7 @@ enum vdpa_command {
 	VDPA_CMD_DEV_DEL,
 	VDPA_CMD_DEV_GET,		/* can dump */
 	VDPA_CMD_DEV_CONFIG_GET,	/* can dump */
+	VDPA_CMD_DEV_CONFIG_SET,
 };
 
 enum vdpa_attr {
-- 
2.26.2

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH linux-next v2 05/14] vdpa: Enable user to set mac and mtu of vdpa device

[Jason Wang]

在 2021/4/7 上午1:04, Parav Pandit 写道:
> $ vdpa dev add name bar mgmtdev vdpasim_net
>
> $ vdpa dev config set bar mac 00:11:22:33:44:55 mtu 9000
>
> $ vdpa dev config show
> bar: mac 00:11:22:33:44:55 link up link_announce false mtu 9000 speed 0 duplex 0
>
> $ vdpa dev config show -jp
> {
>      "config": {
>          "bar": {
>              "mac": "00:11:22:33:44:55",
>              "link ": "up",
>              "link_announce ": false,
>              "mtu": 9000,
>              "speed": 0,
>              "duplex": 0
>          }
>      }
> }
>
> Signed-off-by: Parav Pandit <parav@nvidia.com>
> Reviewed-by: Eli Cohen <elic@nvidia.com>
> ---
> changelog:
> v1->v2:
>   - fixed mtu range checking for max
>   - using NLA_POLICY_ETH_ADDR
>   - set config moved to device ops instead of mgmtdev ops
>   - merged build and set to single routine
>   - ensuring that user has NET_ADMIN capability for configuring network
>     attributes
> ---
>   drivers/vdpa/vdpa.c       | 90 +++++++++++++++++++++++++++++++++++++++
>   include/linux/vdpa.h      | 15 +++++++
>   include/uapi/linux/vdpa.h |  1 +
>   3 files changed, 106 insertions(+)
>
> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
> index 9da8deb8c0f2..6a6ef1a085e8 100644
> --- a/drivers/vdpa/vdpa.c
> +++ b/drivers/vdpa/vdpa.c
> @@ -881,10 +881,94 @@ vdpa_nl_cmd_dev_config_get_dumpit(struct sk_buff *msg, struct netlink_callback *
>   	return msg->len;
>   }
>   
> +static int vdpa_dev_net_config_set(struct vdpa_device *vdev,
> +				   struct sk_buff *skb, struct genl_info *info)
> +{
> +	struct nlattr **nl_attrs = info->attrs;
> +	struct vdpa_dev_set_config config = {};
> +	const u8 *macaddr;
> +	int err;
> +
> +	if (!vdev->config->set_ce_config)
> +		return -EOPNOTSUPP;
> +
> +	if (!netlink_capable(skb, CAP_NET_ADMIN))
> +		return -EPERM;
> +
> +	if (nl_attrs[VDPA_ATTR_DEV_NET_CFG_MACADDR]) {
> +		macaddr = nla_data(nl_attrs[VDPA_ATTR_DEV_NET_CFG_MACADDR]);
> +		memcpy(config.net.mac, macaddr, sizeof(config.net.mac));
> +		config.net_mask.mac_valid = true;
> +	}
> +	if (nl_attrs[VDPA_ATTR_DEV_NET_CFG_MTU]) {
> +		config.net.mtu =
> +			nla_get_u16(nl_attrs[VDPA_ATTR_DEV_NET_CFG_MTU]);
> +		config.net_mask.mtu_valid = true;
> +	}


I think it's better to aovid any device specific stuffs in vdpa.c. Can 
we just make the config data opaque and simply pass them to the driver?


> +
> +	mutex_lock(&vdev->cf_mutex);
> +	err = vdev->config->set_ce_config(vdev, &config);
> +	mutex_unlock(&vdev->cf_mutex);
> +	return err;
> +}
> +
> +static int vdpa_dev_config_set(struct vdpa_device *vdev, struct sk_buff *skb,
> +			       struct genl_info *info)
> +{
> +	int err = -EOPNOTSUPP;
> +	u32 device_id;
> +
> +	if (!vdev->mdev)
> +		return -EOPNOTSUPP;
> +
> +	device_id = vdev->config->get_device_id(vdev);
> +	switch (device_id) {
> +	case VIRTIO_ID_NET:
> +		err = vdpa_dev_net_config_set(vdev, skb, info);
> +		break;
> +	default:
> +		break;
> +	}
> +	return err;
> +}
> +
> +static int vdpa_nl_cmd_dev_config_set_doit(struct sk_buff *skb, struct genl_info *info)
> +{
> +	struct vdpa_device *vdev;
> +	const char *devname;
> +	struct device *dev;
> +	int err;
> +
> +	if (!info->attrs[VDPA_ATTR_DEV_NAME])
> +		return -EINVAL;
> +	devname = nla_data(info->attrs[VDPA_ATTR_DEV_NAME]);
> +
> +	mutex_lock(&vdpa_dev_mutex);
> +	dev = bus_find_device(&vdpa_bus, NULL, devname, vdpa_name_match);
> +	if (!dev) {
> +		mutex_unlock(&vdpa_dev_mutex);
> +		NL_SET_ERR_MSG_MOD(info->extack, "device not found");
> +		return -ENODEV;
> +	}
> +	vdev = container_of(dev, struct vdpa_device, dev);
> +	if (!vdev->mdev) {
> +		mutex_unlock(&vdpa_dev_mutex);
> +		put_device(dev);
> +		return -EINVAL;
> +	}
> +	err = vdpa_dev_config_set(vdev, skb, info);
> +	put_device(dev);
> +	mutex_unlock(&vdpa_dev_mutex);
> +	return err;
> +}
> +
>   static const struct nla_policy vdpa_nl_policy[VDPA_ATTR_MAX + 1] = {
>   	[VDPA_ATTR_MGMTDEV_BUS_NAME] = { .type = NLA_NUL_STRING },
>   	[VDPA_ATTR_MGMTDEV_DEV_NAME] = { .type = NLA_STRING },
>   	[VDPA_ATTR_DEV_NAME] = { .type = NLA_STRING },
> +	[VDPA_ATTR_DEV_NET_CFG_MACADDR] = NLA_POLICY_ETH_ADDR,
> +	/* virtio spec 1.1 section 5.1.4.1 for valid MTU range */
> +	[VDPA_ATTR_DEV_NET_CFG_MTU] = NLA_POLICY_MIN(NLA_U16, 68),
>   };
>   
>   static const struct genl_ops vdpa_nl_ops[] = {
> @@ -919,6 +1003,12 @@ static const struct genl_ops vdpa_nl_ops[] = {
>   		.dumpit = vdpa_nl_cmd_dev_config_get_dumpit,
>   		.flags = GENL_ADMIN_PERM,
>   	},
> +	{
> +		.cmd = VDPA_CMD_DEV_CONFIG_SET,
> +		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
> +		.doit = vdpa_nl_cmd_dev_config_set_doit,
> +		.flags = GENL_ADMIN_PERM,
> +	},
>   };
>   
>   static struct genl_family vdpa_nl_family __ro_after_init = {
> diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
> index dcbbecb5dea8..b59e1a214161 100644
> --- a/include/linux/vdpa.h
> +++ b/include/linux/vdpa.h
> @@ -104,6 +104,14 @@ struct vdpa_dev_config {
>   	struct vdpa_net_dev_config net;
>   };
>   
> +struct vdpa_dev_set_config {
> +	struct vdpa_net_dev_config net;
> +	struct {
> +		u8 mac_valid: 1;
> +		u8 mtu_valid: 1;
> +	} net_mask;
> +};
> +
>   /**
>    * struct vdpa_config_ops - operations for configuring a vDPA device.
>    * Note: vDPA device drivers are required to implement all of the
> @@ -204,6 +212,11 @@ struct vdpa_dev_config {
>    *				cpu endianness.
>    *				@vdev: vdpa device
>    *				@config: pointer to config buffer used to read to
> + * @set_ce_config:		Set one or more device configuration in
> + *				cpu endianness.
> + *				@vdev: vdpa device
> + *				@config: configuration to update
> + *				Returns 0 on success or error code


Can we simply use set_config here?

Thanks


>    * @get_generation:		Get device config generation (optional)
>    *				@vdev: vdpa device
>    *				Returns u32: device generation
> @@ -277,6 +290,8 @@ struct vdpa_config_ops {
>   			   const void *buf, unsigned int len);
>   	void (*get_ce_config)(struct vdpa_device *vdev,
>   			      struct vdpa_dev_config *config);
> +	int (*set_ce_config)(struct vdpa_device *vdev,
> +			     const struct vdpa_dev_set_config *config);
>   	u32 (*get_generation)(struct vdpa_device *vdev);
>   	struct vdpa_iova_range (*get_iova_range)(struct vdpa_device *vdev);
>   
> diff --git a/include/uapi/linux/vdpa.h b/include/uapi/linux/vdpa.h
> index 5c31ecc3b956..ec349789b8d1 100644
> --- a/include/uapi/linux/vdpa.h
> +++ b/include/uapi/linux/vdpa.h
> @@ -18,6 +18,7 @@ enum vdpa_command {
>   	VDPA_CMD_DEV_DEL,
>   	VDPA_CMD_DEV_GET,		/* can dump */
>   	VDPA_CMD_DEV_CONFIG_GET,	/* can dump */
> +	VDPA_CMD_DEV_CONFIG_SET,
>   };
>   
>   enum vdpa_attr {

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH linux-next v2 06/14] vdpa_sim_net: Enable user to set mac address and mtu

[Parav Pandit]

Enable user to set the mac address and mtu so that each vdpa device
can have its own user specified mac address and mtu.
This is done by implementing the management device's configuration
layout fields setting callback routine.

Now that user is enabled to set the mac address, remove the module
parameter for same.

And example of setting mac addr and mtu:
$ vdpa mgmtdev show

$ vdpa dev add name bar mgmtdev vdpasim_net
$ vdpa dev config set bar mac 00:11:22:33:44:55 mtu 9000

View the config after setting:
$ vdpa dev config show
bar: mac 00:11:22:33:44:55 link up link_announce false mtu 9000 speed 0 duplex 0

Signed-off-by: Parav Pandit <parav@nvidia.com>
Reviewed-by: Eli Cohen <elic@nvidia.com>
---
changelog:
v1->v2:
 - using updated interface and callbacks for get/set config
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c     | 26 +++++++++++++++++++
 drivers/vdpa/vdpa_sim/vdpa_sim.h     |  4 +++
 drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 39 +++++++++++++++++-----------
 3 files changed, 54 insertions(+), 15 deletions(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index 5b6b2f87d40c..5d59e06ab224 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -467,6 +467,28 @@ static void vdpasim_set_config(struct vdpa_device *vdpa, unsigned int offset,
 		vdpasim->dev_attr.set_config(vdpasim, vdpasim->config);
 }
 
+static void vdpasim_get_ce_config(struct vdpa_device *vdpa,
+				  struct vdpa_dev_config *config)
+{
+	struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
+
+	if (!vdpasim->dev_attr.get_ce_config)
+		return;
+
+	vdpasim->dev_attr.get_ce_config(vdpasim, config);
+}
+
+static int vdpasim_set_ce_config(struct vdpa_device *vdpa,
+				 const struct vdpa_dev_set_config *config)
+{
+	struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
+
+	if (!vdpasim->dev_attr.set_ce_config)
+		return -EOPNOTSUPP;
+
+	return vdpasim->dev_attr.set_ce_config(vdpasim, config);
+}
+
 static u32 vdpasim_get_generation(struct vdpa_device *vdpa)
 {
 	struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
@@ -568,6 +590,8 @@ static const struct vdpa_config_ops vdpasim_config_ops = {
 	.set_status             = vdpasim_set_status,
 	.get_config             = vdpasim_get_config,
 	.set_config             = vdpasim_set_config,
+	.get_ce_config		= vdpasim_get_ce_config,
+	.set_ce_config		= vdpasim_set_ce_config,
 	.get_generation         = vdpasim_get_generation,
 	.get_iova_range         = vdpasim_get_iova_range,
 	.dma_map                = vdpasim_dma_map,
@@ -595,6 +619,8 @@ static const struct vdpa_config_ops vdpasim_batch_config_ops = {
 	.set_status             = vdpasim_set_status,
 	.get_config             = vdpasim_get_config,
 	.set_config             = vdpasim_set_config,
+	.get_ce_config		= vdpasim_get_ce_config,
+	.set_ce_config		= vdpasim_set_ce_config,
 	.get_generation         = vdpasim_get_generation,
 	.get_iova_range         = vdpasim_get_iova_range,
 	.set_map                = vdpasim_set_map,
diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.h b/drivers/vdpa/vdpa_sim/vdpa_sim.h
index 6d75444f9948..62a21a4567cf 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.h
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.h
@@ -44,6 +44,10 @@ struct vdpasim_dev_attr {
 	work_func_t work_fn;
 	void (*get_config)(struct vdpasim *vdpasim, void *config);
 	void (*set_config)(struct vdpasim *vdpasim, const void *config);
+	void (*get_ce_config)(struct vdpasim *vdpasim,
+			      struct vdpa_dev_config *config);
+	int (*set_ce_config)(struct vdpasim *vdpasim,
+			     const struct vdpa_dev_set_config *config);
 };
 
 /* State of each vdpasim device */
diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
index a1ab6163f7d1..bdc70e9bf3a2 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
@@ -29,12 +29,6 @@
 
 #define VDPASIM_NET_VQ_NUM	2
 
-static char *macaddr;
-module_param(macaddr, charp, 0);
-MODULE_PARM_DESC(macaddr, "Ethernet MAC address");
-
-static u8 macaddr_buf[ETH_ALEN];
-
 static void vdpasim_net_work(struct work_struct *work)
 {
 	struct vdpasim *vdpasim = container_of(work, struct vdpasim, work);
@@ -114,7 +108,28 @@ static void vdpasim_net_get_config(struct vdpasim *vdpasim, void *config)
 
 	net_config->mtu = cpu_to_vdpasim16(vdpasim, 1500);
 	net_config->status = cpu_to_vdpasim16(vdpasim, VIRTIO_NET_S_LINK_UP);
-	memcpy(net_config->mac, macaddr_buf, ETH_ALEN);
+}
+
+static void vdpasim_net_get_ce_config(struct vdpasim *vdpasim,
+				      struct vdpa_dev_config *config)
+{
+	struct virtio_net_config *vio_config = vdpasim->config;
+
+	memcpy(config->net.mac, vio_config->mac, ETH_ALEN);
+	config->net.mtu = vdpasim16_to_cpu(vdpasim, vio_config->mtu);
+	config->net.status = VIRTIO_NET_S_LINK_UP;
+}
+
+static int vdpasim_net_set_ce_config(struct vdpasim *vdpasim,
+				     const struct vdpa_dev_set_config *config)
+{
+	struct virtio_net_config *vio_config = vdpasim->config;
+
+	if (config->net_mask.mac_valid)
+		memcpy(vio_config->mac, config->net.mac, ETH_ALEN);
+	if (config->net_mask.mtu_valid)
+		vio_config->mtu = cpu_to_vdpasim16(vdpasim, config->net.mtu);
+	return 0;
 }
 
 static void vdpasim_net_mgmtdev_release(struct device *dev)
@@ -139,6 +154,8 @@ static int vdpasim_net_dev_add(struct vdpa_mgmt_dev *mdev, const char *name)
 	dev_attr.nvqs = VDPASIM_NET_VQ_NUM;
 	dev_attr.config_size = sizeof(struct virtio_net_config);
 	dev_attr.get_config = vdpasim_net_get_config;
+	dev_attr.get_ce_config = vdpasim_net_get_ce_config;
+	dev_attr.set_ce_config = vdpasim_net_set_ce_config;
 	dev_attr.work_fn = vdpasim_net_work;
 	dev_attr.buffer_size = PAGE_SIZE;
 
@@ -185,14 +202,6 @@ static int __init vdpasim_net_init(void)
 {
 	int ret;
 
-	if (macaddr) {
-		mac_pton(macaddr, macaddr_buf);
-		if (!is_valid_ether_addr(macaddr_buf))
-			return -EADDRNOTAVAIL;
-	} else {
-		eth_random_addr(macaddr_buf);
-	}
-
 	ret = device_register(&vdpasim_net_mgmtdev);
 	if (ret)
 		return ret;
-- 
2.26.2

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH linux-next v2 07/14] vdpa/mlx5: Use the correct dma device when registering memory

[Parav Pandit]

From: Eli Cohen <elic@nvidia.com>

In cases where the vdpa instance uses a SF (sub function), the DMA
device is the parent device. Use a function to retrieve the correct DMA
device.

Signed-off-by: Eli Cohen <elic@nvidia.com>
Reviewed-by: Parav Pandit <parav@nvidia.com>
---
changelog:
v1->v2:
 - new patch
---
 drivers/vdpa/mlx5/core/mr.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/vdpa/mlx5/core/mr.c b/drivers/vdpa/mlx5/core/mr.c
index d300f799efcd..3908ff28eec0 100644
--- a/drivers/vdpa/mlx5/core/mr.c
+++ b/drivers/vdpa/mlx5/core/mr.c
@@ -219,6 +219,11 @@ static void destroy_indirect_key(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_m
 	mlx5_vdpa_destroy_mkey(mvdev, &mkey->mkey);
 }
 
+static struct device *get_dma_device(struct mlx5_vdpa_dev *mvdev)
+{
+	return &mvdev->mdev->pdev->dev;
+}
+
 static int map_direct_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_direct_mr *mr,
 			 struct vhost_iotlb *iotlb)
 {
@@ -234,7 +239,7 @@ static int map_direct_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_direct_mr
 	u64 pa;
 	u64 paend;
 	struct scatterlist *sg;
-	struct device *dma = mvdev->mdev->device;
+	struct device *dma = get_dma_device(mvdev);
 
 	for (map = vhost_iotlb_itree_first(iotlb, mr->start, mr->end - 1);
 	     map; map = vhost_iotlb_itree_next(map, start, mr->end - 1)) {
@@ -291,7 +296,7 @@ static int map_direct_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_direct_mr
 
 static void unmap_direct_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_direct_mr *mr)
 {
-	struct device *dma = mvdev->mdev->device;
+	struct device *dma = get_dma_device(mvdev);
 
 	destroy_direct_mr(mvdev, mr);
 	dma_unmap_sg_attrs(dma, mr->sg_head.sgl, mr->nsg, DMA_BIDIRECTIONAL, 0);
-- 
2.26.2

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH linux-next v2 07/14] vdpa/mlx5: Use the correct dma device when registering memory

[Jason Wang]

在 2021/4/7 上午1:04, Parav Pandit 写道:
> From: Eli Cohen <elic@nvidia.com>
>
> In cases where the vdpa instance uses a SF (sub function), the DMA
> device is the parent device. Use a function to retrieve the correct DMA
> device.
>
> Signed-off-by: Eli Cohen <elic@nvidia.com>
> Reviewed-by: Parav Pandit <parav@nvidia.com>


Acked-by: Jason Wang <jasowang@redhat.com>


> ---
> changelog:
> v1->v2:
>   - new patch
> ---
>   drivers/vdpa/mlx5/core/mr.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/vdpa/mlx5/core/mr.c b/drivers/vdpa/mlx5/core/mr.c
> index d300f799efcd..3908ff28eec0 100644
> --- a/drivers/vdpa/mlx5/core/mr.c
> +++ b/drivers/vdpa/mlx5/core/mr.c
> @@ -219,6 +219,11 @@ static void destroy_indirect_key(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_m
>   	mlx5_vdpa_destroy_mkey(mvdev, &mkey->mkey);
>   }
>   
> +static struct device *get_dma_device(struct mlx5_vdpa_dev *mvdev)
> +{
> +	return &mvdev->mdev->pdev->dev;
> +}
> +
>   static int map_direct_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_direct_mr *mr,
>   			 struct vhost_iotlb *iotlb)
>   {
> @@ -234,7 +239,7 @@ static int map_direct_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_direct_mr
>   	u64 pa;
>   	u64 paend;
>   	struct scatterlist *sg;
> -	struct device *dma = mvdev->mdev->device;
> +	struct device *dma = get_dma_device(mvdev);
>   
>   	for (map = vhost_iotlb_itree_first(iotlb, mr->start, mr->end - 1);
>   	     map; map = vhost_iotlb_itree_next(map, start, mr->end - 1)) {
> @@ -291,7 +296,7 @@ static int map_direct_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_direct_mr
>   
>   static void unmap_direct_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_direct_mr *mr)
>   {
> -	struct device *dma = mvdev->mdev->device;
> +	struct device *dma = get_dma_device(mvdev);
>   
>   	destroy_direct_mr(mvdev, mr);
>   	dma_unmap_sg_attrs(dma, mr->sg_head.sgl, mr->nsg, DMA_BIDIRECTIONAL, 0);

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH linux-next v2 08/14] vdpa/mlx5: Retrieve BAR address suitable any function

[Parav Pandit]

From: Eli Cohen <elic@nvidia.com>

struct mlx5_core_dev has a bar_addr field that contains the correct bar
address for the function regardless of whether it is pci function or sub
function. Use it.

Signed-off-by: Eli Cohen <elic@nvidia.com>
Reviewed-by: Parav Pandit <parav@nvidia.com>
---
changelog:
v1->v2:
 - new patch
---
 drivers/vdpa/mlx5/core/resources.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/vdpa/mlx5/core/resources.c b/drivers/vdpa/mlx5/core/resources.c
index 96e6421c5d1c..6521cbd0f5c2 100644
--- a/drivers/vdpa/mlx5/core/resources.c
+++ b/drivers/vdpa/mlx5/core/resources.c
@@ -246,7 +246,8 @@ int mlx5_vdpa_alloc_resources(struct mlx5_vdpa_dev *mvdev)
 	if (err)
 		goto err_key;
 
-	kick_addr = pci_resource_start(mdev->pdev, 0) + offset;
+	kick_addr = mdev->bar_addr + offset;
+
 	res->kick_addr = ioremap(kick_addr, PAGE_SIZE);
 	if (!res->kick_addr) {
 		err = -ENOMEM;
-- 
2.26.2

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH linux-next v2 08/14] vdpa/mlx5: Retrieve BAR address suitable any function

[Jason Wang]

在 2021/4/7 上午1:04, Parav Pandit 写道:
> From: Eli Cohen <elic@nvidia.com>
>
> struct mlx5_core_dev has a bar_addr field that contains the correct bar
> address for the function regardless of whether it is pci function or sub
> function. Use it.
>
> Signed-off-by: Eli Cohen <elic@nvidia.com>
> Reviewed-by: Parav Pandit <parav@nvidia.com>
> ---
> changelog:
> v1->v2:
>   - new patch
> ---
>   drivers/vdpa/mlx5/core/resources.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vdpa/mlx5/core/resources.c b/drivers/vdpa/mlx5/core/resources.c
> index 96e6421c5d1c..6521cbd0f5c2 100644
> --- a/drivers/vdpa/mlx5/core/resources.c
> +++ b/drivers/vdpa/mlx5/core/resources.c
> @@ -246,7 +246,8 @@ int mlx5_vdpa_alloc_resources(struct mlx5_vdpa_dev *mvdev)
>   	if (err)
>   		goto err_key;
>   
> -	kick_addr = pci_resource_start(mdev->pdev, 0) + offset;
> +	kick_addr = mdev->bar_addr + offset;
> +
>   	res->kick_addr = ioremap(kick_addr, PAGE_SIZE);
>   	if (!res->kick_addr) {
>   		err = -ENOMEM;


Acked-by: Jason Wang <jasowang@redhat.com>


_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH linux-next v2 09/14] vdpa/mlx5: Enable user to add/delete vdpa device

[Parav Pandit]

From: Eli Cohen <elic@nvidia.com>

Allow to control vdpa device creation and destruction using the vdpa
management tool.

Examples:
1. List the management devices
$ vdpa mgmtdev show
pci/0000:3b:00.1:
  supported_classes net

2. Create vdpa instance
$ vdpa dev add mgmtdev pci/0000:3b:00.1 name vdpa0

3. Show vdpa devices
$ vdpa dev show
vdpa0: type network mgmtdev pci/0000:3b:00.1 vendor_id 5555 max_vqs 16 \
max_vq_size 256

Signed-off-by: Eli Cohen <elic@nvidia.com>
Reviewed-by: Parav Pandit <parav@nvidia.com>
Acked-by: Jason Wang <jasowang@redhat.com>
---
 drivers/vdpa/mlx5/net/mlx5_vnet.c | 79 +++++++++++++++++++++++++++----
 1 file changed, 70 insertions(+), 9 deletions(-)

diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index 71397fdafa6a..aaf7f9394af0 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -1966,23 +1966,32 @@ static void init_mvqs(struct mlx5_vdpa_net *ndev)
 	}
 }
 
-static int mlx5v_probe(struct auxiliary_device *adev,
-		       const struct auxiliary_device_id *id)
+struct mlx5_vdpa_mgmtdev {
+	struct vdpa_mgmt_dev mgtdev;
+	struct mlx5_adev *madev;
+	struct mlx5_vdpa_net *ndev;
+};
+
+static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name)
 {
-	struct mlx5_adev *madev = container_of(adev, struct mlx5_adev, adev);
-	struct mlx5_core_dev *mdev = madev->mdev;
+	struct mlx5_vdpa_mgmtdev *mgtdev = container_of(v_mdev, struct mlx5_vdpa_mgmtdev, mgtdev);
 	struct virtio_net_config *config;
 	struct mlx5_vdpa_dev *mvdev;
 	struct mlx5_vdpa_net *ndev;
+	struct mlx5_core_dev *mdev;
 	u32 max_vqs;
 	int err;
 
+	if (mgtdev->ndev)
+		return -ENOSPC;
+
+	mdev = mgtdev->madev->mdev;
 	/* we save one virtqueue for control virtqueue should we require it */
 	max_vqs = MLX5_CAP_DEV_VDPA_EMULATION(mdev, max_num_virtio_queues);
 	max_vqs = min_t(u32, max_vqs, MLX5_MAX_SUPPORTED_VQS);
 
 	ndev = vdpa_alloc_device(struct mlx5_vdpa_net, mvdev.vdev, mdev->device, &mlx5_vdpa_ops,
-				 NULL);
+				 name);
 	if (IS_ERR(ndev))
 		return PTR_ERR(ndev);
 
@@ -2009,11 +2018,12 @@ static int mlx5v_probe(struct auxiliary_device *adev,
 	if (err)
 		goto err_res;
 
-	err = vdpa_register_device(&mvdev->vdev, 2 * mlx5_vdpa_max_qps(max_vqs));
+	mvdev->vdev.mdev = &mgtdev->mgtdev;
+	err = _vdpa_register_device(&mvdev->vdev, 2 * mlx5_vdpa_max_qps(max_vqs));
 	if (err)
 		goto err_reg;
 
-	dev_set_drvdata(&adev->dev, ndev);
+	mgtdev->ndev = ndev;
 	return 0;
 
 err_reg:
@@ -2026,11 +2036,62 @@ static int mlx5v_probe(struct auxiliary_device *adev,
 	return err;
 }
 
+static void mlx5_vdpa_dev_del(struct vdpa_mgmt_dev *v_mdev, struct vdpa_device *dev)
+{
+	struct mlx5_vdpa_mgmtdev *mgtdev = container_of(v_mdev, struct mlx5_vdpa_mgmtdev, mgtdev);
+
+	_vdpa_unregister_device(dev);
+	mgtdev->ndev = NULL;
+}
+
+static const struct vdpa_mgmtdev_ops mdev_ops = {
+	.dev_add = mlx5_vdpa_dev_add,
+	.dev_del = mlx5_vdpa_dev_del,
+};
+
+static struct virtio_device_id id_table[] = {
+	{ VIRTIO_ID_NET, VIRTIO_DEV_ANY_ID },
+	{ 0 },
+};
+
+static int mlx5v_probe(struct auxiliary_device *adev,
+		       const struct auxiliary_device_id *id)
+
+{
+	struct mlx5_adev *madev = container_of(adev, struct mlx5_adev, adev);
+	struct mlx5_core_dev *mdev = madev->mdev;
+	struct mlx5_vdpa_mgmtdev *mgtdev;
+	int err;
+
+	mgtdev = kzalloc(sizeof(*mgtdev), GFP_KERNEL);
+	if (!mgtdev)
+		return -ENOMEM;
+
+	mgtdev->mgtdev.ops = &mdev_ops;
+	mgtdev->mgtdev.device = mdev->device;
+	mgtdev->mgtdev.id_table = id_table;
+	mgtdev->madev = madev;
+
+	err = vdpa_mgmtdev_register(&mgtdev->mgtdev);
+	if (err)
+		goto reg_err;
+
+	dev_set_drvdata(&adev->dev, mgtdev);
+
+	return 0;
+
+reg_err:
+	kfree(mdev);
+	return err;
+}
+
 static void mlx5v_remove(struct auxiliary_device *adev)
 {
-	struct mlx5_vdpa_dev *mvdev = dev_get_drvdata(&adev->dev);
+	struct mlx5_vdpa_mgmtdev *mgtdev;
 
-	vdpa_unregister_device(&mvdev->vdev);
+	mgtdev = dev_get_drvdata(&adev->dev);
+	vdpa_mgmtdev_unregister(&mgtdev->mgtdev);
+	kfree(mgtdev);
 }
 
 static const struct auxiliary_device_id mlx5v_id_table[] = {
-- 
2.26.2

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH linux-next v2 10/14] vdpa/mlx5: Support configuration of MAC

[Parav Pandit]

From: Eli Cohen <elic@nvidia.com>

Add code to accept MAC configuration through vdpa tool. The MAC is
written into the config struct and later can be retrieved through
get_config().

Examples:
1. Configure MAC:
$ vdpa dev config set vdpa0 mac 00:11:22:33:44:55

2. Show configured params:
$ vdpa dev config show
vdpa0: mac 00:11:22:33:44:55 link down link_announce false mtu 0 speed 0 duplex 0

Signed-off-by: Eli Cohen <elic@nvidia.com>
Reviewed-by: Parav Pandit <parav@nvidia.com>
---
changelog:
v1->v2:
 - following new api for config get/set for mgmt tool
---
 drivers/vdpa/mlx5/net/mlx5_vnet.c | 45 ++++++++++++++++++++++++++++++-
 1 file changed, 44 insertions(+), 1 deletion(-)

diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index aaf7f9394af0..949084aac102 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -1493,7 +1493,8 @@ static u64 mlx5_vdpa_get_features(struct vdpa_device *vdev)
 	ndev->mvdev.mlx_features = mlx_to_vritio_features(dev_features);
 	if (MLX5_CAP_DEV_VDPA_EMULATION(mvdev->mdev, virtio_version_1_0))
 		ndev->mvdev.mlx_features |= BIT_ULL(VIRTIO_F_VERSION_1);
-	ndev->mvdev.mlx_features |= BIT_ULL(VIRTIO_F_ACCESS_PLATFORM);
+	ndev->mvdev.mlx_features |= BIT_ULL(VIRTIO_F_ACCESS_PLATFORM) |
+				     BIT_ULL(VIRTIO_NET_F_MAC);
 	print_features(mvdev, ndev->mvdev.mlx_features, false);
 	return ndev->mvdev.mlx_features;
 }
@@ -1562,6 +1563,11 @@ static __virtio16 cpu_to_mlx5vdpa16(struct mlx5_vdpa_dev *mvdev, u16 val)
 	return __cpu_to_virtio16(mlx5_vdpa_is_little_endian(mvdev), val);
 }
 
+static inline u16 mlx5vdpa16_to_cpu(struct mlx5_vdpa_dev *mvdev, __virtio16 val)
+{
+	return __virtio16_to_cpu(mlx5_vdpa_is_little_endian(mvdev), val);
+}
+
 static int mlx5_vdpa_set_features(struct vdpa_device *vdev, u64 features)
 {
 	struct mlx5_vdpa_dev *mvdev = to_mvdev(vdev);
@@ -1880,6 +1886,41 @@ static int mlx5_get_vq_irq(struct vdpa_device *vdv, u16 idx)
 	return -EOPNOTSUPP;
 }
 
+static int mlx5_vdpa_set_ce_config(struct vdpa_device *vdev,
+				   const struct vdpa_dev_set_config *config)
+{
+	struct mlx5_vdpa_dev *mvdev = to_mvdev(vdev);
+	struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev);
+	int err = 0;
+
+	mutex_lock(&ndev->reslock);
+	if (ndev->setup)
+		err = -EBUSY;
+	mutex_unlock(&ndev->reslock);
+
+	if (err)
+		return err;
+
+	if (config->net_mask.mtu_valid)
+		return -EOPNOTSUPP;
+
+	if (config->net_mask.mac_valid)
+		memcpy(ndev->config.mac, config->net.mac, ETH_ALEN);
+
+	return 0;
+}
+
+static void mlx5_vdpa_get_ce_config(struct vdpa_device *vdev,
+				    struct vdpa_dev_config *config)
+{
+	struct mlx5_vdpa_dev *mvdev = to_mvdev(vdev);
+	struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev);
+
+	memcpy(config->net.mac, ndev->config.mac, ETH_ALEN);
+	config->net.mtu = mlx5vdpa16_to_cpu(mvdev, ndev->config.mtu);
+	config->net.status = VIRTIO_NET_S_LINK_UP;
+}
+
 static const struct vdpa_config_ops mlx5_vdpa_ops = {
 	.set_vq_address = mlx5_vdpa_set_vq_address,
 	.set_vq_num = mlx5_vdpa_set_vq_num,
@@ -1902,6 +1943,8 @@ static const struct vdpa_config_ops mlx5_vdpa_ops = {
 	.set_status = mlx5_vdpa_set_status,
 	.get_config = mlx5_vdpa_get_config,
 	.set_config = mlx5_vdpa_set_config,
+	.get_ce_config = mlx5_vdpa_get_ce_config,
+	.set_ce_config = mlx5_vdpa_set_ce_config,
 	.get_generation = mlx5_vdpa_get_generation,
 	.set_map = mlx5_vdpa_set_map,
 	.free = mlx5_vdpa_free,
-- 
2.26.2

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH linux-next v2 10/14] vdpa/mlx5: Support configuration of MAC

[Jason Wang]

在 2021/4/7 上午1:04, Parav Pandit 写道:
> From: Eli Cohen <elic@nvidia.com>
>
> Add code to accept MAC configuration through vdpa tool. The MAC is
> written into the config struct and later can be retrieved through
> get_config().
>
> Examples:
> 1. Configure MAC:
> $ vdpa dev config set vdpa0 mac 00:11:22:33:44:55
>
> 2. Show configured params:
> $ vdpa dev config show
> vdpa0: mac 00:11:22:33:44:55 link down link_announce false mtu 0 speed 0 duplex 0
>
> Signed-off-by: Eli Cohen <elic@nvidia.com>
> Reviewed-by: Parav Pandit <parav@nvidia.com>
> ---
> changelog:
> v1->v2:
>   - following new api for config get/set for mgmt tool
> ---
>   drivers/vdpa/mlx5/net/mlx5_vnet.c | 45 ++++++++++++++++++++++++++++++-
>   1 file changed, 44 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> index aaf7f9394af0..949084aac102 100644
> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> @@ -1493,7 +1493,8 @@ static u64 mlx5_vdpa_get_features(struct vdpa_device *vdev)
>   	ndev->mvdev.mlx_features = mlx_to_vritio_features(dev_features);
>   	if (MLX5_CAP_DEV_VDPA_EMULATION(mvdev->mdev, virtio_version_1_0))
>   		ndev->mvdev.mlx_features |= BIT_ULL(VIRTIO_F_VERSION_1);
> -	ndev->mvdev.mlx_features |= BIT_ULL(VIRTIO_F_ACCESS_PLATFORM);
> +	ndev->mvdev.mlx_features |= BIT_ULL(VIRTIO_F_ACCESS_PLATFORM) |
> +				     BIT_ULL(VIRTIO_NET_F_MAC);
>   	print_features(mvdev, ndev->mvdev.mlx_features, false);
>   	return ndev->mvdev.mlx_features;
>   }
> @@ -1562,6 +1563,11 @@ static __virtio16 cpu_to_mlx5vdpa16(struct mlx5_vdpa_dev *mvdev, u16 val)
>   	return __cpu_to_virtio16(mlx5_vdpa_is_little_endian(mvdev), val);
>   }
>   
> +static inline u16 mlx5vdpa16_to_cpu(struct mlx5_vdpa_dev *mvdev, __virtio16 val)
> +{
> +	return __virtio16_to_cpu(mlx5_vdpa_is_little_endian(mvdev), val);
> +}
> +
>   static int mlx5_vdpa_set_features(struct vdpa_device *vdev, u64 features)
>   {
>   	struct mlx5_vdpa_dev *mvdev = to_mvdev(vdev);
> @@ -1880,6 +1886,41 @@ static int mlx5_get_vq_irq(struct vdpa_device *vdv, u16 idx)
>   	return -EOPNOTSUPP;
>   }
>   
> +static int mlx5_vdpa_set_ce_config(struct vdpa_device *vdev,
> +				   const struct vdpa_dev_set_config *config)
> +{
> +	struct mlx5_vdpa_dev *mvdev = to_mvdev(vdev);
> +	struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev);
> +	int err = 0;
> +
> +	mutex_lock(&ndev->reslock);
> +	if (ndev->setup)
> +		err = -EBUSY;
> +	mutex_unlock(&ndev->reslock);
> +
> +	if (err)
> +		return err;
> +
> +	if (config->net_mask.mtu_valid)
> +		return -EOPNOTSUPP;
> +
> +	if (config->net_mask.mac_valid)
> +		memcpy(ndev->config.mac, config->net.mac, ETH_ALEN);


So it looks to me this assumes that the mac can not be changed by guest 
because the filter is only re-programmed during setting status.

Any chance to trigger the filter programmming here to aovid future 
changes when VIRTIO_NET_F_CTRL_MAC is implemented?

Thanks


> +
> +	return 0;
> +}
> +
> +static void mlx5_vdpa_get_ce_config(struct vdpa_device *vdev,
> +				    struct vdpa_dev_config *config)
> +{
> +	struct mlx5_vdpa_dev *mvdev = to_mvdev(vdev);
> +	struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev);
> +
> +	memcpy(config->net.mac, ndev->config.mac, ETH_ALEN);
> +	config->net.mtu = mlx5vdpa16_to_cpu(mvdev, ndev->config.mtu);
> +	config->net.status = VIRTIO_NET_S_LINK_UP;
> +}
> +
>   static const struct vdpa_config_ops mlx5_vdpa_ops = {
>   	.set_vq_address = mlx5_vdpa_set_vq_address,
>   	.set_vq_num = mlx5_vdpa_set_vq_num,
> @@ -1902,6 +1943,8 @@ static const struct vdpa_config_ops mlx5_vdpa_ops = {
>   	.set_status = mlx5_vdpa_set_status,
>   	.get_config = mlx5_vdpa_get_config,
>   	.set_config = mlx5_vdpa_set_config,
> +	.get_ce_config = mlx5_vdpa_get_ce_config,
> +	.set_ce_config = mlx5_vdpa_set_ce_config,
>   	.get_generation = mlx5_vdpa_get_generation,
>   	.set_map = mlx5_vdpa_set_map,
>   	.free = mlx5_vdpa_free,

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH linux-next v2 11/14] vdpa/mlx5: Forward only packets with allowed MAC address

[Parav Pandit]

From: Eli Cohen <elic@nvidia.com>

Add rules to forward packets to the net device's TIR only if the
destination MAC is equal to the configured MAC. This is required to
prevent the netdevice from receiving traffic not destined to its
configured MAC.

Signed-off-by: Eli Cohen <elic@nvidia.com>
Reviewed-by: Parav Pandit <parav@nvidia.com>
---
 drivers/vdpa/mlx5/net/mlx5_vnet.c | 76 +++++++++++++++++++++++--------
 1 file changed, 58 insertions(+), 18 deletions(-)

diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index 949084aac102..c342cc9355e8 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -147,7 +147,8 @@ struct mlx5_vdpa_net {
 	struct mutex reslock;
 	struct mlx5_flow_table *rxft;
 	struct mlx5_fc *rx_counter;
-	struct mlx5_flow_handle *rx_rule;
+	struct mlx5_flow_handle *rx_rule_ucast;
+	struct mlx5_flow_handle *rx_rule_mcast;
 	bool setup;
 	u16 mtu;
 };
@@ -1294,21 +1295,33 @@ static int add_fwd_to_tir(struct mlx5_vdpa_net *ndev)
 	struct mlx5_flow_table_attr ft_attr = {};
 	struct mlx5_flow_act flow_act = {};
 	struct mlx5_flow_namespace *ns;
+	struct mlx5_flow_spec *spec;
+	void *headers_c;
+	void *headers_v;
+	u8 *dmac_c;
+	u8 *dmac_v;
 	int err;
 
-	/* for now, one entry, match all, forward to tir */
-	ft_attr.max_fte = 1;
-	ft_attr.autogroup.max_num_groups = 1;
+	spec = kvzalloc(sizeof(*spec), GFP_KERNEL);
+	if (!spec)
+		return -ENOMEM;
+
+	spec->match_criteria_enable = MLX5_MATCH_OUTER_HEADERS;
+	ft_attr.max_fte = 2;
+	ft_attr.autogroup.max_num_groups = 2;
 
 	ns = mlx5_get_flow_namespace(ndev->mvdev.mdev, MLX5_FLOW_NAMESPACE_BYPASS);
 	if (!ns) {
-		mlx5_vdpa_warn(&ndev->mvdev, "get flow namespace\n");
-		return -EOPNOTSUPP;
+		mlx5_vdpa_warn(&ndev->mvdev, "failed to get flow namespace\n");
+		err = -EOPNOTSUPP;
+		goto err_ns;
 	}
 
 	ndev->rxft = mlx5_create_auto_grouped_flow_table(ns, &ft_attr);
-	if (IS_ERR(ndev->rxft))
-		return PTR_ERR(ndev->rxft);
+	if (IS_ERR(ndev->rxft)) {
+		err = PTR_ERR(ndev->rxft);
+		goto err_ns;
+	}
 
 	ndev->rx_counter = mlx5_fc_create(ndev->mvdev.mdev, false);
 	if (IS_ERR(ndev->rx_counter)) {
@@ -1316,37 +1329,64 @@ static int add_fwd_to_tir(struct mlx5_vdpa_net *ndev)
 		goto err_fc;
 	}
 
+	headers_c = MLX5_ADDR_OF(fte_match_param, spec->match_criteria, outer_headers);
+	dmac_c = MLX5_ADDR_OF(fte_match_param, headers_c, outer_headers.dmac_47_16);
+	memset(dmac_c, 0xff, ETH_ALEN);
+	headers_v = MLX5_ADDR_OF(fte_match_param, spec->match_value, outer_headers);
+	dmac_v = MLX5_ADDR_OF(fte_match_param, headers_v, outer_headers.dmac_47_16);
+	ether_addr_copy(dmac_v, ndev->config.mac);
+
 	flow_act.action = MLX5_FLOW_CONTEXT_ACTION_FWD_DEST | MLX5_FLOW_CONTEXT_ACTION_COUNT;
 	dest[0].type = MLX5_FLOW_DESTINATION_TYPE_TIR;
 	dest[0].tir_num = ndev->res.tirn;
 	dest[1].type = MLX5_FLOW_DESTINATION_TYPE_COUNTER;
 	dest[1].counter_id = mlx5_fc_id(ndev->rx_counter);
-	ndev->rx_rule = mlx5_add_flow_rules(ndev->rxft, NULL, &flow_act, dest, 2);
-	if (IS_ERR(ndev->rx_rule)) {
-		err = PTR_ERR(ndev->rx_rule);
-		ndev->rx_rule = NULL;
-		goto err_rule;
+	ndev->rx_rule_ucast = mlx5_add_flow_rules(ndev->rxft, spec, &flow_act, dest, 2);
+
+	if (IS_ERR(ndev->rx_rule_ucast)) {
+		err = PTR_ERR(ndev->rx_rule_ucast);
+		ndev->rx_rule_ucast = NULL;
+		goto err_rule_ucast;
+	}
+
+	memset(dmac_c, 0, ETH_ALEN);
+	memset(dmac_v, 0, ETH_ALEN);
+	dmac_c[0] = 1;
+	dmac_v[0] = 1;
+	flow_act.action = MLX5_FLOW_CONTEXT_ACTION_FWD_DEST;
+	ndev->rx_rule_mcast = mlx5_add_flow_rules(ndev->rxft, spec, &flow_act, dest, 1);
+	if (IS_ERR(ndev->rx_rule_mcast)) {
+		err = PTR_ERR(ndev->rx_rule_mcast);
+		ndev->rx_rule_mcast = NULL;
+		goto err_rule_mcast;
 	}
 
+	kvfree(spec);
 	return 0;
 
-err_rule:
+err_rule_mcast:
+	mlx5_del_flow_rules(ndev->rx_rule_ucast);
+	ndev->rx_rule_ucast = NULL;
+err_rule_ucast:
 	mlx5_fc_destroy(ndev->mvdev.mdev, ndev->rx_counter);
 err_fc:
 	mlx5_destroy_flow_table(ndev->rxft);
+err_ns:
+	kvfree(spec);
 	return err;
 }
 
 static void remove_fwd_to_tir(struct mlx5_vdpa_net *ndev)
 {
-	if (!ndev->rx_rule)
+	if (!ndev->rx_rule_ucast)
 		return;
 
-	mlx5_del_flow_rules(ndev->rx_rule);
+	mlx5_del_flow_rules(ndev->rx_rule_mcast);
+	ndev->rx_rule_mcast = NULL;
+	mlx5_del_flow_rules(ndev->rx_rule_ucast);
+	ndev->rx_rule_ucast = NULL;
 	mlx5_fc_destroy(ndev->mvdev.mdev, ndev->rx_counter);
 	mlx5_destroy_flow_table(ndev->rxft);
-
-	ndev->rx_rule = NULL;
 }
 
 static void mlx5_vdpa_kick_vq(struct vdpa_device *vdev, u16 idx)
-- 
2.26.2

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH linux-next v2 12/14] vdpa/mlx5: should exclude header length and fcs from mtu

[Parav Pandit]

From: Si-Wei Liu <si-wei.liu@oracle.com>

When feature VIRTIO_NET_F_MTU is negotiated on mlx5_vdpa,
22 extra bytes worth of MTU length is shown in guest.
This is because the mlx5_query_port_max_mtu API returns
the "hardware" MTU value, which does not just contain the
 Ethernet payload, but includes extra lengths starting
from the Ethernet header up to the FCS altogether.

Fix the MTU so packets won't get dropped silently.

Fixes: 1a86b377aa21 ("vdpa/mlx5: Add VDPA driver for supported mlx5 devices")
Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
Acked-by: Jason Wang <jasowang@redhat.com>
Acked-by: Eli Cohen <elic@nvidia.com>
---
 drivers/vdpa/mlx5/core/mlx5_vdpa.h |  4 +++
 drivers/vdpa/mlx5/net/mlx5_vnet.c  | 42 +++++++++++++++++++++---------
 2 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/drivers/vdpa/mlx5/core/mlx5_vdpa.h b/drivers/vdpa/mlx5/core/mlx5_vdpa.h
index 08f742fd2409..b6cc53ba980c 100644
--- a/drivers/vdpa/mlx5/core/mlx5_vdpa.h
+++ b/drivers/vdpa/mlx5/core/mlx5_vdpa.h
@@ -4,9 +4,13 @@
 #ifndef __MLX5_VDPA_H__
 #define __MLX5_VDPA_H__
 
+#include <linux/etherdevice.h>
+#include <linux/if_vlan.h>
 #include <linux/vdpa.h>
 #include <linux/mlx5/driver.h>
 
+#define MLX5V_ETH_HARD_MTU (ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN)
+
 struct mlx5_vdpa_direct_mr {
 	u64 start;
 	u64 end;
diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index c342cc9355e8..56d463d2be85 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -1526,17 +1526,8 @@ static u64 mlx_to_vritio_features(u16 dev_features)
 static u64 mlx5_vdpa_get_features(struct vdpa_device *vdev)
 {
 	struct mlx5_vdpa_dev *mvdev = to_mvdev(vdev);
-	struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev);
-	u16 dev_features;
 
-	dev_features = MLX5_CAP_DEV_VDPA_EMULATION(mvdev->mdev, device_features_bits_mask);
-	ndev->mvdev.mlx_features = mlx_to_vritio_features(dev_features);
-	if (MLX5_CAP_DEV_VDPA_EMULATION(mvdev->mdev, virtio_version_1_0))
-		ndev->mvdev.mlx_features |= BIT_ULL(VIRTIO_F_VERSION_1);
-	ndev->mvdev.mlx_features |= BIT_ULL(VIRTIO_F_ACCESS_PLATFORM) |
-				     BIT_ULL(VIRTIO_NET_F_MAC);
-	print_features(mvdev, ndev->mvdev.mlx_features, false);
-	return ndev->mvdev.mlx_features;
+	return mvdev->mlx_features;
 }
 
 static int verify_min_features(struct mlx5_vdpa_dev *mvdev, u64 features)
@@ -1834,7 +1825,6 @@ static void mlx5_vdpa_set_status(struct vdpa_device *vdev, u8 status)
 		clear_virtqueues(ndev);
 		mlx5_vdpa_destroy_mr(&ndev->mvdev);
 		ndev->mvdev.status = 0;
-		ndev->mvdev.mlx_features = 0;
 		++mvdev->generation;
 		return;
 	}
@@ -1990,6 +1980,33 @@ static const struct vdpa_config_ops mlx5_vdpa_ops = {
 	.free = mlx5_vdpa_free,
 };
 
+static void query_virtio_features(struct mlx5_vdpa_net *ndev)
+{
+	struct mlx5_vdpa_dev *mvdev = &ndev->mvdev;
+	u16 dev_features;
+
+	dev_features = MLX5_CAP_DEV_VDPA_EMULATION(mvdev->mdev, device_features_bits_mask);
+	mvdev->mlx_features = mlx_to_vritio_features(dev_features);
+	if (MLX5_CAP_DEV_VDPA_EMULATION(mvdev->mdev, virtio_version_1_0))
+		mvdev->mlx_features |= BIT_ULL(VIRTIO_F_VERSION_1) |
+				       BIT_ULL(VIRTIO_NET_F_MAC);
+	mvdev->mlx_features |= BIT_ULL(VIRTIO_F_ACCESS_PLATFORM);
+	print_features(mvdev, mvdev->mlx_features, false);
+}
+
+static int query_mtu(struct mlx5_core_dev *mdev, u16 *mtu)
+{
+	u16 hw_mtu;
+	int err;
+
+	err = mlx5_query_nic_vport_mtu(mdev, &hw_mtu);
+	if (err)
+		return err;
+
+	*mtu = hw_mtu - MLX5V_ETH_HARD_MTU;
+	return 0;
+}
+
 static int alloc_resources(struct mlx5_vdpa_net *ndev)
 {
 	struct mlx5_vdpa_net_resources *res = &ndev->res;
@@ -2084,7 +2101,8 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name)
 	init_mvqs(ndev);
 	mutex_init(&ndev->reslock);
 	config = &ndev->config;
-	err = mlx5_query_nic_vport_mtu(mdev, &ndev->mtu);
+	query_virtio_features(ndev);
+	err = query_mtu(mdev, &ndev->mtu);
 	if (err)
 		goto err_mtu;
 
-- 
2.26.2

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH linux-next v2 13/14] vdpa/mlx5: Fix suspend/resume index restoration

[Parav Pandit]

From: Eli Cohen <elic@nvidia.com>

When we suspend the VM, the VDPA interface will be reset. When the VM is
resumed again, clear_virtqueues() will clear the available and used
indices resulting in hardware virqtqueue objects becoming out of sync.
We can avoid this function alltogether since qemu will clear them if
required, e.g. when the VM went through a reboot.

Moreover, since the hw available and used indices should always be
identical on query and should be restored to the same value same value
for virtqueues that complete in order, we set the single value provided
by set_vq_state(). In get_vq_state() we return the value of hardware
used index.

Fixes: b35ccebe3ef7 ("vdpa/mlx5: Restore the hardware used index after change map")
Fixes: 1a86b377aa21 ("vdpa/mlx5: Add VDPA driver for supported mlx5 devices")
Signed-off-by: Eli Cohen <elic@nvidia.com>
---
 drivers/vdpa/mlx5/net/mlx5_vnet.c | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index 56d463d2be85..a6e6d44b9ca5 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -1170,6 +1170,7 @@ static void suspend_vq(struct mlx5_vdpa_net *ndev, struct mlx5_vdpa_virtqueue *m
 		return;
 	}
 	mvq->avail_idx = attr.available_index;
+	mvq->used_idx = attr.used_index;
 }
 
 static void suspend_vqs(struct mlx5_vdpa_net *ndev)
@@ -1466,6 +1467,7 @@ static int mlx5_vdpa_set_vq_state(struct vdpa_device *vdev, u16 idx,
 		return -EINVAL;
 	}
 
+	mvq->used_idx = state->avail_index;
 	mvq->avail_idx = state->avail_index;
 	return 0;
 }
@@ -1483,7 +1485,7 @@ static int mlx5_vdpa_get_vq_state(struct vdpa_device *vdev, u16 idx, struct vdpa
 	 * that cares about emulating the index after vq is stopped.
 	 */
 	if (!mvq->initialized) {
-		state->avail_index = mvq->avail_idx;
+		state->avail_index = mvq->used_idx;
 		return 0;
 	}
 
@@ -1492,7 +1494,7 @@ static int mlx5_vdpa_get_vq_state(struct vdpa_device *vdev, u16 idx, struct vdpa
 		mlx5_vdpa_warn(mvdev, "failed to query virtqueue\n");
 		return err;
 	}
-	state->avail_index = attr.available_index;
+	state->avail_index = attr.used_index;
 	return 0;
 }
 
@@ -1572,16 +1574,6 @@ static void teardown_virtqueues(struct mlx5_vdpa_net *ndev)
 	}
 }
 
-static void clear_virtqueues(struct mlx5_vdpa_net *ndev)
-{
-	int i;
-
-	for (i = ndev->mvdev.max_vqs - 1; i >= 0; i--) {
-		ndev->vqs[i].avail_idx = 0;
-		ndev->vqs[i].used_idx = 0;
-	}
-}
-
 /* TODO: cross-endian support */
 static inline bool mlx5_vdpa_is_little_endian(struct mlx5_vdpa_dev *mvdev)
 {
@@ -1822,7 +1814,6 @@ static void mlx5_vdpa_set_status(struct vdpa_device *vdev, u8 status)
 	if (!status) {
 		mlx5_vdpa_info(mvdev, "performing device reset\n");
 		teardown_driver(ndev);
-		clear_virtqueues(ndev);
 		mlx5_vdpa_destroy_mr(&ndev->mvdev);
 		ndev->mvdev.status = 0;
 		++mvdev->generation;
-- 
2.26.2

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH linux-next v2 13/14] vdpa/mlx5: Fix suspend/resume index restoration

[Jason Wang]

在 2021/4/7 上午1:04, Parav Pandit 写道:
> From: Eli Cohen <elic@nvidia.com>
>
> When we suspend the VM, the VDPA interface will be reset. When the VM is
> resumed again, clear_virtqueues() will clear the available and used
> indices resulting in hardware virqtqueue objects becoming out of sync.
> We can avoid this function alltogether since qemu will clear them if
> required, e.g. when the VM went through a reboot.
>
> Moreover, since the hw available and used indices should always be
> identical on query and should be restored to the same value same value
> for virtqueues that complete in order, we set the single value provided
> by set_vq_state(). In get_vq_state() we return the value of hardware
> used index.
>
> Fixes: b35ccebe3ef7 ("vdpa/mlx5: Restore the hardware used index after change map")
> Fixes: 1a86b377aa21 ("vdpa/mlx5: Add VDPA driver for supported mlx5 devices")
> Signed-off-by: Eli Cohen <elic@nvidia.com>
> ---
>   drivers/vdpa/mlx5/net/mlx5_vnet.c | 17 ++++-------------
>   1 file changed, 4 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> index 56d463d2be85..a6e6d44b9ca5 100644
> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> @@ -1170,6 +1170,7 @@ static void suspend_vq(struct mlx5_vdpa_net *ndev, struct mlx5_vdpa_virtqueue *m
>   		return;
>   	}
>   	mvq->avail_idx = attr.available_index;
> +	mvq->used_idx = attr.used_index;
>   }
>   
>   static void suspend_vqs(struct mlx5_vdpa_net *ndev)
> @@ -1466,6 +1467,7 @@ static int mlx5_vdpa_set_vq_state(struct vdpa_device *vdev, u16 idx,
>   		return -EINVAL;
>   	}
>   
> +	mvq->used_idx = state->avail_index;
>   	mvq->avail_idx = state->avail_index;
>   	return 0;
>   }
> @@ -1483,7 +1485,7 @@ static int mlx5_vdpa_get_vq_state(struct vdpa_device *vdev, u16 idx, struct vdpa
>   	 * that cares about emulating the index after vq is stopped.
>   	 */
>   	if (!mvq->initialized) {
> -		state->avail_index = mvq->avail_idx;
> +		state->avail_index = mvq->used_idx;


Even if the hardware avail idx is always equal to used idx. I would 
still keep using the avail_idx, this makes it easier to be reviewed 
since it is consistent to e.g kernel vhost bakcend implementations. (The 
last_avail_idx in vhost_virtqueue).

Thanks


>   		return 0;
>   	}
>   
> @@ -1492,7 +1494,7 @@ static int mlx5_vdpa_get_vq_state(struct vdpa_device *vdev, u16 idx, struct vdpa
>   		mlx5_vdpa_warn(mvdev, "failed to query virtqueue\n");
>   		return err;
>   	}
> -	state->avail_index = attr.available_index;
> +	state->avail_index = attr.used_index;
>   	return 0;
>   }
>   
> @@ -1572,16 +1574,6 @@ static void teardown_virtqueues(struct mlx5_vdpa_net *ndev)
>   	}
>   }
>   
> -static void clear_virtqueues(struct mlx5_vdpa_net *ndev)
> -{
> -	int i;
> -
> -	for (i = ndev->mvdev.max_vqs - 1; i >= 0; i--) {
> -		ndev->vqs[i].avail_idx = 0;
> -		ndev->vqs[i].used_idx = 0;
> -	}
> -}
> -
>   /* TODO: cross-endian support */
>   static inline bool mlx5_vdpa_is_little_endian(struct mlx5_vdpa_dev *mvdev)
>   {
> @@ -1822,7 +1814,6 @@ static void mlx5_vdpa_set_status(struct vdpa_device *vdev, u8 status)
>   	if (!status) {
>   		mlx5_vdpa_info(mvdev, "performing device reset\n");
>   		teardown_driver(ndev);
> -		clear_virtqueues(ndev);
>   		mlx5_vdpa_destroy_mr(&ndev->mvdev);
>   		ndev->mvdev.status = 0;
>   		++mvdev->generation;

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH linux-next v2 13/14] vdpa/mlx5: Fix suspend/resume index restoration

[Michael S. Tsirkin]

On Wed, Apr 07, 2021 at 01:16:12PM +0300, Eli Cohen wrote:
> On Wed, Apr 07, 2021 at 03:25:00PM +0800, Jason Wang wrote:
> > 
> > 在 2021/4/7 上午1:04, Parav Pandit 写道:
> > > From: Eli Cohen <elic@nvidia.com>
> > > 
> > > When we suspend the VM, the VDPA interface will be reset. When the VM is
> > > resumed again, clear_virtqueues() will clear the available and used
> > > indices resulting in hardware virqtqueue objects becoming out of sync.
> > > We can avoid this function alltogether since qemu will clear them if
> > > required, e.g. when the VM went through a reboot.
> > > 
> > > Moreover, since the hw available and used indices should always be
> > > identical on query and should be restored to the same value same value
> > > for virtqueues that complete in order, we set the single value provided
> > > by set_vq_state(). In get_vq_state() we return the value of hardware
> > > used index.
> > > 
> > > Fixes: b35ccebe3ef7 ("vdpa/mlx5: Restore the hardware used index after change map")
> > > Fixes: 1a86b377aa21 ("vdpa/mlx5: Add VDPA driver for supported mlx5 devices")
> > > Signed-off-by: Eli Cohen <elic@nvidia.com>
> > > ---
> > >   drivers/vdpa/mlx5/net/mlx5_vnet.c | 17 ++++-------------
> > >   1 file changed, 4 insertions(+), 13 deletions(-)
> > > 
> > > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > index 56d463d2be85..a6e6d44b9ca5 100644
> > > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > @@ -1170,6 +1170,7 @@ static void suspend_vq(struct mlx5_vdpa_net *ndev, struct mlx5_vdpa_virtqueue *m
> > >   		return;
> > >   	}
> > >   	mvq->avail_idx = attr.available_index;
> > > +	mvq->used_idx = attr.used_index;
> > >   }
> > >   static void suspend_vqs(struct mlx5_vdpa_net *ndev)
> > > @@ -1466,6 +1467,7 @@ static int mlx5_vdpa_set_vq_state(struct vdpa_device *vdev, u16 idx,
> > >   		return -EINVAL;
> > >   	}
> > > +	mvq->used_idx = state->avail_index;
> > >   	mvq->avail_idx = state->avail_index;
> > >   	return 0;
> > >   }
> > > @@ -1483,7 +1485,7 @@ static int mlx5_vdpa_get_vq_state(struct vdpa_device *vdev, u16 idx, struct vdpa
> > >   	 * that cares about emulating the index after vq is stopped.
> > >   	 */
> > >   	if (!mvq->initialized) {
> > > -		state->avail_index = mvq->avail_idx;
> > > +		state->avail_index = mvq->used_idx;
> > 
> > 
> > Even if the hardware avail idx is always equal to used idx. I would still
> > keep using the avail_idx, this makes it easier to be reviewed since it is
> > consistent to e.g kernel vhost bakcend implementations. (The last_avail_idx
> > in vhost_virtqueue).
> > 
> 
> The problem is that there is a bug in the firmware such that for RX
> virtqueues firmware returns a wrong value in the avail_idx. The correct
> value is reported in used_idx. That's why we need to take the value from
> used_idx.

Maybe add a code comment here so people can figure it out?

> > Thanks
> > 
> > 
> > >   		return 0;
> > >   	}
> > > @@ -1492,7 +1494,7 @@ static int mlx5_vdpa_get_vq_state(struct vdpa_device *vdev, u16 idx, struct vdpa
> > >   		mlx5_vdpa_warn(mvdev, "failed to query virtqueue\n");
> > >   		return err;
> > >   	}
> > > -	state->avail_index = attr.available_index;
> > > +	state->avail_index = attr.used_index;
> > >   	return 0;
> > >   }
> > > @@ -1572,16 +1574,6 @@ static void teardown_virtqueues(struct mlx5_vdpa_net *ndev)
> > >   	}
> > >   }
> > > -static void clear_virtqueues(struct mlx5_vdpa_net *ndev)
> > > -{
> > > -	int i;
> > > -
> > > -	for (i = ndev->mvdev.max_vqs - 1; i >= 0; i--) {
> > > -		ndev->vqs[i].avail_idx = 0;
> > > -		ndev->vqs[i].used_idx = 0;
> > > -	}
> > > -}
> > > -
> > >   /* TODO: cross-endian support */
> > >   static inline bool mlx5_vdpa_is_little_endian(struct mlx5_vdpa_dev *mvdev)
> > >   {
> > > @@ -1822,7 +1814,6 @@ static void mlx5_vdpa_set_status(struct vdpa_device *vdev, u8 status)
> > >   	if (!status) {
> > >   		mlx5_vdpa_info(mvdev, "performing device reset\n");
> > >   		teardown_driver(ndev);
> > > -		clear_virtqueues(ndev);
> > >   		mlx5_vdpa_destroy_mr(&ndev->mvdev);
> > >   		ndev->mvdev.status = 0;
> > >   		++mvdev->generation;
> > 

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH linux-next v2 14/14] vdpa/mlx5: Fix wrong use of bit numbers

[Parav Pandit]

From: Eli Cohen <elic@nvidia.com>

VIRTIO_F_VERSION_1 is a bit number. Use BIT_ULL() with mask
conditionals.

Also, in mlx5_vdpa_is_little_endian() use BIT_ULL for consistency with
the rest of the code.

Fixes: 1a86b377aa21 ("vdpa/mlx5: Add VDPA driver for supported mlx5 devices")
Signed-off-by: Eli Cohen <elic@nvidia.com>
---
 drivers/vdpa/mlx5/net/mlx5_vnet.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index a6e6d44b9ca5..3b79b5939c7c 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -821,7 +821,7 @@ static int create_virtqueue(struct mlx5_vdpa_net *ndev, struct mlx5_vdpa_virtque
 	MLX5_SET(virtio_q, vq_ctx, event_qpn_or_msix, mvq->fwqp.mqp.qpn);
 	MLX5_SET(virtio_q, vq_ctx, queue_size, mvq->num_ent);
 	MLX5_SET(virtio_q, vq_ctx, virtio_version_1_0,
-		 !!(ndev->mvdev.actual_features & VIRTIO_F_VERSION_1));
+		 !!(ndev->mvdev.actual_features & BIT_ULL(VIRTIO_F_VERSION_1)));
 	MLX5_SET64(virtio_q, vq_ctx, desc_addr, mvq->desc_addr);
 	MLX5_SET64(virtio_q, vq_ctx, used_addr, mvq->device_addr);
 	MLX5_SET64(virtio_q, vq_ctx, available_addr, mvq->driver_addr);
@@ -1578,7 +1578,7 @@ static void teardown_virtqueues(struct mlx5_vdpa_net *ndev)
 static inline bool mlx5_vdpa_is_little_endian(struct mlx5_vdpa_dev *mvdev)
 {
 	return virtio_legacy_is_little_endian() ||
-		(mvdev->actual_features & (1ULL << VIRTIO_F_VERSION_1));
+		(mvdev->actual_features & BIT_ULL(VIRTIO_F_VERSION_1));
 }
 
 static __virtio16 cpu_to_mlx5vdpa16(struct mlx5_vdpa_dev *mvdev, u16 val)
@@ -1985,6 +1985,8 @@ static void query_virtio_features(struct mlx5_vdpa_net *ndev)
 	print_features(mvdev, mvdev->mlx_features, false);
 }
 
+#define MIN_MTU 68
+
 static int query_mtu(struct mlx5_core_dev *mdev, u16 *mtu)
 {
 	u16 hw_mtu;
@@ -1995,6 +1997,9 @@ static int query_mtu(struct mlx5_core_dev *mdev, u16 *mtu)
 		return err;
 
 	*mtu = hw_mtu - MLX5V_ETH_HARD_MTU;
+	if (*mtu < MIN_MTU)
+		return -EINVAL;
+
 	return 0;
 }
 
-- 
2.26.2

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH linux-next v2 14/14] vdpa/mlx5: Fix wrong use of bit numbers

[Jason Wang]

在 2021/4/7 上午1:04, Parav Pandit 写道:
> From: Eli Cohen <elic@nvidia.com>
>
> VIRTIO_F_VERSION_1 is a bit number. Use BIT_ULL() with mask
> conditionals.
>
> Also, in mlx5_vdpa_is_little_endian() use BIT_ULL for consistency with
> the rest of the code.
>
> Fixes: 1a86b377aa21 ("vdpa/mlx5: Add VDPA driver for supported mlx5 devices")
> Signed-off-by: Eli Cohen <elic@nvidia.com>
> ---
>   drivers/vdpa/mlx5/net/mlx5_vnet.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> index a6e6d44b9ca5..3b79b5939c7c 100644
> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> @@ -821,7 +821,7 @@ static int create_virtqueue(struct mlx5_vdpa_net *ndev, struct mlx5_vdpa_virtque
>   	MLX5_SET(virtio_q, vq_ctx, event_qpn_or_msix, mvq->fwqp.mqp.qpn);
>   	MLX5_SET(virtio_q, vq_ctx, queue_size, mvq->num_ent);
>   	MLX5_SET(virtio_q, vq_ctx, virtio_version_1_0,
> -		 !!(ndev->mvdev.actual_features & VIRTIO_F_VERSION_1));
> +		 !!(ndev->mvdev.actual_features & BIT_ULL(VIRTIO_F_VERSION_1)));
>   	MLX5_SET64(virtio_q, vq_ctx, desc_addr, mvq->desc_addr);
>   	MLX5_SET64(virtio_q, vq_ctx, used_addr, mvq->device_addr);
>   	MLX5_SET64(virtio_q, vq_ctx, available_addr, mvq->driver_addr);
> @@ -1578,7 +1578,7 @@ static void teardown_virtqueues(struct mlx5_vdpa_net *ndev)
>   static inline bool mlx5_vdpa_is_little_endian(struct mlx5_vdpa_dev *mvdev)
>   {
>   	return virtio_legacy_is_little_endian() ||
> -		(mvdev->actual_features & (1ULL << VIRTIO_F_VERSION_1));
> +		(mvdev->actual_features & BIT_ULL(VIRTIO_F_VERSION_1));
>   }
>   
>   static __virtio16 cpu_to_mlx5vdpa16(struct mlx5_vdpa_dev *mvdev, u16 val)
> @@ -1985,6 +1985,8 @@ static void query_virtio_features(struct mlx5_vdpa_net *ndev)
>   	print_features(mvdev, mvdev->mlx_features, false);
>   }
>   
> +#define MIN_MTU 68
> +
>   static int query_mtu(struct mlx5_core_dev *mdev, u16 *mtu)
>   {
>   	u16 hw_mtu;
> @@ -1995,6 +1997,9 @@ static int query_mtu(struct mlx5_core_dev *mdev, u16 *mtu)
>   		return err;
>   
>   	*mtu = hw_mtu - MLX5V_ETH_HARD_MTU;
> +	if (*mtu < MIN_MTU)
> +		return -EINVAL;
> +
>   	return 0;
>   }


This looks irrevalant to what is described by the commit log.

Thanks


>   

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

RE: [PATCH linux-next v2 14/14] vdpa/mlx5: Fix wrong use of bit numbers

[Parav Pandit]

> From: Jason Wang <jasowang@redhat.com>
> Sent: Wednesday, April 7, 2021 12:56 PM
> 
> 在 2021/4/7 上午1:04, Parav Pandit 写道:
> > From: Eli Cohen <elic@nvidia.com>
> >
> > VIRTIO_F_VERSION_1 is a bit number. Use BIT_ULL() with mask
> > conditionals.
> >
> > Also, in mlx5_vdpa_is_little_endian() use BIT_ULL for consistency with
> > the rest of the code.
> >
> > Fixes: 1a86b377aa21 ("vdpa/mlx5: Add VDPA driver for supported mlx5
> > devices")
> > Signed-off-by: Eli Cohen <elic@nvidia.com>
> > ---
> >   drivers/vdpa/mlx5/net/mlx5_vnet.c | 9 +++++++--
> >   1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > index a6e6d44b9ca5..3b79b5939c7c 100644
> > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > @@ -821,7 +821,7 @@ static int create_virtqueue(struct mlx5_vdpa_net
> *ndev, struct mlx5_vdpa_virtque
> >   	MLX5_SET(virtio_q, vq_ctx, event_qpn_or_msix, mvq-
> >fwqp.mqp.qpn);
> >   	MLX5_SET(virtio_q, vq_ctx, queue_size, mvq->num_ent);
> >   	MLX5_SET(virtio_q, vq_ctx, virtio_version_1_0,
> > -		 !!(ndev->mvdev.actual_features & VIRTIO_F_VERSION_1));
> > +		 !!(ndev->mvdev.actual_features &
> BIT_ULL(VIRTIO_F_VERSION_1)));
> >   	MLX5_SET64(virtio_q, vq_ctx, desc_addr, mvq->desc_addr);
> >   	MLX5_SET64(virtio_q, vq_ctx, used_addr, mvq->device_addr);
> >   	MLX5_SET64(virtio_q, vq_ctx, available_addr, mvq->driver_addr);
> @@
> > -1578,7 +1578,7 @@ static void teardown_virtqueues(struct
> mlx5_vdpa_net *ndev)
> >   static inline bool mlx5_vdpa_is_little_endian(struct mlx5_vdpa_dev
> *mvdev)
> >   {
> >   	return virtio_legacy_is_little_endian() ||
> > -		(mvdev->actual_features & (1ULL << VIRTIO_F_VERSION_1));
> > +		(mvdev->actual_features & BIT_ULL(VIRTIO_F_VERSION_1));
> >   }
> >
> >   static __virtio16 cpu_to_mlx5vdpa16(struct mlx5_vdpa_dev *mvdev, u16
> > val) @@ -1985,6 +1985,8 @@ static void query_virtio_features(struct
> mlx5_vdpa_net *ndev)
> >   	print_features(mvdev, mvdev->mlx_features, false);
> >   }
> >
> > +#define MIN_MTU 68
> > +
> >   static int query_mtu(struct mlx5_core_dev *mdev, u16 *mtu)
> >   {
> >   	u16 hw_mtu;
> > @@ -1995,6 +1997,9 @@ static int query_mtu(struct mlx5_core_dev
> *mdev, u16 *mtu)
> >   		return err;
> >
> >   	*mtu = hw_mtu - MLX5V_ETH_HARD_MTU;
> > +	if (*mtu < MIN_MTU)
> > +		return -EINVAL;
> > +
> >   	return 0;
> >   }
> 
> 
> This looks irrevalant to what is described by the commit log.
> 
Yes, this hunk came from a wrong patch of mtu.
I will fix it in v3.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH linux-next v2 00/14] vdpa: Enable user to set mac address, mtu

[Jason Wang]

在 2021/4/7 上午1:04, Parav Pandit 写道:
> Currently user cannot set the mac address and mtu of the vdpa device.
> This patchset enables users to set the mac address and mtu of the vdpa
> device once the device is created.
> If a vendor driver supports such configuration user can set it otherwise
> user gets unsupported error.
>
> vdpa mac address and mtu are device configuration layout fields.
> To keep interface generic enough for multiple types of vdpa devices, mac
> address and mtu setting is implemented as configuration layout config
> knobs.
> This enables to use similar config layout for other virtio devices.
>
> An example of query & set of config layout fields for vdpa_sim_net
> driver:
>
> Configuration layout fields are set after device is created.
> This enables user to change such fields at later point without destroying and
> recreating the device for new config.
>
> $ vdpa mgmtdev show
> vdpasim_net:
>    supported_classes net
>
> Add the device:
> $ vdpa dev add name bar mgmtdev vdpasim_net
>
> Configure mac address and mtu:
> $ vdpa dev config set bar mac 00:11:22:33:44:55 mtu 9000
>
> In above command only mac address or only mtu can also be set.
>
> View the config after setting:
> $ vdpa dev config show
> bar: mac 00:11:22:33:44:55 link up link_announce false mtu 9000 speed 0 duplex 0
>
> Patch summary:
> Patch-1 fix kdoc style comments
> Patch-2 fix kdoc style comments
> Patch-3 introduced and use helpers for get/set config and features
> Patch-4 implement query device config layout
> Patch-5 enanble user to set mac and mtu in config space
> Patch-6 vdpa_sim_net implements get and set of config layout
> Patch-7 mlx5 vdpa driver use right dma device for PCI PF,VF,SF
> Patch-8 mlx5 vdpa driver uses right BAR offset for SF
> Patch-9 mlx5 vdpa driver migrates to user created vdpa device
> Patch-10 mlx5 vdpa driver supports user provided mac config
> Patch-11 mlx5 vdpa driver uses user provided mac during rx flow steering
> Patch-12 mlx5 vdpa driver excludes header length and fcs
> Patch-13 mlx5 vdpa driver fixes index restoration during suspend resume
> Patch-14 mlx5 vdpa driver fixes bit usage


Hi Parav:

I think some of the codes should be sent independently: patch 7-8, 
patch13-14 are needed for -stable.

Thanks


>
> changelog:
> v1->v2:
>   - new patches to fix kdoc comment to add new kdoc section
>   - new patch to have synchronized access to features and config space
>   - read whole net config layout instead of individual fields
>   - added error extack for unmanaged vdpa device
>   - fixed several endianness issues
>   - introduced vdpa device ops for get config which is synchronized
>     with other get/set features ops and config ops
>   - fixed mtu range checking for max
>   - using NLA_POLICY_ETH_ADDR
>   - set config moved to device ops instead of mgmtdev ops
>   - merged build and set to single routine
>   - ensuring that user has NET_ADMIN capability for configuring network
>     attributes
>   - using updated interface and callbacks for get/set config
>   - following new api for config get/set for mgmt tool in mlx5 vdpa
>     driver
>   - fixes for accessing right SF dma device and bar address
>   - fix for mtu calculation
>   - fix for bit access in features
>   - fix for index restore with suspend/resume operation
>
> Eli Cohen (7):
>    vdpa/mlx5: Use the correct dma device when registering memory
>    vdpa/mlx5: Retrieve BAR address suitable any function
>    vdpa/mlx5: Enable user to add/delete vdpa device
>    vdpa/mlx5: Support configuration of MAC
>    vdpa/mlx5: Forward only packets with allowed MAC address
>    vdpa/mlx5: Fix suspend/resume index restoration
>    vdpa/mlx5: Fix wrong use of bit numbers
>
> Parav Pandit (6):
>    vdpa: Follow kdoc comment style
>    vdpa: Follow kdoc comment style
>    vdpa: Introduce and use vdpa device get,set config, features helpers
>    vdpa: Introduce query of device config layout
>    vdpa: Enable user to set mac and mtu of vdpa device
>    vdpa_sim_net: Enable user to set mac address and mtu
>
> Si-Wei Liu (1):
>    vdpa/mlx5: should exclude header length and fcs from mtu
>
>   drivers/vdpa/mlx5/core/mlx5_vdpa.h   |   4 +
>   drivers/vdpa/mlx5/core/mr.c          |   9 +-
>   drivers/vdpa/mlx5/core/resources.c   |   3 +-
>   drivers/vdpa/mlx5/net/mlx5_vnet.c    | 264 ++++++++++++++----
>   drivers/vdpa/vdpa.c                  | 383 ++++++++++++++++++++++++++-
>   drivers/vdpa/vdpa_sim/vdpa_sim.c     |  26 ++
>   drivers/vdpa/vdpa_sim/vdpa_sim.h     |   4 +
>   drivers/vdpa/vdpa_sim/vdpa_sim_net.c |  39 +--
>   drivers/vhost/vdpa.c                 |   6 +-
>   include/linux/vdpa.h                 | 123 ++++++---
>   include/uapi/linux/vdpa.h            |  12 +
>   11 files changed, 752 insertions(+), 121 deletions(-)
>

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

RE: [PATCH linux-next v2 00/14] vdpa: Enable user to set mac address, mtu

[Parav Pandit]

Hi  Jason,

> From: Jason Wang <jasowang@redhat.com>
> Sent: Wednesday, April 7, 2021 12:59 PM

> >
> > Patch summary:
> > Patch-1 fix kdoc style comments
> > Patch-2 fix kdoc style comments
> > Patch-3 introduced and use helpers for get/set config and features
> > Patch-4 implement query device config layout
> > Patch-5 enanble user to set mac and mtu in config space
> > Patch-6 vdpa_sim_net implements get and set of config layout
> > Patch-7 mlx5 vdpa driver use right dma device for PCI PF,VF,SF
> > Patch-8 mlx5 vdpa driver uses right BAR offset for SF
> > Patch-9 mlx5 vdpa driver migrates to user created vdpa device
> > Patch-10 mlx5 vdpa driver supports user provided mac config
> > Patch-11 mlx5 vdpa driver uses user provided mac during rx flow
> > steering
> > Patch-12 mlx5 vdpa driver excludes header length and fcs
> > Patch-13 mlx5 vdpa driver fixes index restoration during suspend
> > resume
> > Patch-14 mlx5 vdpa driver fixes bit usage
> 
> 
> Hi Parav:
> 
> I think some of the codes should be sent independently: patch 7-8,
> patch13-14 are needed for -stable.
>
Patch 7-8 are specific to subfunctions. However linux-next branch of tree [1] doesn't contain the SF patches present in [2].

[1] https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git/log/?h=linux-next
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/?h=v5.12-rc6

How to send them through vhost tree with fixes tag?

For 13-14, yes will send them as pre-patch to this series.
I think patch-12 also fall in same category.

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization