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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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().



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


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

Thanks



Thanks,
Stefano


_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization




[Index of Archives]     [KVM Development]     [Libvirt Development]     [Libvirt Users]     [CentOS Virtualization]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux