From a383ef0b5ad8b8c58f2febbec80f10ba1eaaf5c6 Mon Sep 17 00:00:00 2001 From: Hector Martin Date: Thu, 4 Nov 2021 03:53:39 +0900 Subject: [PATCH] iodev: Protect iodevs with spinlocks This makes SMP IRQ reports less flaky Signed-off-by: Hector Martin --- src/fb.c | 1 + src/iodev.c | 52 ++++++++++++++++++++++++++++++++++++++++++---------- src/iodev.h | 1 + src/uart.c | 1 + src/usb.c | 4 ++++ src/utils.c | 6 ++++++ src/utils.h | 7 ++++++- 7 files changed, 61 insertions(+), 11 deletions(-) diff --git a/src/fb.c b/src/fb.c index 08119ce0..8e712cc6 100644 --- a/src/fb.c +++ b/src/fb.c @@ -301,6 +301,7 @@ const struct iodev_ops iodev_fb_ops = { struct iodev iodev_fb = { .ops = &iodev_fb_ops, .usage = USAGE_CONSOLE, + .lock = SPINLOCK_INIT, }; static void fb_clear_console(void) diff --git a/src/iodev.c b/src/iodev.c index ff83f0ce..d4b7fbd3 100644 --- a/src/iodev.c +++ b/src/iodev.c @@ -36,7 +36,10 @@ ssize_t iodev_can_read(iodev_id_t id) if (!iodevs[id]->ops->can_read) return 0; - return iodevs[id]->ops->can_read(iodevs[id]->opaque); + spin_lock(&iodevs[id]->lock); + ssize_t ret = iodevs[id]->ops->can_read(iodevs[id]->opaque); + spin_unlock(&iodevs[id]->lock); + return ret; } bool iodev_can_write(iodev_id_t id) @@ -44,7 +47,12 @@ bool iodev_can_write(iodev_id_t id) if (!iodevs[id]->ops->can_write) return false; - return iodevs[id]->ops->can_write(iodevs[id]->opaque); + if (mmu_active()) + spin_lock(&iodevs[id]->lock); + bool ret = iodevs[id]->ops->can_write(iodevs[id]->opaque); + if (mmu_active()) + spin_unlock(&iodevs[id]->lock); + return ret; } ssize_t iodev_read(iodev_id_t id, void *buf, size_t length) @@ -52,7 +60,10 @@ ssize_t iodev_read(iodev_id_t id, void *buf, size_t length) if (!iodevs[id]->ops->read) return -1; - return iodevs[id]->ops->read(iodevs[id]->opaque, buf, length); + spin_lock(&iodevs[id]->lock); + ssize_t ret = iodevs[id]->ops->read(iodevs[id]->opaque, buf, length); + spin_unlock(&iodevs[id]->lock); + return ret; } ssize_t iodev_write(iodev_id_t id, const void *buf, size_t length) @@ -60,7 +71,12 @@ ssize_t iodev_write(iodev_id_t id, const void *buf, size_t length) if (!iodevs[id]->ops->write) return -1; - return iodevs[id]->ops->write(iodevs[id]->opaque, buf, length); + if (mmu_active()) + spin_lock(&iodevs[id]->lock); + ssize_t ret = iodevs[id]->ops->write(iodevs[id]->opaque, buf, length); + if (mmu_active()) + spin_unlock(&iodevs[id]->lock); + return ret; } ssize_t iodev_queue(iodev_id_t id, const void *buf, size_t length) @@ -68,7 +84,10 @@ ssize_t iodev_queue(iodev_id_t id, const void *buf, size_t length) if (!iodevs[id]->ops->queue) return iodev_write(id, buf, length); - return iodevs[id]->ops->queue(iodevs[id]->opaque, buf, length); + spin_lock(&iodevs[id]->lock); + ssize_t ret = iodevs[id]->ops->queue(iodevs[id]->opaque, buf, length); + spin_unlock(&iodevs[id]->lock); + return ret; } void iodev_flush(iodev_id_t id) @@ -76,7 +95,9 @@ void iodev_flush(iodev_id_t id) if (!iodevs[id]->ops->flush) return; + spin_lock(&iodevs[id]->lock); iodevs[id]->ops->flush(iodevs[id]->opaque); + spin_unlock(&iodevs[id]->lock); } int in_iodev = 0; @@ -89,8 +110,8 @@ void iodev_console_write(const void *buf, size_t length) if (!do_lock && !is_primary_core()) { if (length && iodevs[IODEV_UART]->usage & USAGE_CONSOLE) { - iodev_write(IODEV_UART, "*", 1); - iodev_write(IODEV_UART, buf, length); + iodevs[IODEV_UART]->ops->write(iodevs[IODEV_UART]->opaque, "*", 1); + iodevs[IODEV_UART]->ops->write(iodevs[IODEV_UART]->opaque, buf, length); } return; } @@ -100,8 +121,8 @@ void iodev_console_write(const void *buf, size_t length) if (in_iodev) { if (length && iodevs[IODEV_UART]->usage & USAGE_CONSOLE) { - iodev_write(IODEV_UART, "*", 1); - iodev_write(IODEV_UART, buf, length); + iodevs[IODEV_UART]->ops->write(iodevs[IODEV_UART]->opaque, "+", 1); + iodevs[IODEV_UART]->ops->write(iodevs[IODEV_UART]->opaque, buf, length); } if (do_lock) spin_unlock(&console_lock); @@ -183,8 +204,16 @@ void iodev_console_write(const void *buf, size_t length) void iodev_handle_events(iodev_id_t id) { - if (in_iodev) + bool do_lock = mmu_active(); + + if (do_lock) + spin_lock(&console_lock); + + if (in_iodev) { + if (do_lock) + spin_unlock(&console_lock); return; + } in_iodev++; @@ -195,6 +224,9 @@ void iodev_handle_events(iodev_id_t id) if (iodev_can_write(id)) iodev_console_write(NULL, 0); + + if (do_lock) + spin_unlock(&console_lock); } void iodev_console_kick(void) diff --git a/src/iodev.h b/src/iodev.h index 14648465..b56db60e 100644 --- a/src/iodev.h +++ b/src/iodev.h @@ -34,6 +34,7 @@ struct iodev_ops { struct iodev { const struct iodev_ops *ops; + spinlock_t lock; iodev_usage_t usage; void *opaque; }; diff --git a/src/uart.c b/src/uart.c index 5caf651e..67aa0e3a 100644 --- a/src/uart.c +++ b/src/uart.c @@ -176,4 +176,5 @@ static struct iodev_ops iodev_uart_ops = { struct iodev iodev_uart = { .ops = &iodev_uart_ops, .usage = USAGE_CONSOLE | USAGE_UARTPROXY, + .lock = SPINLOCK_INIT, }; diff --git a/src/usb.c b/src/usb.c index c1c4f8c8..9267f6fb 100644 --- a/src/usb.c +++ b/src/usb.c @@ -210,10 +210,12 @@ struct iodev iodev_usb[USB_INSTANCES] = { { .ops = &iodev_usb_ops, .usage = USAGE_CONSOLE | USAGE_UARTPROXY, + .lock = SPINLOCK_INIT, }, { .ops = &iodev_usb_ops, .usage = USAGE_CONSOLE | USAGE_UARTPROXY, + .lock = SPINLOCK_INIT, }, }; @@ -221,10 +223,12 @@ struct iodev iodev_usb_sec[USB_INSTANCES] = { { .ops = &iodev_usb_sec_ops, .usage = 0, + .lock = SPINLOCK_INIT, }, { .ops = &iodev_usb_sec_ops, .usage = 0, + .lock = SPINLOCK_INIT, }, }; diff --git a/src/utils.c b/src/utils.c index bc719935..65c229ee 100644 --- a/src/utils.c +++ b/src/utils.c @@ -102,6 +102,12 @@ void flush_and_reboot(void) reboot(); } +void spin_init(spinlock_t *lock) +{ + lock->lock = -1; + lock->count = 0; +} + void spin_lock(spinlock_t *lock) { s64 me = smp_id(); diff --git a/src/utils.h b/src/utils.h index 10233276..e3f642d5 100644 --- a/src/utils.h +++ b/src/utils.h @@ -371,8 +371,13 @@ typedef struct { int count; } spinlock_t ALIGNED(64); -#define DECLARE_SPINLOCK(n) spinlock_t n = {-1, 0} +#define SPINLOCK_INIT \ + { \ + -1, 0 \ + } +#define DECLARE_SPINLOCK(n) spinlock_t n = SPINLOCK_INIT; +void spin_init(spinlock_t *lock); void spin_lock(spinlock_t *lock); void spin_unlock(spinlock_t *lock);