From e05c41a1977730824506a1cc5bf9c19cd3b784e4 Mon Sep 17 00:00:00 2001 From: Andrew Davis Date: Wed, 13 Mar 2024 17:49:57 -0500 Subject: [PATCH] virtio: Use physical addresses for buffers in virtio Have the higher level rpmsg_virtio handle the virt-to-phys and back conversion. The rpmsg_virtio layer already has the shared buffer memory metal IO. This removes the need to pass in the buffer region metal_io struct down to the virtio layer, which otherwise does nothing with the contents of the buffers. Signed-off-by: Andrew Davis --- lib/include/openamp/virtqueue.h | 13 ++++++------ lib/rpmsg/rpmsg_virtio.c | 31 +++++++++++++++-------------- lib/virtio/virtqueue.c | 35 ++++++++------------------------- 3 files changed, 31 insertions(+), 48 deletions(-) diff --git a/lib/include/openamp/virtqueue.h b/lib/include/openamp/virtqueue.h index 819439871..dcef05712 100644 --- a/lib/include/openamp/virtqueue.h +++ b/lib/include/openamp/virtqueue.h @@ -65,8 +65,8 @@ extern "C" { /** @brief Buffer descriptor. */ struct virtqueue_buf { - /** Address of the buffer. */ - void *buf; + /** Address (guest-physical) */ + uint64_t buf; /** Size of the buffer. */ int len; @@ -285,12 +285,13 @@ void *virtqueue_get_buffer(struct virtqueue *vq, uint32_t *len, uint16_t *idx); * * @param vq Pointer to VirtIO queue control block * @param avail_idx Pointer to index used in vring desc table + * @param buffer Physical address of buffer * @param len Length of buffer * - * @return Pointer to available buffer + * @return Function status */ -void *virtqueue_get_available_buffer(struct virtqueue *vq, uint16_t *avail_idx, - uint32_t *len); +int virtqueue_get_available_buffer(struct virtqueue *vq, uint16_t *avail_idx, + uint64_t *buffer, uint32_t *len); /** * @internal @@ -381,7 +382,7 @@ void virtqueue_notification(struct virtqueue *vq); uint32_t virtqueue_get_desc_size(struct virtqueue *vq); uint32_t virtqueue_get_buffer_length(struct virtqueue *vq, uint16_t idx); -void *virtqueue_get_buffer_addr(struct virtqueue *vq, uint16_t idx); +uint64_t virtqueue_get_buffer_addr(struct virtqueue *vq, uint16_t idx); /** * @brief Test if virtqueue is empty diff --git a/lib/rpmsg/rpmsg_virtio.c b/lib/rpmsg/rpmsg_virtio.c index 1a9d0bfaa..1d3de1ef0 100644 --- a/lib/rpmsg/rpmsg_virtio.c +++ b/lib/rpmsg/rpmsg_virtio.c @@ -120,7 +120,7 @@ static void rpmsg_virtio_return_buffer(struct rpmsg_virtio_device *rvdev, (void)idx; /* Initialize buffer node */ - vqbuf.buf = buffer; + vqbuf.buf = metal_io_virt_to_phys(rvdev->shbuf_io, buffer); vqbuf.len = len; ret = virtqueue_add_buffer(rvdev->rvq, &vqbuf, 0, 1, buffer); RPMSG_ASSERT(ret == VQUEUE_SUCCESS, "add buffer failed\r\n"); @@ -162,7 +162,7 @@ static int rpmsg_virtio_enqueue_buffer(struct rpmsg_virtio_device *rvdev, (void)idx; /* Initialize buffer node */ - vqbuf.buf = buffer; + vqbuf.buf = metal_io_virt_to_phys(rvdev->shbuf_io, buffer); vqbuf.len = len; return virtqueue_add_buffer(rvdev->svq, &vqbuf, 1, 0, buffer); } @@ -224,7 +224,12 @@ static void *rpmsg_virtio_get_tx_buffer(struct rpmsg_virtio_device *rvdev, #endif /*!VIRTIO_DEVICE_ONLY*/ #ifndef VIRTIO_DRIVER_ONLY } else if (role == RPMSG_REMOTE) { - data = virtqueue_get_available_buffer(rvdev->svq, idx, len); + uint64_t phys_data; + int status; + + status = virtqueue_get_available_buffer(rvdev->svq, idx, &phys_data, len); + if (!status) + data = metal_io_phys_to_virt(rvdev->shbuf_io, phys_data); #endif /*!VIRTIO_DRIVER_ONLY*/ } @@ -256,8 +261,12 @@ static void *rpmsg_virtio_get_rx_buffer(struct rpmsg_virtio_device *rvdev, #ifndef VIRTIO_DRIVER_ONLY if (role == RPMSG_REMOTE) { - data = - virtqueue_get_available_buffer(rvdev->rvq, idx, len); + uint64_t phys_data; + int status; + + status = virtqueue_get_available_buffer(rvdev->rvq, idx, &phys_data, len); + if (!status) + data = metal_io_phys_to_virt(rvdev->shbuf_io, phys_data); } #endif /*!VIRTIO_DRIVER_ONLY*/ @@ -815,7 +824,7 @@ int rpmsg_init_vdev_with_config(struct rpmsg_virtio_device *rvdev, const char *vq_names[RPMSG_NUM_VRINGS]; vq_callback callback[RPMSG_NUM_VRINGS]; int status; - unsigned int i, role; + unsigned int role; if (!rvdev || !vdev || !shm_io) return RPMSG_ERR_PARAM; @@ -918,14 +927,6 @@ int rpmsg_init_vdev_with_config(struct rpmsg_virtio_device *rvdev, */ virtqueue_disable_cb(rvdev->svq); - /* TODO: can have a virtio function to set the shared memory I/O */ - for (i = 0; i < RPMSG_NUM_VRINGS; i++) { - struct virtqueue *vq; - - vq = vdev->vrings_info[i].vq; - vq->shm_io = shm_io; - } - #ifndef VIRTIO_DEVICE_ONLY if (role == RPMSG_HOST) { struct virtqueue_buf vqbuf; @@ -943,7 +944,7 @@ int rpmsg_init_vdev_with_config(struct rpmsg_virtio_device *rvdev, goto err; } - vqbuf.buf = buffer; + vqbuf.buf = metal_io_virt_to_phys(shm_io, buffer); metal_io_block_set(shm_io, metal_io_virt_to_offset(shm_io, diff --git a/lib/virtio/virtqueue.c b/lib/virtio/virtqueue.c index 2544ee360..93069df3c 100644 --- a/lib/virtio/virtqueue.c +++ b/lib/virtio/virtqueue.c @@ -28,24 +28,6 @@ static int virtqueue_nused(struct virtqueue *vq); static int virtqueue_navail(struct virtqueue *vq); #endif -/* Default implementation of P2V based on libmetal */ -static inline void *virtqueue_phys_to_virt(struct virtqueue *vq, - metal_phys_addr_t phys) -{ - struct metal_io_region *io = vq->shm_io; - - return metal_io_phys_to_virt(io, phys); -} - -/* Default implementation of V2P based on libmetal */ -static inline metal_phys_addr_t virtqueue_virt_to_phys(struct virtqueue *vq, - void *buf) -{ - struct metal_io_region *io = vq->shm_io; - - return metal_io_virt_to_phys(io, buf); -} - int virtqueue_create(struct virtio_device *virt_dev, unsigned short id, const char *name, struct vring_alloc_info *ring, void (*callback)(struct virtqueue *vq), @@ -182,11 +164,11 @@ uint32_t virtqueue_get_buffer_length(struct virtqueue *vq, uint16_t idx) return vq->vq_ring.desc[idx].len; } -void *virtqueue_get_buffer_addr(struct virtqueue *vq, uint16_t idx) +uint64_t virtqueue_get_buffer_addr(struct virtqueue *vq, uint16_t idx) { VRING_INVALIDATE(&vq->vq_ring.desc[idx].addr, sizeof(vq->vq_ring.desc[idx].addr)); - return virtqueue_phys_to_virt(vq, vq->vq_ring.desc[idx].addr); + return vq->vq_ring.desc[idx].addr; } void virtqueue_free(struct virtqueue *vq) @@ -202,18 +184,17 @@ void virtqueue_free(struct virtqueue *vq) } } -void *virtqueue_get_available_buffer(struct virtqueue *vq, uint16_t *avail_idx, - uint32_t *len) +int virtqueue_get_available_buffer(struct virtqueue *vq, uint16_t *avail_idx, + uint64_t *buffer, uint32_t *len) { uint16_t head_idx = 0; - void *buffer; atomic_thread_fence(memory_order_seq_cst); /* Avail.idx is updated by driver, invalidate it */ VRING_INVALIDATE(&vq->vq_ring.avail->idx, sizeof(vq->vq_ring.avail->idx)); if (vq->vq_available_idx == vq->vq_ring.avail->idx) { - return NULL; + return ERROR_VRING_NO_BUFF; } VQUEUE_BUSY(vq); @@ -228,12 +209,12 @@ void *virtqueue_get_available_buffer(struct virtqueue *vq, uint16_t *avail_idx, /* Invalidate the desc entry written by driver before accessing it */ VRING_INVALIDATE(&vq->vq_ring.desc[*avail_idx], sizeof(vq->vq_ring.desc[*avail_idx])); - buffer = virtqueue_phys_to_virt(vq, vq->vq_ring.desc[*avail_idx].addr); + *buffer = vq->vq_ring.desc[*avail_idx].addr; *len = vq->vq_ring.desc[*avail_idx].len; VQUEUE_IDLE(vq); - return buffer; + return 0; } int virtqueue_add_consumed_buffer(struct virtqueue *vq, uint16_t head_idx, @@ -414,7 +395,7 @@ static uint16_t vq_ring_add_buffer(struct virtqueue *vq, /* CACHE: No need to invalidate desc because it is only written by driver */ dp = &desc[idx]; - dp->addr = virtqueue_virt_to_phys(vq, buf_list[i].buf); + dp->addr = buf_list[i].buf; dp->len = buf_list[i].len; dp->flags = 0;