linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 00/19] vdpa: generalize vdpa simulator
@ 2020-12-03 17:04 Stefano Garzarella
  2020-12-03 17:04 ` [PATCH v3 01/19] vdpa: remove unnecessary 'default n' in Kconfig entries Stefano Garzarella
                   ` (18 more replies)
  0 siblings, 19 replies; 35+ messages in thread
From: Stefano Garzarella @ 2020-12-03 17:04 UTC (permalink / raw)
  To: virtualization
  Cc: Stefan Hajnoczi, Michael S. Tsirkin, Oren Duer, Jason Wang,
	Laurent Vivier, Stefano Garzarella, linux-kernel, Max Gurtovoy,
	Shahaf Shuler, Eli Cohen

This series moves the network device simulator in a new module
(vdpa_sim_net) and leaves the generic functions in the vdpa_sim core
module, allowing the possibility to add new vDPA device simulators.

For now I removed the vdpa-blk simulator patches, since I'm still working
on them and debugging the iotlb issues.

Thanks to Max that started this work! I took his patches and extended a bit.

v1: https://lists.linuxfoundation.org/pipermail/virtualization/2020-November/050677.html
v2: https://lists.linuxfoundation.org/pipermail/virtualization/2020-November/051036.html

v3:
 - avoided to remove some headers with structures and functions directly
   used [Jason]
 - defined VHOST_IOTLB_UNLIMITED macro in vhost_iotlb.h [Jason]
 - added set_config callback in vdpasim_dev_attr [Jason]
 - cleared notify during reset [Jason]

Max Gurtovoy (2):
  vdpa_sim: remove hard-coded virtq count
  vdpa: split vdpasim to core and net modules

Stefano Garzarella (17):
  vdpa: remove unnecessary 'default n' in Kconfig entries
  vdpa_sim: remove unnecessary headers inclusion
  vhost/iotlb: add VHOST_IOTLB_UNLIMITED macro
  vdpa_sim: remove the limit of IOTLB entries
  vdpa_sim: rename vdpasim_config_ops variables
  vdpa_sim: add struct vdpasim_dev_attr for device attributes
  vdpa_sim: add device id field in vdpasim_dev_attr
  vdpa_sim: add supported_features field in vdpasim_dev_attr
  vdpa_sim: add work_fn in vdpasim_dev_attr
  vdpa_sim: store parsed MAC address in a buffer
  vdpa_sim: make 'config' generic and usable for any device type
  vdpa_sim: add get_config callback in vdpasim_dev_attr
  vdpa_sim: add set_config callback in vdpasim_dev_attr
  vdpa_sim: set vringh notify callback
  vdpa_sim: use kvmalloc to allocate vdpasim->buffer
  vdpa_sim: make vdpasim->buffer size configurable
  vdpa_sim: split vdpasim_virtqueue's iov field in out_iov and in_iov

 drivers/vdpa/vdpa_sim/vdpa_sim.h     | 105 ++++++++++
 include/linux/vhost_iotlb.h          |   2 +
 drivers/vdpa/vdpa_sim/vdpa_sim.c     | 301 +++++++--------------------
 drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 171 +++++++++++++++
 drivers/vhost/iotlb.c                |   3 +-
 drivers/vdpa/Kconfig                 |  16 +-
 drivers/vdpa/vdpa_sim/Makefile       |   1 +
 7 files changed, 368 insertions(+), 231 deletions(-)
 create mode 100644 drivers/vdpa/vdpa_sim/vdpa_sim.h
 create mode 100644 drivers/vdpa/vdpa_sim/vdpa_sim_net.c

-- 
2.26.2


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH v3 01/19] vdpa: remove unnecessary 'default n' in Kconfig entries
  2020-12-03 17:04 [PATCH v3 00/19] vdpa: generalize vdpa simulator Stefano Garzarella
@ 2020-12-03 17:04 ` Stefano Garzarella
  2020-12-03 17:04 ` [PATCH v3 02/19] vdpa_sim: remove unnecessary headers inclusion Stefano Garzarella
                   ` (17 subsequent siblings)
  18 siblings, 0 replies; 35+ messages in thread
From: Stefano Garzarella @ 2020-12-03 17:04 UTC (permalink / raw)
  To: virtualization
  Cc: Stefan Hajnoczi, Michael S. Tsirkin, Oren Duer, Jason Wang,
	Laurent Vivier, Stefano Garzarella, linux-kernel, Max Gurtovoy,
	Shahaf Shuler, Eli Cohen

'default n' is not necessary since it is already the default when
nothing is specified.

Suggested-by: Jason Wang <jasowang@redhat.com>
Acked-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 drivers/vdpa/Kconfig | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/vdpa/Kconfig b/drivers/vdpa/Kconfig
index 6caf539091e5..2c892e890b9e 100644
--- a/drivers/vdpa/Kconfig
+++ b/drivers/vdpa/Kconfig
@@ -14,7 +14,6 @@ config VDPA_SIM
 	select DMA_OPS
 	select VHOST_RING
 	select GENERIC_NET_UTILS
-	default n
 	help
 	  vDPA networking device simulator which loop TX traffic back
 	  to RX. This device is used for testing, prototyping and
@@ -23,7 +22,6 @@ config VDPA_SIM
 config IFCVF
 	tristate "Intel IFC VF vDPA driver"
 	depends on PCI_MSI
-	default n
 	help
 	  This kernel module can drive Intel IFC VF NIC to offload
 	  virtio dataplane traffic to hardware.
@@ -42,7 +40,6 @@ config MLX5_VDPA_NET
 	tristate "vDPA driver for ConnectX devices"
 	select MLX5_VDPA
 	depends on MLX5_CORE
-	default n
 	help
 	  VDPA network driver for ConnectX6 and newer. Provides offloading
 	  of virtio net datapath such that descriptors put on the ring will
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 02/19] vdpa_sim: remove unnecessary headers inclusion
  2020-12-03 17:04 [PATCH v3 00/19] vdpa: generalize vdpa simulator Stefano Garzarella
  2020-12-03 17:04 ` [PATCH v3 01/19] vdpa: remove unnecessary 'default n' in Kconfig entries Stefano Garzarella
@ 2020-12-03 17:04 ` Stefano Garzarella
  2020-12-03 17:37   ` Randy Dunlap
  2020-12-03 17:04 ` [PATCH v3 03/19] vdpa_sim: remove hard-coded virtq count Stefano Garzarella
                   ` (16 subsequent siblings)
  18 siblings, 1 reply; 35+ messages in thread
From: Stefano Garzarella @ 2020-12-03 17:04 UTC (permalink / raw)
  To: virtualization
  Cc: Stefan Hajnoczi, Michael S. Tsirkin, Oren Duer, Jason Wang,
	Laurent Vivier, Stefano Garzarella, linux-kernel, Max Gurtovoy,
	Shahaf Shuler, Eli Cohen

Some headers are not necessary, so let's remove them to do
some cleaning.

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
v3:
- avoided to remove some headers with structures and functions directly
  used (device.h, slab.h, virtio_byteorder.h)[Jason]
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index 6a90fdb9cbfc..b08f28d20d8d 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -7,20 +7,10 @@
  *
  */
 
-#include <linux/init.h>
 #include <linux/module.h>
 #include <linux/device.h>
-#include <linux/kernel.h>
-#include <linux/fs.h>
-#include <linux/poll.h>
 #include <linux/slab.h>
-#include <linux/sched.h>
-#include <linux/wait.h>
-#include <linux/uuid.h>
-#include <linux/iommu.h>
 #include <linux/dma-map-ops.h>
-#include <linux/sysfs.h>
-#include <linux/file.h>
 #include <linux/etherdevice.h>
 #include <linux/vringh.h>
 #include <linux/vdpa.h>
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 03/19] vdpa_sim: remove hard-coded virtq count
  2020-12-03 17:04 [PATCH v3 00/19] vdpa: generalize vdpa simulator Stefano Garzarella
  2020-12-03 17:04 ` [PATCH v3 01/19] vdpa: remove unnecessary 'default n' in Kconfig entries Stefano Garzarella
  2020-12-03 17:04 ` [PATCH v3 02/19] vdpa_sim: remove unnecessary headers inclusion Stefano Garzarella
@ 2020-12-03 17:04 ` Stefano Garzarella
  2020-12-03 17:04 ` [PATCH v3 04/19] vhost/iotlb: add VHOST_IOTLB_UNLIMITED macro Stefano Garzarella
                   ` (15 subsequent siblings)
  18 siblings, 0 replies; 35+ messages in thread
From: Stefano Garzarella @ 2020-12-03 17:04 UTC (permalink / raw)
  To: virtualization
  Cc: Stefan Hajnoczi, Michael S. Tsirkin, Oren Duer, Jason Wang,
	Laurent Vivier, Stefano Garzarella, linux-kernel, Max Gurtovoy,
	Shahaf Shuler, Eli Cohen

From: Max Gurtovoy <mgurtovoy@nvidia.com>

Add a new attribute that will define the number of virt queues to be
created for the vdpasim device.

Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
[sgarzare: replace kmalloc_array() with kcalloc()]
Acked-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
v1:
- use kcalloc() instead of kmalloc_array() since some function expects
  variables initialized to zero
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index b08f28d20d8d..295a770caac0 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -60,7 +60,7 @@ static u64 vdpasim_features = (1ULL << VIRTIO_F_ANY_LAYOUT) |
 /* State of each vdpasim device */
 struct vdpasim {
 	struct vdpa_device vdpa;
-	struct vdpasim_virtqueue vqs[VDPASIM_VQ_NUM];
+	struct vdpasim_virtqueue *vqs;
 	struct work_struct work;
 	/* spinlock to synchronize virtqueue state */
 	spinlock_t lock;
@@ -70,6 +70,7 @@ struct vdpasim {
 	u32 status;
 	u32 generation;
 	u64 features;
+	int nvqs;
 	/* spinlock to synchronize iommu table */
 	spinlock_t iommu_lock;
 };
@@ -134,7 +135,7 @@ static void vdpasim_reset(struct vdpasim *vdpasim)
 {
 	int i;
 
-	for (i = 0; i < VDPASIM_VQ_NUM; i++)
+	for (i = 0; i < vdpasim->nvqs; i++)
 		vdpasim_vq_reset(&vdpasim->vqs[i]);
 
 	spin_lock(&vdpasim->iommu_lock);
@@ -340,7 +341,7 @@ static struct vdpasim *vdpasim_create(void)
 	const struct vdpa_config_ops *ops;
 	struct vdpasim *vdpasim;
 	struct device *dev;
-	int ret = -ENOMEM;
+	int i, ret = -ENOMEM;
 
 	if (batch_mapping)
 		ops = &vdpasim_net_batch_config_ops;
@@ -351,6 +352,7 @@ static struct vdpasim *vdpasim_create(void)
 	if (!vdpasim)
 		goto err_alloc;
 
+	vdpasim->nvqs = VDPASIM_VQ_NUM;
 	INIT_WORK(&vdpasim->work, vdpasim_work);
 	spin_lock_init(&vdpasim->lock);
 	spin_lock_init(&vdpasim->iommu_lock);
@@ -361,6 +363,11 @@ static struct vdpasim *vdpasim_create(void)
 		goto err_iommu;
 	set_dma_ops(dev, &vdpasim_dma_ops);
 
+	vdpasim->vqs = kcalloc(vdpasim->nvqs, sizeof(struct vdpasim_virtqueue),
+			       GFP_KERNEL);
+	if (!vdpasim->vqs)
+		goto err_iommu;
+
 	vdpasim->iommu = vhost_iotlb_alloc(2048, 0);
 	if (!vdpasim->iommu)
 		goto err_iommu;
@@ -379,8 +386,8 @@ static struct vdpasim *vdpasim_create(void)
 		eth_random_addr(vdpasim->config.mac);
 	}
 
-	vringh_set_iotlb(&vdpasim->vqs[0].vring, vdpasim->iommu);
-	vringh_set_iotlb(&vdpasim->vqs[1].vring, vdpasim->iommu);
+	for (i = 0; i < vdpasim->nvqs; i++)
+		vringh_set_iotlb(&vdpasim->vqs[i].vring, vdpasim->iommu);
 
 	vdpasim->vdpa.dma_dev = dev;
 	ret = vdpa_register_device(&vdpasim->vdpa);
@@ -649,6 +656,7 @@ static void vdpasim_free(struct vdpa_device *vdpa)
 	kfree(vdpasim->buffer);
 	if (vdpasim->iommu)
 		vhost_iotlb_free(vdpasim->iommu);
+	kfree(vdpasim->vqs);
 }
 
 static const struct vdpa_config_ops vdpasim_net_config_ops = {
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 04/19] vhost/iotlb: add VHOST_IOTLB_UNLIMITED macro
  2020-12-03 17:04 [PATCH v3 00/19] vdpa: generalize vdpa simulator Stefano Garzarella
                   ` (2 preceding siblings ...)
  2020-12-03 17:04 ` [PATCH v3 03/19] vdpa_sim: remove hard-coded virtq count Stefano Garzarella
@ 2020-12-03 17:04 ` Stefano Garzarella
  2020-12-07  3:55   ` Jason Wang
  2020-12-03 17:04 ` [PATCH v3 05/19] vdpa_sim: remove the limit of IOTLB entries Stefano Garzarella
                   ` (14 subsequent siblings)
  18 siblings, 1 reply; 35+ messages in thread
From: Stefano Garzarella @ 2020-12-03 17:04 UTC (permalink / raw)
  To: virtualization
  Cc: Stefan Hajnoczi, Michael S. Tsirkin, Oren Duer, Jason Wang,
	Laurent Vivier, Stefano Garzarella, linux-kernel, Max Gurtovoy,
	Shahaf Shuler, Eli Cohen

It's possible to allocate an unlimited IOTLB calling
vhost_iotlb_alloc() with 'limit' = 0.

Add a new macro (VHOST_IOTLB_UNLIMITED) for this case and document
it in the vhost_iotlb_alloc() documentation block.

Suggested-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 include/linux/vhost_iotlb.h | 2 ++
 drivers/vhost/iotlb.c       | 3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/linux/vhost_iotlb.h b/include/linux/vhost_iotlb.h
index 6b09b786a762..47019f97f795 100644
--- a/include/linux/vhost_iotlb.h
+++ b/include/linux/vhost_iotlb.h
@@ -4,6 +4,8 @@
 
 #include <linux/interval_tree_generic.h>
 
+#define VHOST_IOTLB_UNLIMITED 0
+
 struct vhost_iotlb_map {
 	struct rb_node rb;
 	struct list_head link;
diff --git a/drivers/vhost/iotlb.c b/drivers/vhost/iotlb.c
index 0fd3f87e913c..80fdde78ee5a 100644
--- a/drivers/vhost/iotlb.c
+++ b/drivers/vhost/iotlb.c
@@ -100,7 +100,8 @@ EXPORT_SYMBOL_GPL(vhost_iotlb_del_range);
 
 /**
  * vhost_iotlb_alloc - add a new vhost IOTLB
- * @limit: maximum number of IOTLB entries
+ * @limit: maximum number of IOTLB entries (use VHOST_IOTLB_UNLIMITED for an
+ *         unlimited IOTLB)
  * @flags: VHOST_IOTLB_FLAG_XXX
  *
  * Returns an error is memory allocation fails
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 05/19] vdpa_sim: remove the limit of IOTLB entries
  2020-12-03 17:04 [PATCH v3 00/19] vdpa: generalize vdpa simulator Stefano Garzarella
                   ` (3 preceding siblings ...)
  2020-12-03 17:04 ` [PATCH v3 04/19] vhost/iotlb: add VHOST_IOTLB_UNLIMITED macro Stefano Garzarella
@ 2020-12-03 17:04 ` Stefano Garzarella
  2020-12-07  4:00   ` Jason Wang
  2020-12-03 17:04 ` [PATCH v3 06/19] vdpa_sim: rename vdpasim_config_ops variables Stefano Garzarella
                   ` (13 subsequent siblings)
  18 siblings, 1 reply; 35+ messages in thread
From: Stefano Garzarella @ 2020-12-03 17:04 UTC (permalink / raw)
  To: virtualization
  Cc: Stefan Hajnoczi, Michael S. Tsirkin, Oren Duer, Jason Wang,
	Laurent Vivier, Stefano Garzarella, linux-kernel, Max Gurtovoy,
	Shahaf Shuler, Eli Cohen

The simulated devices can support multiple queues, so this limit
should be defined according to the number of queues supported by
the device.

Since we are in a simulator, let's simply remove that limit.

Suggested-by: Jason Wang <jasowang@redhat.com>
Acked-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
v3:
- used VHOST_IOTLB_UNLIMITED macro [Jason]
v2:
- added VDPASIM_IOTLB_LIMIT macro [Jason]
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index 295a770caac0..688aceaa6543 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -368,7 +368,7 @@ static struct vdpasim *vdpasim_create(void)
 	if (!vdpasim->vqs)
 		goto err_iommu;
 
-	vdpasim->iommu = vhost_iotlb_alloc(2048, 0);
+	vdpasim->iommu = vhost_iotlb_alloc(VHOST_IOTLB_UNLIMITED, 0);
 	if (!vdpasim->iommu)
 		goto err_iommu;
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 06/19] vdpa_sim: rename vdpasim_config_ops variables
  2020-12-03 17:04 [PATCH v3 00/19] vdpa: generalize vdpa simulator Stefano Garzarella
                   ` (4 preceding siblings ...)
  2020-12-03 17:04 ` [PATCH v3 05/19] vdpa_sim: remove the limit of IOTLB entries Stefano Garzarella
@ 2020-12-03 17:04 ` Stefano Garzarella
  2020-12-03 17:04 ` [PATCH v3 07/19] vdpa_sim: add struct vdpasim_dev_attr for device attributes Stefano Garzarella
                   ` (12 subsequent siblings)
  18 siblings, 0 replies; 35+ messages in thread
From: Stefano Garzarella @ 2020-12-03 17:04 UTC (permalink / raw)
  To: virtualization
  Cc: Stefan Hajnoczi, Michael S. Tsirkin, Oren Duer, Jason Wang,
	Laurent Vivier, Stefano Garzarella, linux-kernel, Max Gurtovoy,
	Shahaf Shuler, Eli Cohen

These variables store generic callbacks used by the vDPA simulator
core, so we can remove the 'net' word in their names.

Co-developed-by: Max Gurtovoy <mgurtovoy@nvidia.com>
Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
Acked-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index 688aceaa6543..625e0f46b672 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -333,8 +333,8 @@ static const struct dma_map_ops vdpasim_dma_ops = {
 	.free = vdpasim_free_coherent,
 };
 
-static const struct vdpa_config_ops vdpasim_net_config_ops;
-static const struct vdpa_config_ops vdpasim_net_batch_config_ops;
+static const struct vdpa_config_ops vdpasim_config_ops;
+static const struct vdpa_config_ops vdpasim_batch_config_ops;
 
 static struct vdpasim *vdpasim_create(void)
 {
@@ -344,9 +344,9 @@ static struct vdpasim *vdpasim_create(void)
 	int i, ret = -ENOMEM;
 
 	if (batch_mapping)
-		ops = &vdpasim_net_batch_config_ops;
+		ops = &vdpasim_batch_config_ops;
 	else
-		ops = &vdpasim_net_config_ops;
+		ops = &vdpasim_config_ops;
 
 	vdpasim = vdpa_alloc_device(struct vdpasim, vdpa, NULL, ops, VDPASIM_VQ_NUM);
 	if (!vdpasim)
@@ -659,7 +659,7 @@ static void vdpasim_free(struct vdpa_device *vdpa)
 	kfree(vdpasim->vqs);
 }
 
