[PATCH v2 00/17] vdpa: generalize vdpa simulator

[PATCH v2 00/17] vdpa: generalize vdpa simulator

[Stefano Garzarella]

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.

As Jason suggested, I simplified the "vdpa: split vdpasim to core and
net modules" patch, moving some changes out in small patches.
@Max: I put your Co-developed-by and Signed-off-by tags on these patches,
let me know if it is okay for you, or if there is a better way to give
credit to your work!

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

v2:
  - moved most of the patches before the vdpa-core/net split [Jason]
  - removed unnecessary headers
  - removed 'default n' in Kconfig entries [Jason]
  - added VDPASIM_IOTLB_LIMIT macro [Jason]
  - set vringh notify callback [Jason]
  - used VIRTIO terminology for in_iov/out_iov [Stefan]
  - simplified "vdpa: split vdpasim to core and net modules" patch,
    moving some changes out in small patches
  - left batch_mapping module parameter in the core [Jason]

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

Stefano Garzarella (15):
  vdpa: remove unnecessary 'default n' in Kconfig entries
  vdpa_sim: remove unnecessary headers inclusion
  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: 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     | 103 ++++++++++
 drivers/vdpa/vdpa_sim/vdpa_sim.c     | 290 +++++++--------------------
 drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 171 ++++++++++++++++
 drivers/vdpa/Kconfig                 |  16 +-
 drivers/vdpa/vdpa_sim/Makefile       |   1 +
 5 files changed, 351 insertions(+), 230 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

[PATCH v2 01/17] vdpa: remove unnecessary 'default n' in Kconfig entries

[Stefano Garzarella]

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

Suggested-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 358f6048dd3c..4019ceb88181 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.
@@ -41,7 +39,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

Re: [PATCH v2 01/17] vdpa: remove unnecessary 'default n' in Kconfig entries

[Jason Wang]

On 2020/11/26 下午10:49, Stefano Garzarella wrote:
> 'default n' is not necessary since it is already the default when
> nothing is specified.
>
> Suggested-by: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>


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


> ---
>   drivers/vdpa/Kconfig | 3 ---
>   1 file changed, 3 deletions(-)
>
> diff --git a/drivers/vdpa/Kconfig b/drivers/vdpa/Kconfig
> index 358f6048dd3c..4019ceb88181 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.
> @@ -41,7 +39,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

[PATCH v2 02/17] vdpa_sim: remove unnecessary headers inclusion

[Stefano Garzarella]

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

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

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index 6a90fdb9cbfc..c6eaf62df8ec 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -7,24 +7,11 @@
  *
  */
 
-#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>
-#include <linux/virtio_byteorder.h>
 #include <linux/vhost_iotlb.h>
 #include <uapi/linux/virtio_config.h>
 #include <uapi/linux/virtio_net.h>
-- 
2.26.2

Re: [PATCH v2 02/17] vdpa_sim: remove unnecessary headers inclusion

[Jason Wang]

On 2020/11/26 下午10:49, 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>
> ---
>   drivers/vdpa/vdpa_sim/vdpa_sim.c | 13 -------------
>   1 file changed, 13 deletions(-)
>
> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> index 6a90fdb9cbfc..c6eaf62df8ec 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> @@ -7,24 +7,11 @@
>    *
>    */
>   
> -#include <linux/init.h>
>   #include <linux/module.h>
> -#include <linux/device.h>


I think the rule is to make sure e.g the structure definition can be via 
direct inclusion. E.g struct device {} is defined in this file.


> -#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>
> -#include <linux/virtio_byteorder.h>


And the  __cpu_to_virtio16 is defined in this file.

Thanks


>   #include <linux/vhost_iotlb.h>
>   #include <uapi/linux/virtio_config.h>
>   #include <uapi/linux/virtio_net.h>

Re: [PATCH v2 02/17] vdpa_sim: remove unnecessary headers inclusion

[Stefano Garzarella]

On Mon, Nov 30, 2020 at 11:04:49AM +0800, Jason Wang wrote:
>
>On 2020/11/26 下午10:49, 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>
>>---
>>  drivers/vdpa/vdpa_sim/vdpa_sim.c | 13 -------------
>>  1 file changed, 13 deletions(-)
>>
>>diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
>>index 6a90fdb9cbfc..c6eaf62df8ec 100644
>>--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
>>+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
>>@@ -7,24 +7,11 @@
>>   *
>>   */
>>-#include <linux/init.h>
>>  #include <linux/module.h>
>>-#include <linux/device.h>
>
>
>I think the rule is to make sure e.g the structure definition can be 
>via direct inclusion. E.g struct device {} is defined in this file.
>
>
>>-#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>
>>-#include <linux/virtio_byteorder.h>
>
>
>And the  __cpu_to_virtio16 is defined in this file.
>

Okay, I'll keep this two includes and check better the others.

Thanks,
Stefano

[PATCH v2 03/17] vdpa_sim: remove hard-coded virtq count

[Stefano Garzarella]

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()]
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 c6eaf62df8ec..ad72f7b1a4eb 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -57,7 +57,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;
@@ -67,6 +67,7 @@ struct vdpasim {
 	u32 status;
 	u32 generation;
 	u64 features;
+	int nvqs;
 	/* spinlock to synchronize iommu table */
 	spinlock_t iommu_lock;
 };
@@ -131,7 +132,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);
@@ -337,7 +338,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;
@@ -348,6 +349,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);
@@ -358,6 +360,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;
@@ -376,8 +383,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);
@@ -646,6 +653,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

Re: [PATCH v2 03/17] vdpa_sim: remove hard-coded virtq count

[Jason Wang]

On 2020/11/26 下午10:49, Stefano Garzarella wrote:
> 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()]
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>


Acked-by: Jason Wang <jasowang@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 c6eaf62df8ec..ad72f7b1a4eb 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> @@ -57,7 +57,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;
> @@ -67,6 +67,7 @@ struct vdpasim {
>   	u32 status;
>   	u32 generation;
>   	u64 features;
> +	int nvqs;
>   	/* spinlock to synchronize iommu table */
>   	spinlock_t iommu_lock;
>   };
> @@ -131,7 +132,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);
> @@ -337,7 +338,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;
> @@ -348,6 +349,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);
> @@ -358,6 +360,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;
> @@ -376,8 +383,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);
> @@ -646,6 +653,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 = {

[PATCH v2 04/17] vdpa_sim: remove the limit of IOTLB entries

[Stefano Garzarella]

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>
---
v2:
- added VDPASIM_IOTLB_LIMIT macro [Jason]
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index ad72f7b1a4eb..40664d87f303 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -46,6 +46,7 @@ struct vdpasim_virtqueue {
 #define VDPASIM_QUEUE_MAX 256
 #define VDPASIM_DEVICE_ID 0x1
 #define VDPASIM_VENDOR_ID 0
+#define VDPASIM_IOTLB_LIMIT 0 /* unlimited */
 #define VDPASIM_VQ_NUM 0x2
 #define VDPASIM_NAME "vdpasim-netdev"
 
@@ -365,7 +366,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(VDPASIM_IOTLB_LIMIT, 0);
 	if (!vdpasim->iommu)
 		goto err_iommu;
 
-- 
2.26.2

Re: [PATCH v2 04/17] vdpa_sim: remove the limit of IOTLB entries

[Jason Wang]

On 2020/11/26 下午10:49, 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>
> ---
> v2:
> - added VDPASIM_IOTLB_LIMIT macro [Jason]


Sorry for being unclear. I meant adding a macro like

VHOST_IOTLB_UNLIMITED 0 in vhost_iotlb.h.

And use that in vdpa_sim.

Thanks


> ---
>   drivers/vdpa/vdpa_sim/vdpa_sim.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> index ad72f7b1a4eb..40664d87f303 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> @@ -46,6 +46,7 @@ struct vdpasim_virtqueue {
>   #define VDPASIM_QUEUE_MAX 256
>   #define VDPASIM_DEVICE_ID 0x1
>   #define VDPASIM_VENDOR_ID 0
> +#define VDPASIM_IOTLB_LIMIT 0 /* unlimited */
>   #define VDPASIM_VQ_NUM 0x2
>   #define VDPASIM_NAME "vdpasim-netdev"
>   
> @@ -365,7 +366,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(VDPASIM_IOTLB_LIMIT, 0);
>   	if (!vdpasim->iommu)
>   		goto err_iommu;
>   

Re: [PATCH v2 04/17] vdpa_sim: remove the limit of IOTLB entries

[Stefano Garzarella]

On Mon, Nov 30, 2020 at 11:07:08AM +0800, Jason Wang wrote:
>
>On 2020/11/26 下午10:49, 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>
>>---
>>v2:
>>- added VDPASIM_IOTLB_LIMIT macro [Jason]
>
>
>Sorry for being unclear. I meant adding a macro like
>
>VHOST_IOTLB_UNLIMITED 0 in vhost_iotlb.h.
>
>And use that in vdpa_sim.

Got it :-) I'll fix adding the macro in another patch and using it in 
this one.

Thanks,
Stefano

[PATCH v2 05/17] vdpa_sim: rename vdpasim_config_ops variables

[Stefano Garzarella]

These variables stores 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>
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 40664d87f303..62204e064841 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -331,8 +331,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)
 {
@@ -342,9 +342,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)
@@ -657,7 +657,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,
@@ -684,7 +684,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

Re: [PATCH v2 05/17] vdpa_sim: rename vdpasim_config_ops variables

[Jason Wang]

On 2020/11/26 下午10:49, Stefano Garzarella wrote:
> These variables stores 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>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>   drivers/vdpa/vdpa_sim/vdpa_sim.c | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 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 40664d87f303..62204e064841 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> @@ -331,8 +331,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)
>   {
> @@ -342,9 +342,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)
> @@ -657,7 +657,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,
> @@ -684,7 +684,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,

