On Wed, 11 Sep 2024 11:46:30 +0800, Jason Wang [off-list ref] wrote:

On Tue, Aug 20, 2024 at 3:33 PM Xuan Zhuo [off-list ref] wrote:

quoted

1. this commit hardens dma unmap for indirect

I think we need to explain why we need such hardening. For example
indirect use stream mapping which is read-only from the device. So it
looks to me like it doesn't require hardening by itself.

quoted

2. the subsequent commit uses the struct extra to record whether the
   buffers need to be unmapped or not.

It's better to explain why such a decision could not be implied with
the existing metadata.

quoted

 So we need a struct extra for
   every desc, whatever it is indirect or not.

If this is the real reason, we need to tweak the title.

YES. It is.

I will tweak the title in next version.

quoted

Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 drivers/virtio/virtio_ring.c | 122 ++++++++++++++++-------------------
 1 file changed, 57 insertions(+), 65 deletions(-)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 228e9fbcba3f..582d2c05498a 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c

@@ -67,9 +67,16 @@
 #define LAST_ADD_TIME_INVALID(vq)
 #endif

+struct vring_desc_extra {
+       dma_addr_t addr;                /* Descriptor DMA addr. */
+       u32 len;                        /* Descriptor length. */
+       u16 flags;                      /* Descriptor flags. */
+       u16 next;                       /* The next desc state in a list. */
+};
+
 struct vring_desc_state_split {
        void *data;                     /* Data for callback. */
-       struct vring_desc *indir_desc;  /* Indirect descriptor, if any. */
+       struct vring_desc_extra *indir; /* Indirect descriptor, if any. */

Btw, it might be worth explaining that this will be allocated with an
indirect descriptor table so we won't stress more to the memory
allocator.

Will do.

Thanks.

Thanks