From 50dc25943cba7f6d666c5607fb76b517a145e847 Mon Sep 17 00:00:00 2001 From: Iuliana Prodan Date: Tue, 18 Jul 2023 13:19:19 +0300 Subject: [PATCH 1/5] options: fix typo Fix typo for WITH_DCACHE_BUFFERS option. Signed-off-by: Iuliana Prodan --- cmake/options.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmake/options.cmake b/cmake/options.cmake index a605f20ee..0c2b80aa8 100644 --- a/cmake/options.cmake +++ b/cmake/options.cmake @@ -90,7 +90,7 @@ if (WITH_DCACHE_VRINGS) add_definitions(-DVIRTIO_CACHED_VRINGS) endif (WITH_DCACHE_VRINGS) -option (WITH_DCACHE_BUFFERS "Build with vrings cache operations enabled" OFF) +option (WITH_DCACHE_BUFFERS "Build with buffers cache operations enabled" OFF) if (WITH_DCACHE_BUFFERS) add_definitions(-DVIRTIO_CACHED_BUFFERS) From ef91f0c52190590b6fb8dd2d02ddead48057753e Mon Sep 17 00:00:00 2001 From: Iuliana Prodan Date: Tue, 18 Jul 2023 13:23:35 +0300 Subject: [PATCH 2/5] virtqueue: move VRING_FLUSH and VRING_INVALIDATE to header Move VRING_FLUSH and VRING_INVALIDATE defines to header file. Signed-off-by: Iuliana Prodan --- lib/include/openamp/virtqueue.h | 9 ++++ lib/virtio/virtqueue.c | 92 +++++++++++++++++++-------------- 2 files changed, 61 insertions(+), 40 deletions(-) diff --git a/lib/include/openamp/virtqueue.h b/lib/include/openamp/virtqueue.h index 0b6d24b10..93bc81612 100644 --- a/lib/include/openamp/virtqueue.h +++ b/lib/include/openamp/virtqueue.h @@ -20,6 +20,7 @@ extern "C" { #include #include #include +#include /* Error Codes */ #define VQ_ERROR_BASE -3000 @@ -47,6 +48,14 @@ extern "C" { /* Support to suppress interrupt until specific index is reached. */ #define VIRTIO_RING_F_EVENT_IDX (1 << 29) +#ifdef VIRTIO_CACHED_VRINGS +#define VRING_FLUSH(x, s) metal_cache_flush(&x, s) +#define VRING_INVALIDATE(x, s) metal_cache_invalidate(&x, s) +#else +#define VRING_FLUSH(x, s) do { } while (0) +#define VRING_INVALIDATE(x, s) do { } while (0) +#endif /* VIRTIO_CACHED_VRINGS */ + struct virtqueue_buf { void *buf; int len; diff --git a/lib/virtio/virtqueue.c b/lib/virtio/virtqueue.c index 39e25a209..c5ef51c6c 100644 --- a/lib/virtio/virtqueue.c +++ b/lib/virtio/virtqueue.c @@ -11,7 +11,6 @@ #include #include #include -#include /* Prototype for internal functions. */ static void vq_ring_init(struct virtqueue *, void *, int); @@ -29,14 +28,6 @@ static int virtqueue_nused(struct virtqueue *vq); static int virtqueue_navail(struct virtqueue *vq); #endif -#ifdef VIRTIO_CACHED_VRINGS -#define VRING_FLUSH(x) metal_cache_flush(&x, sizeof(x)) -#define VRING_INVALIDATE(x) metal_cache_invalidate(&x, sizeof(x)) -#else -#define VRING_FLUSH(x) do { } while (0) -#define VRING_INVALIDATE(x) do { } while (0) -#endif /* VIRTIO_CACHED_VRINGS */ - /* Default implementation of P2V based on libmetal */ static inline void *virtqueue_phys_to_virt(struct virtqueue *vq, metal_phys_addr_t phys) @@ -152,7 +143,7 @@ void *virtqueue_get_buffer(struct virtqueue *vq, uint32_t *len, uint16_t *idx) uint16_t used_idx, desc_idx; /* Used.idx is updated by the virtio device, so we need to invalidate */ - VRING_INVALIDATE(vq->vq_ring.used->idx); + VRING_INVALIDATE(vq->vq_ring.used->idx, sizeof(vq->vq_ring.used->idx)); if (!vq || vq->vq_used_cons_idx == vq->vq_ring.used->idx) return NULL; @@ -165,7 +156,8 @@ void *virtqueue_get_buffer(struct virtqueue *vq, uint32_t *len, uint16_t *idx) atomic_thread_fence(memory_order_seq_cst); /* Used.ring is written by remote, invalidate it */ - VRING_INVALIDATE(vq->vq_ring.used->ring[used_idx]); + VRING_INVALIDATE(vq->vq_ring.used->ring[used_idx], + sizeof(vq->vq_ring.used->ring[used_idx])); desc_idx = (uint16_t)uep->id; if (len) @@ -185,13 +177,15 @@ void *virtqueue_get_buffer(struct virtqueue *vq, uint32_t *len, uint16_t *idx) uint32_t virtqueue_get_buffer_length(struct virtqueue *vq, uint16_t idx) { - VRING_INVALIDATE(vq->vq_ring.desc[idx].len); + VRING_INVALIDATE(vq->vq_ring.desc[idx].len, + sizeof(vq->vq_ring.desc[idx].len)); return vq->vq_ring.desc[idx].len; } void *virtqueue_get_buffer_addr(struct virtqueue *vq, uint16_t idx) { - VRING_INVALIDATE(vq->vq_ring.desc[idx].addr); + 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); } @@ -217,7 +211,7 @@ void *virtqueue_get_available_buffer(struct virtqueue *vq, uint16_t *avail_idx, atomic_thread_fence(memory_order_seq_cst); /* Avail.idx is updated by driver, invalidate it */ - VRING_INVALIDATE(vq->vq_ring.avail->idx); + 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; } @@ -227,11 +221,13 @@ void *virtqueue_get_available_buffer(struct virtqueue *vq, uint16_t *avail_idx, head_idx = vq->vq_available_idx++ & (vq->vq_nentries - 1); /* Avail.ring is updated by driver, invalidate it */ - VRING_INVALIDATE(vq->vq_ring.avail->ring[head_idx]); + VRING_INVALIDATE(vq->vq_ring.avail->ring[head_idx], + sizeof(vq->vq_ring.avail->ring[head_idx])); *avail_idx = vq->vq_ring.avail->ring[head_idx]; /* Invalidate the desc entry written by driver before accessing it */ - VRING_INVALIDATE(vq->vq_ring.desc[*avail_idx]); + 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); *len = vq->vq_ring.desc[*avail_idx].len; @@ -259,14 +255,15 @@ int virtqueue_add_consumed_buffer(struct virtqueue *vq, uint16_t head_idx, used_desc->len = len; /* We still need to flush it because this is read by driver */ - VRING_FLUSH(vq->vq_ring.used->ring[used_idx]); + VRING_FLUSH(vq->vq_ring.used->ring[used_idx], + sizeof(vq->vq_ring.used->ring[used_idx])); atomic_thread_fence(memory_order_seq_cst); vq->vq_ring.used->idx++; /* Used.idx is read by driver, so we need to flush it */ - VRING_FLUSH(vq->vq_ring.used->idx); + VRING_FLUSH(vq->vq_ring.used->idx, sizeof(vq->vq_ring.used->idx)); /* Keep pending count until virtqueue_notify(). */ vq->vq_queued_cnt++; @@ -290,27 +287,31 @@ void virtqueue_disable_cb(struct virtqueue *vq) if (vq->vq_dev->role == VIRTIO_DEV_DRIVER) { vring_used_event(&vq->vq_ring) = vq->vq_used_cons_idx - vq->vq_nentries - 1; - VRING_FLUSH(vring_used_event(&vq->vq_ring)); + VRING_FLUSH(vring_used_event(&vq->vq_ring), + sizeof(vring_used_event(&vq->vq_ring))); } #endif /*VIRTIO_DEVICE_ONLY*/ #ifndef VIRTIO_DRIVER_ONLY if (vq->vq_dev->role == VIRTIO_DEV_DEVICE) { vring_avail_event(&vq->vq_ring) = vq->vq_available_idx - vq->vq_nentries - 1; - VRING_FLUSH(vring_avail_event(&vq->vq_ring)); + VRING_FLUSH(vring_avail_event(&vq->vq_ring), + sizeof(vring_avail_event(&vq->vq_ring))); } #endif /*VIRTIO_DRIVER_ONLY*/ } else { #ifndef VIRTIO_DEVICE_ONLY if (vq->vq_dev->role == VIRTIO_DEV_DRIVER) { vq->vq_ring.avail->flags |= VRING_AVAIL_F_NO_INTERRUPT; - VRING_FLUSH(vq->vq_ring.avail->flags); + VRING_FLUSH(vq->vq_ring.avail->flags, + sizeof(vq->vq_ring.avail->flags)); } #endif /*VIRTIO_DEVICE_ONLY*/ #ifndef VIRTIO_DRIVER_ONLY if (vq->vq_dev->role == VIRTIO_DEV_DEVICE) { vq->vq_ring.used->flags |= VRING_USED_F_NO_NOTIFY; - VRING_FLUSH(vq->vq_ring.used->flags); + VRING_FLUSH(vq->vq_ring.used->flags, + sizeof(vq->vq_ring.used->flags)); } #endif /*VIRTIO_DRIVER_ONLY*/ } @@ -338,8 +339,8 @@ void virtqueue_dump(struct virtqueue *vq) if (!vq) return; - VRING_INVALIDATE(vq->vq_ring.avail); - VRING_INVALIDATE(vq->vq_ring.used); + VRING_INVALIDATE(vq->vq_ring.avail, sizeof(vq->vq_ring.avail)); + VRING_INVALIDATE(vq->vq_ring.used, sizeof(vq->vq_ring.used)); metal_log(METAL_LOG_DEBUG, "VQ: %s - size=%d; free=%d; queued=%d; desc_head_idx=%d; " @@ -359,7 +360,7 @@ uint32_t virtqueue_get_desc_size(struct virtqueue *vq) uint32_t len = 0; /* Avail.idx is updated by driver, invalidate it */ - VRING_INVALIDATE(vq->vq_ring.avail->idx); + VRING_INVALIDATE(vq->vq_ring.avail->idx, sizeof(vq->vq_ring.avail->idx)); if (vq->vq_available_idx == vq->vq_ring.avail->idx) { return 0; @@ -370,11 +371,13 @@ uint32_t virtqueue_get_desc_size(struct virtqueue *vq) head_idx = vq->vq_available_idx & (vq->vq_nentries - 1); /* Avail.ring is updated by driver, invalidate it */ - VRING_INVALIDATE(vq->vq_ring.avail->ring[head_idx]); + VRING_INVALIDATE(vq->vq_ring.avail->ring[head_idx], + sizeof(vq->vq_ring.avail->ring[head_idx])); avail_idx = vq->vq_ring.avail->ring[head_idx]; /* Invalidate the desc entry written by driver before accessing it */ - VRING_INVALIDATE(vq->vq_ring.desc[avail_idx].len); + VRING_INVALIDATE(vq->vq_ring.desc[avail_idx].len, + sizeof(vq->vq_ring.desc[avail_idx].len)); len = vq->vq_ring.desc[avail_idx].len; @@ -429,7 +432,7 @@ static uint16_t vq_ring_add_buffer(struct virtqueue *vq, * Instead of flushing the whole desc region, we flush only the * single entry hopefully saving some cycles */ - VRING_FLUSH(desc[idx]); + VRING_FLUSH(desc[idx], sizeof(desc[idx])); } @@ -528,14 +531,15 @@ static void vq_ring_update_avail(struct virtqueue *vq, uint16_t desc_idx) vq->vq_ring.avail->ring[avail_idx] = desc_idx; /* We still need to flush the ring */ - VRING_FLUSH(vq->vq_ring.avail->ring[avail_idx]); + VRING_FLUSH(vq->vq_ring.avail->ring[avail_idx], + sizeof(vq->vq_ring.avail->ring[avail_idx])); atomic_thread_fence(memory_order_seq_cst); vq->vq_ring.avail->idx++; /* And the index */ - VRING_FLUSH(vq->vq_ring.avail->idx); + VRING_FLUSH(vq->vq_ring.avail->idx, sizeof(vq->vq_ring.avail->idx)); /* Keep pending count until virtqueue_notify(). */ vq->vq_queued_cnt++; @@ -557,27 +561,31 @@ static int vq_ring_enable_interrupt(struct virtqueue *vq, uint16_t ndesc) if (vq->vq_dev->role == VIRTIO_DEV_DRIVER) { vring_used_event(&vq->vq_ring) = vq->vq_used_cons_idx + ndesc; - VRING_FLUSH(vring_used_event(&vq->vq_ring)); + VRING_FLUSH(vring_used_event(&vq->vq_ring), + sizeof(vring_used_event(&vq->vq_ring))); } #endif /*VIRTIO_DEVICE_ONLY*/ #ifndef VIRTIO_DRIVER_ONLY if (vq->vq_dev->role == VIRTIO_DEV_DEVICE) { vring_avail_event(&vq->vq_ring) = vq->vq_available_idx + ndesc; - VRING_FLUSH(vring_avail_event(&vq->vq_ring)); + VRING_FLUSH(vring_avail_event(&vq->vq_ring), + sizeof(vring_avail_event(&vq->vq_ring))); } #endif /*VIRTIO_DRIVER_ONLY*/ } else { #ifndef VIRTIO_DEVICE_ONLY if (vq->vq_dev->role == VIRTIO_DEV_DRIVER) { vq->vq_ring.avail->flags &= ~VRING_AVAIL_F_NO_INTERRUPT; - VRING_FLUSH(vq->vq_ring.avail->flags); + VRING_FLUSH(vq->vq_ring.avail->flags, + sizeof(vq->vq_ring.avail->flags)); } #endif /*VIRTIO_DEVICE_ONLY*/ #ifndef VIRTIO_DRIVER_ONLY if (vq->vq_dev->role == VIRTIO_DEV_DEVICE) { vq->vq_ring.used->flags &= ~VRING_USED_F_NO_NOTIFY; - VRING_FLUSH(vq->vq_ring.used->flags); + VRING_FLUSH(vq->vq_ring.used->flags, + sizeof(vq->vq_ring.used->flags)); } #endif /*VIRTIO_DRIVER_ONLY*/ } @@ -634,7 +642,8 @@ static int vq_ring_must_notify(struct virtqueue *vq) /* CACHE: no need to invalidate avail */ new_idx = vq->vq_ring.avail->idx; prev_idx = new_idx - vq->vq_queued_cnt; - VRING_INVALIDATE(vring_avail_event(&vq->vq_ring)); + VRING_INVALIDATE(vring_avail_event(&vq->vq_ring), + sizeof(vring_avail_event(&vq->vq_ring))); event_idx = vring_avail_event(&vq->vq_ring); return vring_need_event(event_idx, new_idx, prev_idx) != 0; @@ -645,7 +654,8 @@ static int vq_ring_must_notify(struct virtqueue *vq) /* CACHE: no need to invalidate used */ new_idx = vq->vq_ring.used->idx; prev_idx = new_idx - vq->vq_queued_cnt; - VRING_INVALIDATE(vring_used_event(&vq->vq_ring)); + VRING_INVALIDATE(vring_used_event(&vq->vq_ring), + sizeof(vring_used_event(&vq->vq_ring))); event_idx = vring_used_event(&vq->vq_ring); return vring_need_event(event_idx, new_idx, prev_idx) != 0; @@ -654,14 +664,16 @@ static int vq_ring_must_notify(struct virtqueue *vq) } else { #ifndef VIRTIO_DEVICE_ONLY if (vq->vq_dev->role == VIRTIO_DEV_DRIVER) { - VRING_INVALIDATE(vq->vq_ring.used->flags); + 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; } #endif /*VIRTIO_DEVICE_ONLY*/ #ifndef VIRTIO_DRIVER_ONLY if (vq->vq_dev->role == VIRTIO_DEV_DEVICE) { - VRING_INVALIDATE(vq->vq_ring.avail->flags); + VRING_INVALIDATE(vq->vq_ring.avail->flags, + sizeof(vq->vq_ring.avail->flags)); return (vq->vq_ring.avail->flags & VRING_AVAIL_F_NO_INTERRUPT) == 0; } @@ -693,7 +705,7 @@ static int virtqueue_nused(struct virtqueue *vq) uint16_t used_idx, nused; /* Used is written by remote */ - VRING_INVALIDATE(vq->vq_ring.used->idx); + VRING_INVALIDATE(vq->vq_ring.used->idx, sizeof(vq->vq_ring.used->idx)); used_idx = vq->vq_ring.used->idx; nused = (uint16_t)(used_idx - vq->vq_used_cons_idx); @@ -714,7 +726,7 @@ static int virtqueue_navail(struct virtqueue *vq) uint16_t avail_idx, navail; /* Avail is written by driver */ - VRING_INVALIDATE(vq->vq_ring.avail->idx); + VRING_INVALIDATE(vq->vq_ring.avail->idx, sizeof(vq->vq_ring.avail->idx)); avail_idx = vq->vq_ring.avail->idx; From 03b7c761a45f16f704f379a4558b2c1e02a6b05c Mon Sep 17 00:00:00 2001 From: Iuliana Prodan Date: Tue, 18 Jul 2023 14:52:34 +0300 Subject: [PATCH 3/5] rpmsg: buffers flush/invalidate Do buffers flush and invalidate the same as with vrings and resource table: - add defines in header file; - call BUFFER_FLUSH/BUFFER_INVALIDATE where necessary. Signed-off-by: Iuliana Prodan --- lib/include/openamp/rpmsg_virtio.h | 9 +++++++++ lib/rpmsg/rpmsg_virtio.c | 13 +++---------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/lib/include/openamp/rpmsg_virtio.h b/lib/include/openamp/rpmsg_virtio.h index e4da54833..cd74498fa 100644 --- a/lib/include/openamp/rpmsg_virtio.h +++ b/lib/include/openamp/rpmsg_virtio.h @@ -14,6 +14,7 @@ #include #include +#include #include #include @@ -29,6 +30,14 @@ extern "C" { /* The feature bitmap for virtio rpmsg */ #define VIRTIO_RPMSG_F_NS 0 /* RP supports name service notifications */ +#ifdef VIRTIO_CACHED_BUFFERS +#define BUFFER_FLUSH(x, s) metal_cache_flush(x, s) +#define BUFFER_INVALIDATE(x, s) metal_cache_invalidate(x, s) +#else +#define BUFFER_FLUSH(x, s) do { } while (0) +#define BUFFER_INVALIDATE(x, s) do { } while (0) +#endif /* VIRTIO_CACHED_BUFFERS */ + /** * struct rpmsg_virtio_shm_pool - shared memory pool used for rpmsg buffers * @base: base address of the memory pool diff --git a/lib/rpmsg/rpmsg_virtio.c b/lib/rpmsg/rpmsg_virtio.c index 0ec519390..ea4cc0d9e 100644 --- a/lib/rpmsg/rpmsg_virtio.c +++ b/lib/rpmsg/rpmsg_virtio.c @@ -9,7 +9,6 @@ */ #include -#include #include #include #include @@ -93,9 +92,7 @@ static void rpmsg_virtio_return_buffer(struct rpmsg_virtio_device *rvdev, { unsigned int role = rpmsg_virtio_get_role(rvdev); -#ifdef VIRTIO_CACHED_BUFFERS - metal_cache_invalidate(buffer, len); -#endif + BUFFER_INVALIDATE(buffer, len); #ifndef VIRTIO_DEVICE_ONLY if (role == RPMSG_HOST) { @@ -135,9 +132,7 @@ static int rpmsg_virtio_enqueue_buffer(struct rpmsg_virtio_device *rvdev, { unsigned int role = rpmsg_virtio_get_role(rvdev); -#ifdef VIRTIO_CACHED_BUFFERS - metal_cache_flush(buffer, len); -#endif /* VIRTIO_CACHED_BUFFERS */ + BUFFER_FLUSH(buffer, len); #ifndef VIRTIO_DEVICE_ONLY if (role == RPMSG_HOST) { @@ -245,11 +240,9 @@ static void *rpmsg_virtio_get_rx_buffer(struct rpmsg_virtio_device *rvdev, } #endif /*!VIRTIO_DRIVER_ONLY*/ -#ifdef VIRTIO_CACHED_BUFFERS /* Invalidate the buffer before returning it */ if (data) - metal_cache_invalidate(data, *len); -#endif /* VIRTIO_CACHED_BUFFERS */ + BUFFER_INVALIDATE(data, *len); return data; } From 99b9ed388fcf58d7147318e787a6180c698a0a7d Mon Sep 17 00:00:00 2001 From: Iuliana Prodan Date: Tue, 18 Jul 2023 19:17:05 +0300 Subject: [PATCH 4/5] virtqueue: define a common cache flush/invalidate Since all cache operations, for vrings, buffer and resource table are using metal_cache_flush and metal_cache_invalidate, define a common function for all. Signed-off-by: Iuliana Prodan --- lib/include/openamp/remoteproc_virtio.h | 4 +- lib/include/openamp/rpmsg_virtio.h | 4 +- lib/include/openamp/virtqueue.h | 8 +++- lib/virtio/virtqueue.c | 62 ++++++++++++------------- 4 files changed, 41 insertions(+), 37 deletions(-) diff --git a/lib/include/openamp/remoteproc_virtio.h b/lib/include/openamp/remoteproc_virtio.h index 31e575efc..770b428cc 100644 --- a/lib/include/openamp/remoteproc_virtio.h +++ b/lib/include/openamp/remoteproc_virtio.h @@ -26,8 +26,8 @@ extern "C" { /* cache invalidation helpers for resource table */ #ifdef VIRTIO_CACHED_RSC_TABLE -#define RSC_TABLE_FLUSH(x, s) metal_cache_flush(x, s) -#define RSC_TABLE_INVALIDATE(x, s) metal_cache_invalidate(x, s) +#define RSC_TABLE_FLUSH(x, s) CACHE_FLUSH(x, s) +#define RSC_TABLE_INVALIDATE(x, s) CACHE_INVALIDATE(x, s) #else #define RSC_TABLE_FLUSH(x, s) do { } while (0) #define RSC_TABLE_INVALIDATE(x, s) do { } while (0) diff --git a/lib/include/openamp/rpmsg_virtio.h b/lib/include/openamp/rpmsg_virtio.h index cd74498fa..9179add20 100644 --- a/lib/include/openamp/rpmsg_virtio.h +++ b/lib/include/openamp/rpmsg_virtio.h @@ -31,8 +31,8 @@ extern "C" { #define VIRTIO_RPMSG_F_NS 0 /* RP supports name service notifications */ #ifdef VIRTIO_CACHED_BUFFERS -#define BUFFER_FLUSH(x, s) metal_cache_flush(x, s) -#define BUFFER_INVALIDATE(x, s) metal_cache_invalidate(x, s) +#define BUFFER_FLUSH(x, s) CACHE_FLUSH(x, s) +#define BUFFER_INVALIDATE(x, s) CACHE_INVALIDATE(x, s) #else #define BUFFER_FLUSH(x, s) do { } while (0) #define BUFFER_INVALIDATE(x, s) do { } while (0) diff --git a/lib/include/openamp/virtqueue.h b/lib/include/openamp/virtqueue.h index 93bc81612..2bc1ff1e8 100644 --- a/lib/include/openamp/virtqueue.h +++ b/lib/include/openamp/virtqueue.h @@ -48,9 +48,13 @@ extern "C" { /* Support to suppress interrupt until specific index is reached. */ #define VIRTIO_RING_F_EVENT_IDX (1 << 29) +/* cache invalidation helpers */ +#define CACHE_FLUSH(x, s) metal_cache_flush(x, s) +#define CACHE_INVALIDATE(x, s) metal_cache_invalidate(x, s) + #ifdef VIRTIO_CACHED_VRINGS -#define VRING_FLUSH(x, s) metal_cache_flush(&x, s) -#define VRING_INVALIDATE(x, s) metal_cache_invalidate(&x, s) +#define VRING_FLUSH(x, s) CACHE_FLUSH(x, s) +#define VRING_INVALIDATE(x, s) CACHE_INVALIDATE(x, s) #else #define VRING_FLUSH(x, s) do { } while (0) #define VRING_INVALIDATE(x, s) do { } while (0) diff --git a/lib/virtio/virtqueue.c b/lib/virtio/virtqueue.c index c5ef51c6c..2544ee360 100644 --- a/lib/virtio/virtqueue.c +++ b/lib/virtio/virtqueue.c @@ -143,7 +143,7 @@ void *virtqueue_get_buffer(struct virtqueue *vq, uint32_t *len, uint16_t *idx) uint16_t used_idx, desc_idx; /* Used.idx is updated by the virtio device, so we need to invalidate */ - VRING_INVALIDATE(vq->vq_ring.used->idx, sizeof(vq->vq_ring.used->idx)); + VRING_INVALIDATE(&vq->vq_ring.used->idx, sizeof(vq->vq_ring.used->idx)); if (!vq || vq->vq_used_cons_idx == vq->vq_ring.used->idx) return NULL; @@ -156,7 +156,7 @@ void *virtqueue_get_buffer(struct virtqueue *vq, uint32_t *len, uint16_t *idx) atomic_thread_fence(memory_order_seq_cst); /* Used.ring is written by remote, invalidate it */ - VRING_INVALIDATE(vq->vq_ring.used->ring[used_idx], + VRING_INVALIDATE(&vq->vq_ring.used->ring[used_idx], sizeof(vq->vq_ring.used->ring[used_idx])); desc_idx = (uint16_t)uep->id; @@ -177,14 +177,14 @@ void *virtqueue_get_buffer(struct virtqueue *vq, uint32_t *len, uint16_t *idx) uint32_t virtqueue_get_buffer_length(struct virtqueue *vq, uint16_t idx) { - VRING_INVALIDATE(vq->vq_ring.desc[idx].len, + VRING_INVALIDATE(&vq->vq_ring.desc[idx].len, sizeof(vq->vq_ring.desc[idx].len)); return vq->vq_ring.desc[idx].len; } void *virtqueue_get_buffer_addr(struct virtqueue *vq, uint16_t idx) { - VRING_INVALIDATE(vq->vq_ring.desc[idx].addr, + 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); } @@ -211,7 +211,7 @@ void *virtqueue_get_available_buffer(struct virtqueue *vq, uint16_t *avail_idx, 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)); + 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; } @@ -221,12 +221,12 @@ void *virtqueue_get_available_buffer(struct virtqueue *vq, uint16_t *avail_idx, head_idx = vq->vq_available_idx++ & (vq->vq_nentries - 1); /* Avail.ring is updated by driver, invalidate it */ - VRING_INVALIDATE(vq->vq_ring.avail->ring[head_idx], + VRING_INVALIDATE(&vq->vq_ring.avail->ring[head_idx], sizeof(vq->vq_ring.avail->ring[head_idx])); *avail_idx = vq->vq_ring.avail->ring[head_idx]; /* Invalidate the desc entry written by driver before accessing it */ - VRING_INVALIDATE(vq->vq_ring.desc[*avail_idx], + 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); *len = vq->vq_ring.desc[*avail_idx].len; @@ -255,7 +255,7 @@ int virtqueue_add_consumed_buffer(struct virtqueue *vq, uint16_t head_idx, used_desc->len = len; /* We still need to flush it because this is read by driver */ - VRING_FLUSH(vq->vq_ring.used->ring[used_idx], + VRING_FLUSH(&vq->vq_ring.used->ring[used_idx], sizeof(vq->vq_ring.used->ring[used_idx])); atomic_thread_fence(memory_order_seq_cst); @@ -263,7 +263,7 @@ int virtqueue_add_consumed_buffer(struct virtqueue *vq, uint16_t head_idx, vq->vq_ring.used->idx++; /* Used.idx is read by driver, so we need to flush it */ - VRING_FLUSH(vq->vq_ring.used->idx, sizeof(vq->vq_ring.used->idx)); + VRING_FLUSH(&vq->vq_ring.used->idx, sizeof(vq->vq_ring.used->idx)); /* Keep pending count until virtqueue_notify(). */ vq->vq_queued_cnt++; @@ -287,7 +287,7 @@ void virtqueue_disable_cb(struct virtqueue *vq) if (vq->vq_dev->role == VIRTIO_DEV_DRIVER) { vring_used_event(&vq->vq_ring) = vq->vq_used_cons_idx - vq->vq_nentries - 1; - VRING_FLUSH(vring_used_event(&vq->vq_ring), + VRING_FLUSH(&vring_used_event(&vq->vq_ring), sizeof(vring_used_event(&vq->vq_ring))); } #endif /*VIRTIO_DEVICE_ONLY*/ @@ -295,7 +295,7 @@ void virtqueue_disable_cb(struct virtqueue *vq) if (vq->vq_dev->role == VIRTIO_DEV_DEVICE) { vring_avail_event(&vq->vq_ring) = vq->vq_available_idx - vq->vq_nentries - 1; - VRING_FLUSH(vring_avail_event(&vq->vq_ring), + VRING_FLUSH(&vring_avail_event(&vq->vq_ring), sizeof(vring_avail_event(&vq->vq_ring))); } #endif /*VIRTIO_DRIVER_ONLY*/ @@ -303,14 +303,14 @@ void virtqueue_disable_cb(struct virtqueue *vq) #ifndef VIRTIO_DEVICE_ONLY if (vq->vq_dev->role == VIRTIO_DEV_DRIVER) { vq->vq_ring.avail->flags |= VRING_AVAIL_F_NO_INTERRUPT; - VRING_FLUSH(vq->vq_ring.avail->flags, + VRING_FLUSH(&vq->vq_ring.avail->flags, sizeof(vq->vq_ring.avail->flags)); } #endif /*VIRTIO_DEVICE_ONLY*/ #ifndef VIRTIO_DRIVER_ONLY if (vq->vq_dev->role == VIRTIO_DEV_DEVICE) { vq->vq_ring.used->flags |= VRING_USED_F_NO_NOTIFY; - VRING_FLUSH(vq->vq_ring.used->flags, + VRING_FLUSH(&vq->vq_ring.used->flags, sizeof(vq->vq_ring.used->flags)); } #endif /*VIRTIO_DRIVER_ONLY*/ @@ -339,8 +339,8 @@ void virtqueue_dump(struct virtqueue *vq) if (!vq) return; - VRING_INVALIDATE(vq->vq_ring.avail, sizeof(vq->vq_ring.avail)); - VRING_INVALIDATE(vq->vq_ring.used, sizeof(vq->vq_ring.used)); + VRING_INVALIDATE(&vq->vq_ring.avail, sizeof(vq->vq_ring.avail)); + VRING_INVALIDATE(&vq->vq_ring.used, sizeof(vq->vq_ring.used)); metal_log(METAL_LOG_DEBUG, "VQ: %s - size=%d; free=%d; queued=%d; desc_head_idx=%d; " @@ -360,7 +360,7 @@ uint32_t virtqueue_get_desc_size(struct virtqueue *vq) uint32_t len = 0; /* Avail.idx is updated by driver, invalidate it */ - VRING_INVALIDATE(vq->vq_ring.avail->idx, sizeof(vq->vq_ring.avail->idx)); + VRING_INVALIDATE(&vq->vq_ring.avail->idx, sizeof(vq->vq_ring.avail->idx)); if (vq->vq_available_idx == vq->vq_ring.avail->idx) { return 0; @@ -371,12 +371,12 @@ uint32_t virtqueue_get_desc_size(struct virtqueue *vq) head_idx = vq->vq_available_idx & (vq->vq_nentries - 1); /* Avail.ring is updated by driver, invalidate it */ - VRING_INVALIDATE(vq->vq_ring.avail->ring[head_idx], + VRING_INVALIDATE(&vq->vq_ring.avail->ring[head_idx], sizeof(vq->vq_ring.avail->ring[head_idx])); avail_idx = vq->vq_ring.avail->ring[head_idx]; /* Invalidate the desc entry written by driver before accessing it */ - VRING_INVALIDATE(vq->vq_ring.desc[avail_idx].len, + VRING_INVALIDATE(&vq->vq_ring.desc[avail_idx].len, sizeof(vq->vq_ring.desc[avail_idx].len)); len = vq->vq_ring.desc[avail_idx].len; @@ -432,7 +432,7 @@ static uint16_t vq_ring_add_buffer(struct virtqueue *vq, * Instead of flushing the whole desc region, we flush only the * single entry hopefully saving some cycles */ - VRING_FLUSH(desc[idx], sizeof(desc[idx])); + VRING_FLUSH(&desc[idx], sizeof(desc[idx])); } @@ -531,7 +531,7 @@ static void vq_ring_update_avail(struct virtqueue *vq, uint16_t desc_idx) vq->vq_ring.avail->ring[avail_idx] = desc_idx; /* We still need to flush the ring */ - VRING_FLUSH(vq->vq_ring.avail->ring[avail_idx], + VRING_FLUSH(&vq->vq_ring.avail->ring[avail_idx], sizeof(vq->vq_ring.avail->ring[avail_idx])); atomic_thread_fence(memory_order_seq_cst); @@ -539,7 +539,7 @@ static void vq_ring_update_avail(struct virtqueue *vq, uint16_t desc_idx) vq->vq_ring.avail->idx++; /* And the index */ - VRING_FLUSH(vq->vq_ring.avail->idx, sizeof(vq->vq_ring.avail->idx)); + VRING_FLUSH(&vq->vq_ring.avail->idx, sizeof(vq->vq_ring.avail->idx)); /* Keep pending count until virtqueue_notify(). */ vq->vq_queued_cnt++; @@ -561,7 +561,7 @@ static int vq_ring_enable_interrupt(struct virtqueue *vq, uint16_t ndesc) if (vq->vq_dev->role == VIRTIO_DEV_DRIVER) { vring_used_event(&vq->vq_ring) = vq->vq_used_cons_idx + ndesc; - VRING_FLUSH(vring_used_event(&vq->vq_ring), + VRING_FLUSH(&vring_used_event(&vq->vq_ring), sizeof(vring_used_event(&vq->vq_ring))); } #endif /*VIRTIO_DEVICE_ONLY*/ @@ -569,7 +569,7 @@ static int vq_ring_enable_interrupt(struct virtqueue *vq, uint16_t ndesc) if (vq->vq_dev->role == VIRTIO_DEV_DEVICE) { vring_avail_event(&vq->vq_ring) = vq->vq_available_idx + ndesc; - VRING_FLUSH(vring_avail_event(&vq->vq_ring), + VRING_FLUSH(&vring_avail_event(&vq->vq_ring), sizeof(vring_avail_event(&vq->vq_ring))); } #endif /*VIRTIO_DRIVER_ONLY*/ @@ -577,14 +577,14 @@ static int vq_ring_enable_interrupt(struct virtqueue *vq, uint16_t ndesc) #ifndef VIRTIO_DEVICE_ONLY if (vq->vq_dev->role == VIRTIO_DEV_DRIVER) { vq->vq_ring.avail->flags &= ~VRING_AVAIL_F_NO_INTERRUPT; - VRING_FLUSH(vq->vq_ring.avail->flags, + VRING_FLUSH(&vq->vq_ring.avail->flags, sizeof(vq->vq_ring.avail->flags)); } #endif /*VIRTIO_DEVICE_ONLY*/ #ifndef VIRTIO_DRIVER_ONLY if (vq->vq_dev->role == VIRTIO_DEV_DEVICE) { vq->vq_ring.used->flags &= ~VRING_USED_F_NO_NOTIFY; - VRING_FLUSH(vq->vq_ring.used->flags, + VRING_FLUSH(&vq->vq_ring.used->flags, sizeof(vq->vq_ring.used->flags)); } #endif /*VIRTIO_DRIVER_ONLY*/ @@ -642,7 +642,7 @@ static int vq_ring_must_notify(struct virtqueue *vq) /* CACHE: no need to invalidate avail */ new_idx = vq->vq_ring.avail->idx; prev_idx = new_idx - vq->vq_queued_cnt; - VRING_INVALIDATE(vring_avail_event(&vq->vq_ring), + VRING_INVALIDATE(&vring_avail_event(&vq->vq_ring), sizeof(vring_avail_event(&vq->vq_ring))); event_idx = vring_avail_event(&vq->vq_ring); return vring_need_event(event_idx, new_idx, @@ -654,7 +654,7 @@ static int vq_ring_must_notify(struct virtqueue *vq) /* CACHE: no need to invalidate used */ new_idx = vq->vq_ring.used->idx; prev_idx = new_idx - vq->vq_queued_cnt; - VRING_INVALIDATE(vring_used_event(&vq->vq_ring), + VRING_INVALIDATE(&vring_used_event(&vq->vq_ring), sizeof(vring_used_event(&vq->vq_ring))); event_idx = vring_used_event(&vq->vq_ring); return vring_need_event(event_idx, new_idx, @@ -664,7 +664,7 @@ static int vq_ring_must_notify(struct virtqueue *vq) } else { #ifndef VIRTIO_DEVICE_ONLY if (vq->vq_dev->role == VIRTIO_DEV_DRIVER) { - VRING_INVALIDATE(vq->vq_ring.used->flags, + 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; @@ -672,7 +672,7 @@ static int vq_ring_must_notify(struct virtqueue *vq) #endif /*VIRTIO_DEVICE_ONLY*/ #ifndef VIRTIO_DRIVER_ONLY if (vq->vq_dev->role == VIRTIO_DEV_DEVICE) { - VRING_INVALIDATE(vq->vq_ring.avail->flags, + VRING_INVALIDATE(&vq->vq_ring.avail->flags, sizeof(vq->vq_ring.avail->flags)); return (vq->vq_ring.avail->flags & VRING_AVAIL_F_NO_INTERRUPT) == 0; @@ -705,7 +705,7 @@ static int virtqueue_nused(struct virtqueue *vq) uint16_t used_idx, nused; /* Used is written by remote */ - VRING_INVALIDATE(vq->vq_ring.used->idx, sizeof(vq->vq_ring.used->idx)); + VRING_INVALIDATE(&vq->vq_ring.used->idx, sizeof(vq->vq_ring.used->idx)); used_idx = vq->vq_ring.used->idx; nused = (uint16_t)(used_idx - vq->vq_used_cons_idx); @@ -726,7 +726,7 @@ static int virtqueue_navail(struct virtqueue *vq) uint16_t avail_idx, navail; /* Avail is written by driver */ - VRING_INVALIDATE(vq->vq_ring.avail->idx, sizeof(vq->vq_ring.avail->idx)); + VRING_INVALIDATE(&vq->vq_ring.avail->idx, sizeof(vq->vq_ring.avail->idx)); avail_idx = vq->vq_ring.avail->idx; From 003311e7f7363af65a300f6ffda0cbb92cb0ecb9 Mon Sep 17 00:00:00 2001 From: Iuliana Prodan Date: Tue, 18 Jul 2023 15:19:06 +0300 Subject: [PATCH 5/5] options: add option for all cache operations Add WITH_DCACHE operation used for all cache operations: vrings, buffers and resource table. The other options will be deprecated - add warning message for this. Add info for WITH_DCACHE option in README. Signed-off-by: Iuliana Prodan --- README.md | 3 +++ cmake/options.cmake | 9 +++++++++ lib/include/openamp/remoteproc_virtio.h | 5 ++++- lib/include/openamp/rpmsg_virtio.h | 5 ++++- lib/include/openamp/virtqueue.h | 5 ++++- 5 files changed, 24 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 35ce24308..74b9b7157 100644 --- a/README.md +++ b/README.md @@ -96,6 +96,9 @@ library for it project: enabled on buffers. * **WITH_DCACHE_RSC_TABLE** (default OFF): Build with data cache operations enabled on resource table. +* **WITH_DCACHE** (default OFF): Build with all cache operations + enabled. When set to ON, cache operations for vrings, buffers and resource + table are enabled. * **RPMSG_BUFFER_SIZE** (default 512): adjust the size of the RPMsg buffers. The default value of the RPMsg size is compatible with the Linux Kernel hard coded value. If you AMP configuration is Linux kernel host/ OpenAMP remote, diff --git a/cmake/options.cmake b/cmake/options.cmake index 0c2b80aa8..35a3333fd 100644 --- a/cmake/options.cmake +++ b/cmake/options.cmake @@ -84,22 +84,31 @@ if (WITH_VIRTIO_MMIO_DRV) add_definitions(-DWITH_VIRTIO_MMIO_DRV) endif (WITH_VIRTIO_MMIO_DRV) +option (WITH_DCACHE "Build with all cache operations enabled" OFF) + +if (WITH_DCACHE) + add_definitions(-DVIRTIO_USE_DCACHE) +endif (WITH_DCACHE) + option (WITH_DCACHE_VRINGS "Build with vrings cache operations enabled" OFF) if (WITH_DCACHE_VRINGS) add_definitions(-DVIRTIO_CACHED_VRINGS) + message(DEPRECATION "deprecated cmake option replaced by WITH_DCACHE" ...) endif (WITH_DCACHE_VRINGS) option (WITH_DCACHE_BUFFERS "Build with buffers cache operations enabled" OFF) if (WITH_DCACHE_BUFFERS) add_definitions(-DVIRTIO_CACHED_BUFFERS) + message(DEPRECATION "deprecated cmake option replaced by WITH_DCACHE" ...) endif (WITH_DCACHE_BUFFERS) option (WITH_DCACHE_RSC_TABLE "Build with resource table cache operations enabled" OFF) if (WITH_DCACHE_RSC_TABLE) add_definitions(-DVIRTIO_CACHED_RSC_TABLE) + message(DEPRECATION "deprecated cmake option replaced by WITH_DCACHE" ...) endif (WITH_DCACHE_RSC_TABLE) # Set the complication flags diff --git a/lib/include/openamp/remoteproc_virtio.h b/lib/include/openamp/remoteproc_virtio.h index 770b428cc..ab06bb5a7 100644 --- a/lib/include/openamp/remoteproc_virtio.h +++ b/lib/include/openamp/remoteproc_virtio.h @@ -26,12 +26,15 @@ extern "C" { /* cache invalidation helpers for resource table */ #ifdef VIRTIO_CACHED_RSC_TABLE +#warning "VIRTIO_CACHED_RSC_TABLE is deprecated, please use VIRTIO_USE_DCACHE" +#endif +#if defined(VIRTIO_CACHED_RSC_TABLE) || defined(VIRTIO_USE_DCACHE) #define RSC_TABLE_FLUSH(x, s) CACHE_FLUSH(x, s) #define RSC_TABLE_INVALIDATE(x, s) CACHE_INVALIDATE(x, s) #else #define RSC_TABLE_FLUSH(x, s) do { } while (0) #define RSC_TABLE_INVALIDATE(x, s) do { } while (0) -#endif /* VIRTIO_CACHED_RSC_TABLE */ +#endif /* VIRTIO_CACHED_RSC_TABLE || VIRTIO_USE_DCACHE */ /* define vdev notification function user should implement */ typedef int (*rpvdev_notify_func)(void *priv, uint32_t id); diff --git a/lib/include/openamp/rpmsg_virtio.h b/lib/include/openamp/rpmsg_virtio.h index 9179add20..6dfe905a1 100644 --- a/lib/include/openamp/rpmsg_virtio.h +++ b/lib/include/openamp/rpmsg_virtio.h @@ -31,12 +31,15 @@ extern "C" { #define VIRTIO_RPMSG_F_NS 0 /* RP supports name service notifications */ #ifdef VIRTIO_CACHED_BUFFERS +#warning "VIRTIO_CACHED_BUFFERS is deprecated, please use VIRTIO_USE_DCACHE" +#endif +#if defined(VIRTIO_CACHED_BUFFERS) || defined(VIRTIO_USE_DCACHE) #define BUFFER_FLUSH(x, s) CACHE_FLUSH(x, s) #define BUFFER_INVALIDATE(x, s) CACHE_INVALIDATE(x, s) #else #define BUFFER_FLUSH(x, s) do { } while (0) #define BUFFER_INVALIDATE(x, s) do { } while (0) -#endif /* VIRTIO_CACHED_BUFFERS */ +#endif /* VIRTIO_CACHED_BUFFERS || VIRTIO_USE_DCACHE */ /** * struct rpmsg_virtio_shm_pool - shared memory pool used for rpmsg buffers diff --git a/lib/include/openamp/virtqueue.h b/lib/include/openamp/virtqueue.h index 2bc1ff1e8..1b44e5417 100644 --- a/lib/include/openamp/virtqueue.h +++ b/lib/include/openamp/virtqueue.h @@ -53,12 +53,15 @@ extern "C" { #define CACHE_INVALIDATE(x, s) metal_cache_invalidate(x, s) #ifdef VIRTIO_CACHED_VRINGS +#warning "VIRTIO_CACHED_VRINGS is deprecated, please use VIRTIO_USE_DCACHE" +#endif +#if defined(VIRTIO_CACHED_VRINGS) || defined(VIRTIO_USE_DCACHE) #define VRING_FLUSH(x, s) CACHE_FLUSH(x, s) #define VRING_INVALIDATE(x, s) CACHE_INVALIDATE(x, s) #else #define VRING_FLUSH(x, s) do { } while (0) #define VRING_INVALIDATE(x, s) do { } while (0) -#endif /* VIRTIO_CACHED_VRINGS */ +#endif /* VIRTIO_CACHED_VRINGS || VIRTIO_USE_DCACHE */ struct virtqueue_buf { void *buf;