Re: [Minios-devel] [UNIKRAFT PATCH v3 02/14] plat/virtio: Introduce virtqueue_hasdata function

[Justin He <Justin.He@xxxxxxx>]

virtqueue_hasdata should use virtio16_to_cpu to support both big and little 
endian.
But given that this is not relevant to this patch itself, LGTM. Just comment 
here maybe
we need improve the unikraft virtio to support big/little endian arches.

Reviewed-by: Jia He <justin.he@xxxxxxx>

--
Cheers,
Justin (Jia He)


> -----Original Message-----
> From: Roxana Nicolescu <nicolescu.roxana1996@xxxxxxxxx>
> Sent: Friday, March 6, 2020 4:30 AM
> To: minios-devel@xxxxxxxxxxxxx
> Cc: Justin He <Justin.He@xxxxxxx>; Roxana Nicolescu
> <nicolescu.roxana1996@xxxxxxxxx>
> Subject: [UNIKRAFT PATCH v3 02/14] plat/virtio: Introduce
> virtqueue_hasdata function
>
> This patch introduces the function which checks
> if a queue has some pending requests.
>
> Signed-off-by: Roxana Nicolescu <nicolescu.roxana1996@xxxxxxxxx>
> ---
>  plat/drivers/include/virtio/virtqueue.h | 10 ++++++++++
>  plat/drivers/virtio/virtio_ring.c       | 20 +++++++++++---------
>  2 files changed, 21 insertions(+), 9 deletions(-)
>
> diff --git a/plat/drivers/include/virtio/virtqueue.h
> b/plat/drivers/include/virtio/virtqueue.h
> index f8b36348..14798288 100644
> --- a/plat/drivers/include/virtio/virtqueue.h
> +++ b/plat/drivers/include/virtio/virtqueue.h
> @@ -198,6 +198,16 @@ struct virtqueue *virtqueue_create(__u16 queue_id,
> __u16 nr_descs, __u16 align,
>   */
>  int virtqueue_is_full(struct virtqueue *vq);
>
> +/**
> + * Check the virtqueue if has any pending responses.
> + * @param vq
> + *A reference to the virtqueue.
> + * @return int
> + *1 on true,
> + *0 otherwise
> + */
> +int virtqueue_hasdata(struct virtqueue *vq);
> +
>  /*
>   * Destroy a virtual queue
>   * @param vq
> diff --git a/plat/drivers/virtio/virtio_ring.c 
> b/plat/drivers/virtio/virtio_ring.c
> index 235679d2..ab9287ac 100644
> --- a/plat/drivers/virtio/virtio_ring.c
> +++ b/plat/drivers/virtio/virtio_ring.c
> @@ -78,7 +78,6 @@ static inline void virtqueue_ring_update_avail(struct
> virtqueue_vring *vrq,
>         __u16 idx);
>  static inline void virtqueue_detach_desc(struct virtqueue_vring *vrq,
>   __u16 head_idx);
> -static inline int virtqueue_hasdata(struct virtqueue_vring *vrq);
>  static inline int virtqueue_buffer_enqueue_segments(
>      struct virtqueue_vring *vrq,
>      __u16 head,
> @@ -110,7 +109,7 @@ int virtqueue_intr_enable(struct virtqueue *vq)
>
>  vrq = to_virtqueue_vring(vq);
>  /* Check if there are no more packets enabled */
> -if (!virtqueue_hasdata(vrq)) {
> +if (!virtqueue_hasdata(vq)) {
>  if (vrq->vring.avail->flags | VRING_AVAIL_F_NO_INTERRUPT) {
>  vrq->vring.avail->flags &=
>  (~VRING_AVAIL_F_NO_INTERRUPT);
> @@ -125,7 +124,7 @@ int virtqueue_intr_enable(struct virtqueue *vq)
>   */
>  mb();
>  /* Check if there are further descriptors */
> -if (virtqueue_hasdata(vrq)) {
> +if (virtqueue_hasdata(vq)) {
>  virtqueue_intr_disable(vq);
>  rc = 1;
>  }
> @@ -216,9 +215,14 @@ static inline int
> virtqueue_buffer_enqueue_segments(
>  return idx;
>  }
>
> -static inline int virtqueue_hasdata(struct virtqueue_vring *vrq)
> +int virtqueue_hasdata(struct virtqueue *vq)
>  {
> -return (vrq->last_used_desc_idx != vrq->vring.used->idx);
> +struct virtqueue_vring *vring;
> +
> +UK_ASSERT(vq);
> +
> +vring = to_virtqueue_vring(vq);
> +return (vring->last_used_desc_idx != vring->vring.used->idx);
>  }
>
>  __u64 virtqueue_feature_negotiate(__u64 feature_set)
> @@ -235,14 +239,12 @@ __u64 virtqueue_feature_negotiate(__u64
> feature_set)
>
>  int virtqueue_ring_interrupt(void *obj)
>  {
> -struct virtqueue_vring *vrq = NULL;
>  struct virtqueue *vq = (struct virtqueue *)obj;
>  int rc = 0;
>
>  UK_ASSERT(vq);
>
> -vrq = to_virtqueue_vring(vq);
> -if (!virtqueue_hasdata(vrq))
> +if (!virtqueue_hasdata(vq))
>  return rc;
>
>  if (likely(vq->vq_callback))
> @@ -271,7 +273,7 @@ int virtqueue_buffer_dequeue(struct virtqueue *vq,
> void **cookie, __u32 *len)
>  vrq = to_virtqueue_vring(vq);
>
>  /* No new descriptor since last dequeue operation */
> -if (!virtqueue_hasdata(vrq))
> +if (!virtqueue_hasdata(vq))
>  return -ENOMSG;
>  used_idx = vrq->last_used_desc_idx++ & (vrq->vring.num - 1);
>  elem = &vrq->vring.used->ring[used_idx];
> --
> 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.

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel