Skip to content

Commit

Permalink
bhyve: Move lock of uart frontend to uart backend
Browse files Browse the repository at this point in the history
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
  • Loading branch information
aokblast authored and markjdb committed May 1, 2024
1 parent aa34b1d commit e10b9d6
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 21 deletions.
22 changes: 21 additions & 1 deletion usr.sbin/bhyve/uart_backend.c
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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 */
Expand Down Expand Up @@ -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
Expand All @@ -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);
}
3 changes: 2 additions & 1 deletion usr.sbin/bhyve/uart_backend.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_ */
15 changes: 6 additions & 9 deletions usr.sbin/bhyve/uart_emul.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) */
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -329,15 +328,15 @@ 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
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
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
Expand Down
16 changes: 6 additions & 10 deletions usr.sbin/bhyve/uart_pl011.c
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
#include <sys/param.h>

#include <assert.h>
#include <pthread.h>
#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
Expand Down Expand Up @@ -106,7 +105,6 @@

struct uart_pl011_softc {
struct uart_softc *backend;
pthread_mutex_t mtx; /* protects all softc elements */

uint16_t irq_state;

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

Expand All @@ -202,15 +200,15 @@ 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
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;
Expand Down Expand Up @@ -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
Expand All @@ -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);
Expand Down Expand Up @@ -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);
}
Expand All @@ -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);
Expand Down

0 comments on commit e10b9d6

Please sign in to comment.