Re: [dpdk-dev] [PATCH v5 01/11] net/virtio: vring init for packed queues

["Gavin Hu (Arm Technology China)" <Gavin.Hu@arm.com>]

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Jens Freimann
> Sent: Friday, September 7, 2018 2:20 AM
> To: dev@dpdk.org
> Cc: tiwei.bie@intel.com; maxime.coquelin@redhat.com
> Subject: [dpdk-dev] [PATCH v5 01/11] net/virtio: vring init for packed queues
>
> Add and initialize descriptor data structures.
>
> Signed-off-by: Jens Freimann <jfreimann@redhat.com>
> ---
>  drivers/net/virtio/virtio_ethdev.c | 22 ++++++------
>  drivers/net/virtio/virtio_pci.h    |  8 +++++
>  drivers/net/virtio/virtio_ring.h   | 55 +++++++++++++++++++++++++++---
>  drivers/net/virtio/virtqueue.h     | 10 ++++++
>  4 files changed, 80 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/net/virtio/virtio_ethdev.c
> b/drivers/net/virtio/virtio_ethdev.c
> index 614357da7..ad91f7f82 100644
> --- a/drivers/net/virtio/virtio_ethdev.c
> +++ b/drivers/net/virtio/virtio_ethdev.c
> @@ -299,19 +299,21 @@ virtio_init_vring(struct virtqueue *vq)
>
>  PMD_INIT_FUNC_TRACE();
>
> -/*
> - * Reinitialise since virtio port might have been stopped and restarted
> - */
>  memset(ring_mem, 0, vq->vq_ring_size);
> -vring_init(vr, size, ring_mem, VIRTIO_PCI_VRING_ALIGN);
> -vq->vq_used_cons_idx = 0;
> -vq->vq_desc_head_idx = 0;
> -vq->vq_avail_idx = 0;
> -vq->vq_desc_tail_idx = (uint16_t)(vq->vq_nentries - 1);
> +vring_init(vq->hw, vr, size, ring_mem, VIRTIO_PCI_VRING_ALIGN);
> +
>  vq->vq_free_cnt = vq->vq_nentries;
>  memset(vq->vq_descx, 0, sizeof(struct vq_desc_extra) * vq-
> >vq_nentries);
> +vq->vq_used_cons_idx = 0;
> +vq->vq_avail_idx     = 0;
> +if (vtpci_packed_queue(vq->hw)) {
> +vring_desc_init_packed(vr, size);
> +} else {
> +vq->vq_desc_head_idx = 0;
> +vq->vq_desc_tail_idx = (uint16_t)(vq->vq_nentries - 1);
>
> -vring_desc_init(vr->desc, size);
> +vring_desc_init(vr->desc, size);
> +}
>
>  /*
>   * Disable device(host) interrupting guest @@ -386,7 +388,7 @@
> virtio_init_queue(struct rte_eth_dev *dev, uint16_t vtpci_queue_idx)
>  /*
>   * Reserve a memzone for vring elements
>   */
> -size = vring_size(vq_size, VIRTIO_PCI_VRING_ALIGN);
> +size = vring_size(hw, vq_size, VIRTIO_PCI_VRING_ALIGN);
>  vq->vq_ring_size = RTE_ALIGN_CEIL(size,
> VIRTIO_PCI_VRING_ALIGN);
>  PMD_INIT_LOG(DEBUG, "vring_size: %d, rounded_vring_size: %d",
>       size, vq->vq_ring_size);
> diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
> index 58fdd3d45..90204d281 100644
> --- a/drivers/net/virtio/virtio_pci.h
> +++ b/drivers/net/virtio/virtio_pci.h
> @@ -113,6 +113,8 @@ struct virtnet_ctl;
>
>  #define VIRTIO_F_VERSION_132
>  #define VIRTIO_F_IOMMU_PLATFORM33
> +#define VIRTIO_F_RING_PACKED34
> +#define VIRTIO_F_IN_ORDER35
>
>  /*
>   * Some VirtIO feature bits (currently bits 28 through 31) are @@ -314,6
> +316,12 @@ vtpci_with_feature(struct virtio_hw *hw, uint64_t bit)
>  return (hw->guest_features & (1ULL << bit)) != 0;  }
>
> +static inline int
> +vtpci_packed_queue(struct virtio_hw *hw) {
> +return vtpci_with_feature(hw, VIRTIO_F_RING_PACKED); }
> +
>  /*
>   * Function declaration from virtio_pci.c
>   */
> diff --git a/drivers/net/virtio/virtio_ring.h b/drivers/net/virtio/virtio_ring.h
> index 9e3c2a015..cea4d441e 100644
> --- a/drivers/net/virtio/virtio_ring.h
> +++ b/drivers/net/virtio/virtio_ring.h
> @@ -54,11 +54,38 @@ struct vring_used {
>  struct vring_used_elem ring[0];
>  };
>
> +/* For support of packed virtqueues in Virtio 1.1 the format of
> +descriptors
> + * looks like this.
> + */
> +struct vring_desc_packed {
> +uint64_t addr;

Should be __le64 here, according to spec. same as follows.

> +uint32_t len;
> +uint16_t index;
> +uint16_t flags;
> +};