[PATCH v2 06/17] vdpa_sim: add struct vdpasim_dev_attr for device attributes

[Stefano Garzarella]

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>
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 62204e064841..f98262add0e1 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -55,11 +55,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;
@@ -68,7 +73,6 @@ struct vdpasim {
 	u32 status;
 	u32 generation;
 	u64 features;
-	int nvqs;
 	/* spinlock to synchronize iommu table */
 	spinlock_t iommu_lock;
 };
@@ -133,7 +137,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);
@@ -334,7 +338,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;
@@ -346,11 +350,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);
@@ -361,7 +366,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;
@@ -384,7 +389,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;
@@ -712,7 +717,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

Re: [PATCH v2 06/17] vdpa_sim: add struct vdpasim_dev_attr for device attributes

[Jason Wang]

On 2020/11/26 下午10:49, Stefano Garzarella wrote:
> 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>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>   drivers/vdpa/vdpa_sim/vdpa_sim.c | 25 +++++++++++++++++--------
>   1 file changed, 17 insertions(+), 8 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 62204e064841..f98262add0e1 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> @@ -55,11 +55,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;
> @@ -68,7 +73,6 @@ struct vdpasim {
>   	u32 status;
>   	u32 generation;
>   	u64 features;
> -	int nvqs;
>   	/* spinlock to synchronize iommu table */
>   	spinlock_t iommu_lock;
>   };
> @@ -133,7 +137,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);
> @@ -334,7 +338,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;
> @@ -346,11 +350,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);
> @@ -361,7 +366,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;
> @@ -384,7 +389,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;
> @@ -712,7 +717,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;

[PATCH v2 07/17] vdpa_sim: add device id field in vdpasim_dev_attr

[Stefano Garzarella]

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>
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 f98262add0e1..393b54a9f0e4 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -44,7 +44,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_IOTLB_LIMIT 0 /* unlimited */
 #define VDPASIM_VQ_NUM 0x2
@@ -57,6 +56,7 @@ static u64 vdpasim_features = (1ULL << VIRTIO_F_ANY_LAYOUT) |
 
 struct vdpasim_dev_attr {
 	int nvqs;
+	u32 id;
 };
 
 /* State of each vdpasim device */
@@ -536,7 +536,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)
@@ -719,6 +721,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

Re: [PATCH v2 07/17] vdpa_sim: add device id field in vdpasim_dev_attr

[Jason Wang]

