Skip to content

Commit

Permalink
lib: Add and use VIRTIO_ROLE_IS_{DRIVER,DEVICE}() macros
Browse files Browse the repository at this point in the history 
There is a common pattern of checking the virtio role while also checking
that this role is supported in this build, which can help optimize away
unusable code. Add a couple macros for this. This has two main benefits,
first being shorter and easier to read if statements, and also makes it
easier to not forget to always do both checks.

Use these everywhere except rpmsg_virtio.c which needs one more refactor
before we can switch it over.

Signed-off-by: Andrew Davis <[email protected]>
  • Loading branch information
@glneo @arnopo
glneo authored and arnopo committed Apr 23, 2024
1 parent 7a8c292 commit f939a86
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 18 deletions.
5 changes: 5 additions & 0 deletions lib/include/openamp/virtio.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,11 @@ __deprecated static inline int deprecated_virtio_dev_slave(void)

#define VIRTIO_ENABLED(option) (option == 1)

#define VIRTIO_ROLE_IS_DRIVER(vdev) \
(VIRTIO_ENABLED(VIRTIO_DRIVER_SUPPORT) && (vdev->role) == VIRTIO_DEV_DRIVER)
#define VIRTIO_ROLE_IS_DEVICE(vdev) \
(VIRTIO_ENABLED(VIRTIO_DEVICE_SUPPORT) && (vdev->role) == VIRTIO_DEV_DEVICE)