-static const struct vdpa_config_ops vdpasim_net_config_ops = {
+static const struct vdpa_config_ops vdpasim_config_ops = {
 	.set_vq_address         = vdpasim_set_vq_address,
 	.set_vq_num             = vdpasim_set_vq_num,
 	.kick_vq                = vdpasim_kick_vq,
@@ -686,7 +686,7 @@ static const struct vdpa_config_ops vdpasim_net_config_ops = {
 	.free                   = vdpasim_free,
 };
 
-static const struct vdpa_config_ops vdpasim_net_batch_config_ops = {
+static const struct vdpa_config_ops vdpasim_batch_config_ops = {
 	.set_vq_address         = vdpasim_set_vq_address,
 	.set_vq_num             = vdpasim_set_vq_num,
 	.kick_vq                = vdpasim_kick_vq,
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 07/19] vdpa_sim: add struct vdpasim_dev_attr for device attributes
  2020-12-03 17:04 [PATCH v3 00/19] vdpa: generalize vdpa simulator Stefano Garzarella
                   ` (5 preceding siblings ...)
  2020-12-03 17:04 ` [PATCH v3 06/19] vdpa_sim: rename vdpasim_config_ops variables Stefano Garzarella
@ 2020-12-03 17:04 ` Stefano Garzarella
  2020-12-03 17:05 ` [PATCH v3 08/19] vdpa_sim: add device id field in vdpasim_dev_attr Stefano Garzarella
                   ` (11 subsequent siblings)
  18 siblings, 0 replies; 35+ messages in thread
From: Stefano Garzarella @ 2020-12-03 17:04 UTC (permalink / raw)
  To: virtualization
  Cc: Stefan Hajnoczi, Michael S. Tsirkin, Oren Duer, Jason Wang,
	Laurent Vivier, Stefano Garzarella, linux-kernel, Max Gurtovoy,
	Shahaf Shuler, Eli Cohen

vdpasim_dev_attr will contain device specific attributes. We starting
moving the number of virtqueues (i.e. nvqs) to vdpasim_dev_attr.

vdpasim_create() creates a new vDPA simulator following the device
attributes defined in the vdpasim_dev_attr parameter.

Co-developed-by: Max Gurtovoy <mgurtovoy@nvidia.com>
Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
Acked-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index 625e0f46b672..e132b886cb84 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -57,11 +57,16 @@ static u64 vdpasim_features = (1ULL << VIRTIO_F_ANY_LAYOUT) |
 			      (1ULL << VIRTIO_F_ACCESS_PLATFORM) |
 			      (1ULL << VIRTIO_NET_F_MAC);
 
+struct vdpasim_dev_attr {
+	int nvqs;
+};
+
 /* State of each vdpasim device */
 struct vdpasim {
 	struct vdpa_device vdpa;
 	struct vdpasim_virtqueue *vqs;
 	struct work_struct work;
+	struct vdpasim_dev_attr dev_attr;
 	/* spinlock to synchronize virtqueue state */
 	spinlock_t lock;
 	struct virtio_net_config config;
@@ -70,7 +75,6 @@ struct vdpasim {
 	u32 status;
 	u32 generation;
 	u64 features;
-	int nvqs;
 	/* spinlock to synchronize iommu table */
 	spinlock_t iommu_lock;
 };
@@ -135,7 +139,7 @@ static void vdpasim_reset(struct vdpasim *vdpasim)
 {
 	int i;
 
-	for (i = 0; i < vdpasim->nvqs; i++)
+	for (i = 0; i < vdpasim->dev_attr.nvqs; i++)
 		vdpasim_vq_reset(&vdpasim->vqs[i]);
 
 	spin_lock(&vdpasim->iommu_lock);
@@ -336,7 +340,7 @@ static const struct dma_map_ops vdpasim_dma_ops = {
 static const struct vdpa_config_ops vdpasim_config_ops;
 static const struct vdpa_config_ops vdpasim_batch_config_ops;
 
-static struct vdpasim *vdpasim_create(void)
+static struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr)
 {
 	const struct vdpa_config_ops *ops;
 	struct vdpasim *vdpasim;
@@ -348,11 +352,12 @@ static struct vdpasim *vdpasim_create(void)
 	else
 		ops = &vdpasim_config_ops;
 
-	vdpasim = vdpa_alloc_device(struct vdpasim, vdpa, NULL, ops, VDPASIM_VQ_NUM);
+	vdpasim = vdpa_alloc_device(struct vdpasim, vdpa, NULL, ops,
+				    dev_attr->nvqs);
 	if (!vdpasim)
 		goto err_alloc;
 
-	vdpasim->nvqs = VDPASIM_VQ_NUM;
+	vdpasim->dev_attr = *dev_attr;
 	INIT_WORK(&vdpasim->work, vdpasim_work);
 	spin_lock_init(&vdpasim->lock);
 	spin_lock_init(&vdpasim->iommu_lock);
@@ -363,7 +368,7 @@ static struct vdpasim *vdpasim_create(void)
 		goto err_iommu;
 	set_dma_ops(dev, &vdpasim_dma_ops);
 
-	vdpasim->vqs = kcalloc(vdpasim->nvqs, sizeof(struct vdpasim_virtqueue),
+	vdpasim->vqs = kcalloc(dev_attr->nvqs, sizeof(struct vdpasim_virtqueue),
 			       GFP_KERNEL);
 	if (!vdpasim->vqs)
 		goto err_iommu;
@@ -386,7 +391,7 @@ static struct vdpasim *vdpasim_create(void)
 		eth_random_addr(vdpasim->config.mac);
 	}
 
-	for (i = 0; i < vdpasim->nvqs; i++)
+	for (i = 0; i < dev_attr->nvqs; i++)
 		vringh_set_iotlb(&vdpasim->vqs[i].vring, vdpasim->iommu);
 
 	vdpasim->vdpa.dma_dev = dev;
@@ -714,7 +719,11 @@ static const struct vdpa_config_ops vdpasim_batch_config_ops = {
 
 static int __init vdpasim_dev_init(void)
 {
-	vdpasim_dev = vdpasim_create();
+	struct vdpasim_dev_attr dev_attr = {};
+
+	dev_attr.nvqs = VDPASIM_VQ_NUM;
+
+	vdpasim_dev = vdpasim_create(&dev_attr);
 
 	if (!IS_ERR(vdpasim_dev))
 		return 0;
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 08/19] vdpa_sim: add device id field in vdpasim_dev_attr
  2020-12-03 17:04 [PATCH v3 00/19] vdpa: generalize vdpa simulator Stefano Garzarella
                   ` (6 preceding siblings ...)
  2020-12-03 17:04 ` [PATCH v3 07/19] vdpa_sim: add struct vdpasim_dev_attr for device attributes Stefano Garzarella
@ 2020-12-03 17:05 ` Stefano Garzarella
  2020-12-03 17:05 ` [PATCH v3 09/19] vdpa_sim: add supported_features " Stefano Garzarella
                   ` (10 subsequent siblings)
  18 siblings, 0 replies; 35+ messages in thread
From: Stefano Garzarella @ 2020-12-03 17:05 UTC (permalink / raw)
  To: virtualization
  Cc: Stefan Hajnoczi, Michael S. Tsirkin, Oren Duer, Jason Wang,
	Laurent Vivier, Stefano Garzarella, linux-kernel, Max Gurtovoy,
	Shahaf Shuler, Eli Cohen

Remove VDPASIM_DEVICE_ID macro and add 'id' field in vdpasim_dev_attr,
that will be returned by vdpasim_get_device_id().

Use VIRTIO_ID_NET for vDPA-net simulator device id.

Co-developed-by: Max Gurtovoy <mgurtovoy@nvidia.com>
Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
Acked-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index e132b886cb84..3d97bc709fb9 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -47,7 +47,6 @@ struct vdpasim_virtqueue {
 
 #define VDPASIM_QUEUE_ALIGN PAGE_SIZE
 #define VDPASIM_QUEUE_MAX 256
-#define VDPASIM_DEVICE_ID 0x1
 #define VDPASIM_VENDOR_ID 0
 #define VDPASIM_VQ_NUM 0x2
 #define VDPASIM_NAME "vdpasim-netdev"
@@ -59,6 +58,7 @@ static u64 vdpasim_features = (1ULL << VIRTIO_F_ANY_LAYOUT) |
 
 struct vdpasim_dev_attr {
 	int nvqs;
+	u32 id;
 };
 
 /* State of each vdpasim device */
@@ -538,7 +538,9 @@ static u16 vdpasim_get_vq_num_max(struct vdpa_device *vdpa)
 
 static u32 vdpasim_get_device_id(struct vdpa_device *vdpa)
 {
-	return VDPASIM_DEVICE_ID;
+	struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
+
+	return vdpasim->dev_attr.id;
 }
 
 static u32 vdpasim_get_vendor_id(struct vdpa_device *vdpa)
@@ -721,6 +723,7 @@ static int __init vdpasim_dev_init(void)
 {
 	struct vdpasim_dev_attr dev_attr = {};
 
+	dev_attr.id = VIRTIO_ID_NET;
 	dev_attr.nvqs = VDPASIM_VQ_NUM;
 
 	vdpasim_dev = vdpasim_create(&dev_attr);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 09/19] vdpa_sim: add supported_features field in vdpasim_dev_attr
  2020-12-03 17:04 [PATCH v3 00/19] vdpa: generalize vdpa simulator Stefano Garzarella
                   ` (7 preceding siblings ...)
  2020-12-03 17:05 ` [PATCH v3 08/19] vdpa_sim: add device id field in vdpasim_dev_attr Stefano Garzarella
@ 2020-12-03 17:05 ` Stefano Garzarella
  2020-12-03 17:05 ` [PATCH v3 10/19] vdpa_sim: add work_fn " Stefano Garzarella
                   ` (9 subsequent siblings)
  18 siblings, 0 replies; 35+ messages in thread
From: Stefano Garzarella @ 2020-12-03 17:05 UTC (permalink / raw)
  To: virtualization
  Cc: Stefan Hajnoczi, Michael S. Tsirkin, Oren Duer, Jason Wang,
	Laurent Vivier, Stefano Garzarella, linux-kernel, Max Gurtovoy,
	Shahaf Shuler, Eli Cohen

Introduce a new VDPASIM_FEATURES macro with the generic features
supported by the vDPA simulator, and VDPASIM_NET_FEATURES macro with
vDPA-net features.

Add 'supported_features' field in vdpasim_dev_attr, to allow devices
to specify their features.

Co-developed-by: Max Gurtovoy <mgurtovoy@nvidia.com>
Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
Acked-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index 3d97bc709fb9..569c5213ee01 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -51,12 +51,15 @@ struct vdpasim_virtqueue {
 #define VDPASIM_VQ_NUM 0x2
 #define VDPASIM_NAME "vdpasim-netdev"
 
-static u64 vdpasim_features = (1ULL << VIRTIO_F_ANY_LAYOUT) |
-			      (1ULL << VIRTIO_F_VERSION_1)  |
-			      (1ULL << VIRTIO_F_ACCESS_PLATFORM) |
-			      (1ULL << VIRTIO_NET_F_MAC);
+#define VDPASIM_FEATURES	((1ULL << VIRTIO_F_ANY_LAYOUT) | \
+				 (1ULL << VIRTIO_F_VERSION_1)  | \
+				 (1ULL << VIRTIO_F_ACCESS_PLATFORM))
+
+#define VDPASIM_NET_FEATURES	(VDPASIM_FEATURES | \
+				 (1ULL << VIRTIO_NET_F_MAC))
 
 struct vdpasim_dev_attr {
+	u64 supported_features;
 	int nvqs;
 	u32 id;
 };
@@ -114,7 +117,7 @@ static void vdpasim_queue_ready(struct vdpasim *vdpasim, unsigned int idx)
 {
 	struct vdpasim_virtqueue *vq = &vdpasim->vqs[idx];
 
-	vringh_init_iotlb(&vq->vring, vdpasim_features,
+	vringh_init_iotlb(&vq->vring, vdpasim->dev_attr.supported_features,
 			  VDPASIM_QUEUE_MAX, false,
 			  (struct vring_desc *)(uintptr_t)vq->desc_addr,
 			  (struct vring_avail *)
@@ -123,7 +126,8 @@ static void vdpasim_queue_ready(struct vdpasim *vdpasim, unsigned int idx)
 			  (uintptr_t)vq->device_addr);
 }
 
-static void vdpasim_vq_reset(struct vdpasim_virtqueue *vq)
+static void vdpasim_vq_reset(struct vdpasim *vdpasim,
+			     struct vdpasim_virtqueue *vq)
 {
 	vq->ready = false;
 	vq->desc_addr = 0;
@@ -131,8 +135,8 @@ static void vdpasim_vq_reset(struct vdpasim_virtqueue *vq)
 	vq->device_addr = 0;
 	vq->cb = NULL;
 	vq->private = NULL;
-	vringh_init_iotlb(&vq->vring, vdpasim_features, VDPASIM_QUEUE_MAX,
-			  false, NULL, NULL, NULL);
+	vringh_init_iotlb(&vq->vring, vdpasim->dev_attr.supported_features,
+			  VDPASIM_QUEUE_MAX, false, NULL, NULL, NULL);
 }
 
 static void vdpasim_reset(struct vdpasim *vdpasim)
@@ -140,7 +144,7 @@ static void vdpasim_reset(struct vdpasim *vdpasim)
 	int i;
 
 	for (i = 0; i < vdpasim->dev_attr.nvqs; i++)
-		vdpasim_vq_reset(&vdpasim->vqs[i]);
+		vdpasim_vq_reset(vdpasim, &vdpasim->vqs[i]);
 
 	spin_lock(&vdpasim->iommu_lock);
 	vhost_iotlb_reset(vdpasim->iommu);
@@ -500,7 +504,9 @@ static u32 vdpasim_get_vq_align(struct vdpa_device *vdpa)
 
 static u64 vdpasim_get_features(struct vdpa_device *vdpa)
 {
-	return vdpasim_features;
+	struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
+
+	return vdpasim->dev_attr.supported_features;
 }
 
 static int vdpasim_set_features(struct vdpa_device *vdpa, u64 features)
@@ -512,7 +518,7 @@ static int vdpasim_set_features(struct vdpa_device *vdpa, u64 features)
 	if (!(features & (1ULL << VIRTIO_F_ACCESS_PLATFORM)))
 		return -EINVAL;
 
-	vdpasim->features = features & vdpasim_features;
+	vdpasim->features = features & vdpasim->dev_attr.supported_features;
 
 	/* We generally only know whether guest is using the legacy interface
 	 * here, so generally that's the earliest we can set config fields.
@@ -724,6 +730,7 @@ static int __init vdpasim_dev_init(void)
 	struct vdpasim_dev_attr dev_attr = {};
 
 	dev_attr.id = VIRTIO_ID_NET;
+	dev_attr.supported_features = VDPASIM_NET_FEATURES;
 	dev_attr.nvqs = VDPASIM_VQ_NUM;
 
 	vdpasim_dev = vdpasim_create(&dev_attr);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 10/19] vdpa_sim: add work_fn in vdpasim_dev_attr
  2020-12-03 17:04 [PATCH v3 00/19] vdpa: generalize vdpa simulator Stefano Garzarella
                   ` (8 preceding siblings ...)
  2020-12-03 17:05 ` [PATCH v3 09/19] vdpa_sim: add supported_features " Stefano Garzarella
@ 2020-12-03 17:05 ` Stefano Garzarella
  2020-12-03 17:05 ` [PATCH v3 11/19] vdpa_sim: store parsed MAC address in a buffer Stefano Garzarella
                   ` (8 subsequent siblings)
  18 siblings, 0 replies; 35+ messages in thread
From: Stefano Garzarella @ 2020-12-03 17:05 UTC (permalink / raw)
  To: virtualization
  Cc: Stefan Hajnoczi, Michael S. Tsirkin, Oren Duer, Jason Wang,
	Laurent Vivier, Stefano Garzarella, linux-kernel, Max Gurtovoy,
	Shahaf Shuler, Eli Cohen

Rename vdpasim_work() in vdpasim_net_work() and add it to
the vdpasim_dev_attr structure.

Co-developed-by: Max Gurtovoy <mgurtovoy@nvidia.com>
Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
Acked-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index 569c5213ee01..e7d366f63090 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -62,6 +62,8 @@ struct vdpasim_dev_attr {
 	u64 supported_features;
 	int nvqs;
 	u32 id;
+
+	work_func_t work_fn;
 };
 
 /* State of each vdpasim device */
@@ -155,7 +157,7 @@ static void vdpasim_reset(struct vdpasim *vdpasim)
 	++vdpasim->generation;
 }
 
-static void vdpasim_work(struct work_struct *work)
+static void vdpasim_net_work(struct work_struct *work)
 {
 	struct vdpasim *vdpasim = container_of(work, struct
 						 vdpasim, work);
@@ -362,7 +364,7 @@ static struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr)
 		goto err_alloc;
 
 	vdpasim->dev_attr = *dev_attr;
-	INIT_WORK(&vdpasim->work, vdpasim_work);
+	INIT_WORK(&vdpasim->work, dev_attr->work_fn);
 	spin_lock_init(&vdpasim->lock);
 	spin_lock_init(&vdpasim->iommu_lock);
 
@@ -732,6 +734,7 @@ static int __init vdpasim_dev_init(void)
 	dev_attr.id = VIRTIO_ID_NET;
 	dev_attr.supported_features = VDPASIM_NET_FEATURES;
 	dev_attr.nvqs = VDPASIM_VQ_NUM;
+	dev_attr.work_fn = vdpasim_net_work;
 
 	vdpasim_dev = vdpasim_create(&dev_attr);
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 11/19] vdpa_sim: store parsed MAC address in a buffer
  2020-12-03 17:04 [PATCH v3 00/19] vdpa: generalize vdpa simulator Stefano Garzarella
                   ` (9 preceding siblings ...)
  2020-12-03 17:05 ` [PATCH v3 10/19] vdpa_sim: add work_fn " Stefano Garzarella
@ 2020-12-03 17:05 ` Stefano Garzarella
  2020-12-03 17:05 ` [PATCH v3 12/19] vdpa_sim: make 'config' generic and usable for any device type Stefano Garzarella
                   ` (7 subsequent siblings)
  18 siblings, 0 replies; 35+ messages in thread
From: Stefano Garzarella @ 2020-12-03 17:05 UTC (permalink / raw)
  To: virtualization
  Cc: Stefan Hajnoczi, Michael S. Tsirkin, Oren Duer, Jason Wang,
	Laurent Vivier, Stefano Garzarella, linux-kernel, Max Gurtovoy,
	Shahaf Shuler, Eli Cohen

As preparation for the next patches, we store the MAC address,
parsed during the vdpasim_create(), in a buffer that will be used
to fill 'config' together with other configurations.

Acked-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index e7d366f63090..949f4231d08a 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -32,6 +32,8 @@ static char *macaddr;
 module_param(macaddr, charp, 0);
 MODULE_PARM_DESC(macaddr, "Ethernet MAC address");
 
+u8 macaddr_buf[ETH_ALEN];
+
 struct vdpasim_virtqueue {
 	struct vringh vring;
 	struct vringh_kiov iov;
@@ -388,13 +390,13 @@ static struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr)
 		goto err_iommu;
 
 	if (macaddr) {
-		mac_pton(macaddr, vdpasim->config.mac);
-		if (!is_valid_ether_addr(vdpasim->config.mac)) {
+		mac_pton(macaddr, macaddr_buf);
+		if (!is_valid_ether_addr(macaddr_buf)) {
 			ret = -EADDRNOTAVAIL;
 			goto err_iommu;
 		}
 	} else {
-		eth_random_addr(vdpasim->config.mac);
+		eth_random_addr(macaddr_buf);
 	}
 
 	for (i = 0; i < dev_attr->nvqs; i++)
@@ -530,6 +532,8 @@ static int vdpasim_set_features(struct vdpa_device *vdpa, u64 features)
 
 	config->mtu = cpu_to_vdpasim16(vdpasim, 1500);
 	config->status = cpu_to_vdpasim16(vdpasim, VIRTIO_NET_S_LINK_UP);
+	memcpy(config->mac, macaddr_buf, ETH_ALEN);
+
 	return 0;
 }
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 12/19] vdpa_sim: make 'config' generic and usable for any device type
  2020-12-03 17:04 [PATCH v3 00/19] vdpa: generalize vdpa simulator Stefano Garzarella
                   ` (10 preceding siblings ...)
  2020-12-03 17:05 ` [PATCH v3 11/19] vdpa_sim: store parsed MAC address in a buffer Stefano Garzarella
@ 2020-12-03 17:05 ` Stefano Garzarella
  2020-12-07  5:22   ` Jason Wang
  2020-12-03 17:05 ` [PATCH v3 13/19] vdpa_sim: add get_config callback in vdpasim_dev_attr Stefano Garzarella
                   ` (6 subsequent siblings)
  18 siblings, 1 reply; 35+ messages in thread
From: Stefano Garzarella @ 2020-12-03 17:05 UTC (permalink / raw)
  To: virtualization
  Cc: Stefan Hajnoczi, Michael S. Tsirkin, Oren Duer, Jason Wang,
	Laurent Vivier, Stefano Garzarella, linux-kernel, Max Gurtovoy,
	Shahaf Shuler, Eli Cohen

Add new 'config_size' attribute in 'vdpasim_dev_attr' and allocates
'config' dynamically to support any device types.

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index 949f4231d08a..fe71ed7890e1 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -62,6 +62,7 @@ struct vdpasim_virtqueue {
 
 struct vdpasim_dev_attr {
 	u64 supported_features;
+	size_t config_size;
 	int nvqs;
 	u32 id;
 
@@ -76,7 +77,8 @@ struct vdpasim {
 	struct vdpasim_dev_attr dev_attr;
 	/* spinlock to synchronize virtqueue state */
 	spinlock_t lock;
-	struct virtio_net_config config;
+	/* virtio config according to device type */
+	void *config;
 	struct vhost_iotlb *iommu;
 	void *buffer;
 	u32 status;
@@ -376,6 +378,10 @@ static struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr)
 		goto err_iommu;
 	set_dma_ops(dev, &vdpasim_dma_ops);
 
+	vdpasim->config = kzalloc(dev_attr->config_size, GFP_KERNEL);
+	if (!vdpasim->config)
+		goto err_iommu;
+
 	vdpasim->vqs = kcalloc(dev_attr->nvqs, sizeof(struct vdpasim_virtqueue),
 			       GFP_KERNEL);
 	if (!vdpasim->vqs)
@@ -516,7 +522,8 @@ static u64 vdpasim_get_features(struct vdpa_device *vdpa)
 static int vdpasim_set_features(struct vdpa_device *vdpa, u64 features)
 {
 	struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
-	struct virtio_net_config *config = &vdpasim->config;
+	struct virtio_net_config *config =
+		(struct virtio_net_config *)vdpasim->config;
 
 	/* DMA mapping must be done by driver */
 	if (!(features & (1ULL << VIRTIO_F_ACCESS_PLATFORM)))
@@ -588,8 +595,8 @@ static void vdpasim_get_config(struct vdpa_device *vdpa, unsigned int offset,
 {
 	struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
 
-	if (offset + len < sizeof(struct virtio_net_config))
-		memcpy(buf, (u8 *)&vdpasim->config + offset, len);
+	if (offset + len < vdpasim->dev_attr.config_size)
+		memcpy(buf, vdpasim->config + offset, len);
 }
 
 static void vdpasim_set_config(struct vdpa_device *vdpa, unsigned int offset,
@@ -676,6 +683,7 @@ static void vdpasim_free(struct vdpa_device *vdpa)
 	if (vdpasim->iommu)
 		vhost_iotlb_free(vdpasim->iommu);
 	kfree(vdpasim->vqs);
+	kfree(vdpasim->config);
 }
 
 static const struct vdpa_config_ops vdpasim_config_ops = {
@@ -738,6 +746,7 @@ static int __init vdpasim_dev_init(void)
 	dev_attr.id = VIRTIO_ID_NET;
 	dev_attr.supported_features = VDPASIM_NET_FEATURES;
 	dev_attr.nvqs = VDPASIM_VQ_NUM;
+	dev_attr.config_size = sizeof(struct virtio_net_config);
 	dev_attr.work_fn = vdpasim_net_work;
 
 	vdpasim_dev = vdpasim_create(&dev_attr);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 13/19] vdpa_sim: add get_config callback in vdpasim_dev_attr
  2020-12-03 17:04 [PATCH v3 00/19] vdpa: generalize vdpa simulator Stefano Garzarella
                   ` (11 preceding siblings ...)
  2020-12-03 17:05 ` [PATCH v3 12/19] vdpa_sim: make 'config' generic and usable for any device type Stefano Garzarella
@ 2020-12-03 17:05 ` Stefano Garzarella
  2020-12-07  5:29   ` Jason Wang
  2020-12-03 17:05 ` [PATCH v3 14/19] vdpa_sim: add set_config " Stefano Garzarella
                   ` (5 subsequent siblings)
  18 siblings, 1 reply; 35+ messages in thread
From: Stefano Garzarella @ 2020-12-03 17:05 UTC (permalink / raw)
  To: virtualization
  Cc: Stefan Hajnoczi, Michael S. Tsirkin, Oren Duer, Jason Wang,
	Laurent Vivier, Stefano Garzarella, linux-kernel, Max Gurtovoy,
	Shahaf Shuler, Eli Cohen

The get_config callback can be used by the device to fill the
config structure.
The callback will be invoked in vdpasim_get_config() before copying
bytes into caller buffer.

Move vDPA-net config updates from vdpasim_set_features() in the
new vdpasim_net_get_config() callback.

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
v3:
- checked if get_config callback is set before call it
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 35 +++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index fe71ed7890e1..f935ade0806b 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -60,6 +60,8 @@ struct vdpasim_virtqueue {
 #define VDPASIM_NET_FEATURES	(VDPASIM_FEATURES | \
 				 (1ULL << VIRTIO_NET_F_MAC))
 
+struct vdpasim;
+
 struct vdpasim_dev_attr {
 	u64 supported_features;
 	size_t config_size;
@@ -67,6 +69,7 @@ struct vdpasim_dev_attr {
 	u32 id;
 
 	work_func_t work_fn;
+	void (*get_config)(struct vdpasim *vdpasim, void *config);
 };
 
 /* State of each vdpasim device */
@@ -522,8 +525,6 @@ static u64 vdpasim_get_features(struct vdpa_device *vdpa)
 static int vdpasim_set_features(struct vdpa_device *vdpa, u64 features)
 {
 	struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
-	struct virtio_net_config *config =
-		(struct virtio_net_config *)vdpasim->config;
 
 	/* DMA mapping must be done by driver */
 	if (!(features & (1ULL << VIRTIO_F_ACCESS_PLATFORM)))
@@ -531,16 +532,6 @@ static int vdpasim_set_features(struct vdpa_device *vdpa, u64 features)
 
 	vdpasim->features = features & vdpasim->dev_attr.supported_features;
 
-	/* We generally only know whether guest is using the legacy interface
-	 * here, so generally that's the earliest we can set config fields.
-	 * Note: We actually require VIRTIO_F_ACCESS_PLATFORM above which
-	 * implies VIRTIO_F_VERSION_1, but let's not try to be clever here.
-	 */
-
-	config->mtu = cpu_to_vdpasim16(vdpasim, 1500);
-	config->status = cpu_to_vdpasim16(vdpasim, VIRTIO_NET_S_LINK_UP);
-	memcpy(config->mac, macaddr_buf, ETH_ALEN);
-
 	return 0;
 }
 
@@ -595,8 +586,13 @@ static void vdpasim_get_config(struct vdpa_device *vdpa, unsigned int offset,
 {
 	struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
 
-	if (offset + len < vdpasim->dev_attr.config_size)
-		memcpy(buf, vdpasim->config + offset, len);
+	if (offset + len > vdpasim->dev_attr.config_size)
+		return;
+
+	if (vdpasim->dev_attr.get_config)
+		vdpasim->dev_attr.get_config(vdpasim, vdpasim->config);
+
+	memcpy(buf, vdpasim->config + offset, len);
 }
 
 static void vdpasim_set_config(struct vdpa_device *vdpa, unsigned int offset,
@@ -739,6 +735,16 @@ static const struct vdpa_config_ops vdpasim_batch_config_ops = {
 	.free                   = vdpasim_free,
 };
 
+static void vdpasim_net_get_config(struct vdpasim *vdpasim, void *config)
+{
+	struct virtio_net_config *net_config =
+		(struct virtio_net_config *)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 int __init vdpasim_dev_init(void)
 {
 	struct vdpasim_dev_attr dev_attr = {};
@@ -747,6 +753,7 @@ static int __init vdpasim_dev_init(void)
 	dev_attr.supported_features = VDPASIM_NET_FEATURES;
 	dev_attr.nvqs = VDPASIM_VQ_NUM;
 	dev_attr.config_size = sizeof(struct virtio_net_config);
+	dev_attr.get_config = vdpasim_net_get_config;
 	dev_attr.work_fn = vdpasim_net_work;
 
 	vdpasim_dev = vdpasim_create(&dev_attr);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 14/19] vdpa_sim: add set_config callback in vdpasim_dev_attr
  2020-12-03 17:04 [PATCH v3 00/19] vdpa: generalize vdpa simulator Stefano Garzarella
                   ` (12 preceding siblings ...)
  2020-12-03 17:05 ` [PATCH v3 13/19] vdpa_sim: add get_config callback in vdpasim_dev_attr Stefano Garzarella
@ 2020-12-03 17:05 ` Stefano Garzarella
  2020-12-07  5:30   ` Jason Wang
  2020-12-03 17:05 ` [PATCH v3 15/19] vdpa_sim: set vringh notify callback Stefano Garzarella
                   ` (4 subsequent siblings)
  18 siblings, 1 reply; 35+ messages in thread
From: Stefano Garzarella @ 2020-12-03 17:05 UTC (permalink / raw)
  To: virtualization
  Cc: Stefan Hajnoczi, Michael S. Tsirkin, Oren Duer, Jason Wang,
	Laurent Vivier, Stefano Garzarella, linux-kernel, Max Gurtovoy,
	Shahaf Shuler, Eli Cohen

The set_config callback can be used by the device to parse the
config structure modified by the driver.

The callback will be invoked, if set, in vdpasim_set_config() after
copying bytes from caller buffer into vdpasim->config buffer.

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

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index f935ade0806b..03a8717f80ea 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -70,6 +70,7 @@ 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);
 };
 
 /* State of each vdpasim device */
@@ -598,7 +599,15 @@ static void vdpasim_get_config(struct vdpa_device *vdpa, unsigned int offset,
 static void vdpasim_set_config(struct vdpa_device *vdpa, unsigned int offset,
 			     const void *buf, unsigned int len)
 {
-	/* No writable config supportted by vdpasim */
+	struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
+
+	if (offset + len > vdpasim->dev_attr.config_size)
+		return;
+
+	memcpy(vdpasim->config + offset, buf, len);
+
+	if (vdpasim->dev_attr.set_config)
+		vdpasim->dev_attr.set_config(vdpasim, vdpasim->config);
 }
 
 static u32 vdpasim_get_generation(struct vdpa_device *vdpa)
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 15/19] vdpa_sim: set vringh notify callback
  2020-12-03 17:04 [PATCH v3 00/19] vdpa: generalize vdpa simulator Stefano Garzarella
                   ` (13 preceding siblings ...)
  2020-12-03 17:05 ` [PATCH v3 14/19] vdpa_sim: add set_config " Stefano Garzarella
@ 2020-12-03 17:05 ` Stefano Garzarella
  2020-12-07  5:30   ` Jason Wang
  2020-12-03 17:05 ` [PATCH v3 16/19] vdpa_sim: use kvmalloc to allocate vdpasim->buffer Stefano Garzarella
                   ` (3 subsequent siblings)
  18 siblings, 1 reply; 35+ messages in thread
From: Stefano Garzarella @ 2020-12-03 17:05 UTC (permalink / raw)
  To: virtualization
  Cc: Stefan Hajnoczi, Michael S. Tsirkin, Oren Duer, Jason Wang,
	Laurent Vivier, Stefano Garzarella, linux-kernel, Max Gurtovoy,
	Shahaf Shuler, Eli Cohen

Instead of calling the vq callback directly, we can leverage the
vringh_notify() function, adding vdpasim_vq_notify() and setting it
in the vringh notify callback.

Suggested-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
v3:
- cleared notify during reset [Jason]
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index 03a8717f80ea..1243b02488f7 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -123,6 +123,17 @@ static struct vdpasim *dev_to_sim(struct device *dev)
 	return vdpa_to_sim(vdpa);
 }
 
+static void vdpasim_vq_notify(struct vringh *vring)
+{
+	struct vdpasim_virtqueue *vq =
+		container_of(vring, struct vdpasim_virtqueue, vring);
+
+	if (!vq->cb)
+		return;
+
+	vq->cb(vq->private);
+}
+
 static void vdpasim_queue_ready(struct vdpasim *vdpasim, unsigned int idx)
 {
 	struct vdpasim_virtqueue *vq = &vdpasim->vqs[idx];
@@ -134,6 +145,8 @@ static void vdpasim_queue_ready(struct vdpasim *vdpasim, unsigned int idx)
 			  (uintptr_t)vq->driver_addr,
 			  (struct vring_used *)
 			  (uintptr_t)vq->device_addr);
+
+	vq->vring.notify = vdpasim_vq_notify;
 }
 
 static void vdpasim_vq_reset(struct vdpasim *vdpasim,
@@ -147,6 +160,8 @@ static void vdpasim_vq_reset(struct vdpasim *vdpasim,
 	vq->private = NULL;
 	vringh_init_iotlb(&vq->vring, vdpasim->dev_attr.supported_features,
 			  VDPASIM_QUEUE_MAX, false, NULL, NULL, NULL);
+
+	vq->vring.notify = NULL;
 }
 
 static void vdpasim_reset(struct vdpasim *vdpasim)
@@ -223,10 +238,10 @@ static void vdpasim_net_work(struct work_struct *work)
 		smp_wmb();
 
 		local_bh_disable();
-		if (txq->cb)
-			txq->cb(txq->private);
-		if (rxq->cb)
-			rxq->cb(rxq->private);
+		if (vringh_need_notify_iotlb(&txq->vring) > 0)
+			vringh_notify(&txq->vring);
+		if (vringh_need_notify_iotlb(&rxq->vring) > 0)
+			vringh_notify(&rxq->vring);
 		local_bh_enable();
 
 		if (++pkts > 4) {
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 16/19] vdpa_sim: use kvmalloc to allocate vdpasim->buffer
  2020-12-03 17:04 [PATCH v3 00/19] vdpa: generalize vdpa simulator Stefano Garzarella
                   ` (14 preceding siblings ...)
  2020-12-03 17:05 ` [PATCH v3 15/19] vdpa_sim: set vringh notify callback Stefano Garzarella
@ 2020-12-03 17:05 ` Stefano Garzarella
  2020-12-03 17:05 ` [PATCH v3 17/19] vdpa_sim: make vdpasim->buffer size configurable Stefano Garzarella
                   ` (2 subsequent siblings)
  18 siblings, 0 replies; 35+ messages in thread
From: Stefano Garzarella @ 2020-12-03 17:05 UTC (permalink / raw)
  To: virtualization
  Cc: Stefan Hajnoczi, Michael S. Tsirkin, Oren Duer, Jason Wang,
	Laurent Vivier, Stefano Garzarella, linux-kernel, Max Gurtovoy,
	Shahaf Shuler, Eli Cohen

The next patch will make the buffer size configurable from each
device.
Since the buffer could be larger than a page, we use kvmalloc()
instead of kmalloc().

Acked-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index 1243b02488f7..fb714d88e77f 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -410,7 +410,7 @@ static struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr)
 	if (!vdpasim->iommu)
 		goto err_iommu;
 
-	vdpasim->buffer = kmalloc(PAGE_SIZE, GFP_KERNEL);
+	vdpasim->buffer = kvmalloc(PAGE_SIZE, GFP_KERNEL);
 	if (!vdpasim->buffer)
 		goto err_iommu;
 
@@ -699,7 +699,7 @@ static void vdpasim_free(struct vdpa_device *vdpa)
 	struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
 
 	cancel_work_sync(&vdpasim->work);
-	kfree(vdpasim->buffer);
+	kvfree(vdpasim->buffer);
 	if (vdpasim->iommu)
 		vhost_iotlb_free(vdpasim->iommu);
 	kfree(vdpasim->vqs);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 17/19] vdpa_sim: make vdpasim->buffer size configurable
  2020-12-03 17:04 [PATCH v3 00/19] vdpa: generalize vdpa simulator Stefano Garzarella
                   ` (15 preceding siblings ...)
  2020-12-03 17:05 ` [PATCH v3 16/19] vdpa_sim: use kvmalloc to allocate vdpasim->buffer Stefano Garzarella