On 2020/11/26 下午10:49, Stefano Garzarella wrote:
> 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>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>   drivers/vdpa/vdpa_sim/vdpa_sim.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 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 f98262add0e1..393b54a9f0e4 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> @@ -44,7 +44,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_IOTLB_LIMIT 0 /* unlimited */
>   #define VDPASIM_VQ_NUM 0x2
> @@ -57,6 +56,7 @@ static u64 vdpasim_features = (1ULL << VIRTIO_F_ANY_LAYOUT) |
>   
>   struct vdpasim_dev_attr {
>   	int nvqs;
> +	u32 id;
>   };
>   
>   /* State of each vdpasim device */
> @@ -536,7 +536,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)
> @@ -719,6 +721,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);

[PATCH v2 08/17] vdpa_sim: add supported_features field in vdpasim_dev_attr

[Stefano Garzarella]

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>
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 393b54a9f0e4..36677fc3631b 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -49,12 +49,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;
 };
@@ -112,7 +115,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 *)
@@ -121,7 +124,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;
@@ -129,8 +133,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)
@@ -138,7 +142,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);
@@ -498,7 +502,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)
@@ -510,7 +516,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.
@@ -722,6 +728,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

Re: [PATCH v2 08/17] vdpa_sim: add supported_features field in vdpasim_dev_attr

[Jason Wang]

On 2020/11/26 下午10:49, Stefano Garzarella wrote:
> 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>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>   drivers/vdpa/vdpa_sim/vdpa_sim.c | 29 ++++++++++++++++++-----------
>   1 file changed, 18 insertions(+), 11 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 393b54a9f0e4..36677fc3631b 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> @@ -49,12 +49,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;
>   };
> @@ -112,7 +115,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 *)
> @@ -121,7 +124,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;
> @@ -129,8 +133,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)
> @@ -138,7 +142,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);
> @@ -498,7 +502,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)
> @@ -510,7 +516,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.
> @@ -722,6 +728,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);

[PATCH v2 09/17] vdpa_sim: add work_fn in vdpasim_dev_attr

[Stefano Garzarella]

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>
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 36677fc3631b..b84d9acd130c 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -60,6 +60,8 @@ struct vdpasim_dev_attr {
 	u64 supported_features;
 	int nvqs;
 	u32 id;
+
+	work_func_t work_fn;
 };
 
 /* State of each vdpasim device */
@@ -153,7 +155,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);
@@ -360,7 +362,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);
 
@@ -730,6 +732,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

Re: [PATCH v2 09/17] vdpa_sim: add work_fn in vdpasim_dev_attr

[Jason Wang]

On 2020/11/26 下午10:49, Stefano Garzarella wrote:
> 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>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>   drivers/vdpa/vdpa_sim/vdpa_sim.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 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 36677fc3631b..b84d9acd130c 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> @@ -60,6 +60,8 @@ struct vdpasim_dev_attr {
>   	u64 supported_features;
>   	int nvqs;
>   	u32 id;
> +
> +	work_func_t work_fn;
>   };
>   
>   /* State of each vdpasim device */
> @@ -153,7 +155,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);
> @@ -360,7 +362,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);
>   
> @@ -730,6 +732,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);
>   

[PATCH v2 10/17] vdpa_sim: store parsed MAC address in a buffer

[Stefano Garzarella]

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.

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 b84d9acd130c..9f2ca3a77025 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -29,6 +29,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;
@@ -386,13 +388,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++)
@@ -528,6 +530,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

Re: [PATCH v2 10/17] vdpa_sim: store parsed MAC address in a buffer

[Jason Wang]

On 2020/11/26 下午10:49, Stefano Garzarella wrote:
> 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.
>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>   drivers/vdpa/vdpa_sim/vdpa_sim.c | 10 +++++++---
>   1 file changed, 7 insertions(+), 3 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 b84d9acd130c..9f2ca3a77025 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> @@ -29,6 +29,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;
> @@ -386,13 +388,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++)
> @@ -528,6 +530,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;
>   }
>   

[PATCH v2 11/17] vdpa_sim: make 'config' generic and usable for any device type

