On 4/8/24 10:02, ni.liqiang wrote: > In our testing of the virtio hardware accelerator, we found that > configuring the flags of the descriptor after addr and len, > as implemented in DPDK, seems to be more friendly to the hardware. please describe in detail "friendly to the hardware" means ..
> In our Virtio hardware implementation tests, using the default > open-source code, the hardware's bulk reads ensure performance > but correctness is compromised. If we refer to the implementation code > of DPDK, placing the flags configuration of the descriptor > after addr and len, virtio backend can function properly based on > our hardware accelerator. you are not specifying how the correctness is compromised .. again what the "properly" mean here ? what is the exact failure that you are seeing ? please document .. > I am somewhat puzzled by this. From a software process perspective, > it seems that there should be no difference whether > the flags configuration of the descriptor is before or after addr and len. > However, this is not the case according to experimental test results. > We would like to know if such a change in the configuration order > is reasonable and acceptable? where are the experimental results ? any particular reason those results are not documented here ? > > Thanks. > > Signed-off-by: ni.liqiang <[email protected]> > Reviewed-by: jin.qi <[email protected]> > Tested-by: jin.qi <[email protected]> > Cc: ni.liqiang <[email protected]> > --- > drivers/virtio/virtio_ring.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c > index 6f7e5010a673..bea2c2fb084e 100644 > --- a/drivers/virtio/virtio_ring.c > +++ b/drivers/virtio/virtio_ring.c > @@ -1472,15 +1472,16 @@ static inline int virtqueue_add_packed(struct > virtqueue *_vq, > flags = cpu_to_le16(vq->packed.avail_used_flags | > (++c == total_sg ? 0 : VRING_DESC_F_NEXT) | > (n < out_sgs ? 0 : VRING_DESC_F_WRITE)); > - if (i == head) > - head_flags = flags; > - else > - desc[i].flags = flags; > > desc[i].addr = cpu_to_le64(addr); > desc[i].len = cpu_to_le32(sg->length); > desc[i].id = cpu_to_le16(id); > > + if (i == head) > + head_flags = flags; > + else > + desc[i].flags = flags; > + > if (unlikely(vq->use_dma_api)) { > vq->packed.desc_extra[curr].addr = addr; > vq->packed.desc_extra[curr].len = sg->length; -ck