@ 2020-12-03 17:05 ` Stefano Garzarella
  2020-12-03 17:05 ` [PATCH v3 18/19] vdpa_sim: split vdpasim_virtqueue's iov field in out_iov and in_iov Stefano Garzarella
  2020-12-03 17:05 ` [PATCH v3 19/19] vdpa: split vdpasim to core and net modules Stefano Garzarella
  18 siblings, 0 replies; 35+ messages in thread
From: Stefano Garzarella @ 2020-12-03 17:05 UTC (permalink / raw)
  To: virtualization
  Cc: Stefan Hajnoczi, Michael S. Tsirkin, Oren Duer, Jason Wang,
	Laurent Vivier, Stefano Garzarella, linux-kernel, Max Gurtovoy,
	Shahaf Shuler, Eli Cohen

Allow each device to specify the size of the buffer allocated
in vdpa_sim.

Acked-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index fb714d88e77f..38b6b5e7348c 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -65,6 +65,7 @@ struct vdpasim;
 struct vdpasim_dev_attr {
 	u64 supported_features;
 	size_t config_size;
+	size_t buffer_size;
 	int nvqs;
 	u32 id;
 
@@ -410,7 +411,7 @@ static struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr)
 	if (!vdpasim->iommu)
 		goto err_iommu;
 
-	vdpasim->buffer = kvmalloc(PAGE_SIZE, GFP_KERNEL);
+	vdpasim->buffer = kvmalloc(dev_attr->buffer_size, GFP_KERNEL);
 	if (!vdpasim->buffer)
 		goto err_iommu;
 
@@ -779,6 +780,7 @@ static int __init vdpasim_dev_init(void)
 	dev_attr.config_size = sizeof(struct virtio_net_config);
 	dev_attr.get_config = vdpasim_net_get_config;
 	dev_attr.work_fn = vdpasim_net_work;
+	dev_attr.buffer_size = PAGE_SIZE;
 
 	vdpasim_dev = vdpasim_create(&dev_attr);
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 18/19] vdpa_sim: split vdpasim_virtqueue's iov field in out_iov and in_iov
  2020-12-03 17:04 [PATCH v3 00/19] vdpa: generalize vdpa simulator Stefano Garzarella
                   ` (16 preceding siblings ...)
  2020-12-03 17:05 ` [PATCH v3 17/19] vdpa_sim: make vdpasim->buffer size configurable Stefano Garzarella
@ 2020-12-03 17:05 ` Stefano Garzarella
  2020-12-03 17:05 ` [PATCH v3 19/19] vdpa: split vdpasim to core and net modules Stefano Garzarella
  18 siblings, 0 replies; 35+ messages in thread
From: Stefano Garzarella @ 2020-12-03 17:05 UTC (permalink / raw)
  To: virtualization
  Cc: Stefan Hajnoczi, Michael S. Tsirkin, Oren Duer, Jason Wang,
	Laurent Vivier, Stefano Garzarella, linux-kernel, Max Gurtovoy,
	Shahaf Shuler, Eli Cohen

vringh_getdesc_iotlb() manages 2 iovs for writable and readable
descriptors. This is very useful for the block device, where for
each request we have both types of descriptor.

Let's split the vdpasim_virtqueue's iov field in out_iov and
in_iov to use them with vringh_getdesc_iotlb().

We are using VIRTIO terminology for "out" (readable by the device)
and "in" (writable by the device) descriptors.

Acked-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
v2:
- used VIRTIO terminology [Stefan]
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index 38b6b5e7348c..557b2129b41b 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -36,7 +36,8 @@ u8 macaddr_buf[ETH_ALEN];
 
 struct vdpasim_virtqueue {
 	struct vringh vring;
-	struct vringh_kiov iov;
+	struct vringh_kiov in_iov;
+	struct vringh_kiov out_iov;
 	unsigned short head;
 	bool ready;
 	u64 desc_addr;
@@ -202,12 +203,12 @@ static void vdpasim_net_work(struct work_struct *work)
 
 	while (true) {
 		total_write = 0;
-		err = vringh_getdesc_iotlb(&txq->vring, &txq->iov, NULL,
+		err = vringh_getdesc_iotlb(&txq->vring, &txq->out_iov, NULL,
 					   &txq->head, GFP_ATOMIC);
 		if (err <= 0)
 			break;
 
-		err = vringh_getdesc_iotlb(&rxq->vring, NULL, &rxq->iov,
+		err = vringh_getdesc_iotlb(&rxq->vring, NULL, &rxq->in_iov,
 					   &rxq->head, GFP_ATOMIC);
 		if (err <= 0) {
 			vringh_complete_iotlb(&txq->vring, txq->head, 0);
@@ -215,13 +216,13 @@ static void vdpasim_net_work(struct work_struct *work)
 		}
 
 		while (true) {
-			read = vringh_iov_pull_iotlb(&txq->vring, &txq->iov,
+			read = vringh_iov_pull_iotlb(&txq->vring, &txq->out_iov,
 						     vdpasim->buffer,
 						     PAGE_SIZE);
 			if (read <= 0)
 				break;
 
-			write = vringh_iov_push_iotlb(&rxq->vring, &rxq->iov,
+			write = vringh_iov_push_iotlb(&rxq->vring, &rxq->in_iov,
 						      vdpasim->buffer, read);
 			if (write <= 0)
 				break;
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v3 19/19] vdpa: split vdpasim to core and net modules
  2020-12-03 17:04 [PATCH v3 00/19] vdpa: generalize vdpa simulator Stefano Garzarella
                   ` (17 preceding siblings ...)
  2020-12-03 17:05 ` [PATCH v3 18/19] vdpa_sim: split vdpasim_virtqueue's iov field in out_iov and in_iov Stefano Garzarella
@ 2020-12-03 17:05 ` Stefano Garzarella
  2020-12-03 17:25   ` Randy Dunlap
  2020-12-07  5:33   ` Jason Wang
  18 siblings, 2 replies; 35+ messages in thread
From: Stefano Garzarella @ 2020-12-03 17:05 UTC (permalink / raw)
  To: virtualization
  Cc: Stefan Hajnoczi, Michael S. Tsirkin, Oren Duer, Jason Wang,
	Laurent Vivier, Stefano Garzarella, linux-kernel, Max Gurtovoy,
	Shahaf Shuler, Eli Cohen

From: Max Gurtovoy <mgurtovoy@nvidia.com>

Introduce new vdpa_sim_net and vdpa_sim (core) drivers. This is a
preparation for adding a vdpa simulator module for block devices.

Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
[sgarzare: various cleanups/fixes]
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
v2:
- Fixed "warning: variable 'dev' is used uninitialized" reported by
  'kernel test robot' and Dan Carpenter
- rebased on top of other changes (dev_attr, get_config(), notify(), etc.)
- left batch_mapping module parameter in the core [Jason]

v1:
- Removed unused headers
- Removed empty module_init() module_exit()
- Moved vdpasim_is_little_endian() in vdpa_sim.h
- Moved vdpasim16_to_cpu/cpu_to_vdpasim16() in vdpa_sim.h
- Added vdpasim*_to_cpu/cpu_to_vdpasim*() also for 32 and 64
- Replaced 'select VDPA_SIM' with 'depends on VDPA_SIM' since selected
  option can not depend on other [Jason]
---
 drivers/vdpa/vdpa_sim/vdpa_sim.h     | 105 +++++++++++++
 drivers/vdpa/vdpa_sim/vdpa_sim.c     | 224 +--------------------------
 drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 171 ++++++++++++++++++++
 drivers/vdpa/Kconfig                 |  13 +-
 drivers/vdpa/vdpa_sim/Makefile       |   1 +
 5 files changed, 292 insertions(+), 222 deletions(-)
 create mode 100644 drivers/vdpa/vdpa_sim/vdpa_sim.h
 create mode 100644 drivers/vdpa/vdpa_sim/vdpa_sim_net.c

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.h b/drivers/vdpa/vdpa_sim/vdpa_sim.h
new file mode 100644
index 000000000000..b02142293d5b
--- /dev/null
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.h
@@ -0,0 +1,105 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2020, Red Hat Inc. All rights reserved.
+ */
+
+#ifndef _VDPA_SIM_H
+#define _VDPA_SIM_H
+
+#include <linux/vringh.h>
+#include <linux/vdpa.h>
+#include <linux/virtio_byteorder.h>
+#include <linux/vhost_iotlb.h>
+#include <uapi/linux/virtio_config.h>
+
+#define VDPASIM_FEATURES	((1ULL << VIRTIO_F_ANY_LAYOUT) | \
+				 (1ULL << VIRTIO_F_VERSION_1)  | \
+				 (1ULL << VIRTIO_F_ACCESS_PLATFORM))
+
+struct vdpasim;
+
+struct vdpasim_virtqueue {
+	struct vringh vring;
+	struct vringh_kiov in_iov;
+	struct vringh_kiov out_iov;
+	unsigned short head;
+	bool ready;
+	u64 desc_addr;
+	u64 device_addr;
+	u64 driver_addr;
+	u32 num;
+	void *private;
+	irqreturn_t (*cb)(void *data);
+};
+
+struct vdpasim_dev_attr {
+	u64 supported_features;
+	size_t config_size;
+	size_t buffer_size;
+	int nvqs;
+	u32 id;
+
+	work_func_t work_fn;
+	void (*get_config)(struct vdpasim *vdpasim, void *config);
+	void (*set_config)(struct vdpasim *vdpasim, const void *config);
+};
+
+/* State of each vdpasim device */
+struct vdpasim {
+	struct vdpa_device vdpa;
+	struct vdpasim_virtqueue *vqs;
+	struct work_struct work;
+	struct vdpasim_dev_attr dev_attr;
+	/* spinlock to synchronize virtqueue state */
+	spinlock_t lock;
+	/* virtio config according to device type */
+	void *config;
+	struct vhost_iotlb *iommu;
+	void *buffer;
+	u32 status;
+	u32 generation;
+	u64 features;
+	/* spinlock to synchronize iommu table */
+	spinlock_t iommu_lock;
+};
+
+struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *attr);
+
+/* TODO: cross-endian support */
+static inline bool vdpasim_is_little_endian(struct vdpasim *vdpasim)
+{
+	return virtio_legacy_is_little_endian() ||
+		(vdpasim->features & (1ULL << VIRTIO_F_VERSION_1));
+}
+
+static inline u16 vdpasim16_to_cpu(struct vdpasim *vdpasim, __virtio16 val)
+{
+	return __virtio16_to_cpu(vdpasim_is_little_endian(vdpasim), val);
+}
+
+static inline __virtio16 cpu_to_vdpasim16(struct vdpasim *vdpasim, u16 val)
+{
+	return __cpu_to_virtio16(vdpasim_is_little_endian(vdpasim), val);
+}
+
+static inline u32 vdpasim32_to_cpu(struct vdpasim *vdpasim, __virtio32 val)
+{
+	return __virtio32_to_cpu(vdpasim_is_little_endian(vdpasim), val);
+}
+
+static inline __virtio32 cpu_to_vdpasim32(struct vdpasim *vdpasim, u32 val)
+{
+	return __cpu_to_virtio32(vdpasim_is_little_endian(vdpasim), val);
+}
+
+static inline u64 vdpasim64_to_cpu(struct vdpasim *vdpasim, __virtio64 val)
+{
+	return __virtio64_to_cpu(vdpasim_is_little_endian(vdpasim), val);
+}
+
+static inline __virtio64 cpu_to_vdpasim64(struct vdpasim *vdpasim, u64 val)
+{
+	return __cpu_to_virtio64(vdpasim_is_little_endian(vdpasim), val);
+}
+
+#endif
diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index 557b2129b41b..7ff074ea04ea 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * VDPA networking device simulator.
+ * VDPA device simulator core.
  *
  * Copyright (c) 2020, Red Hat Inc. All rights reserved.
  *     Author: Jason Wang <jasowang@redhat.com>