[Stefano Garzarella]

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 9f2ca3a77025..c07ddf6af720 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -60,6 +60,7 @@ struct vdpasim_virtqueue {
 
 struct vdpasim_dev_attr {
 	u64 supported_features;
+	size_t config_size;
 	int nvqs;
 	u32 id;
 
@@ -74,7 +75,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;
@@ -374,6 +376,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)
@@ -514,7 +520,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)))
@@ -586,8 +593,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,
@@ -674,6 +681,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 = {
@@ -736,6 +744,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

[PATCH v2 12/17] vdpa_sim: add get_config callback in vdpasim_dev_attr

[Stefano Garzarella]

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>
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 33 +++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index c07ddf6af720..8b87ce0485b6 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -58,6 +58,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;
@@ -65,6 +67,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 */
@@ -520,8 +523,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)))
@@ -529,15 +530,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;
 }
@@ -593,8 +585,12 @@ 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;
+
+	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,
@@ -737,6 +733,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 = {};
@@ -745,6 +751,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

Re: [PATCH v2 12/17] vdpa_sim: add get_config callback in vdpasim_dev_attr

[Jason Wang]

On 2020/11/26 下午10:49, 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>
> ---
>   drivers/vdpa/vdpa_sim/vdpa_sim.c | 33 +++++++++++++++++++-------------
>   1 file changed, 20 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> index c07ddf6af720..8b87ce0485b6 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> @@ -58,6 +58,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;
> @@ -65,6 +67,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 */
> @@ -520,8 +523,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)))
> @@ -529,15 +530,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;
>   }
> @@ -593,8 +585,12 @@ 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;
> +
> +	vdpasim->dev_attr.get_config(vdpasim, vdpasim->config);
> +
> +	memcpy(buf, vdpasim->config + offset, len);
>   }


I wonder how much value we can get from vdpasim->config consider we've 
already had get_config() method.

Is it possible to copy to the buffer directly here?

Thanks


>   
>   static void vdpasim_set_config(struct vdpa_device *vdpa, unsigned int offset,
> @@ -737,6 +733,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 = {};
> @@ -745,6 +751,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);

Re: [PATCH v2 12/17] vdpa_sim: add get_config callback in vdpasim_dev_attr

[Stefano Garzarella]

On Mon, Nov 30, 2020 at 11:25:31AM +0800, Jason Wang wrote:
>
>On 2020/11/26 下午10:49, 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>
>>---
>>  drivers/vdpa/vdpa_sim/vdpa_sim.c | 33 +++++++++++++++++++-------------
>>  1 file changed, 20 insertions(+), 13 deletions(-)
>>
>>diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
>>index c07ddf6af720..8b87ce0485b6 100644
>>--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
>>+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
>>@@ -58,6 +58,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;
>>@@ -65,6 +67,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 */
>>@@ -520,8 +523,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)))
>>@@ -529,15 +530,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;
>>  }
>>@@ -593,8 +585,12 @@ 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;
>>+
>>+	vdpasim->dev_attr.get_config(vdpasim, vdpasim->config);
>>+
>>+	memcpy(buf, vdpasim->config + offset, len);
>>  }
>
>
>I wonder how much value we can get from vdpasim->config consider we've 
>already had get_config() method.
>
>Is it possible to copy to the buffer directly here?

I had thought of eliminating it too, but then I wanted to do something 
similar to what we do in QEMU (hw/virtio/virtio.c), leaving in the 
simulator core the buffer, the memory copy (handling offset and len), 
and the boundary checks.

In this way each device should simply fill the entire configuration and 
we can avoid code duplication.

Storing the configuration in the core may also be useful if some device 
needs to support config writes.

Do you think it makes sense, or is it better to move everything in the 
device?

Thanks,
Stefano

Re: [PATCH v2 12/17] vdpa_sim: add get_config callback in vdpasim_dev_attr

[Jason Wang]

