Re: [Qemu-devel] [[RFC v3 02/12] virtio: redefine structure & memory cache for packed ring

[Wei Xu]

On Mon, Oct 15, 2018 at 11:03:52AM +0800, Jason Wang wrote:
> 
> 
> On 2018年10月11日 22:08, address@hidden wrote:
> >From: Wei Xu <address@hidden>
> >
> >Redefine packed ring structure according to qemu nomenclature,
> >also supported data(event index, wrap counter, etc) are introduced.
> >
> >Signed-off-by: Wei Xu <address@hidden>
> >---
> >  hw/virtio/virtio.c | 26 ++++++++++++++++++++++++--
> >  1 file changed, 24 insertions(+), 2 deletions(-)
> >
> >diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> >index 94f5c8e..500eecf 100644
> >--- a/hw/virtio/virtio.c
> >+++ b/hw/virtio/virtio.c
> >@@ -39,6 +39,13 @@ typedef struct VRingDesc
> >      uint16_t next;
> >  } VRingDesc;
> >+typedef struct VRingPackedDesc {
> >+    uint64_t addr;
> >+    uint32_t len;
> >+    uint16_t id;
> >+    uint16_t flags;
> >+} VRingPackedDesc;
> >+
> >  typedef struct VRingAvail
> >  {
> >      uint16_t flags;
> >@@ -62,8 +69,14 @@ typedef struct VRingUsed
> >  typedef struct VRingMemoryRegionCaches {
> >      struct rcu_head rcu;
> >      MemoryRegionCache desc;
> >-    MemoryRegionCache avail;
> >-    MemoryRegionCache used;
> >+    union {
> >+        MemoryRegionCache avail;
> >+        MemoryRegionCache driver;
> >+    };
> 
> Can we reuse avail and used?

Sure.

> 
> >+    union {
> >+        MemoryRegionCache used;
> >+        MemoryRegionCache device;
> >+    };
> >  } VRingMemoryRegionCaches;
> >  typedef struct VRing
> >@@ -77,6 +90,11 @@ typedef struct VRing
> >      VRingMemoryRegionCaches *caches;
> >  } VRing;
> >+typedef struct VRingPackedDescEvent {
> >+    uint16_t off_wrap;
> >+    uint16_t flags;
> >+} VRingPackedDescEvent ;
> >+
> >  struct VirtQueue
> >  {
> >      VRing vring;
> >@@ -87,6 +105,10 @@ struct VirtQueue
> >      /* Last avail_idx read from VQ. */
> >      uint16_t shadow_avail_idx;
> >+    uint16_t event_idx;
> 
> Need a comment to explain this field.

Yes, it is the unified name for interrupt which is what I want to see
if we can reuse 'shadow' and 'used' index in current code, for Tx
queue, it should be the 'used' index after finished sending the last
desc. For Rx queue, it should be the 'shadow' index when no enough
descriptors which might be a few descriptors ahead of the 'used' index,
there are a few indexes already so this makes code a bit redundant.

Will see if I can remove this in next version, any comments?

Wei


> 
> Thanks
> 
> >+    bool event_wrap_counter;
> >+    bool avail_wrap_counter;
> >+
> >      uint16_t used_idx;
> >      /* Last used index value we have signalled on */
> 
>