[PATCH v3 00/19] vdpa: generalize vdpa simulator

[PATCH v3 00/19] 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.

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

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

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

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

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

-- 
2.26.2

[PATCH v3 01/19] 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>
Acked-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 drivers/vdpa/Kconfig | 3 ---
 1 file changed, 3 deletions(-)

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

[PATCH v3 02/19] 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>
---
v3:
- avoided to remove some headers with structures and functions directly
  used (device.h, slab.h, virtio_byteorder.h)[Jason]
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 10 ----------
 1 file changed, 10 deletions(-)

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

Re: [PATCH v3 02/19] vdpa_sim: remove unnecessary headers inclusion

[Randy Dunlap]

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

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

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

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


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


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

above is used by __init and __exit.

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

Looks OK to remove poll.h.

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

Might be OK for sched.h.

> -#include <linux/wait.h>

Might be OK for wait.h.

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

I didn't check the others.


-- 
~Randy

Re: [PATCH v3 02/19] vdpa_sim: remove unnecessary headers inclusion

[Stefano Garzarella]

Hi Randy,

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

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

I'll check better for the next version!

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

Thanks,
Stefano

[PATCH v3 03/19] 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()]
Acked-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
v1:
- use kcalloc() instead of kmalloc_array() since some function expects
  variables initialized to zero
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

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

[PATCH v3 04/19] vhost/iotlb: add VHOST_IOTLB_UNLIMITED macro

[Stefano Garzarella]

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

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

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

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

Re: [PATCH v3 04/19] vhost/iotlb: add VHOST_IOTLB_UNLIMITED macro

[Jason Wang]

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


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


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

[PATCH v3 05/19] 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>
---
v3:
- used VHOST_IOTLB_UNLIMITED macro [Jason]
v2:
- added VDPASIM_IOTLB_LIMIT macro [Jason]
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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

Re: [PATCH v3 05/19] vdpa_sim: remove the limit of IOTLB entries

[Jason Wang]

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


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

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

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


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

Re: [PATCH v3 05/19] vdpa_sim: remove the limit of IOTLB entries

[Stefano Garzarella]

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

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

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

Thanks,
Stefano

Re: [PATCH v3 05/19] vdpa_sim: remove the limit of IOTLB entries

[Jason Wang]

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


Per-device should be ok.

Thanks


>
> Thanks,
> Stefano
>

[PATCH v3 06/19] vdpa_sim: rename vdpasim_config_ops variables

[Stefano Garzarella]

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

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

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

[PATCH v3 07/19] 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>
Acked-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

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

[PATCH v3 08/19] 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>
Acked-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

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

[PATCH v3 09/19] 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>
Acked-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

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

[PATCH v3 10/19] 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>
Acked-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

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

[PATCH v3 11/19] 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.

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

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

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

Re: [PATCH v3 12/19] vdpa_sim: make 'config' generic and usable for any device type

[Jason Wang]

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


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


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

[PATCH v3 13/19] 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>
---
v3:
- checked if get_config callback is set before call it
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 35 +++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 14 deletions(-)

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

Re: [PATCH v3 13/19] vdpa_sim: add get_config callback in vdpasim_dev_attr

[Jason Wang]

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


Patch looks good to me.

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

Thanks


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

Re: [PATCH v3 13/19] vdpa_sim: add get_config callback in vdpasim_dev_attr

[Stefano Garzarella]

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

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

@Michael any suggestion?

Thanks,
Stefano

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

Re: [PATCH v3 13/19] vdpa_sim: add get_config callback in vdpasim_dev_attr

[Stefano Garzarella]

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

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

I'll put these considerations in the commit message.

Thanks,
Stefano

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

[PATCH v3 14/19] vdpa_sim: add set_config callback in vdpasim_dev_attr

[Stefano Garzarella]

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

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

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

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

Re: [PATCH v3 14/19] vdpa_sim: add set_config callback in vdpasim_dev_attr

[Jason Wang]

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


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


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

[PATCH v3 15/19] 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>
---
v3:
- cleared notify during reset [Jason]
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

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

Re: [PATCH v3 15/19] vdpa_sim: set vringh notify callback

[Jason Wang]

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


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


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

[PATCH v3 16/19] 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 1243b02488f7..fb714d88e77f 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -410,7 +410,7 @@ static struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr)
 	if (!vdpasim->iommu)
 		goto err_iommu;
 
-	vdpasim->buffer = kmalloc(PAGE_SIZE, GFP_KERNEL);
+	vdpasim->buffer = kvmalloc(PAGE_SIZE, GFP_KERNEL);
 	if (!vdpasim->buffer)
 		goto err_iommu;
 
@@ -699,7 +699,7 @@ static void vdpasim_free(struct vdpa_device *vdpa)
 	struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
 
 	cancel_work_sync(&vdpasim->work);
-	kfree(vdpasim->buffer);
+	kvfree(vdpasim->buffer);
 	if (vdpasim->iommu)
 		vhost_iotlb_free(vdpasim->iommu);
 	kfree(vdpasim->vqs);
-- 
2.26.2

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

[PATCH v3 18/19] 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.

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

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

[PATCH v3 19/19] 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     | 105 +++++++++++++
 drivers/vdpa/vdpa_sim/vdpa_sim.c     | 224 +--------------------------
 drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 171 ++++++++++++++++++++
 drivers/vdpa/Kconfig                 |  13 +-
 drivers/vdpa/vdpa_sim/Makefile       |   1 +
 5 files changed, 292 insertions(+), 222 deletions(-)
 create mode 100644 drivers/vdpa/vdpa_sim/vdpa_sim.h
 create mode 100644 drivers/vdpa/vdpa_sim/vdpa_sim_net.c

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

Re: [PATCH v3 19/19] vdpa: split vdpasim to core and net modules

[Randy Dunlap]

Hi,

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

	                                         loops


thanks.
-- 
~Randy

Re: [PATCH v3 19/19] vdpa: split vdpasim to core and net modules

[Stefano Garzarella]

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

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

Stefano

Re: [PATCH v3 19/19] vdpa: split vdpasim to core and net modules

[Jason Wang]

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


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