On 2020/11/30 下午10:14, Stefano Garzarella wrote:
> On Mon, Nov 30, 2020 at 11:25:31AM +0800, Jason Wang wrote:
>>
>> On 2020/11/26 下午10:49, 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>
>>> ---
>>>  drivers/vdpa/vdpa_sim/vdpa_sim.c | 33 +++++++++++++++++++-------------
>>>  1 file changed, 20 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c 
>>> b/drivers/vdpa/vdpa_sim/vdpa_sim.c
>>> index c07ddf6af720..8b87ce0485b6 100644
>>> --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
>>> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
>>> @@ -58,6 +58,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;
>>> @@ -65,6 +67,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 */
>>> @@ -520,8 +523,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)))
>>> @@ -529,15 +530,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;
>>>  }
>>> @@ -593,8 +585,12 @@ 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;
>>> +
>>> +    vdpasim->dev_attr.get_config(vdpasim, vdpasim->config);
>>> +
>>> +    memcpy(buf, vdpasim->config + offset, len);
>>>  }
>>
>>
>> I wonder how much value we can get from vdpasim->config consider 
>> we've already had get_config() method.
>>
>> Is it possible to copy to the buffer directly here?
>
> I had thought of eliminating it too, but then I wanted to do something 
> similar to what we do in QEMU (hw/virtio/virtio.c), leaving in the 
> simulator core the buffer, the memory copy (handling offset and len), 
> and the boundary checks.
>
> In this way each device should simply fill the entire configuration 
> and we can avoid code duplication.
>
> Storing the configuration in the core may also be useful if some 
> device needs to support config writes.


I think in that way we need should provide config_write().


>
> Do you think it makes sense, or is it better to move everything in the 
> device?


I prefer to do that in the device but it's also fine keep what the patch 
has done.

Thanks


>
> Thanks,
> Stefano
>

Re: [PATCH v2 12/17] vdpa_sim: add get_config callback in vdpasim_dev_attr

[Stefano Garzarella]

On Tue, Dec 01, 2020 at 11:44:19AM +0800, Jason Wang wrote:
>
>On 2020/11/30 下午10:14, Stefano Garzarella wrote:
>>On Mon, Nov 30, 2020 at 11:25:31AM +0800, Jason Wang wrote:
>>>
>>>On 2020/11/26 下午10:49, 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>
>>>>---
>>>> drivers/vdpa/vdpa_sim/vdpa_sim.c | 33 +++++++++++++++++++-------------
>>>> 1 file changed, 20 insertions(+), 13 deletions(-)
>>>>
>>>>diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c 
>>>>b/drivers/vdpa/vdpa_sim/vdpa_sim.c
>>>>index c07ddf6af720..8b87ce0485b6 100644
>>>>--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
>>>>+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
>>>>@@ -58,6 +58,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;
>>>>@@ -65,6 +67,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 */
>>>>@@ -520,8 +523,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)))
>>>>@@ -529,15 +530,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;
>>>> }
>>>>@@ -593,8 +585,12 @@ 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;
>>>>+
>>>>+    vdpasim->dev_attr.get_config(vdpasim, vdpasim->config);
>>>>+
>>>>+    memcpy(buf, vdpasim->config + offset, len);
>>>> }
>>>
>>>
>>>I wonder how much value we can get from vdpasim->config consider 
>>>we've already had get_config() method.
>>>
>>>Is it possible to copy to the buffer directly here?
>>
>>I had thought of eliminating it too, but then I wanted to do something 
>>similar to what we do in QEMU (hw/virtio/virtio.c), leaving in the 
>>simulator core the buffer, the memory copy (handling offset and len), 
>>and the boundary checks.
>>
>>In this way each device should simply fill the entire configuration 
>>and we can avoid code duplication.
>>
>>Storing the configuration in the core may also be useful if some 
>>device needs to support config writes.
>
>
>I think in that way we need should provide config_write().

Yes, I'll add set_config() callback.

>
>
>>
>>Do you think it makes sense, or is it better to move everything in the 
>>device?
>
>
>I prefer to do that in the device but it's also fine keep what the 
>patch has done.

Okay, for now I'll keep it and add the set_config() callback, but I'm 
open to move it in the device.

Thanks,
Stefano

[PATCH v2 13/17] vdpa_sim: set vringh notify callback

[Stefano Garzarella]

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>
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index 8b87ce0485b6..4327efd6d41e 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -120,6 +120,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];
@@ -131,6 +142,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,
@@ -220,10 +233,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

Re: [PATCH v2 13/17] vdpa_sim: set vringh notify callback

[Jason Wang]

