On Thu, 28 Aug 2025, dmuk...@xen.org wrote: > From: Denis Mukhin <dmuk...@ford.com> > > Introduce a driver framework to abstract UART emulators in the hypervisor. > > That allows for architecture-independent handling of virtual UARTs in the > console driver and simplifies enabling new UART emulators. > > The framework is built under CONFIG_VUART_FRAMEWORK, which will be > automatically enabled once the user enables any UART emulator. > > Current implementation supports maximum of one vUART of each kind per domain. > > Use new domain_has_vuart() in the console driver code to check whether to > forward console input to the domain using vUART. > > Enable console forwarding over vUART for hardware domains with a vUART. That > enables console forwarding to dom0 on x86, since console can be forwarded only > to Xen, dom0 and pvshim on x86 as of now. > > Note: existing vUARTs are deliberately *not* hooked to the new framework to > minimize the scope of the patch: vpl011 (i.e. SBSA) emulator and "vuart" (i.e. > minimalistic MMIO-mapped dtuart for hwdoms on Arm) are kept unmodified. > > No functional changes for non-x86 architectures. > > Signed-off-by: Denis Mukhin <dmuk...@ford.com> > --- > Changes since v4: > - addressed feedback > - Link to v4: > https://lore.kernel.org/xen-devel/20250731192130.3948419-3-dmuk...@ford.com/ > --- > xen/arch/arm/xen.lds.S | 1 + > xen/arch/ppc/xen.lds.S | 1 + > xen/arch/riscv/xen.lds.S | 1 + > xen/arch/x86/xen.lds.S | 1 + > xen/common/Kconfig | 2 + > xen/common/Makefile | 1 + > xen/common/emul/Kconfig | 6 ++ > xen/common/emul/Makefile | 1 + > xen/common/emul/vuart/Kconfig | 6 ++ > xen/common/emul/vuart/Makefile | 1 + > xen/common/emul/vuart/vuart.c | 156 +++++++++++++++++++++++++++++++++ > xen/common/keyhandler.c | 3 + > xen/drivers/char/console.c | 6 +- > xen/include/xen/serial.h | 3 + > xen/include/xen/vuart.h | 116 ++++++++++++++++++++++++ > xen/include/xen/xen.lds.h | 10 +++ > 16 files changed, 314 insertions(+), 1 deletion(-) > create mode 100644 xen/common/emul/Kconfig > create mode 100644 xen/common/emul/Makefile > create mode 100644 xen/common/emul/vuart/Kconfig > create mode 100644 xen/common/emul/vuart/Makefile > create mode 100644 xen/common/emul/vuart/vuart.c > create mode 100644 xen/include/xen/vuart.h > > diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S > index db17ff1efa98..cd05b18770f4 100644 > --- a/xen/arch/arm/xen.lds.S > +++ b/xen/arch/arm/xen.lds.S > @@ -58,6 +58,7 @@ SECTIONS > *(.rodata) > *(.rodata.*) > VPCI_ARRAY > + VUART_ARRAY > *(.data.rel.ro) > *(.data.rel.ro.*) > > diff --git a/xen/arch/ppc/xen.lds.S b/xen/arch/ppc/xen.lds.S > index 1de0b77fc6b9..f9d4e5b0dcd8 100644 > --- a/xen/arch/ppc/xen.lds.S > +++ b/xen/arch/ppc/xen.lds.S > @@ -52,6 +52,7 @@ SECTIONS > *(.rodata) > *(.rodata.*) > VPCI_ARRAY > + VUART_ARRAY > *(.data.rel.ro) > *(.data.rel.ro.*) > > diff --git a/xen/arch/riscv/xen.lds.S b/xen/arch/riscv/xen.lds.S > index edcadff90bfe..59dcaa5fef9a 100644 > --- a/xen/arch/riscv/xen.lds.S > +++ b/xen/arch/riscv/xen.lds.S > @@ -47,6 +47,7 @@ SECTIONS > *(.rodata) > *(.rodata.*) > VPCI_ARRAY > + VUART_ARRAY > *(.data.rel.ro) > *(.data.rel.ro.*) > > diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S > index 966e514f2034..d877b93a6964 100644 > --- a/xen/arch/x86/xen.lds.S > +++ b/xen/arch/x86/xen.lds.S > @@ -132,6 +132,7 @@ SECTIONS > *(.rodata) > *(.rodata.*) > VPCI_ARRAY > + VUART_ARRAY > *(.data.rel.ro) > *(.data.rel.ro.*) > > diff --git a/xen/common/Kconfig b/xen/common/Kconfig > index 76f9ce705f7a..78a32b69e2b2 100644 > --- a/xen/common/Kconfig > +++ b/xen/common/Kconfig > @@ -676,4 +676,6 @@ config PM_STATS > Enable collection of performance management statistics to aid in > analyzing and tuning power/performance characteristics of the system > > +source "common/emul/Kconfig" > + > endmenu > diff --git a/xen/common/Makefile b/xen/common/Makefile > index c316957fcb36..c0734480ee4b 100644 > --- a/xen/common/Makefile > +++ b/xen/common/Makefile > @@ -11,6 +11,7 @@ obj-$(filter-out $(CONFIG_X86),$(CONFIG_ACPI)) += device.o > obj-$(CONFIG_DEVICE_TREE_PARSE) += device-tree/ > obj-$(CONFIG_IOREQ_SERVER) += dm.o > obj-y += domain.o > +obj-y += emul/ > obj-y += event_2l.o > obj-y += event_channel.o > obj-$(CONFIG_EVTCHN_FIFO) += event_fifo.o > diff --git a/xen/common/emul/Kconfig b/xen/common/emul/Kconfig > new file mode 100644 > index 000000000000..7c6764d1756b > --- /dev/null > +++ b/xen/common/emul/Kconfig > @@ -0,0 +1,6 @@ > +menu "Domain Emulation Features" > + visible if EXPERT > + > +source "common/emul/vuart/Kconfig" > + > +endmenu > diff --git a/xen/common/emul/Makefile b/xen/common/emul/Makefile > new file mode 100644 > index 000000000000..ae0b575c3901 > --- /dev/null > +++ b/xen/common/emul/Makefile > @@ -0,0 +1 @@ > +obj-$(CONFIG_VUART_FRAMEWORK) += vuart/ > diff --git a/xen/common/emul/vuart/Kconfig b/xen/common/emul/vuart/Kconfig > new file mode 100644 > index 000000000000..ce1b976b7da7 > --- /dev/null > +++ b/xen/common/emul/vuart/Kconfig > @@ -0,0 +1,6 @@ > +config VUART_FRAMEWORK > + bool > + > +menu "UART Emulation" > + > +endmenu > diff --git a/xen/common/emul/vuart/Makefile b/xen/common/emul/vuart/Makefile > new file mode 100644 > index 000000000000..97f792dc6641 > --- /dev/null > +++ b/xen/common/emul/vuart/Makefile > @@ -0,0 +1 @@ > +obj-y += vuart.o > diff --git a/xen/common/emul/vuart/vuart.c b/xen/common/emul/vuart/vuart.c > new file mode 100644 > index 000000000000..7b277d00d5c7 > --- /dev/null > +++ b/xen/common/emul/vuart/vuart.c > @@ -0,0 +1,156 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * UART emulator framework. > + * > + * Copyright 2025 Ford Motor Company > + */ > + > +#include <xen/err.h> > +#include <xen/sched.h> > +#include <xen/vuart.h> > +#include <xen/xvmalloc.h> > + > +#define for_each_emulator(e) \ > + for ( e = vuart_array_start; e < vuart_array_end; e++ ) > + > +extern const struct vuart_emulator vuart_array_start[]; > +extern const struct vuart_emulator vuart_array_end[]; > + > +static const struct vuart_emulator * > +vuart_match_by_compatible(struct domain *d, const char *compat) > +{ > + const struct vuart_emulator *emulator; > + > + if ( d->console.vuart ) > + return NULL; > + > + for_each_emulator(emulator) > + if ( emulator->compatible && > + !strncmp(emulator->compatible, compat, > + strlen(emulator->compatible)) )
strncmp will continue until the given count even if compat is shorter > + return emulator; > + > + return NULL; > +} > + > +static struct vuart *vuart_find_by_console_permission(const struct domain *d) > +{ > + struct vuart *vuart = d->console.vuart; > + > + ASSERT(d->console.input_allowed); This ASSERT looks suspicious as we haven't even checked that vuart!=NULL yet. I would remove it > + if ( !vuart || !vuart->emulator || !vuart->emulator->put_rx || > + !(vuart->flags & VUART_CONSOLE_INPUT)) > + return NULL; > + > + return vuart; > +} > + > +struct vuart *vuart_find_by_io_range(struct domain *d, unsigned long addr, > + unsigned long size) > +{ > + struct vuart *vuart = d->console.vuart; > + > + if ( !vuart || !vuart->info ) > + return NULL; You might as well call vuart_find_by_console_permission that has all the checks already > + > + if ( addr >= vuart->info->base_addr && > + addr + size - 1 <= vuart->info->base_addr + vuart->info->size - 1 ) > + return vuart; > + > + return NULL; > +} > + > +int vuart_init(struct domain *d, struct vuart_info *info) > +{ > + const struct vuart_emulator *emulator; > + struct vuart *vuart; > + int rc; > + > + emulator = vuart_match_by_compatible(d, info->compatible); > + if ( !emulator ) > + return -ENODEV; > + > + vuart = xzalloc(typeof(*vuart)); > + if ( !vuart ) > + return -ENOMEM; > + > + vuart->info = xvzalloc(typeof(*info)); > + if ( !vuart->info ) > + { > + rc = -ENOMEM; > + goto err_out; > + } > + memcpy(vuart->info, info, sizeof(*info)); one thing to note is that the fields (strings) compatible and name are copied by address, I am not if it is OK but FYI > + vuart->vdev = emulator->alloc(d, vuart->info); > + if ( IS_ERR(vuart->vdev) ) > + { > + rc = PTR_ERR(vuart->vdev); > + goto err_out; this path leads to vuart->info not being freed > + } > + > + vuart->emulator = emulator; > + vuart->owner = d; > + vuart->flags |= VUART_CONSOLE_INPUT; > + > + d->console.input_allowed = true; > + d->console.vuart = vuart; > + > + return 0; > + > + err_out: > + XVFREE(vuart); > + return rc; > +} > + > +/* > + * Release any resources taken by UART emulators. > + * > + * NB: no flags are cleared, since currently exit() is called only during > + * domain destroy. > + */ > +void vuart_deinit(struct domain *d) > +{ > + struct vuart *vuart = d->console.vuart; > + > + if ( vuart ) > + { > + vuart->emulator->free(vuart); should we pass vuart or vuart->vdev here? The emulator state is supposed to be vuart->vdev? > + XVFREE(vuart->info); > + } > + > + XVFREE(d->console.vuart); > +} > + > +void vuart_dump_state(const struct domain *d) > +{ > + struct vuart *vuart = d->console.vuart; > + > + if ( vuart ) > + vuart->emulator->dump_state(vuart); also here vuart->vdev? > +} > + > +/* > + * Put character to the *first* suitable emulated UART's FIFO. > + */ > +int vuart_put_rx(struct domain *d, char c) > +{ > + struct vuart *vuart = vuart_find_by_console_permission(d); > + > + return vuart ? vuart->emulator->put_rx(vuart, c) : -ENODEV; and here? > +} > + > +bool domain_has_vuart(const struct domain *d) > +{ > + return vuart_find_by_console_permission(d); > +} > + > +/* > + * Local variables: > + * mode: C > + * c-file-style: "BSD" > + * c-basic-offset: 4 > + * indent-tabs-mode: nil > + * End: > + */ > diff --git a/xen/common/keyhandler.c b/xen/common/keyhandler.c > index cb6df2823b00..156e64d9eb58 100644 > --- a/xen/common/keyhandler.c > +++ b/xen/common/keyhandler.c > @@ -22,6 +22,7 @@ > #include <xen/mm.h> > #include <xen/watchdog.h> > #include <xen/init.h> > +#include <xen/vuart.h> > #include <asm/div64.h> > > static unsigned char keypress_key; > @@ -352,6 +353,8 @@ static void cf_check dump_domains(unsigned char key) > v->periodic_period / 1000000); > } > } > + > + vuart_dump_state(d); > } > > for_each_domain ( d ) > diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c > index 9bd5b4825da6..d5164897a776 100644 > --- a/xen/drivers/char/console.c > +++ b/xen/drivers/char/console.c > @@ -33,6 +33,7 @@ > #include <asm/setup.h> > #include <xen/sections.h> > #include <xen/consoled.h> > +#include <xen/vuart.h> > > #ifdef CONFIG_X86 > #include <asm/guest.h> > @@ -596,11 +597,12 @@ static void __serial_rx(char c) > if ( !d ) > return; > > - if ( is_hardware_domain(d) ) > + if ( is_hardware_domain(d) && !domain_has_vuart(d) ) > { > /* > * Deliver input to the hardware domain buffer, unless it is > * already full. > + * NB: must be the first check: hardware domain may have emulated > UART. > */ > if ( (serial_rx_prod - serial_rx_cons) != SERIAL_RX_SIZE ) > serial_rx_ring[SERIAL_RX_MASK(serial_rx_prod++)] = c; > @@ -611,6 +613,8 @@ static void __serial_rx(char c) > */ > send_global_virq(VIRQ_CONSOLE); > } > + else if ( domain_has_vuart(d) ) > + rc = vuart_put_rx(d, c); > #ifdef CONFIG_SBSA_VUART_CONSOLE > else > /* Deliver input to the emulated UART. */ > diff --git a/xen/include/xen/serial.h b/xen/include/xen/serial.h > index 8e1844555208..d7e81f098359 100644 > --- a/xen/include/xen/serial.h > +++ b/xen/include/xen/serial.h > @@ -36,6 +36,9 @@ struct vuart_info { > unsigned long data_off; /* Data register offset */ > unsigned long status_off; /* Status register offset */ > unsigned long status; /* Ready status value */ > + unsigned int irq; /* Interrupt */ > + const char *compatible; /* Compatible string */ > + const char *name; /* User-friendly name */ > }; > > struct serial_port { > diff --git a/xen/include/xen/vuart.h b/xen/include/xen/vuart.h > new file mode 100644 > index 000000000000..ca025b4179be > --- /dev/null > +++ b/xen/include/xen/vuart.h > @@ -0,0 +1,116 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * UART emulator framework. > + * > + * Copyright 2025 Ford Motor Company > + */ > + > +#ifndef XEN_VUART_H > +#define XEN_VUART_H > + > +#include <xen/serial.h> > +#include <public/xen.h> > + > +struct vuart_emulator; > + > +enum { > + VUART_CONSOLE_INPUT = 0U << 1, /** Physical console input forwarding. */ code style: single * This flag is zero, is that intended? > +}; > + > + > +/* > + * FIXME: #ifdef is temporary to avoid clash with > + * arch/arm/include/asm/domain.h > + */ > +#ifdef CONFIG_VUART_FRAMEWORK > +struct vuart { > + const struct vuart_emulator *emulator; > + struct vuart_info *info; > + struct domain *owner; > + uint32_t flags; > + void *vdev; > +}; > +#endif > + > +struct vuart_emulator { > + /* UART compatible string. Cannot be NULL or empty. */ > + const char *compatible; > + > + /* > + * Allocate emulated UART state (RX/TX FIFOs, locks, initialize > registers, > + * hook I/O handlers, etc.) > + * Cannot be NULL. > + */ > + void *(*alloc)(struct domain *d, const struct vuart_info *info); > + > + /* > + * Release resources used to emulate UART state (flush RX/TX FIFOs, > unhook > + * I/O handlers, etc.). > + * Cannot be NULL. > + */ > + void (*free)(void *arg); > + > + /* > + * Print emulated UART state, including registers, on the console. > + * Can be NULL. > + */ > + void (*dump_state)(void *arg); > + > + /* > + * Place character to the emulated RX FIFO. > + * Used to forward physical console input to the guest OS. > + * Can be NULL. > + */ > + int (*put_rx)(void *arg, char c); > +}; > + > +#define VUART_REGISTER(name, x) \ > + static const struct vuart_emulator name##_entry \ > + __used_section(".data.rel.ro.vuart") = x > + > +struct vuart *vuart_find_by_io_range(struct domain *d, > + unsigned long base_addr, > + unsigned long size); > + > +int vuart_put_rx(struct domain *d, char c); > + > +#ifdef CONFIG_VUART_FRAMEWORK > + > +int vuart_init(struct domain *d, struct vuart_info *info); > +void vuart_deinit(struct domain *d); > +void vuart_dump_state(const struct domain *d); > +bool domain_has_vuart(const struct domain *d); > + > +#else > + > +static inline int vuart_init(struct domain *d, struct vuart_info *info) > +{ > + return 0; > +} > + > +static inline void vuart_deinit(struct domain *d) > +{ > +} > + > +static inline void vuart_dump_state(const struct domain *d) > +{ > +} > + > +static inline bool domain_has_vuart(const struct domain *d) > +{ > + return false; > +} > + > +#endif /* CONFIG_VUART_FRAMEWORK */ > + > +#endif /* XEN_VUART_H */ > + > +/* > + * Local variables: > + * mode: C > + * c-file-style: "BSD" > + * c-basic-offset: 4 > + * indent-tabs-mode: nil > + * End: > + */ > + > diff --git a/xen/include/xen/xen.lds.h b/xen/include/xen/xen.lds.h > index b126dfe88792..2d65f32ddad3 100644 > --- a/xen/include/xen/xen.lds.h > +++ b/xen/include/xen/xen.lds.h > @@ -194,4 +194,14 @@ > #define VPCI_ARRAY > #endif > > +#ifdef CONFIG_VUART_FRAMEWORK > +#define VUART_ARRAY \ > + . = ALIGN(POINTER_ALIGN); \ > + vuart_array_start = .; \ > + *(.data.rel.ro.vuart) \ > + vuart_array_end = .; > +#else > +#define VUART_ARRAY > +#endif > + > #endif /* __XEN_LDS_H__ */ > -- > 2.51.0 >