RE: [RFCv4,01/35] plat/virtio: Introduce several helpers for virtio ring

[Justin He <Justin.He@xxxxxxx>]

Hi Razvan

> -----Original Message-----
> From: Minios-devel <minios-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of
> Razvan Virtan
> Sent: Tuesday, March 16, 2021 12:55 AM
> To: Justin He <Justin.He@xxxxxxx>
> Cc: minios-devel@xxxxxxxxxxxxx
> Subject: Re: [RFCv4,01/35] plat/virtio: Introduce several helpers for
> virtio ring
>
> This patch looks good to me, I have only 2 inline sugestions for
> "virito_ring.c".
>
> diff --git a/plat/drivers/virtio/virtio_ring.c
> b/plat/drivers/virtio/virtio_ring.c
> index 0a5c3f3..bda7549 100644
> --- a/plat/drivers/virtio/virtio_ring.c
> +++ b/plat/drivers/virtio/virtio_ring.c
> @@ -262,6 +262,38 @@  __phys_addr virtqueue_physaddr(struct virtqueue *vq)
>       return ukplat_virt_to_phys(vrq->vring_mem);
>  }
>
> +__phys_addr virtqueue_get_avail_addr(struct virtqueue *vq)
> +{
> +     struct virtqueue_vring *vrq = NULL;
> +
> +     UK_ASSERT(vq);
> +
> +     vrq = to_virtqueue_vring(vq);
> Maybe `return ukplat_virt_to_phys((void *) vrq->vring.avail);` would be
> more straightforward?
> +     return virtqueue_physaddr(vq) +
> +             ((char *)vrq->vring.avail - (char *)vrq->vring.desc);
> +}
> +
> +__phys_addr virtqueue_get_used_addr(struct virtqueue *vq)
> +{
> +     struct virtqueue_vring *vrq = NULL;
> +
> +     UK_ASSERT(vq);
> +
> +     vrq = to_virtqueue_vring(vq);
> Maybe `return ukplat_virt_to_phys((void *) vrq->vring.used);` would be more
> straightforward?

I don't quite understand you suggestion here. Seems they are not equal.
ukplat_virt_to_phys((void *) vrq->vring.used) != virtqueue_physaddr(vq) + 
((char *)vrq->vring.used - (char *)vrq->vring.desc)


--
Cheers,
Justin (Jia He)


> +     return virtqueue_physaddr(vq) +
> +             ((char *)vrq->vring.used - (char *)vrq->vring.desc);
> +}
> +
> +unsigned int virtqueue_vring_get_num(struct virtqueue *vq)
> +{
> +     struct virtqueue_vring *vrq = NULL;
> +
> +     UK_ASSERT(vq);
> +
> +     vrq = to_virtqueue_vring(vq);
> +     return vrq->vring.num;
> +}
> +
>
> Reviewed-by: Razvan Virtan <virtanrazvan@xxxxxxxxx>

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.