The branch main has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=e10b9d6602dfd876a47f85d8866cabb5a26b8482
commit e10b9d6602dfd876a47f85d8866cabb5a26b8482 Author: SHENG-YI HONG <[email protected]> AuthorDate: 2024-05-01 15:09:31 +0000 Commit: Mark Johnston <[email protected]> CommitDate: 2024-05-01 15:09:31 +0000 bhyve: Move lock of uart frontend to uart backend Currently, lock of uart in bhyve is placed in frontend. There are some problems about it: 1. If every frontend should has a lock, why not move it inside backend as they all have same uart_softc. 2. If backend needs to modify the information of uart after initialize, it will be impossible as backend cannot use lock. For example, if we want implement a telnet support for uart in backend, It should wait for connection when initialize. After some remote process connect it, it needs to modify rfd and wfd in backend. So I decide to move it to backend. Reviewed by: corvink, jhb, markj Differential Revision: https://reviews.freebsd.org/D44947 --- usr.sbin/bhyve/uart_backend.c | 22 +++++++++++++++++++++- usr.sbin/bhyve/uart_backend.h | 3 ++- usr.sbin/bhyve/uart_emul.c | 15 ++++++--------- usr.sbin/bhyve/uart_pl011.c | 16 ++++++---------- 4 files changed, 35 insertions(+), 21 deletions(-) diff --git a/usr.sbin/bhyve/uart_backend.c b/usr.sbin/bhyve/uart_backend.c index 8d91f4f671e1..51844cbf1170 100644 --- a/usr.sbin/bhyve/uart_backend.c +++ b/usr.sbin/bhyve/uart_backend.c @@ -35,6 +35,7 @@ #include <assert.h> #include <capsicum_helpers.h> #include <err.h> +#include <pthread.h> #include <stdbool.h> #include <stdlib.h> #include <string.h> @@ -66,6 +67,7 @@ struct uart_softc { struct ttyfd tty; struct fifo rxfifo; struct mevent *mev; + pthread_mutex_t mtx; }; static bool uart_stdio; /* stdio in use for i/o */ @@ -325,7 +327,13 @@ uart_tty_backend(struct uart_softc *sc, const char *path) struct uart_softc * uart_init(void) { - return (calloc(1, sizeof(struct uart_softc))); + struct uart_softc *sc = calloc(1, sizeof(struct uart_softc)); + if (sc == NULL) + return (NULL); + + pthread_mutex_init(&sc->mtx, NULL); + + return (sc); } int @@ -346,3 +354,15 @@ uart_tty_open(struct uart_softc *sc, const char *path, return (retval); } + +void +uart_softc_lock(struct uart_softc *sc) +{ + pthread_mutex_lock(&sc->mtx); +} + +void +uart_softc_unlock(struct uart_softc *sc) +{ + pthread_mutex_unlock(&sc->mtx); +} diff --git a/usr.sbin/bhyve/uart_backend.h b/usr.sbin/bhyve/uart_backend.h index fa7949ad6d1c..ecf785ffd893 100644 --- a/usr.sbin/bhyve/uart_backend.h +++ b/usr.sbin/bhyve/uart_backend.h @@ -51,5 +51,6 @@ int uart_rxfifo_snapshot(struct uart_softc *sc, struct uart_softc *uart_init(void); int uart_tty_open(struct uart_softc *sc, const char *path, void (*drain)(int, enum ev_type, void *), void *arg); - +void uart_softc_lock(struct uart_softc *sc); +void uart_softc_unlock(struct uart_softc *sc); #endif /* _UART_BACKEND_H_ */ diff --git a/usr.sbin/bhyve/uart_emul.c b/usr.sbin/bhyve/uart_emul.c index 58d6697e4fea..bc71d4760634 100644 --- a/usr.sbin/bhyve/uart_emul.c +++ b/usr.sbin/bhyve/uart_emul.c @@ -83,7 +83,6 @@ static struct { struct uart_ns16550_softc { struct uart_softc *backend; - pthread_mutex_t mtx; /* protects all softc elements */ uint8_t data; /* Data register (R/W) */ uint8_t ier; /* Interrupt enable register (R/W) */ uint8_t lcr; /* Line control register (R/W) */ @@ -204,14 +203,14 @@ uart_drain(int fd __unused, enum ev_type ev, void *arg) * to take out the softc lock to protect against concurrent * access from a vCPU i/o exit */ - pthread_mutex_lock(&sc->mtx); + uart_softc_lock(sc->backend); loopback = (sc->mcr & MCR_LOOPBACK) != 0; uart_rxfifo_drain(sc->backend, loopback); if (!loopback) uart_toggle_intr(sc); - pthread_mutex_unlock(&sc->mtx); + uart_softc_unlock(sc->backend); } void @@ -220,7 +219,7 @@ uart_ns16550_write(struct uart_ns16550_softc *sc, int offset, uint8_t value) int fifosz; uint8_t msr; - pthread_mutex_lock(&sc->mtx); + uart_softc_lock(sc->backend); /* * Take care of the special case DLAB accesses first @@ -329,7 +328,7 @@ uart_ns16550_write(struct uart_ns16550_softc *sc, int offset, uint8_t value) done: uart_toggle_intr(sc); - pthread_mutex_unlock(&sc->mtx); + uart_softc_unlock(sc->backend); } uint8_t @@ -337,7 +336,7 @@ uart_ns16550_read(struct uart_ns16550_softc *sc, int offset) { uint8_t iir, intr_reason, reg; - pthread_mutex_lock(&sc->mtx); + uart_softc_lock(sc->backend); /* * Take care of the special case DLAB accesses first @@ -414,7 +413,7 @@ uart_ns16550_read(struct uart_ns16550_softc *sc, int offset) done: uart_toggle_intr(sc); - pthread_mutex_unlock(&sc->mtx); + uart_softc_unlock(sc->backend); return (reg); } @@ -446,8 +445,6 @@ uart_ns16550_init(uart_intr_func_t intr_assert, uart_intr_func_t intr_deassert, sc->intr_deassert = intr_deassert; sc->backend = uart_init(); - pthread_mutex_init(&sc->mtx, NULL); - uart_reset(sc); return (sc); diff --git a/usr.sbin/bhyve/uart_pl011.c b/usr.sbin/bhyve/uart_pl011.c index e2d5c8bf5657..fb576451c544 100644 --- a/usr.sbin/bhyve/uart_pl011.c +++ b/usr.sbin/bhyve/uart_pl011.c @@ -31,7 +31,6 @@ #include <sys/param.h> #include <assert.h> -#include <pthread.h> #include <stdbool.h> #include <stdio.h> #include <stdlib.h> @@ -106,7 +105,6 @@ struct uart_pl011_softc { struct uart_softc *backend; - pthread_mutex_t mtx; /* protects all softc elements */ uint16_t irq_state; @@ -184,7 +182,7 @@ uart_drain(int fd __unused, enum ev_type ev, void *arg) * to take out the softc lock to protect against concurrent * access from a vCPU i/o exit */ - pthread_mutex_lock(&sc->mtx); + uart_softc_lock(sc->backend); old_size = uart_rxfifo_numchars(sc->backend); @@ -202,7 +200,7 @@ uart_drain(int fd __unused, enum ev_type ev, void *arg) if (!loopback) uart_toggle_intr(sc); - pthread_mutex_unlock(&sc->mtx); + uart_softc_unlock(sc->backend); } void @@ -210,7 +208,7 @@ uart_pl011_write(struct uart_pl011_softc *sc, int offset, uint32_t value) { bool loopback; - pthread_mutex_lock(&sc->mtx); + uart_softc_lock(sc->backend); switch (offset) { case UARTDR: loopback = (sc->cr & UARTCR_LBE) != 0; @@ -266,7 +264,7 @@ uart_pl011_write(struct uart_pl011_softc *sc, int offset, uint32_t value) break; } uart_toggle_intr(sc); - pthread_mutex_unlock(&sc->mtx); + uart_softc_unlock(sc->backend); } uint32_t @@ -276,7 +274,7 @@ uart_pl011_read(struct uart_pl011_softc *sc, int offset) int fifo_sz; reg = 0; - pthread_mutex_lock(&sc->mtx); + uart_softc_lock(sc->backend); switch (offset) { case UARTDR: reg = uart_rxfifo_getchar(sc->backend); @@ -362,7 +360,7 @@ uart_pl011_read(struct uart_pl011_softc *sc, int offset) break; } uart_toggle_intr(sc); - pthread_mutex_unlock(&sc->mtx); + uart_softc_unlock(sc->backend); return (reg); } @@ -380,8 +378,6 @@ uart_pl011_init(uart_intr_func_t intr_assert, uart_intr_func_t intr_deassert, sc->intr_deassert = intr_deassert; sc->backend = uart_init(); - pthread_mutex_init(&sc->mtx, NULL); - uart_reset(sc); return (sc);