@@ -11,107 +11,21 @@
 #include <linux/device.h>
 #include <linux/slab.h>
 #include <linux/dma-map-ops.h>
-#include <linux/etherdevice.h>
-#include <linux/vringh.h>
-#include <linux/vdpa.h>
-#include <linux/virtio_byteorder.h>
-#include <linux/vhost_iotlb.h>
-#include <uapi/linux/virtio_config.h>
-#include <uapi/linux/virtio_net.h>
+
+#include "vdpa_sim.h"
 
 #define DRV_VERSION  "0.1"
 #define DRV_AUTHOR   "Jason Wang <jasowang@redhat.com>"
-#define DRV_DESC     "vDPA Device Simulator"
+#define DRV_DESC     "vDPA Device Simulator core"
 #define DRV_LICENSE  "GPL v2"
 
 static int batch_mapping = 1;
 module_param(batch_mapping, int, 0444);
 MODULE_PARM_DESC(batch_mapping, "Batched mapping 1 -Enable; 0 - Disable");
 
-static char *macaddr;
-module_param(macaddr, charp, 0);
-MODULE_PARM_DESC(macaddr, "Ethernet MAC address");
-
-u8 macaddr_buf[ETH_ALEN];
-
-struct vdpasim_virtqueue {
-	struct vringh vring;
-	struct vringh_kiov in_iov;
-	struct vringh_kiov out_iov;
-	unsigned short head;
-	bool ready;
-	u64 desc_addr;
-	u64 device_addr;
-	u64 driver_addr;
-	u32 num;
-	void *private;
-	irqreturn_t (*cb)(void *data);
-};
-
 #define VDPASIM_QUEUE_ALIGN PAGE_SIZE
 #define VDPASIM_QUEUE_MAX 256
 #define VDPASIM_VENDOR_ID 0
-#define VDPASIM_VQ_NUM 0x2
-#define VDPASIM_NAME "vdpasim-netdev"
-
-#define VDPASIM_FEATURES	((1ULL << VIRTIO_F_ANY_LAYOUT) | \
-				 (1ULL << VIRTIO_F_VERSION_1)  | \
-				 (1ULL << VIRTIO_F_ACCESS_PLATFORM))
-
-#define VDPASIM_NET_FEATURES	(VDPASIM_FEATURES | \
-				 (1ULL << VIRTIO_NET_F_MAC))
-
-struct vdpasim;
-
-struct vdpasim_dev_attr {
-	u64 supported_features;
-	size_t config_size;
-	size_t buffer_size;
-	int nvqs;
-	u32 id;
-
-	work_func_t work_fn;
-	void (*get_config)(struct vdpasim *vdpasim, void *config);
-	void (*set_config)(struct vdpasim *vdpasim, const void *config);
-};
-
-/* State of each vdpasim device */
-struct vdpasim {
-	struct vdpa_device vdpa;
-	struct vdpasim_virtqueue *vqs;
-	struct work_struct work;
-	struct vdpasim_dev_attr dev_attr;
-	/* spinlock to synchronize virtqueue state */
-	spinlock_t lock;
-	/* virtio config according to device type */
-	void *config;
-	struct vhost_iotlb *iommu;
-	void *buffer;
-	u32 status;
-	u32 generation;
-	u64 features;
-	/* spinlock to synchronize iommu table */
-	spinlock_t iommu_lock;
-};
-
-/* TODO: cross-endian support */
-static inline bool vdpasim_is_little_endian(struct vdpasim *vdpasim)
-{
-	return virtio_legacy_is_little_endian() ||
-		(vdpasim->features & (1ULL << VIRTIO_F_VERSION_1));
-}
-
-static inline u16 vdpasim16_to_cpu(struct vdpasim *vdpasim, __virtio16 val)
-{
-	return __virtio16_to_cpu(vdpasim_is_little_endian(vdpasim), val);
-}
-
-static inline __virtio16 cpu_to_vdpasim16(struct vdpasim *vdpasim, u16 val)
-{
-	return __cpu_to_virtio16(vdpasim_is_little_endian(vdpasim), val);
-}
-
-static struct vdpasim *vdpasim_dev;
 
 static struct vdpasim *vdpa_to_sim(struct vdpa_device *vdpa)
 {
@@ -182,80 +96,6 @@ static void vdpasim_reset(struct vdpasim *vdpasim)
 	++vdpasim->generation;
 }
 
-static void vdpasim_net_work(struct work_struct *work)
-{
-	struct vdpasim *vdpasim = container_of(work, struct
-						 vdpasim, work);
-	struct vdpasim_virtqueue *txq = &vdpasim->vqs[1];
-	struct vdpasim_virtqueue *rxq = &vdpasim->vqs[0];
-	ssize_t read, write;
-	size_t total_write;
-	int pkts = 0;
-	int err;
-
-	spin_lock(&vdpasim->lock);
-
-	if (!(vdpasim->status & VIRTIO_CONFIG_S_DRIVER_OK))
-		goto out;
-
-	if (!txq->ready || !rxq->ready)
-		goto out;
-
-	while (true) {
-		total_write = 0;
-		err = vringh_getdesc_iotlb(&txq->vring, &txq->out_iov, NULL,
-					   &txq->head, GFP_ATOMIC);
-		if (err <= 0)
-			break;
-
-		err = vringh_getdesc_iotlb(&rxq->vring, NULL, &rxq->in_iov,
-					   &rxq->head, GFP_ATOMIC);
-		if (err <= 0) {
-			vringh_complete_iotlb(&txq->vring, txq->head, 0);
-			break;
-		}
-
-		while (true) {
-			read = vringh_iov_pull_iotlb(&txq->vring, &txq->out_iov,
-						     vdpasim->buffer,
-						     PAGE_SIZE);
-			if (read <= 0)
-				break;
-
-			write = vringh_iov_push_iotlb(&rxq->vring, &rxq->in_iov,
-						      vdpasim->buffer, read);
-			if (write <= 0)
-				break;
-
-			total_write += write;
-		}
-
-		/* Make sure data is wrote before advancing index */
-		smp_wmb();
-
-		vringh_complete_iotlb(&txq->vring, txq->head, 0);
-		vringh_complete_iotlb(&rxq->vring, rxq->head, total_write);
-
-		/* Make sure used is visible before rasing the interrupt. */
-		smp_wmb();
-
-		local_bh_disable();
-		if (vringh_need_notify_iotlb(&txq->vring) > 0)
-			vringh_notify(&txq->vring);
-		if (vringh_need_notify_iotlb(&rxq->vring) > 0)
-			vringh_notify(&rxq->vring);
-		local_bh_enable();
-
-		if (++pkts > 4) {
-			schedule_work(&vdpasim->work);
-			goto out;
-		}
-	}
-
-out:
-	spin_unlock(&vdpasim->lock);
-}
-
 static int dir_to_perm(enum dma_data_direction dir)
 {
 	int perm = -EFAULT;
@@ -371,7 +211,7 @@ static const struct dma_map_ops vdpasim_dma_ops = {
 static const struct vdpa_config_ops vdpasim_config_ops;
 static const struct vdpa_config_ops vdpasim_batch_config_ops;
 
-static struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr)
+struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr)
 {
 	const struct vdpa_config_ops *ops;
 	struct vdpasim *vdpasim;
@@ -416,23 +256,10 @@ static struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr)
 	if (!vdpasim->buffer)
 		goto err_iommu;
 
-	if (macaddr) {
-		mac_pton(macaddr, macaddr_buf);
-		if (!is_valid_ether_addr(macaddr_buf)) {
-			ret = -EADDRNOTAVAIL;
-			goto err_iommu;
-		}
-	} else {
-		eth_random_addr(macaddr_buf);
-	}
-
 	for (i = 0; i < dev_attr->nvqs; i++)
 		vringh_set_iotlb(&vdpasim->vqs[i].vring, vdpasim->iommu);
 
 	vdpasim->vdpa.dma_dev = dev;
-	ret = vdpa_register_device(&vdpasim->vdpa);
-	if (ret)
-		goto err_iommu;
 
 	return vdpasim;
 
@@ -441,6 +268,7 @@ static struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr)
 err_alloc:
 	return ERR_PTR(ret);
 }
+EXPORT_SYMBOL_GPL(vdpasim_create);
 
 static int vdpasim_set_vq_address(struct vdpa_device *vdpa, u16 idx,
 				  u64 desc_area, u64 driver_area,
@@ -761,46 +589,6 @@ static const struct vdpa_config_ops vdpasim_batch_config_ops = {
 	.free                   = vdpasim_free,
 };
 
-static void vdpasim_net_get_config(struct vdpasim *vdpasim, void *config)
-{
-	struct virtio_net_config *net_config =
-		(struct virtio_net_config *)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 int __init vdpasim_dev_init(void)
-{
-	struct vdpasim_dev_attr dev_attr = {};
-
-	dev_attr.id = VIRTIO_ID_NET;
-	dev_attr.supported_features = VDPASIM_NET_FEATURES;
-	dev_attr.nvqs = VDPASIM_VQ_NUM;
-	dev_attr.config_size = sizeof(struct virtio_net_config);
-	dev_attr.get_config = vdpasim_net_get_config;
-	dev_attr.work_fn = vdpasim_net_work;
-	dev_attr.buffer_size = PAGE_SIZE;
-
-	vdpasim_dev = vdpasim_create(&dev_attr);
-
-	if (!IS_ERR(vdpasim_dev))
-		return 0;
-
-	return PTR_ERR(vdpasim_dev);
-}
-
-static void __exit vdpasim_dev_exit(void)
-{
-	struct vdpa_device *vdpa = &vdpasim_dev->vdpa;
-
-	vdpa_unregister_device(vdpa);
-}
-
-module_init(vdpasim_dev_init)
-module_exit(vdpasim_dev_exit)
-
 MODULE_VERSION(DRV_VERSION);
 MODULE_LICENSE(DRV_LICENSE);
 MODULE_AUTHOR(DRV_AUTHOR);
diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
new file mode 100644
index 000000000000..d278f6bd34ac
--- /dev/null
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
@@ -0,0 +1,171 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * VDPA simulator for networking device.
+ *
+ * Copyright (c) 2020, Red Hat Inc. All rights reserved.
+ *     Author: Jason Wang <jasowang@redhat.com>
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/etherdevice.h>
+#include <uapi/linux/virtio_net.h>
+
+#include "vdpa_sim.h"
+
+#define DRV_VERSION  "0.1"
+#define DRV_AUTHOR   "Jason Wang <jasowang@redhat.com>"
+#define DRV_DESC     "vDPA Device Simulator for networking device"
+#define DRV_LICENSE  "GPL v2"
+
+#define VDPASIM_NET_FEATURES	(VDPASIM_FEATURES | \
+				 (1ULL << VIRTIO_NET_F_MAC))
+
+#define VDPASIM_NET_VQ_NUM	2
+
+static char *macaddr;
+module_param(macaddr, charp, 0);
+MODULE_PARM_DESC(macaddr, "Ethernet MAC address");
+
+u8 macaddr_buf[ETH_ALEN];
+
+static struct vdpasim *vdpasim_net_dev;
+
+static void vdpasim_net_work(struct work_struct *work)
+{
+	struct vdpasim *vdpasim = container_of(work, struct vdpasim, work);
+	struct vdpasim_virtqueue *txq = &vdpasim->vqs[1];
+	struct vdpasim_virtqueue *rxq = &vdpasim->vqs[0];
+	ssize_t read, write;
+	size_t total_write;
+	int pkts = 0;
+	int err;
+
+	spin_lock(&vdpasim->lock);
+
+	if (!(vdpasim->status & VIRTIO_CONFIG_S_DRIVER_OK))
+		goto out;
+
+	if (!txq->ready || !rxq->ready)
+		goto out;
+
+	while (true) {
+		total_write = 0;
+		err = vringh_getdesc_iotlb(&txq->vring, &txq->out_iov, NULL,
+					   &txq->head, GFP_ATOMIC);
+		if (err <= 0)
+			break;
+
+		err = vringh_getdesc_iotlb(&rxq->vring, NULL, &rxq->in_iov,
+					   &rxq->head, GFP_ATOMIC);
+		if (err <= 0) {
+			vringh_complete_iotlb(&txq->vring, txq->head, 0);
+			break;
+		}
+
+		while (true) {
+			read = vringh_iov_pull_iotlb(&txq->vring, &txq->out_iov,
+						     vdpasim->buffer,
+						     PAGE_SIZE);
+			if (read <= 0)
+				break;
+
+			write = vringh_iov_push_iotlb(&rxq->vring, &rxq->in_iov,
+						      vdpasim->buffer, read);
+			if (write <= 0)
+				break;
+
+			total_write += write;
+		}
+
+		/* Make sure data is wrote before advancing index */
+		smp_wmb();
+
+		vringh_complete_iotlb(&txq->vring, txq->head, 0);
+		vringh_complete_iotlb(&rxq->vring, rxq->head, total_write);
+
+		/* Make sure used is visible before rasing the interrupt. */
+		smp_wmb();
+
+		local_bh_disable();
+		if (vringh_need_notify_iotlb(&txq->vring) > 0)
+			vringh_notify(&txq->vring);
+		if (vringh_need_notify_iotlb(&rxq->vring) > 0)
+			vringh_notify(&rxq->vring);
+		local_bh_enable();
+
+		if (++pkts > 4) {
+			schedule_work(&vdpasim->work);
+			goto out;
+		}
+	}
+
+out:
+	spin_unlock(&vdpasim->lock);
+}
+
+static void vdpasim_net_get_config(struct vdpasim *vdpasim, void *config)
+{
+	struct virtio_net_config *net_config =
+		(struct virtio_net_config *)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 int __init vdpasim_net_init(void)
+{
+	struct vdpasim_dev_attr dev_attr = {};
+	int ret;
+
+	if (macaddr) {
+		mac_pton(macaddr, macaddr_buf);
+		if (!is_valid_ether_addr(macaddr_buf)) {
+			ret = -EADDRNOTAVAIL;
+			goto out;
+		}
+	} else {
+		eth_random_addr(macaddr_buf);
+	}
+
+	dev_attr.id = VIRTIO_ID_NET;
+	dev_attr.supported_features = VDPASIM_NET_FEATURES;
+	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.work_fn = vdpasim_net_work;
+	dev_attr.buffer_size = PAGE_SIZE;
+
+	vdpasim_net_dev = vdpasim_create(&dev_attr);
+	if (IS_ERR(vdpasim_net_dev)) {
+		ret = PTR_ERR(vdpasim_net_dev);
+		goto out;
+	}
+
+	ret = vdpa_register_device(&vdpasim_net_dev->vdpa);
+	if (ret)
+		goto put_dev;
+
+	return 0;
+
+put_dev:
+	put_device(&vdpasim_net_dev->vdpa.dev);
+out:
+	return ret;
+}
+
+static void __exit vdpasim_net_exit(void)
+{
+	struct vdpa_device *vdpa = &vdpasim_net_dev->vdpa;
+
+	vdpa_unregister_device(vdpa);
+}
+
+module_init(vdpasim_net_init);
+module_exit(vdpasim_net_exit);
+
+MODULE_VERSION(DRV_VERSION);
+MODULE_LICENSE(DRV_LICENSE);
+MODULE_AUTHOR(DRV_AUTHOR);
+MODULE_DESCRIPTION(DRV_DESC);
diff --git a/drivers/vdpa/Kconfig b/drivers/vdpa/Kconfig
index 2c892e890b9e..b0f91ad8eb47 100644
--- a/drivers/vdpa/Kconfig
+++ b/drivers/vdpa/Kconfig
@@ -9,15 +9,20 @@ menuconfig VDPA
 if VDPA
 
 config VDPA_SIM
-	tristate "vDPA device simulator"
+	tristate "vDPA device simulator core"
 	depends on RUNTIME_TESTING_MENU && HAS_DMA
 	select DMA_OPS
 	select VHOST_RING
+	help
+	  Enable this module to support vDPA device simulators. These devices
+	  are used for testing, prototyping and development of vDPA.
+
+config VDPA_SIM_NET
+	tristate "vDPA simulator for networking device"
+	depends on VDPA_SIM
 	select GENERIC_NET_UTILS
 	help
-	  vDPA networking device simulator which loop TX traffic back
-	  to RX. This device is used for testing, prototyping and
-	  development of vDPA.
+	  vDPA networking device simulator which loop TX traffic back to RX.
 
 config IFCVF
 	tristate "Intel IFC VF vDPA driver"
diff --git a/drivers/vdpa/vdpa_sim/Makefile b/drivers/vdpa/vdpa_sim/Makefile
index b40278f65e04..79d4536d347e 100644
--- a/drivers/vdpa/vdpa_sim/Makefile
+++ b/drivers/vdpa/vdpa_sim/Makefile
@@ -1,2 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_VDPA_SIM) += vdpa_sim.o
+obj-$(CONFIG_VDPA_SIM_NET) += vdpa_sim_net.o
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 19/19] vdpa: split vdpasim to core and net modules
  2020-12-03 17:05 ` [PATCH v3 19/19] vdpa: split vdpasim to core and net modules Stefano Garzarella
@ 2020-12-03 17:25   ` Randy Dunlap
  2020-12-04  7:48     ` Stefano Garzarella
  2020-12-07  5:33   ` Jason Wang
  1 sibling, 1 reply; 35+ messages in thread
