From: Tiwei Bie <tiwei.bie@intel.com>
To: Jens Freimann <jfreimann@redhat.com>
Cc: dev@dpdk.org, maxime.coquelin@redhat.com, Gavin.Hu@arm.com
Subject: Re: [PATCH v10 1/9] net/virtio: vring init for packed queues
Date: Fri, 2 Nov 2018 22:00:32 +0800 [thread overview]
Message-ID: <20181102140032.GB18161@debian> (raw)
In-Reply-To: <20181102090749.17316-2-jfreimann@redhat.com>
On Fri, Nov 02, 2018 at 10:07:41AM +0100, Jens Freimann wrote:
> 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 | 7 ++++
> drivers/net/virtio/virtio_ring.h | 60 ++++++++++++++++++++++++++----
> drivers/net/virtio/virtqueue.h | 19 +++++++++-
> 4 files changed, 92 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
> index 27088e3a6..0f5a3b80f 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
> - */
This comment is supposed to be kept in this version.
> 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);
> vq->vq_free_cnt = vq->vq_nentries;
> memset(vq->vq_descx, 0, sizeof(struct vq_desc_extra) * vq->vq_nentries);
> -
> - vring_desc_init(vr->desc, size);
> + if (vtpci_packed_queue(vq->hw)) {
> + vring_init_packed(&vq->ring_packed, ring_mem, VIRTIO_PCI_VRING_ALIGN, size);
> + vring_desc_init_packed(vq, size);
> + } else {
> + vring_init_split(vr, ring_mem, VIRTIO_PCI_VRING_ALIGN, size);
> + vring_desc_init_split(vr->desc, size);
> + }
>
> /*
> * Disable device(host) interrupting guest
[...]
> diff --git a/drivers/net/virtio/virtio_ring.h b/drivers/net/virtio/virtio_ring.h
> index 9e3c2a015..69ec84d1e 100644
> --- a/drivers/net/virtio/virtio_ring.h
> +++ b/drivers/net/virtio/virtio_ring.h
> @@ -54,9 +54,35 @@ 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 {
Please change the name to vring_packed_desc to make the name
consistent with the name in vhost code.
> + uint64_t addr;
> + uint32_t len;
> + uint16_t index;
The `index` is supposed to be changed to `id` in this version.
> + uint16_t flags;
> +} __attribute__ ((aligned(16)));
When defining the structure for vring desc, I think the
__attribute__ ((aligned(16))) isn't needed.
> +
> +#define RING_EVENT_FLAGS_ENABLE 0x0
> +#define RING_EVENT_FLAGS_DISABLE 0x1
> +#define RING_EVENT_FLAGS_DESC 0x2
An empty line is supposed to be added here in this version.
> +struct vring_packed_desc_event {
> + uint16_t desc_event_off_wrap;
> + uint16_t desc_event_flags;
> +};
> +
> +struct vring_packed {
> + unsigned int num;
> + struct vring_desc_packed *desc_packed;
> + struct vring_packed_desc_event *driver_event;
> + struct vring_packed_desc_event *device_event;
> +
> +};
> +
> struct vring {
> unsigned int num;
> - struct vring_desc *desc;
> + struct vring_desc *desc;
Above change is unnecessary.
> struct vring_avail *avail;
> struct vring_used *used;
> };
> @@ -95,10 +121,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);
> @@ -106,17 +140,29 @@ vring_size(unsigned int num, unsigned long align)
> (num * sizeof(struct vring_used_elem));
> return size;
> }
> -
Why above empty line is removed?
> static inline void
> -vring_init(struct vring *vr, unsigned int num, uint8_t *p,
> - unsigned long align)
> +vring_init_split(struct vring *vr, uint8_t *p, unsigned long align,
> + unsigned int num)
Above change to the order of the params is unnecessary.
> {
> vr->num = num;
> vr->desc = (struct vring_desc *) p;
> vr->avail = (struct vring_avail *) (p +
> - num * sizeof(struct vring_desc));
> + vr->num * sizeof(struct vring_desc));
Above change is unnecessary.
> vr->used = (void *)
> - RTE_ALIGN_CEIL((uintptr_t)(&vr->avail->ring[num]), align);
> + RTE_ALIGN_CEIL((uintptr_t)(&vr->avail->ring[vr->num]), align);
Above change is unnecessary.
> +}
> +
> +static inline void
> +vring_init_packed(struct vring_packed *vr, uint8_t *p, unsigned long align,
> + unsigned int num)
> +{
> + vr->num = num;
> + vr->desc_packed = (struct vring_desc_packed *)p;
> + vr->driver_event = (struct vring_packed_desc_event *)(p +
> + vr->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);
> }
>
> /*
> diff --git a/drivers/net/virtio/virtqueue.h b/drivers/net/virtio/virtqueue.h
> index 26518ed98..90bff7e38 100644
> --- a/drivers/net/virtio/virtqueue.h
> +++ b/drivers/net/virtio/virtqueue.h
> @@ -161,11 +161,16 @@ struct virtio_pmd_ctrl {
> struct vq_desc_extra {
> void *cookie;
> uint16_t ndescs;
> + uint16_t next;
> };
>
> struct virtqueue {
> struct virtio_hw *hw; /**< virtio_hw structure pointer. */
> struct vring vq_ring; /**< vring keeping desc, used and avail */
> + struct vring_packed ring_packed; /**< vring keeping desc, used and avail */
The comment should be updated for packed ring.
> + bool avail_wrap_counter;
> + bool used_wrap_counter;
> +
> /**
> * Last consumed descriptor in the used table,
> * trails vq_ring.used->idx.
> @@ -245,9 +250,21 @@ struct virtio_tx_region {
> __attribute__((__aligned__(16)));
> };
>
> +static inline void
> +vring_desc_init_packed(struct virtqueue *vq, int n)
> +{
> + int i;
> + for (i = 0; i < n - 1; i++) {
> + vq->ring_packed.desc_packed[i].index = i;
> + vq->vq_descx[i].next = i + 1;
> + }
> + vq->ring_packed.desc_packed[i].index = i;
> + vq->vq_descx[i].next = VQ_RING_DESC_CHAIN_END;
> +}
> +
> /* Chain all the descriptors in the ring with an END */
> static inline void
> -vring_desc_init(struct vring_desc *dp, uint16_t n)
> +vring_desc_init_split(struct vring_desc *dp, uint16_t n)
> {
> uint16_t i;
>
> --
> 2.17.1
>
next prev parent reply other threads:[~2018-11-02 14:02 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-02 9:07 [PATCH v10 0/9] implement packed virtqueues Jens Freimann
2018-11-02 9:07 ` [PATCH v10 1/9] net/virtio: vring init for packed queues Jens Freimann
2018-11-02 14:00 ` Tiwei Bie [this message]
2018-11-02 9:07 ` [PATCH v10 2/9] net/virtio: add packed virtqueue defines Jens Freimann
2018-11-02 9:07 ` [PATCH v10 3/9] net/virtio: add packed virtqueue helpers Jens Freimann
2018-11-02 9:07 ` [PATCH v10 4/9] net/virtio: dump packed virtqueue data Jens Freimann
2018-11-02 9:07 ` [PATCH v10 5/9] net/virtio: implement transmit path for packed queues Jens Freimann
2018-11-02 9:07 ` [PATCH v10 6/9] net/virtio: implement receive " Jens Freimann
2018-11-02 9:07 ` [PATCH v10 7/9] net/virtio: add virtio send command packed queue support Jens Freimann
2018-11-28 17:25 ` Maxime Coquelin
2018-11-02 9:07 ` [PATCH v10 8/9] net/virtio: enable packed virtqueues by default Jens Freimann
2018-11-02 9:07 ` [PATCH v10 9/9] net/virtio-user: disable packed virtqueues when CTRL_VQ enabled Jens Freimann
2018-11-02 14:02 ` [PATCH v10 0/9] implement packed virtqueues Tiwei Bie
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20181102140032.GB18161@debian \
--to=tiwei.bie@intel.com \
--cc=Gavin.Hu@arm.com \
--cc=dev@dpdk.org \
--cc=jfreimann@redhat.com \
--cc=maxime.coquelin@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.