Should have __attribute__ ((aligned(16))) for alignment according to the spec? Not sure if it automatically aligned at 16bytes boundary.

> +#define RING_EVENT_FLAGS_ENABLE 0x0
> +#define RING_EVENT_FLAGS_DISABLE 0x1
> +#define RING_EVENT_FLAGS_DESC 0x2
> +struct vring_packed_desc_event {
> +uint16_t desc_event_off_wrap;
> +uint16_t desc_event_flags;
> +};
> +
>  struct vring {
>  unsigned int num;
> -struct vring_desc  *desc;
> -struct vring_avail *avail;
> -struct vring_used  *used;
> +union {
> +struct vring_desc_packed *desc_packed;
> +struct vring_desc *desc;
> +};
> +union {
> +struct vring_avail *avail;
> +struct vring_packed_desc_event *driver_event;
> +};
> +union {
> +struct vring_used  *used;
> +struct vring_packed_desc_event *device_event;
> +};
>  };
>
>  /* The standard layout for the ring is a continuous chunk of memory which
> @@ -95,10 +122,18 @@ struct vring {  #define vring_avail_event(vr)
> (*(uint16_t *)&(vr)->used->ring[(vr)->num])
>
>  static inline size_t
> -vring_size(unsigned int num, unsigned long align)
> +vring_size(struct virtio_hw *hw, unsigned int num, unsigned long align)
>  {
>  size_t size;
>
> +if (vtpci_packed_queue(hw)) {
> +size = num * sizeof(struct vring_desc_packed);
> +size += sizeof(struct vring_packed_desc_event);
> +size = RTE_ALIGN_CEIL(size, align);
> +size += sizeof(struct vring_packed_desc_event);
> +return size;
> +}
> +
>  size = num * sizeof(struct vring_desc);
>  size += sizeof(struct vring_avail) + (num * sizeof(uint16_t));
>  size = RTE_ALIGN_CEIL(size, align);
> @@ -108,10 +143,20 @@ vring_size(unsigned int num, unsigned long align)  }
>
>  static inline void
> -vring_init(struct vring *vr, unsigned int num, uint8_t *p,
> +vring_init(struct virtio_hw *hw, struct vring *vr, unsigned int num,
> +uint8_t *p,
>  unsigned long align)
>  {
>  vr->num = num;
> +if (vtpci_packed_queue(hw)) {
> +vr->desc_packed = (struct vring_desc_packed *)p;
> +vr->driver_event = (struct vring_packed_desc_event *)(p +
> +num * sizeof(struct vring_desc_packed));
> +vr->device_event = (struct vring_packed_desc_event *)
> +RTE_ALIGN_CEIL((uintptr_t)(vr-
> >driver_event +
> +sizeof(struct vring_packed_desc_event)),
> align);
> +return;
> +}
> +
>  vr->desc = (struct vring_desc *) p;
>  vr->avail = (struct vring_avail *) (p +
>  num * sizeof(struct vring_desc));
> diff --git a/drivers/net/virtio/virtqueue.h b/drivers/net/virtio/virtqueue.h
> index 26518ed98..d2a0b651a 100644
> --- a/drivers/net/virtio/virtqueue.h
> +++ b/drivers/net/virtio/virtqueue.h
> @@ -245,6 +245,16 @@ struct virtio_tx_region {
>     __attribute__((__aligned__(16)));  };
>
> +static inline void
> +vring_desc_init_packed(struct vring *vr, int n) {
> +int i;
> +for (i = 0; i < n; i++) {
> +struct vring_desc_packed *desc = &vr->desc_packed[i];
> +desc->index = i;
> +}
> +}
> +
>  /* Chain all the descriptors in the ring with an END */  static inline void
> vring_desc_init(struct vring_desc *dp, uint16_t n)
> --
> 2.17.1

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.