On 2020/11/18 下午9:14, Stefano Garzarella wrote:
Hi Jason,
I just discovered that I missed the other questions in this email,
sorry for that!
No problem :)
On Mon, Nov 16, 2020 at 12:00:11PM +0800, Jason Wang wrote:
On 2020/11/13 下午9:47, Stefano Garzarella wrote:
From: Max Gurtovoy <mgurtovoy@xxxxxxxxxx>
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@xxxxxxxxxx>
[sgarzare: various cleanups/fixes]
Signed-off-by: Stefano Garzarella <sgarzare@xxxxxxxxxx>
---
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]
If possible, I would suggest to split this patch further:
1) convert to use void *config, and an attribute for setting config
size during allocation
2) introduce supported_features
3) other attributes (#vqs)
4) rename config ops (more generic one)
5) introduce ops for set|get_config, set_get_features
6) real split
[...]
-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)
+struct vdpasim *vdpasim_create(struct vdpasim_init_attr *attr)
{
const struct vdpa_config_ops *ops;
struct vdpasim *vdpasim;
+ u32 device_id;
struct device *dev;
- int ret = -ENOMEM;
+ int i, size, ret = -ENOMEM;
- if (batch_mapping)
- ops = &vdpasim_net_batch_config_ops;
+ device_id = attr->device_id;
+ /* Currently, we only accept the network and block devices. */
+ if (device_id != VIRTIO_ID_NET && device_id != VIRTIO_ID_BLOCK)
+ return ERR_PTR(-EOPNOTSUPP);
+
+ if (attr->batch_mapping)
+ 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)
goto err_alloc;
- INIT_WORK(&vdpasim->work, vdpasim_work);
+ if (device_id == VIRTIO_ID_NET)
+ size = sizeof(struct virtio_net_config);
+ else
+ size = sizeof(struct virtio_blk_config);
It's better to avoid such if/else consider we may introduce more type
of devices.
Can we have an attribute of config size instead?
Yes, I'll move the patch 7 before this.
About config size and set/get_config ops, I'm not sure if it is better
to hidden everything under the new set/get_config ops, allocating the
config structure in each device, or leave the allocation in the core
and update it like now.
I think we'd better to avoid having any type specific codes in generic
sim codes.
[...]
+config VDPA_SIM_NET
+ tristate "vDPA simulator for networking device"
+ depends on VDPA_SIM
+ default n
I remember somebody told me that if we don't enable a module it was
disabled by default.
So, should I remove "default n" from vdpa_sim* entries?
Yes, but please do that in another patch.
Thanks
Thanks,
Stefano
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