From: Randy Dunlap @ 2020-12-03 17:25 UTC (permalink / raw)
  To: Stefano Garzarella, virtualization
  Cc: Stefan Hajnoczi, Michael S. Tsirkin, Oren Duer, Jason Wang,
	Laurent Vivier, linux-kernel, Max Gurtovoy, Shahaf Shuler,
	Eli Cohen

Hi,

On 12/3/20 9:05 AM, Stefano Garzarella wrote:
> diff --git a/drivers/vdpa/Kconfig b/drivers/vdpa/Kconfig
> index 2c892e890b9e..b0f91ad8eb47 100644
> --- a/drivers/vdpa/Kconfig
> +++ b/drivers/vdpa/Kconfig
> @@ -9,15 +9,20 @@ menuconfig VDPA
>  if VDPA
>  
>  config VDPA_SIM
> -	tristate "vDPA device simulator"
> +	tristate "vDPA device simulator core"
>  	depends on RUNTIME_TESTING_MENU && HAS_DMA
>  	select DMA_OPS
>  	select VHOST_RING
> +	help
> +	  Enable this module to support vDPA device simulators. These devices
> +	  are used for testing, prototyping and development of vDPA.
> +
> +config VDPA_SIM_NET
> +	tristate "vDPA simulator for networking device"
> +	depends on VDPA_SIM
>  	select GENERIC_NET_UTILS
>  	help
> -	  vDPA networking device simulator which loop TX traffic back
> -	  to RX. This device is used for testing, prototyping and
> -	  development of vDPA.
> +	  vDPA networking device simulator which loop TX traffic back to RX.

	                                         loops


thanks.
-- 
~Randy


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 02/19] vdpa_sim: remove unnecessary headers inclusion
  2020-12-03 17:04 ` [PATCH v3 02/19] vdpa_sim: remove unnecessary headers inclusion Stefano Garzarella
@ 2020-12-03 17:37   ` Randy Dunlap
  2020-12-04  7:57     ` Stefano Garzarella
  0 siblings, 1 reply; 35+ messages in thread
From: Randy Dunlap @ 2020-12-03 17:37 UTC (permalink / raw)
  To: Stefano Garzarella, virtualization
  Cc: Stefan Hajnoczi, Michael S. Tsirkin, Oren Duer, Jason Wang,
	Laurent Vivier, linux-kernel, Max Gurtovoy, Shahaf Shuler,
	Eli Cohen

On 12/3/20 9:04 AM, Stefano Garzarella wrote:
> Some headers are not necessary, so let's remove them to do
> some cleaning.
> 
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>

Hi,
What makes you say that some of these are unnecessary?

Please use Rule #1 from Documentation/process/submit-checklist.rst:

1) If you use a facility then #include the file that defines/declares
   that facility.  Don't depend on other header files pulling in ones
   that you use.


so just because it will compile without these headers being explictly
#included does not mean that you should remove them.


> ---
> v3:
> - avoided to remove some headers with structures and functions directly
>   used (device.h, slab.h, virtio_byteorder.h)[Jason]
> ---
>  drivers/vdpa/vdpa_sim/vdpa_sim.c | 10 ----------
>  1 file changed, 10 deletions(-)
> 
> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> index 6a90fdb9cbfc..b08f28d20d8d 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> @@ -7,20 +7,10 @@
>   *
>   */
>  
> -#include <linux/init.h>

above is used by __init and __exit.

>  #include <linux/module.h>
>  #include <linux/device.h>
> -#include <linux/kernel.h>
> -#include <linux/fs.h>
> -#include <linux/poll.h>

Looks OK to remove poll.h.

>  #include <linux/slab.h>
> -#include <linux/sched.h>

Might be OK for sched.h.

> -#include <linux/wait.h>

Might be OK for wait.h.

> -#include <linux/uuid.h>
> -#include <linux/iommu.h>
>  #include <linux/dma-map-ops.h>
> -#include <linux/sysfs.h>
> -#include <linux/file.h>
>  #include <linux/etherdevice.h>
>  #include <linux/vringh.h>
>  #include <linux/vdpa.h>
> 

I didn't check the others.


-- 
~Randy


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 19/19] vdpa: split vdpasim to core and net modules
  2020-12-03 17:25   ` Randy Dunlap
@ 2020-12-04  7:48     ` Stefano Garzarella
  0 siblings, 0 replies; 35+ messages in thread
From: Stefano Garzarella @ 2020-12-04  7:48 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: virtualization, Stefan Hajnoczi, Michael S. Tsirkin, Oren Duer,
	Jason Wang, Laurent Vivier, linux-kernel, Max Gurtovoy,
	Shahaf Shuler, Eli Cohen

On Thu, Dec 03, 2020 at 09:25:52AM -0800, Randy Dunlap wrote:
>Hi,
>
>On 12/3/20 9:05 AM, Stefano Garzarella wrote:
>> diff --git a/drivers/vdpa/Kconfig b/drivers/vdpa/Kconfig
>> index 2c892e890b9e..b0f91ad8eb47 100644
>> --- a/drivers/vdpa/Kconfig
>> +++ b/drivers/vdpa/Kconfig
>> @@ -9,15 +9,20 @@ menuconfig VDPA
>>  if VDPA
>>
>>  config VDPA_SIM
>> -	tristate "vDPA device simulator"
>> +	tristate "vDPA device simulator core"
>>  	depends on RUNTIME_TESTING_MENU && HAS_DMA
>>  	select DMA_OPS
>>  	select VHOST_RING
>> +	help
>> +	  Enable this module to support vDPA device simulators. These devices
>> +	  are used for testing, prototyping and development of vDPA.
>> +
>> +config VDPA_SIM_NET
>> +	tristate "vDPA simulator for networking device"
>> +	depends on VDPA_SIM
>>  	select GENERIC_NET_UTILS
>>  	help
>> -	  vDPA networking device simulator which loop TX traffic back
>> -	  to RX. This device is used for testing, prototyping and
>> -	  development of vDPA.
>> +	  vDPA networking device simulator which loop TX traffic back to RX.
>
>	                                         loops

It was pre-existing, but since I'm there I'll fix it, thanks!

Stefano


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 02/19] vdpa_sim: remove unnecessary headers inclusion
  2020-12-03 17:37   ` Randy Dunlap
@ 2020-12-04  7:57     ` Stefano Garzarella
  0 siblings, 0 replies; 35+ messages in thread
From: Stefano Garzarella @ 2020-12-04  7:57 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: virtualization, Stefan Hajnoczi, Michael S. Tsirkin, Oren Duer,
	Jason Wang, Laurent Vivier, linux-kernel, Max Gurtovoy,
	Shahaf Shuler, Eli Cohen

Hi Randy,

On Thu, Dec 03, 2020 at 09:37:48AM -0800, Randy Dunlap wrote:
>On 12/3/20 9:04 AM, Stefano Garzarella wrote:
>> Some headers are not necessary, so let's remove them to do
>> some cleaning.
>>
>> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>
>Hi,
>What makes you say that some of these are unnecessary?
>
>Please use Rule #1 from Documentation/process/submit-checklist.rst:
>
>1) If you use a facility then #include the file that defines/declares
>   that facility.  Don't depend on other header files pulling in ones
>   that you use.
>
>
>so just because it will compile without these headers being explictly
>#included does not mean that you should remove them.

Thanks for the clarification. I tried to follow this rule already 
pointed out by Jason, but of course I missed the __init and __exit 
macros...

I'll check better for the next version!

>
>
>> ---
>> v3:
>> - avoided to remove some headers with structures and functions directly
>>   used (device.h, slab.h, virtio_byteorder.h)[Jason]
>> ---
>>  drivers/vdpa/vdpa_sim/vdpa_sim.c | 10 ----------
>>  1 file changed, 10 deletions(-)
>>
>> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
>> index 6a90fdb9cbfc..b08f28d20d8d 100644
>> --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
>> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
>> @@ -7,20 +7,10 @@
>>   *
>>   */
>>
>> -#include <linux/init.h>
>
>above is used by __init and __exit.
>
>>  #include <linux/module.h>
>>  #include <linux/device.h>
>> -#include <linux/kernel.h>
>> -#include <linux/fs.h>
>> -#include <linux/poll.h>
>
>Looks OK to remove poll.h.
>
>>  #include <linux/slab.h>
>> -#include <linux/sched.h>
>
>Might be OK for sched.h.
>
>> -#include <linux/wait.h>
>
>Might be OK for wait.h.
>
>> -#include <linux/uuid.h>
>> -#include <linux/iommu.h>
>>  #include <linux/dma-map-ops.h>
>> -#include <linux/sysfs.h>
>> -#include <linux/file.h>
>>  #include <linux/etherdevice.h>
>>  #include <linux/vringh.h>
>>  #include <linux/vdpa.h>
>>
>
>I didn't check the others.
>

Thanks,
Stefano


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 04/19] vhost/iotlb: add VHOST_IOTLB_UNLIMITED macro
  2020-12-03 17:04 ` [PATCH v3 04/19] vhost/iotlb: add VHOST_IOTLB_UNLIMITED macro Stefano Garzarella
@ 2020-12-07  3:55   ` Jason Wang
  0 siblings, 0 replies; 35+ messages in thread
From: Jason Wang @ 2020-12-07  3:55 UTC (permalink / raw)
  To: Stefano Garzarella, virtualization
  Cc: Stefan Hajnoczi, Michael S. Tsirkin, Oren Duer, Laurent Vivier,
	linux-kernel, Max Gurtovoy, Shahaf Shuler, Eli Cohen


On 2020/12/4 上午1:04, Stefano Garzarella wrote:
> It's possible to allocate an unlimited IOTLB calling
> vhost_iotlb_alloc() with 'limit' = 0.
>
> Add a new macro (VHOST_IOTLB_UNLIMITED) for this case and document
> it in the vhost_iotlb_alloc() documentation block.
>
> Suggested-by: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>


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


> ---
>   include/linux/vhost_iotlb.h | 2 ++
>   drivers/vhost/iotlb.c       | 3 ++-
>   2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/vhost_iotlb.h b/include/linux/vhost_iotlb.h
> index 6b09b786a762..47019f97f795 100644
> --- a/include/linux/vhost_iotlb.h
> +++ b/include/linux/vhost_iotlb.h
> @@ -4,6 +4,8 @@
>   
>   #include <linux/interval_tree_generic.h>
>   
> +#define VHOST_IOTLB_UNLIMITED 0
> +
>   struct vhost_iotlb_map {
>   	struct rb_node rb;
>   	struct list_head link;
> diff --git a/drivers/vhost/iotlb.c b/drivers/vhost/iotlb.c
> index 0fd3f87e913c..80fdde78ee5a 100644
> --- a/drivers/vhost/iotlb.c
> +++ b/drivers/vhost/iotlb.c
> @@ -100,7 +100,8 @@ EXPORT_SYMBOL_GPL(vhost_iotlb_del_range);
>   
>   /**
>    * vhost_iotlb_alloc - add a new vhost IOTLB
> - * @limit: maximum number of IOTLB entries
> + * @limit: maximum number of IOTLB entries (use VHOST_IOTLB_UNLIMITED for an
> + *         unlimited IOTLB)
>    * @flags: VHOST_IOTLB_FLAG_XXX
>    *
>    * Returns an error is memory allocation fails


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 05/19] vdpa_sim: remove the limit of IOTLB entries
  2020-12-03 17:04 ` [PATCH v3 05/19] vdpa_sim: remove the limit of IOTLB entries Stefano Garzarella
@ 2020-12-07  4:00   ` Jason Wang
  2020-12-09 10:58     ` Stefano Garzarella
  0 siblings, 1 reply; 35+ messages in thread
From: Jason Wang @ 2020-12-07  4:00 UTC (permalink / raw)
  To: Stefano Garzarella, virtualization
  Cc: Stefan Hajnoczi, Michael S. Tsirkin, Oren Duer, Laurent Vivier,
	linux-kernel, Max Gurtovoy, Shahaf Shuler, Eli Cohen


On 2020/12/4 上午1:04, Stefano Garzarella wrote:
> The simulated devices can support multiple queues, so this limit
> should be defined according to the number of queues supported by
> the device.
>
> Since we are in a simulator, let's simply remove that limit.
>
> Suggested-by: Jason Wang <jasowang@redhat.com>
> Acked-by: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>


Rethink about this, since simulator can be used by VM, so the allocation 
is actually guest trigger-able when vIOMMU is enabled.

This means we need a limit somehow, (e.g I remember swiotlb is about 
64MB by default). Or having a module parameter for this.

Btw, have you met any issue when using 2048, I guess it can happen when 
we run several processes in parallel?


> ---
> v3:
> - used VHOST_IOTLB_UNLIMITED macro [Jason]
> v2:
> - added VDPASIM_IOTLB_LIMIT macro [Jason]
> ---
>   drivers/vdpa/vdpa_sim/vdpa_sim.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> index 295a770caac0..688aceaa6543 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> @@ -368,7 +368,7 @@ static struct vdpasim *vdpasim_create(void)
>   	if (!vdpasim->vqs)
>   		goto err_iommu;
>   
> -	vdpasim->iommu = vhost_iotlb_alloc(2048, 0);
> +	vdpasim->iommu = vhost_iotlb_alloc(VHOST_IOTLB_UNLIMITED, 0);
>   	if (!vdpasim->iommu)
>   		goto err_iommu;
>   


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 12/19] vdpa_sim: make 'config' generic and usable for any device type
  2020-12-03 17:05 ` [PATCH v3 12/19] vdpa_sim: make 'config' generic and usable for any device type Stefano Garzarella
@ 2020-12-07  5:22   ` Jason Wang
  0 siblings, 0 replies; 35+ messages in thread
From: Jason Wang @ 2020-12-07  5:22 UTC (permalink / raw)
  To: Stefano Garzarella, virtualization
  Cc: Stefan Hajnoczi, Michael S. Tsirkin, Oren Duer, Laurent Vivier,
	linux-kernel, Max Gurtovoy, Shahaf Shuler, Eli Cohen


On 2020/12/4 上午1:05, Stefano Garzarella wrote:
> Add new 'config_size' attribute in 'vdpasim_dev_attr' and allocates
> 'config' dynamically to support any device types.
>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>   drivers/vdpa/vdpa_sim/vdpa_sim.c | 17 +++++++++++++----
>   1 file changed, 13 insertions(+), 4 deletions(-)


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


