On Tue, Aug 20, 2024 at 3:33 PM Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx> wrote: > > 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. > 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. > 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. > > Signed-off-by: Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx> > --- > 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. Thanks