On Tue, Dec 01, 2020 at 11:44:19AM +0800, Jason Wang wrote:
On 2020/11/30 下午10:14, Stefano Garzarella wrote:
On Mon, Nov 30, 2020 at 11:25:31AM +0800, Jason Wang wrote:
On 2020/11/26 下午10:49, Stefano Garzarella wrote:
The get_config callback can be used by the device to fill the
config structure.
The callback will be invoked in vdpasim_get_config() before copying
bytes into caller buffer.
Move vDPA-net config updates from vdpasim_set_features() in the
new vdpasim_net_get_config() callback.
Signed-off-by: Stefano Garzarella <sgarzare@xxxxxxxxxx>
---
drivers/vdpa/vdpa_sim/vdpa_sim.c | 33 +++++++++++++++++++-------------
1 file changed, 20 insertions(+), 13 deletions(-)
diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c
b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index c07ddf6af720..8b87ce0485b6 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -58,6 +58,8 @@ struct vdpasim_virtqueue {
#define VDPASIM_NET_FEATURES (VDPASIM_FEATURES | \
(1ULL << VIRTIO_NET_F_MAC))
+struct vdpasim;
+
struct vdpasim_dev_attr {
u64 supported_features;
size_t config_size;
@@ -65,6 +67,7 @@ struct vdpasim_dev_attr {
u32 id;
work_func_t work_fn;
+ void (*get_config)(struct vdpasim *vdpasim, void *config);
};
/* State of each vdpasim device */
@@ -520,8 +523,6 @@ static u64 vdpasim_get_features(struct
vdpa_device *vdpa)
static int vdpasim_set_features(struct vdpa_device *vdpa, u64
features)
{
struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
- struct virtio_net_config *config =
- (struct virtio_net_config *)vdpasim->config;
/* DMA mapping must be done by driver */
if (!(features & (1ULL << VIRTIO_F_ACCESS_PLATFORM)))
@@ -529,15 +530,6 @@ static int vdpasim_set_features(struct
vdpa_device *vdpa, u64 features)
vdpasim->features = features &
vdpasim->dev_attr.supported_features;
- /* We generally only know whether guest is using the legacy
interface
- * here, so generally that's the earliest we can set config
fields.
- * Note: We actually require VIRTIO_F_ACCESS_PLATFORM above which
- * implies VIRTIO_F_VERSION_1, but let's not try to be
clever here.
- */
-
- config->mtu = cpu_to_vdpasim16(vdpasim, 1500);
- config->status = cpu_to_vdpasim16(vdpasim, VIRTIO_NET_S_LINK_UP);
- memcpy(config->mac, macaddr_buf, ETH_ALEN);
return 0;
}
@@ -593,8 +585,12 @@ static void vdpasim_get_config(struct
vdpa_device *vdpa, unsigned int offset,
{
struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
- if (offset + len < vdpasim->dev_attr.config_size)
- memcpy(buf, vdpasim->config + offset, len);
+ if (offset + len > vdpasim->dev_attr.config_size)
+ return;
+
+ vdpasim->dev_attr.get_config(vdpasim, vdpasim->config);
+
+ memcpy(buf, vdpasim->config + offset, len);
}
I wonder how much value we can get from vdpasim->config consider
we've already had get_config() method.
Is it possible to copy to the buffer directly here?
I had thought of eliminating it too, but then I wanted to do something
similar to what we do in QEMU (hw/virtio/virtio.c), leaving in the
simulator core the buffer, the memory copy (handling offset and len),
and the boundary checks.
In this way each device should simply fill the entire configuration
and we can avoid code duplication.
Storing the configuration in the core may also be useful if some
device needs to support config writes.
I think in that way we need should provide config_write().
Yes, I'll add set_config() callback.
Do you think it makes sense, or is it better to move everything in the
device?
I prefer to do that in the device but it's also fine keep what the
patch has done.
Okay, for now I'll keep it and add the set_config() callback, but I'm
open to move it in the device.
Thanks,
Stefano
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