/** @brief Virtio device identifier. */
struct virtio_device_id {
/** Virtio subsystem device ID. */
Expand Down
4 changes: 2 additions & 2 deletions lib/remoteproc/remoteproc_virtio.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ static int rproc_virtio_create_virtqueue(struct virtio_device *vdev,
if (!vring_info->vq)
return ERROR_NO_MEM;

if (VIRTIO_ENABLED(VIRTIO_DRIVER_SUPPORT) && vdev->role == VIRTIO_DEV_DRIVER) {
if (VIRTIO_ROLE_IS_DRIVER(vdev)) {
size_t offset = metal_io_virt_to_offset(vring_info->io, vring_alloc->vaddr);
size_t size = vring_size(vring_alloc->num_descs, vring_alloc->align);

Expand Down Expand Up @@ -405,7 +405,7 @@ void rproc_virtio_wait_remote_ready(struct virtio_device *vdev)
* remote action, we can return. Behavior should be updated
* in future if a remote status is added.
*/
if (VIRTIO_ENABLED(VIRTIO_DRIVER_SUPPORT) && vdev->role == VIRTIO_DEV_DRIVER)
if (VIRTIO_ROLE_IS_DRIVER(vdev))
return;

while (1) {
Expand Down
2 changes: 1 addition & 1 deletion lib/virtio/virtio.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ int virtio_create_virtqueues(struct virtio_device *vdev, unsigned int flags,
vring_info = &vdev->vrings_info[i];

vring_alloc = &vring_info->info;
if (VIRTIO_ENABLED(VIRTIO_DRIVER_SUPPORT) && vdev->role == VIRTIO_DEV_DRIVER) {
if (VIRTIO_ROLE_IS_DRIVER(vdev)) {
size_t offset;
struct metal_io_region *io = vring_info->io;

Expand Down
30 changes: 15 additions & 15 deletions lib/virtio/virtqueue.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -278,25 +278,25 @@ void virtqueue_disable_cb(struct virtqueue *vq)
VQUEUE_BUSY(vq);

if (vq->vq_dev->features & VIRTIO_RING_F_EVENT_IDX) {
if (VIRTIO_ENABLED(VIRTIO_DRIVER_SUPPORT) && vq->vq_dev->role == VIRTIO_DEV_DRIVER) {
if (VIRTIO_ROLE_IS_DRIVER(vq->vq_dev)) {
vring_used_event(&vq->vq_ring) =
vq->vq_used_cons_idx - vq->vq_nentries - 1;
VRING_FLUSH(&vring_used_event(&vq->vq_ring),
sizeof(vring_used_event(&vq->vq_ring)));
}
if (VIRTIO_ENABLED(VIRTIO_DEVICE_SUPPORT) && vq->vq_dev->role == VIRTIO_DEV_DEVICE) {
if (VIRTIO_ROLE_IS_DEVICE(vq->vq_dev)) {
vring_avail_event(&vq->vq_ring) =
vq->vq_available_idx - vq->vq_nentries - 1;
VRING_FLUSH(&vring_avail_event(&vq->vq_ring),
sizeof(vring_avail_event(&vq->vq_ring)));
}
} else {
if (VIRTIO_ENABLED(VIRTIO_DRIVER_SUPPORT) && vq->vq_dev->role == VIRTIO_DEV_DRIVER) {
if (VIRTIO_ROLE_IS_DRIVER(vq->vq_dev)) {
vq->vq_ring.avail->flags |= VRING_AVAIL_F_NO_INTERRUPT;
VRING_FLUSH(&vq->vq_ring.avail->flags,
sizeof(vq->vq_ring.avail->flags));
}
if (VIRTIO_ENABLED(VIRTIO_DEVICE_SUPPORT) && vq->vq_dev->role == VIRTIO_DEV_DEVICE) {
if (VIRTIO_ROLE_IS_DEVICE(vq->vq_dev)) {
vq->vq_ring.used->flags |= VRING_USED_F_NO_NOTIFY;
VRING_FLUSH(&vq->vq_ring.used->flags,
sizeof(vq->vq_ring.used->flags));
Expand Down Expand Up @@ -485,7 +485,7 @@ static void vq_ring_init(struct virtqueue *vq, void *ring_mem, int alignment)

vring_init(vr, size, ring_mem, alignment);

if (VIRTIO_ENABLED(VIRTIO_DRIVER_SUPPORT) && vq->vq_dev->role == VIRTIO_DEV_DRIVER) {
if (VIRTIO_ROLE_IS_DRIVER(vq->vq_dev)) {
int i;

for (i = 0; i < size - 1; i++)
Expand Down Expand Up @@ -542,25 +542,25 @@ static int vq_ring_enable_interrupt(struct virtqueue *vq, uint16_t ndesc)
* what's already been consumed.
*/
if (vq->vq_dev->features & VIRTIO_RING_F_EVENT_IDX) {
if (VIRTIO_ENABLED(VIRTIO_DRIVER_SUPPORT) && vq->vq_dev->role == VIRTIO_DEV_DRIVER) {
if (VIRTIO_ROLE_IS_DRIVER(vq->vq_dev)) {
vring_used_event(&vq->vq_ring) =
vq->vq_used_cons_idx + ndesc;
VRING_FLUSH(&vring_used_event(&vq->vq_ring),
sizeof(vring_used_event(&vq->vq_ring)));
}
if (VIRTIO_ENABLED(VIRTIO_DEVICE_SUPPORT) && vq->vq_dev->role == VIRTIO_DEV_DEVICE) {
if (VIRTIO_ROLE_IS_DEVICE(vq->vq_dev)) {
vring_avail_event(&vq->vq_ring) =
vq->vq_available_idx + ndesc;
VRING_FLUSH(&vring_avail_event(&vq->vq_ring),
sizeof(vring_avail_event(&vq->vq_ring)));
}
} else {
if (VIRTIO_ENABLED(VIRTIO_DRIVER_SUPPORT) && vq->vq_dev->role == VIRTIO_DEV_DRIVER) {
if (VIRTIO_ROLE_IS_DRIVER(vq->vq_dev)) {
vq->vq_ring.avail->flags &= ~VRING_AVAIL_F_NO_INTERRUPT;
VRING_FLUSH(&vq->vq_ring.avail->flags,
sizeof(vq->vq_ring.avail->flags));
}
if (VIRTIO_ENABLED(VIRTIO_DEVICE_SUPPORT) && vq->vq_dev->role == VIRTIO_DEV_DEVICE) {
if (VIRTIO_ROLE_IS_DEVICE(vq->vq_dev)) {
vq->vq_ring.used->flags &= ~VRING_USED_F_NO_NOTIFY;
VRING_FLUSH(&vq->vq_ring.used->flags,
sizeof(vq->vq_ring.used->flags));
Expand All @@ -574,12 +574,12 @@ static int vq_ring_enable_interrupt(struct virtqueue *vq, uint16_t ndesc)
* since we last checked. Let our caller know so it processes the new
* entries.
*/
if (VIRTIO_ENABLED(VIRTIO_DRIVER_SUPPORT) && vq->vq_dev->role == VIRTIO_DEV_DRIVER) {
if (VIRTIO_ROLE_IS_DRIVER(vq->vq_dev)) {
if (virtqueue_nused(vq) > ndesc) {
return 1;
}
}
if (VIRTIO_ENABLED(VIRTIO_DEVICE_SUPPORT) && vq->vq_dev->role == VIRTIO_DEV_DEVICE) {
if (VIRTIO_ROLE_IS_DEVICE(vq->vq_dev)) {
if (virtqueue_navail(vq) > ndesc) {
return 1;
}
Expand Down Expand Up @@ -610,7 +610,7 @@ static int vq_ring_must_notify(struct virtqueue *vq)
uint16_t new_idx, prev_idx, event_idx;

if (vq->vq_dev->features & VIRTIO_RING_F_EVENT_IDX) {
if (VIRTIO_ENABLED(VIRTIO_DRIVER_SUPPORT) && vq->vq_dev->role == VIRTIO_DEV_DRIVER) {
if (VIRTIO_ROLE_IS_DRIVER(vq->vq_dev)) {
/* CACHE: no need to invalidate avail */
new_idx = vq->vq_ring.avail->idx;
prev_idx = new_idx - vq->vq_queued_cnt;
Expand All @@ -620,7 +620,7 @@ static int vq_ring_must_notify(struct virtqueue *vq)
return vring_need_event(event_idx, new_idx,
prev_idx) != 0;
}
if (VIRTIO_ENABLED(VIRTIO_DEVICE_SUPPORT) && vq->vq_dev->role == VIRTIO_DEV_DEVICE) {
if (VIRTIO_ROLE_IS_DEVICE(vq->vq_dev)) {
/* CACHE: no need to invalidate used */
new_idx = vq->vq_ring.used->idx;
prev_idx = new_idx - vq->vq_queued_cnt;
Expand All @@ -631,13 +631,13 @@ static int vq_ring_must_notify(struct virtqueue *vq)
prev_idx) != 0;
}
} else {
if (VIRTIO_ENABLED(VIRTIO_DRIVER_SUPPORT) && vq->vq_dev->role == VIRTIO_DEV_DRIVER) {
if (VIRTIO_ROLE_IS_DRIVER(vq->vq_dev)) {
VRING_INVALIDATE(&vq->vq_ring.used->flags,
sizeof(vq->vq_ring.used->flags));
return (vq->vq_ring.used->flags &
VRING_USED_F_NO_NOTIFY) == 0;
}
if (VIRTIO_ENABLED(VIRTIO_DEVICE_SUPPORT) && vq->vq_dev->role == VIRTIO_DEV_DEVICE) {
if (VIRTIO_ROLE_IS_DEVICE(vq->vq_dev)) {
VRING_INVALIDATE(&vq->vq_ring.avail->flags,
sizeof(vq->vq_ring.avail->flags));
return (vq->vq_ring.avail->flags &
Expand Down

0 comments on commit f939a86

Please sign in to comment.