Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Move RPMSG buffer address translation out of VirtIO layer #575

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+34 −52
Open

Move RPMSG buffer address translation out of VirtIO layer #575

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3dbbb25
virtqueue: Fix comment on shm_io and fix type
glneo Mar 15, 2024
e05c41a
virtio: Use physical addresses for buffers in virtio
glneo Mar 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions lib/include/openamp/virtqueue.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
31 changes: 16 additions & 15 deletions lib/rpmsg/rpmsg_virtio.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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*/
}

Expand Down Expand Up @@ -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*/

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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,
Expand Down
35 changes: 8 additions & 27 deletions lib/virtio/virtqueue.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we remove shm_io from virtqueue too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we should, I'm just waiting for this PR to go in first to remove the last (accidental) user: #570


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),
Expand Down Expand Up @@ -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)
Expand All @@ -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);
Expand All @@ -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,
Expand Down Expand Up @@ -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;

Expand Down
Loading