>
> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> index 949f4231d08a..fe71ed7890e1 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> @@ -62,6 +62,7 @@ struct vdpasim_virtqueue {
>   
>   struct vdpasim_dev_attr {
>   	u64 supported_features;
> +	size_t config_size;
>   	int nvqs;
>   	u32 id;
>   
> @@ -76,7 +77,8 @@ struct vdpasim {
>   	struct vdpasim_dev_attr dev_attr;
>   	/* spinlock to synchronize virtqueue state */
>   	spinlock_t lock;
> -	struct virtio_net_config config;
> +	/* virtio config according to device type */
> +	void *config;
>   	struct vhost_iotlb *iommu;
>   	void *buffer;
>   	u32 status;
> @@ -376,6 +378,10 @@ static struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr)
>   		goto err_iommu;
>   	set_dma_ops(dev, &vdpasim_dma_ops);
>   
> +	vdpasim->config = kzalloc(dev_attr->config_size, GFP_KERNEL);
> +	if (!vdpasim->config)
> +		goto err_iommu;
> +
>   	vdpasim->vqs = kcalloc(dev_attr->nvqs, sizeof(struct vdpasim_virtqueue),
>   			       GFP_KERNEL);
>   	if (!vdpasim->vqs)
> @@ -516,7 +522,8 @@ static u64 vdpasim_get_features(struct vdpa_device *vdpa)
>   static int vdpasim_set_features(struct vdpa_device *vdpa, u64 features)
>   {
>   	struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
> -	struct virtio_net_config *config = &vdpasim->config;
> +	struct virtio_net_config *config =
> +		(struct virtio_net_config *)vdpasim->config;
>   
>   	/* DMA mapping must be done by driver */
>   	if (!(features & (1ULL << VIRTIO_F_ACCESS_PLATFORM)))
> @@ -588,8 +595,8 @@ static void vdpasim_get_config(struct vdpa_device *vdpa, unsigned int offset,
>   {
>   	struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
>   
> -	if (offset + len < sizeof(struct virtio_net_config))
> -		memcpy(buf, (u8 *)&vdpasim->config + offset, len);
> +	if (offset + len < vdpasim->dev_attr.config_size)
> +		memcpy(buf, vdpasim->config + offset, len);
>   }
>   
>   static void vdpasim_set_config(struct vdpa_device *vdpa, unsigned int offset,
> @@ -676,6 +683,7 @@ static void vdpasim_free(struct vdpa_device *vdpa)
>   	if (vdpasim->iommu)
>   		vhost_iotlb_free(vdpasim->iommu);
>   	kfree(vdpasim->vqs);
> +	kfree(vdpasim->config);
>   }
>   
>   static const struct vdpa_config_ops vdpasim_config_ops = {
> @@ -738,6 +746,7 @@ static int __init vdpasim_dev_init(void)
>   	dev_attr.id = VIRTIO_ID_NET;
>   	dev_attr.supported_features = VDPASIM_NET_FEATURES;
>   	dev_attr.nvqs = VDPASIM_VQ_NUM;
> +	dev_attr.config_size = sizeof(struct virtio_net_config);
>   	dev_attr.work_fn = vdpasim_net_work;
>   
>   	vdpasim_dev = vdpasim_create(&dev_attr);


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 13/19] vdpa_sim: add get_config callback in vdpasim_dev_attr
  2020-12-03 17:05 ` [PATCH v3 13/19] vdpa_sim: add get_config callback in vdpasim_dev_attr Stefano Garzarella
@ 2020-12-07  5:29   ` Jason Wang
  2020-12-09 11:07     ` Stefano Garzarella
  0 siblings, 1 reply; 35+ messages in thread
From: Jason Wang @ 2020-12-07  5:29 UTC (permalink / raw)
  To: Stefano Garzarella, virtualization
  Cc: Stefan Hajnoczi, Michael S. Tsirkin, Oren Duer, Laurent Vivier,
	linux-kernel, Max Gurtovoy, Shahaf Shuler, Eli Cohen


On 2020/12/4 上午1:05, Stefano Garzarella wrote:
> The get_config callback can be used by the device to fill the
> config structure.
> The callback will be invoked in vdpasim_get_config() before copying
> bytes into caller buffer.
>
> Move vDPA-net config updates from vdpasim_set_features() in the
> new vdpasim_net_get_config() callback.
>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
> v3:
> - checked if get_config callback is set before call it
> ---
>   drivers/vdpa/vdpa_sim/vdpa_sim.c | 35 +++++++++++++++++++-------------
>   1 file changed, 21 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> index fe71ed7890e1..f935ade0806b 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> @@ -60,6 +60,8 @@ struct vdpasim_virtqueue {
>   #define VDPASIM_NET_FEATURES	(VDPASIM_FEATURES | \
>   				 (1ULL << VIRTIO_NET_F_MAC))
>   
> +struct vdpasim;
> +
>   struct vdpasim_dev_attr {
>   	u64 supported_features;
>   	size_t config_size;
> @@ -67,6 +69,7 @@ struct vdpasim_dev_attr {
>   	u32 id;
>   
>   	work_func_t work_fn;
> +	void (*get_config)(struct vdpasim *vdpasim, void *config);
>   };
>   
>   /* State of each vdpasim device */
> @@ -522,8 +525,6 @@ static u64 vdpasim_get_features(struct vdpa_device *vdpa)
>   static int vdpasim_set_features(struct vdpa_device *vdpa, u64 features)
>   {
>   	struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
> -	struct virtio_net_config *config =
> -		(struct virtio_net_config *)vdpasim->config;
>   
>   	/* DMA mapping must be done by driver */
>   	if (!(features & (1ULL << VIRTIO_F_ACCESS_PLATFORM)))
> @@ -531,16 +532,6 @@ static int vdpasim_set_features(struct vdpa_device *vdpa, u64 features)
>   
>   	vdpasim->features = features & vdpasim->dev_attr.supported_features;
>   
> -	/* We generally only know whether guest is using the legacy interface
> -	 * here, so generally that's the earliest we can set config fields.
> -	 * Note: We actually require VIRTIO_F_ACCESS_PLATFORM above which
> -	 * implies VIRTIO_F_VERSION_1, but let's not try to be clever here.
> -	 */
> -
> -	config->mtu = cpu_to_vdpasim16(vdpasim, 1500);
> -	config->status = cpu_to_vdpasim16(vdpasim, VIRTIO_NET_S_LINK_UP);
> -	memcpy(config->mac, macaddr_buf, ETH_ALEN);


Patch looks good to me.

But we need Michael to confirm whether doing moving like this is safe. I 
guess what has been done were trying to make sure get_config() fail 
before set_features(), but it's not clear to me whether it's useful.

Thanks


> -
>   	return 0;
>   }
>   
> @@ -595,8 +586,13 @@ static void vdpasim_get_config(struct vdpa_device *vdpa, unsigned int offset,
>   {
>   	struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
>   
> -	if (offset + len < vdpasim->dev_attr.config_size)
> -		memcpy(buf, vdpasim->config + offset, len);
> +	if (offset + len > vdpasim->dev_attr.config_size)
> +		return;
> +
> +	if (vdpasim->dev_attr.get_config)
> +		vdpasim->dev_attr.get_config(vdpasim, vdpasim->config);
> +
> +	memcpy(buf, vdpasim->config + offset, len);
>   }
>   
>   static void vdpasim_set_config(struct vdpa_device *vdpa, unsigned int offset,
> @@ -739,6 +735,16 @@ static const struct vdpa_config_ops vdpasim_batch_config_ops = {
>   	.free                   = vdpasim_free,
>   };
>   
> +static void vdpasim_net_get_config(struct vdpasim *vdpasim, void *config)
> +{
> +	struct virtio_net_config *net_config =
> +		(struct virtio_net_config *)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 int __init vdpasim_dev_init(void)
>   {
>   	struct vdpasim_dev_attr dev_attr = {};
> @@ -747,6 +753,7 @@ static int __init vdpasim_dev_init(void)
>   	dev_attr.supported_features = VDPASIM_NET_FEATURES;
>   	dev_attr.nvqs = VDPASIM_VQ_NUM;
>   	dev_attr.config_size = sizeof(struct virtio_net_config);
> +	dev_attr.get_config = vdpasim_net_get_config;
>   	dev_attr.work_fn = vdpasim_net_work;
>   
>   	vdpasim_dev = vdpasim_create(&dev_attr);


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 14/19] vdpa_sim: add set_config callback in vdpasim_dev_attr
  2020-12-03 17:05 ` [PATCH v3 14/19] vdpa_sim: add set_config " Stefano Garzarella
@ 2020-12-07  5:30   ` Jason Wang
  0 siblings, 0 replies; 35+ messages in thread
From: Jason Wang @ 2020-12-07  5:30 UTC (permalink / raw)
  To: Stefano Garzarella, virtualization
  Cc: Stefan Hajnoczi, Michael S. Tsirkin, Oren Duer, Laurent Vivier,
	linux-kernel, Max Gurtovoy, Shahaf Shuler, Eli Cohen


On 2020/12/4 上午1:05, Stefano Garzarella wrote:
> The set_config callback can be used by the device to parse the
> config structure modified by the driver.
>
> The callback will be invoked, if set, in vdpasim_set_config() after
> copying bytes from caller buffer into vdpasim->config buffer.
>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>


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


> ---
>   drivers/vdpa/vdpa_sim/vdpa_sim.c | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> index f935ade0806b..03a8717f80ea 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> @@ -70,6 +70,7 @@ 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);
>   };
>   
>   /* State of each vdpasim device */
> @@ -598,7 +599,15 @@ static void vdpasim_get_config(struct vdpa_device *vdpa, unsigned int offset,
>   static void vdpasim_set_config(struct vdpa_device *vdpa, unsigned int offset,
>   			     const void *buf, unsigned int len)
>   {
> -	/* No writable config supportted by vdpasim */
> +	struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
> +
> +	if (offset + len > vdpasim->dev_attr.config_size)
> +		return;
> +
> +	memcpy(vdpasim->config + offset, buf, len);
> +
> +	if (vdpasim->dev_attr.set_config)
> +		vdpasim->dev_attr.set_config(vdpasim, vdpasim->config);
>   }
>   
>   static u32 vdpasim_get_generation(struct vdpa_device *vdpa)


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 15/19] vdpa_sim: set vringh notify callback
  2020-12-03 17:05 ` [PATCH v3 15/19] vdpa_sim: set vringh notify callback Stefano Garzarella
@ 2020-12-07  5:30   ` Jason Wang
  0 siblings, 0 replies; 35+ messages in thread
From: Jason Wang @ 2020-12-07  5:30 UTC (permalink / raw)
  To: Stefano Garzarella, virtualization
  Cc: Stefan Hajnoczi, Michael S. Tsirkin, Oren Duer, Laurent Vivier,
	linux-kernel, Max Gurtovoy, Shahaf Shuler, Eli Cohen


On 2020/12/4 上午1:05, Stefano Garzarella wrote:
> Instead of calling the vq callback directly, we can leverage the
> vringh_notify() function, adding vdpasim_vq_notify() and setting it
> in the vringh notify callback.
>
> Suggested-by: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>


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


> ---
> v3:
> - cleared notify during reset [Jason]
> ---
>   drivers/vdpa/vdpa_sim/vdpa_sim.c | 23 +++++++++++++++++++----
>   1 file changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> index 03a8717f80ea..1243b02488f7 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> @@ -123,6 +123,17 @@ static struct vdpasim *dev_to_sim(struct device *dev)
>   	return vdpa_to_sim(vdpa);
>   }
>   
> +static void vdpasim_vq_notify(struct vringh *vring)
> +{
> +	struct vdpasim_virtqueue *vq =
> +		container_of(vring, struct vdpasim_virtqueue, vring);
> +
> +	if (!vq->cb)
> +		return;
> +
> +	vq->cb(vq->private);
> +}
> +
>   static void vdpasim_queue_ready(struct vdpasim *vdpasim, unsigned int idx)
>   {
>   	struct vdpasim_virtqueue *vq = &vdpasim->vqs[idx];
> @@ -134,6 +145,8 @@ static void vdpasim_queue_ready(struct vdpasim *vdpasim, unsigned int idx)
>   			  (uintptr_t)vq->driver_addr,
>   			  (struct vring_used *)
>   			  (uintptr_t)vq->device_addr);
> +
> +	vq->vring.notify = vdpasim_vq_notify;
>   }
>   
>   static void vdpasim_vq_reset(struct vdpasim *vdpasim,
> @@ -147,6 +160,8 @@ static void vdpasim_vq_reset(struct vdpasim *vdpasim,
>   	vq->private = NULL;
>   	vringh_init_iotlb(&vq->vring, vdpasim->dev_attr.supported_features,
>   			  VDPASIM_QUEUE_MAX, false, NULL, NULL, NULL);
> +
> +	vq->vring.notify = NULL;
>   }
>   
>   static void vdpasim_reset(struct vdpasim *vdpasim)
> @@ -223,10 +238,10 @@ static void vdpasim_net_work(struct work_struct *work)
>   		smp_wmb();
>   
>   		local_bh_disable();
> -		if (txq->cb)
> -			txq->cb(txq->private);
> -		if (rxq->cb)
> -			rxq->cb(rxq->private);
> +		if (vringh_need_notify_iotlb(&txq->vring) > 0)
> +			vringh_notify(&txq->vring);
> +		if (vringh_need_notify_iotlb(&rxq->vring) > 0)
> +			vringh_notify(&rxq->vring);
>   		local_bh_enable();
>   
>   		if (++pkts > 4) {


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 19/19] vdpa: split vdpasim to core and net modules
  2020-12-03 17:05 ` [PATCH v3 19/19] vdpa: split vdpasim to core and net modules Stefano Garzarella
  2020-12-03 17:25   ` Randy Dunlap
@ 2020-12-07  5:33   ` Jason Wang
  1 sibling, 0 replies; 35+ messages in thread
From: Jason Wang @ 2020-12-07  5:33 UTC (permalink / raw)
  To: Stefano Garzarella, virtualization
  Cc: Stefan Hajnoczi, Michael S. Tsirkin, Oren Duer, Laurent Vivier,
	linux-kernel, Max Gurtovoy, Shahaf Shuler, Eli Cohen


On 2020/12/4 上午1:05, Stefano Garzarella wrote:
> From: Max Gurtovoy<mgurtovoy@nvidia.com>
>
> Introduce new vdpa_sim_net and vdpa_sim (core) drivers. This is a
> preparation for adding a vdpa simulator module for block devices.
>
> Signed-off-by: Max Gurtovoy<mgurtovoy@nvidia.com>
> [sgarzare: various cleanups/fixes]
> Signed-off-by: Stefano Garzarella<sgarzare@redhat.com>
> ---
> v2:
> - Fixed "warning: variable 'dev' is used uninitialized" reported by
>    'kernel test robot' and Dan Carpenter
> - rebased on top of other changes (dev_attr, get_config(), notify(), etc.)
> - left batch_mapping module parameter in the core [Jason]
>
> v1:
> - Removed unused headers
> - Removed empty module_init() module_exit()
> - Moved vdpasim_is_little_endian() in vdpa_sim.h
> - Moved vdpasim16_to_cpu/cpu_to_vdpasim16() in vdpa_sim.h
> - Added vdpasim*_to_cpu/cpu_to_vdpasim*() also for 32 and 64
> - Replaced 'select VDPA_SIM' with 'depends on VDPA_SIM' since selected
>    option can not depend on other [Jason]
> ---
>   drivers/vdpa/vdpa_sim/vdpa_sim.h     | 105 +++++++++++++
>   drivers/vdpa/vdpa_sim/vdpa_sim.c     | 224 +--------------------------
>   drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 171 ++++++++++++++++++++
>   drivers/vdpa/Kconfig                 |  13 +-
>   drivers/vdpa/vdpa_sim/Makefile       |   1 +
>   5 files changed, 292 insertions(+), 222 deletions(-)
>   create mode 100644 drivers/vdpa/vdpa_sim/vdpa_sim.h
>   create mode 100644 drivers/vdpa/vdpa_sim/vdpa_sim_net.c


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



^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 05/19] vdpa_sim: remove the limit of IOTLB entries
  2020-12-07  4:00   ` Jason Wang
@ 2020-12-09 10:58     ` Stefano Garzarella
  2020-12-10  4:03       ` Jason Wang
  0 siblings, 1 reply; 35+ messages in thread
From: Stefano Garzarella @ 2020-12-09 10:58 UTC (permalink / raw)
  To: Jason Wang
  Cc: virtualization, Stefan Hajnoczi, Michael S. Tsirkin, Oren Duer,
	Laurent Vivier, linux-kernel, Max Gurtovoy, Shahaf Shuler,
	Eli Cohen

On Mon, Dec 07, 2020 at 12:00:07PM +0800, Jason Wang wrote:
>
>On 2020/12/4 上午1:04, Stefano Garzarella wrote:
>>The simulated devices can support multiple queues, so this limit
>>should be defined according to the number of queues supported by
>>the device.
>>
>>Since we are in a simulator, let's simply remove that limit.
>>
>>Suggested-by: Jason Wang <jasowang@redhat.com>
>>Acked-by: Jason Wang <jasowang@redhat.com>
>>Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>
>
>Rethink about this, since simulator can be used by VM, so the 
>allocation is actually guest trigger-able when vIOMMU is enabled.
>
>This means we need a limit somehow, (e.g I remember swiotlb is about 
>64MB by default). Or having a module parameter for this.
>
>Btw, have you met any issue when using 2048, I guess it can happen 
>when we run several processes in parallel?
>

No, I didn't try with the limit.
This came from the reviews to Max's patches.

Anyway I can add a module parameter to control that limit, do you think 
is better to set a limit per queue (the parameter per number of queues), 
or just a value for the entire device?

Thanks,
Stefano


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 13/19] vdpa_sim: add get_config callback in vdpasim_dev_attr
  2020-12-07  5:29   ` Jason Wang
@ 2020-12-09 11:07     ` Stefano Garzarella
  2020-12-15 11:43       ` Stefano Garzarella
  0 siblings, 1 reply; 35+ messages in thread
From: Stefano Garzarella @ 2020-12-09 11:07 UTC (permalink / raw)
  To: Jason Wang, Michael S. Tsirkin
  Cc: virtualization, Stefan Hajnoczi, Oren Duer, Laurent Vivier,
	linux-kernel, Max Gurtovoy, Shahaf Shuler, Eli Cohen

On Mon, Dec 07, 2020 at 01:29:17PM +0800, Jason Wang wrote:
>
>On 2020/12/4 上午1:05, Stefano Garzarella wrote:
>>The get_config callback can be used by the device to fill the
>>config structure.
>>The callback will be invoked in vdpasim_get_config() before copying
>>bytes into caller buffer.
>>
>>Move vDPA-net config updates from vdpasim_set_features() in the
>>new vdpasim_net_get_config() callback.
>>
>>Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>>---
>>v3:
>>- checked if get_config callback is set before call it
>>---
>>  drivers/vdpa/vdpa_sim/vdpa_sim.c | 35 +++++++++++++++++++-------------
>>  1 file changed, 21 insertions(+), 14 deletions(-)
>>
>>diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
>>index fe71ed7890e1..f935ade0806b 100644
>>--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
>>+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
>>@@ -60,6 +60,8 @@ struct vdpasim_virtqueue {
>>  #define VDPASIM_NET_FEATURES	(VDPASIM_FEATURES | \
>>  				 (1ULL << VIRTIO_NET_F_MAC))
>>+struct vdpasim;
>>+
>>  struct vdpasim_dev_attr {
>>  	u64 supported_features;
>>  	size_t config_size;
>>@@ -67,6 +69,7 @@ struct vdpasim_dev_attr {
>>  	u32 id;
>>  	work_func_t work_fn;
>>+	void (*get_config)(struct vdpasim *vdpasim, void *config);
>>  };
>>  /* State of each vdpasim device */
>>@@ -522,8 +525,6 @@ static u64 vdpasim_get_features(struct vdpa_device *vdpa)
>>  static int vdpasim_set_features(struct vdpa_device *vdpa, u64 features)
>>  {
>>  	struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
>>-	struct virtio_net_config *config =
>>-		(struct virtio_net_config *)vdpasim->config;
>>  	/* DMA mapping must be done by driver */
>>  	if (!(features & (1ULL << VIRTIO_F_ACCESS_PLATFORM)))
>>@@ -531,16 +532,6 @@ static int vdpasim_set_features(struct vdpa_device *vdpa, u64 features)
>>  	vdpasim->features = features & vdpasim->dev_attr.supported_features;
>>-	/* We generally only know whether guest is using the legacy interface
>>-	 * here, so generally that's the earliest we can set config fields.
>>-	 * Note: We actually require VIRTIO_F_ACCESS_PLATFORM above which
>>-	 * implies VIRTIO_F_VERSION_1, but let's not try to be clever here.
>>-	 */
>>-
>>-	config->mtu = cpu_to_vdpasim16(vdpasim, 1500);
>>-	config->status = cpu_to_vdpasim16(vdpasim, VIRTIO_NET_S_LINK_UP);
>>-	memcpy(config->mac, macaddr_buf, ETH_ALEN);
>
>
>Patch looks good to me.
>
>But we need Michael to confirm whether doing moving like this is safe. 
>I guess what has been done were trying to make sure get_config() fail 
>before set_features(), but it's not clear to me whether it's useful.

IIUC, also looking the QEMU code, the set_features() should be called 
every time before get_config(), but to be sure, in get_config(), I can 
check for example if 'vdpasim->features' is not zero (we require 
VIRTIO_F_ACCESS_PLATFORM set).

@Michael any suggestion?

Thanks,
Stefano

>
>Thanks
>
>
>>-
>>  	return 0;
>>  }
>>@@ -595,8 +586,13 @@ static void vdpasim_get_config(struct vdpa_device *vdpa, unsigned int offset,
>>  {
>>  	struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
>>-	if (offset + len < vdpasim->dev_attr.config_size)
>>-		memcpy(buf, vdpasim->config + offset, len);
>>+	if (offset + len > vdpasim->dev_attr.config_size)
>>+		return;
>>+
>>+	if (vdpasim->dev_attr.get_config)
>>+		vdpasim->dev_attr.get_config(vdpasim, vdpasim->config);
>>+
>>+	memcpy(buf, vdpasim->config + offset, len);
>>  }
>>  static void vdpasim_set_config(struct vdpa_device *vdpa, unsigned int offset,
>>@@ -739,6 +735,16 @@ static const struct vdpa_config_ops vdpasim_batch_config_ops = {
>>  	.free                   = vdpasim_free,
>>  };
>>+static void vdpasim_net_get_config(struct vdpasim *vdpasim, void *config)
>>+{
>>+	struct virtio_net_config *net_config =
>>+		(struct virtio_net_config *)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 int __init vdpasim_dev_init(void)
>>  {
>>  	struct vdpasim_dev_attr dev_attr = {};
>>@@ -747,6 +753,7 @@ static int __init vdpasim_dev_init(void)
>>  	dev_attr.supported_features = VDPASIM_NET_FEATURES;
>>  	dev_attr.nvqs = VDPASIM_VQ_NUM;
>>  	dev_attr.config_size = sizeof(struct virtio_net_config);
>>+	dev_attr.get_config = vdpasim_net_get_config;
>>  	dev_attr.work_fn = vdpasim_net_work;
>>  	vdpasim_dev = vdpasim_create(&dev_attr);
>


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 05/19] vdpa_sim: remove the limit of IOTLB entries
  2020-12-09 10:58     ` Stefano Garzarella
@ 2020-12-10  4:03       ` Jason Wang
  0 siblings, 0 replies; 35+ messages in thread
From: Jason Wang @ 2020-12-10  4:03 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: virtualization, Stefan Hajnoczi, Michael S. Tsirkin, Oren Duer,
	Laurent Vivier, linux-kernel, Max Gurtovoy, Shahaf Shuler,
	Eli Cohen


On 2020/12/9 下午6:58, Stefano Garzarella wrote:
> On Mon, Dec 07, 2020 at 12:00:07PM +0800, Jason Wang wrote:
>>
>> On 2020/12/4 上午1:04, Stefano Garzarella wrote:
>>> The simulated devices can support multiple queues, so this limit
>>> should be defined according to the number of queues supported by
>>> the device.
>>>
>>> Since we are in a simulator, let's simply remove that limit.
>>>
>>> Suggested-by: Jason Wang <jasowang@redhat.com>
>>> Acked-by: Jason Wang <jasowang@redhat.com>
>>> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>>
>>
>> Rethink about this, since simulator can be used by VM, so the 
>> allocation is actually guest trigger-able when vIOMMU is enabled.
>>
>> This means we need a limit somehow, (e.g I remember swiotlb is about 
>> 64MB by default). Or having a module parameter for this.
>>
>> Btw, have you met any issue when using 2048, I guess it can happen 
>> when we run several processes in parallel?
>>
>
> No, I didn't try with the limit.
> This came from the reviews to Max's patches.
>
> Anyway I can add a module parameter to control that limit, do you 
> think is better to set a limit per queue (the parameter per number of 
> queues), or just a value for the entire device?


Per-device should be ok.

Thanks


>
> Thanks,
> Stefano
>


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v3 13/19] vdpa_sim: add get_config callback in vdpasim_dev_attr
  2020-12-09 11:07     ` Stefano Garzarella
@ 2020-12-15 11:43       ` Stefano Garzarella
  0 siblings, 0 replies; 35+ messages in thread
From: Stefano Garzarella @ 2020-12-15 11:43 UTC (permalink / raw)
  To: Jason Wang, Michael S. Tsirkin
  Cc: Linux Virtualization, Stefan Hajnoczi, Oren Duer, Laurent Vivier,
	kernel list, Max Gurtovoy, Shahaf Shuler, Eli Cohen


On Wed, Dec 9, 2020 at 12:07 PM Stefano Garzarella <sgarzare@redhat.com> wrote:
> On Mon, Dec 07, 2020 at 01:29:17PM +0800, Jason Wang wrote:
> >
> >On 2020/12/4 上午1:05, Stefano Garzarella wrote:
> >>The get_config callback can be used by the device to fill the
> >>config structure.
> >>The callback will be invoked in vdpasim_get_config() before copying
> >>bytes into caller buffer.
> >>
> >>Move vDPA-net config updates from vdpasim_set_features() in the
> >>new vdpasim_net_get_config() callback.
> >>
> >>Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> >>---
> >>v3:
> >>- checked if get_config callback is set before call it
> >>---
> >>  drivers/vdpa/vdpa_sim/vdpa_sim.c | 35 +++++++++++++++++++-------------
> >>  1 file changed, 21 insertions(+), 14 deletions(-)
> >>
> >>diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> >>index fe71ed7890e1..f935ade0806b 100644
> >>--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
> >>+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> >>@@ -60,6 +60,8 @@ struct vdpasim_virtqueue {
> >>  #define VDPASIM_NET_FEATURES        (VDPASIM_FEATURES | \
> >>                               (1ULL << VIRTIO_NET_F_MAC))
> >>+struct vdpasim;
> >>+
> >>  struct vdpasim_dev_attr {
> >>      u64 supported_features;
> >>      size_t config_size;
> >>@@ -67,6 +69,7 @@ struct vdpasim_dev_attr {
> >>      u32 id;
> >>      work_func_t work_fn;
> >>+     void (*get_config)(struct vdpasim *vdpasim, void *config);
> >>  };
> >>  /* State of each vdpasim device */
> >>@@ -522,8 +525,6 @@ static u64 vdpasim_get_features(struct vdpa_device *vdpa)
> >>  static int vdpasim_set_features(struct vdpa_device *vdpa, u64 features)
> >>  {
> >>      struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
> >>-     struct virtio_net_config *config =
> >>-             (struct virtio_net_config *)vdpasim->config;
> >>      /* DMA mapping must be done by driver */
> >>      if (!(features & (1ULL << VIRTIO_F_ACCESS_PLATFORM)))
> >>@@ -531,16 +532,6 @@ static int vdpasim_set_features(struct vdpa_device *vdpa, u64 features)
> >>      vdpasim->features = features & vdpasim->dev_attr.supported_features;
> >>-     /* We generally only know whether guest is using the legacy interface
> >>-      * here, so generally that's the earliest we can set config fields.
> >>-      * Note: We actually require VIRTIO_F_ACCESS_PLATFORM above which
> >>-      * implies VIRTIO_F_VERSION_1, but let's not try to be clever here.
> >>-      */
> >>-
> >>-     config->mtu = cpu_to_vdpasim16(vdpasim, 1500);
> >>-     config->status = cpu_to_vdpasim16(vdpasim, VIRTIO_NET_S_LINK_UP);
> >>-     memcpy(config->mac, macaddr_buf, ETH_ALEN);
> >
> >
> >Patch looks good to me.
> >
> >But we need Michael to confirm whether doing moving like this is safe.
> >I guess what has been done were trying to make sure get_config() fail
> >before set_features(), but it's not clear to me whether it's useful.
>
> IIUC, also looking the QEMU code, the set_features() should be called
> every time before get_config(), but to be sure, in get_config(), I can
> check for example if 'vdpasim->features' is not zero (we require
> VIRTIO_F_ACCESS_PLATFORM set).

Working on this I just realized that we already check in 
vdpa_get_config() that set_features() is called, so I think the moving 
is safe.

I'll put these considerations in the commit message.

Thanks,
Stefano

>
> @Michael any suggestion?
>
> Thanks,
> Stefano
>
> >
> >Thanks
> >
> >
> >>-
> >>      return 0;
> >>  }
> >>@@ -595,8 +586,13 @@ static void vdpasim_get_config(struct vdpa_device *vdpa, unsigned int offset,
> >>  {
> >>      struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
> >>-     if (offset + len < vdpasim->dev_attr.config_size)
> >>-             memcpy(buf, vdpasim->config + offset, len);
> >>+     if (offset + len > vdpasim->dev_attr.config_size)
> >>+             return;
> >>+
> >>+     if (vdpasim->dev_attr.get_config)
> >>+             vdpasim->dev_attr.get_config(vdpasim, vdpasim->config);
> >>+
> >>+     memcpy(buf, vdpasim->config + offset, len);
> >>  }
> >>  static void vdpasim_set_config(struct vdpa_device *vdpa, unsigned int offset,
> >>@@ -739,6 +735,16 @@ static const struct vdpa_config_ops vdpasim_batch_config_ops = {
> >>      .free                   = vdpasim_free,
> >>  };
> >>+static void vdpasim_net_get_config(struct vdpasim *vdpasim, void *config)
> >>+{
> >>+     struct virtio_net_config *net_config =
> >>+             (struct virtio_net_config *)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 int __init vdpasim_dev_init(void)
> >>  {
> >>      struct vdpasim_dev_attr dev_attr = {};
> >>@@ -747,6 +753,7 @@ static int __init vdpasim_dev_init(void)
> >>      dev_attr.supported_features = VDPASIM_NET_FEATURES;
> >>      dev_attr.nvqs = VDPASIM_VQ_NUM;
> >>      dev_attr.config_size = sizeof(struct virtio_net_config);
> >>+     dev_attr.get_config = vdpasim_net_get_config;
> >>      dev_attr.work_fn = vdpasim_net_work;
> >>      vdpasim_dev = vdpasim_create(&dev_attr);
> >


^ permalink raw reply	[flat|nested] 35+ messages in thread

end of thread, other threads:[~2020-12-15 11:45 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-03 17:04 [PATCH v3 00/19] vdpa: generalize vdpa simulator Stefano Garzarella
2020-12-03 17:04 ` [PATCH v3 01/19] vdpa: remove unnecessary 'default n' in Kconfig entries Stefano Garzarella
2020-12-03 17:04 ` [PATCH v3 02/19] vdpa_sim: remove unnecessary headers inclusion Stefano Garzarella
2020-12-03 17:37   ` Randy Dunlap
2020-12-04  7:57     ` Stefano Garzarella
2020-12-03 17:04 ` [PATCH v3 03/19] vdpa_sim: remove hard-coded virtq count Stefano Garzarella
2020-12-03 17:04 ` [PATCH v3 04/19] vhost/iotlb: add VHOST_IOTLB_UNLIMITED macro Stefano Garzarella
2020-12-07  3:55   ` Jason Wang
2020-12-03 17:04 ` [PATCH v3 05/19] vdpa_sim: remove the limit of IOTLB entries Stefano Garzarella
2020-12-07  4:00   ` Jason Wang
2020-12-09 10:58     ` Stefano Garzarella
2020-12-10  4:03       ` Jason Wang
2020-12-03 17:04 ` [PATCH v3 06/19] vdpa_sim: rename vdpasim_config_ops variables Stefano Garzarella
2020-12-03 17:04 ` [PATCH v3 07/19] vdpa_sim: add struct vdpasim_dev_attr for device attributes Stefano Garzarella
2020-12-03 17:05 ` [PATCH v3 08/19] vdpa_sim: add device id field in vdpasim_dev_attr Stefano Garzarella
2020-12-03 17:05 ` [PATCH v3 09/19] vdpa_sim: add supported_features " Stefano Garzarella
2020-12-03 17:05 ` [PATCH v3 10/19] vdpa_sim: add work_fn " Stefano Garzarella
2020-12-03 17:05 ` [PATCH v3 11/19] vdpa_sim: store parsed MAC address in a buffer Stefano Garzarella
2020-12-03 17:05 ` [PATCH v3 12/19] vdpa_sim: make 'config' generic and usable for any device type Stefano Garzarella
2020-12-07  5:22   ` Jason Wang
2020-12-03 17:05 ` [PATCH v3 13/19] vdpa_sim: add get_config callback in vdpasim_dev_attr Stefano Garzarella
2020-12-07  5:29   ` Jason Wang
2020-12-09 11:07     ` Stefano Garzarella
2020-12-15 11:43       ` Stefano Garzarella
2020-12-03 17:05 ` [PATCH v3 14/19] vdpa_sim: add set_config " Stefano Garzarella
2020-12-07  5:30   ` Jason Wang
2020-12-03 17:05 ` [PATCH v3 15/19] vdpa_sim: set vringh notify callback Stefano Garzarella
2020-12-07  5:30   ` Jason Wang
2020-12-03 17:05 ` [PATCH v3 16/19] vdpa_sim: use kvmalloc to allocate vdpasim->buffer Stefano Garzarella
2020-12-03 17:05 ` [PATCH v3 17/19] vdpa_sim: make vdpasim->buffer size configurable Stefano Garzarella
2020-12-03 17:05 ` [PATCH v3 18/19] vdpa_sim: split vdpasim_virtqueue's iov field in out_iov and in_iov Stefano Garzarella
2020-12-03 17:05 ` [PATCH v3 19/19] vdpa: split vdpasim to core and net modules Stefano Garzarella
2020-12-03 17:25   ` Randy Dunlap
2020-12-04  7:48     ` Stefano Garzarella
2020-12-07  5:33   ` Jason Wang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).