On 2020/11/26 下午10:49, 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>
> ---
>   drivers/vdpa/vdpa_sim/vdpa_sim.c | 21 +++++++++++++++++----
>   1 file changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> index 8b87ce0485b6..4327efd6d41e 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> @@ -120,6 +120,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];
> @@ -131,6 +142,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;


Do we need to clear notify during reset?

Other looks good.


>   }
>   
>   static void vdpasim_vq_reset(struct vdpasim *vdpasim,
> @@ -220,10 +233,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) {

Re: [PATCH v2 13/17] vdpa_sim: set vringh notify callback

[Stefano Garzarella]

On Mon, Nov 30, 2020 at 11:27:51AM +0800, Jason Wang wrote:
>
>On 2020/11/26 下午10:49, 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>
>>---
>>  drivers/vdpa/vdpa_sim/vdpa_sim.c | 21 +++++++++++++++++----
>>  1 file changed, 17 insertions(+), 4 deletions(-)
>>
>>diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
>>index 8b87ce0485b6..4327efd6d41e 100644
>>--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
>>+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
>>@@ -120,6 +120,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];
>>@@ -131,6 +142,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;
>
>
>Do we need to clear notify during reset?

Right, I'll clear it.

>
>Other looks good.
>

Thanks,
Stefano

[PATCH v2 14/17] vdpa_sim: use kvmalloc to allocate vdpasim->buffer

[Stefano Garzarella]

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 4327efd6d41e..3bcf622949c8 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -405,7 +405,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;
 
@@ -686,7 +686,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

[PATCH v2 15/17] vdpa_sim: make vdpasim->buffer size configurable

[Stefano Garzarella]

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 3bcf622949c8..f5f41f20ee0b 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -63,6 +63,7 @@ struct vdpasim;
 struct vdpasim_dev_attr {
 	u64 supported_features;
 	size_t config_size;
+	size_t buffer_size;
 	int nvqs;
 	u32 id;
 
@@ -405,7 +406,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;
 
@@ -766,6 +767,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

[PATCH v2 16/17] vdpa_sim: split vdpasim_virtqueue's iov field in out_iov and in_iov

[Stefano Garzarella]

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.

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 f5f41f20ee0b..f8ee261ef4ae 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -33,7 +33,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;
@@ -197,12 +198,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);
@@ -210,13 +211,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

Re: [PATCH v2 16/17] vdpa_sim: split vdpasim_virtqueue's iov field in out_iov and in_iov

[Jason Wang]

On 2020/11/26 下午10:49, Stefano Garzarella wrote:
> 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.
>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
> v2:
> - used VIRTIO terminology [Stefan]
> ---


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


>   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 f5f41f20ee0b..f8ee261ef4ae 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> @@ -33,7 +33,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;
> @@ -197,12 +198,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);
> @@ -210,13 +211,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;

[PATCH v2 17/17] vdpa: split vdpasim to core and net modules

[Stefano Garzarella]

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     | 103 +++++++++++++
 drivers/vdpa/vdpa_sim/vdpa_sim.c     | 222 +--------------------------
 drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 171 +++++++++++++++++++++
 drivers/vdpa/Kconfig                 |  13 +-
 drivers/vdpa/vdpa_sim/Makefile       |   1 +
 5 files changed, 290 insertions(+), 220 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..1662f3bf3eac
--- /dev/null
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.h
@@ -0,0 +1,103 @@
+/* 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/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);
+};
+
+/* 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 f8ee261ef4ae..cbdd419de1ef 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>
@@ -9,106 +9,22 @@
 
 #include <linux/module.h>
 #include <linux/dma-map-ops.h>
-#include <linux/etherdevice.h>
-#include <linux/vringh.h>
-#include <linux/vdpa.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_IOTLB_LIMIT 0 /* unlimited */
-#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);
-};
-
-/* 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)
 {
@@ -177,80 +93,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;
@@ -366,7 +208,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;
@@ -411,23 +253,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;
 
@@ -436,6 +265,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,
@@ -748,46 +578,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 4019ceb88181..6d63f10e6741 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

Re: [PATCH v2 17/17] vdpa: split vdpasim to core and net modules

[Jason Wang]

On 2020/11/26 下午10:49, 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     | 103 +++++++++++++
>   drivers/vdpa/vdpa_sim/vdpa_sim.c     | 222 +--------------------------
>   drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 171 +++++++++++++++++++++
>   drivers/vdpa/Kconfig                 |  13 +-
>   drivers/vdpa/vdpa_sim/Makefile       |   1 +
>   5 files changed, 290 insertions(+), 220 deletions(-)
>   create mode 100644 drivers/vdpa/vdpa_sim/vdpa_sim.h
>   create mode 100644 drivers/vdpa/vdpa_sim/vdpa_sim_net.c


Looks good, consider there are some still some questions left. I will 
probably ack for the next version.

Thanks

Re: [PATCH v2 17/17] vdpa: split vdpasim to core and net modules

[Stefano Garzarella]

On Mon, Nov 30, 2020 at 11:31:43AM +0800, Jason Wang wrote:
>
>On 2020/11/26 下午10:49, 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     | 103 +++++++++++++
>>  drivers/vdpa/vdpa_sim/vdpa_sim.c     | 222 +--------------------------
>>  drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 171 +++++++++++++++++++++
>>  drivers/vdpa/Kconfig                 |  13 +-
>>  drivers/vdpa/vdpa_sim/Makefile       |   1 +
>>  5 files changed, 290 insertions(+), 220 deletions(-)
>>  create mode 100644 drivers/vdpa/vdpa_sim/vdpa_sim.h
>>  create mode 100644 drivers/vdpa/vdpa_sim/vdpa_sim_net.c
>
>
>Looks good, consider there are some still some questions left. I will 
>probably ack for the next version.
>

Sure, thanks for your feedback!

Stefano

Re: [PATCH v2 00/17] vdpa: generalize vdpa simulator

[Max Gurtovoy]

On 11/26/2020 4:49 PM, Stefano Garzarella wrote:
> 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.
>
> As Jason suggested, I simplified the "vdpa: split vdpasim to core and
> net modules" patch, moving some changes out in small patches.
> @Max: I put your Co-developed-by and Signed-off-by tags on these patches,
> let me know if it is okay for you, or if there is a better way to give
> credit to your work!

Stefano,

thanks for taking my initial series and bringing it to upstream level 
and thanks Jason for your reviews.

I'm ok with the tags and hopefully I'll be able to help a bit in the 
submission in couple of weeks.

great progress !


> v1: https://lists.linuxfoundation.org/pipermail/virtualization/2020-November/050677.html
>
> v2:
>    - moved most of the patches before the vdpa-core/net split [Jason]
>    - removed unnecessary headers
>    - removed 'default n' in Kconfig entries [Jason]
>    - added VDPASIM_IOTLB_LIMIT macro [Jason]
>    - set vringh notify callback [Jason]
>    - used VIRTIO terminology for in_iov/out_iov [Stefan]
>    - simplified "vdpa: split vdpasim to core and net modules" patch,
>      moving some changes out in small patches
>    - left batch_mapping module parameter in the core [Jason]
>
> Max Gurtovoy (2):
>    vdpa_sim: remove hard-coded virtq count
>    vdpa: split vdpasim to core and net modules
>
> Stefano Garzarella (15):
>    vdpa: remove unnecessary 'default n' in Kconfig entries
>    vdpa_sim: remove unnecessary headers inclusion
>    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: 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

Re: [PATCH v2 00/17] vdpa: generalize vdpa simulator

[Stefano Garzarella]

On Thu, Nov 26, 2020 at 05:12:30PM +0200, Max Gurtovoy wrote:
>
>On 11/26/2020 4:49 PM, Stefano Garzarella wrote:
>>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.
>>
>>As Jason suggested, I simplified the "vdpa: split vdpasim to core and
>>net modules" patch, moving some changes out in small patches.
>>@Max: I put your Co-developed-by and Signed-off-by tags on these patches,
>>let me know if it is okay for you, or if there is a better way to give
>>credit to your work!
>
>Stefano,
>
>thanks for taking my initial series and bringing it to upstream level 
>and thanks Jason for your reviews.
>
>I'm ok with the tags and hopefully I'll be able to help a bit in the 
>submission in couple of weeks.

Great! :-)
I'll keep you updated.

>
>great progress !

Thanks,
Stefano