Xen project Mailing List

Re: [PATCH v4 6/8] tools/xl: enable NS16550-compatible UART emulator for HVM (x86)

On Thu, Jul 31, 2025 at 07:22:12PM +0000, dmkhn@xxxxxxxxx wrote: > From: Denis Mukhin <dmukhin@xxxxxxxx> > > Enable UART emulator to be individually configured per HVM-domain. > > Signed-off-by: Denis Mukhin <dmukhin@xxxxxxxx> > --- > Changes since v3: > - new patch > --- > docs/man/xl.cfg.5.pod.in | 9 ++++-- > tools/golang/xenlight/helpers.gen.go | 4 +-- > tools/golang/xenlight/types.gen.go | 3 +- > tools/libs/light/libxl_arm.c | 26 ++++++++++++----- > tools/libs/light/libxl_create.c | 2 +- > tools/libs/light/libxl_types.idl | 3 +- > tools/libs/light/libxl_x86.c | 42 ++++++++++++++++++++++++++++ > tools/ocaml/libs/xc/xenctrl.ml | 1 + > tools/ocaml/libs/xc/xenctrl.mli | 1 + > tools/xl/xl_parse.c | 2 +- > xen/arch/x86/domain.c | 5 ++-- > 11 files changed, 80 insertions(+), 18 deletions(-) > > diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in > index 5362fb0e9a6f..e1d012274eaf 100644 > --- a/docs/man/xl.cfg.5.pod.in > +++ b/docs/man/xl.cfg.5.pod.in > @@ -3032,14 +3032,17 @@ the domain was created. > This requires hardware compatibility with the requested version, either > natively or via hardware backwards compatibility support. > > -=item B<vuart="uart"> > +=item B<vuart=[ "sbsa_uart", "ns16550" ]> > > To enable vuart console, user must specify the following option in the > -VM config file: > +VM config file, e.g: > > +``` > vuart = "sbsa_uart" > +``` > > -Currently, only the "sbsa_uart" model is supported for ARM. > +Currently, "sbsa_uart" (ARM) and "ns16550" (x86) are the only supported > +UART models. > > =back > > diff --git a/tools/golang/xenlight/helpers.gen.go > b/tools/golang/xenlight/helpers.gen.go > index b43aad7d0064..e56af8a8a8c5 100644 > --- a/tools/golang/xenlight/helpers.gen.go > +++ b/tools/golang/xenlight/helpers.gen.go > @@ -1160,7 +1160,6 @@ x.TypeUnion = &typePvh > default: > return fmt.Errorf("invalid union key '%v'", x.Type)} > x.ArchArm.GicVersion = GicVersion(xc.arch_arm.gic_version) > -x.ArchArm.Vuart = VuartType(xc.arch_arm.vuart) > x.ArchArm.SveVl = SveType(xc.arch_arm.sve_vl) > x.ArchArm.NrSpis = uint32(xc.arch_arm.nr_spis) > if err := x.ArchX86.MsrRelaxed.fromC(&xc.arch_x86.msr_relaxed);err != nil { > @@ -1169,6 +1168,7 @@ return fmt.Errorf("converting field ArchX86.MsrRelaxed: > %v", err) > x.Altp2M = Altp2MMode(xc.altp2m) > x.Altp2MCount = uint32(xc.altp2m_count) > x.VmtraceBufKb = int(xc.vmtrace_buf_kb) > +x.Vuart = VuartType(xc.vuart) > if err := x.Vpmu.fromC(&xc.vpmu);err != nil { > return fmt.Errorf("converting field Vpmu: %v", err) > } > @@ -1695,7 +1695,6 @@ break > default: > return fmt.Errorf("invalid union key '%v'", x.Type)} > xc.arch_arm.gic_version = C.libxl_gic_version(x.ArchArm.GicVersion) > -xc.arch_arm.vuart = C.libxl_vuart_type(x.ArchArm.Vuart) > xc.arch_arm.sve_vl = C.libxl_sve_type(x.ArchArm.SveVl) > xc.arch_arm.nr_spis = C.uint32_t(x.ArchArm.NrSpis) > if err := x.ArchX86.MsrRelaxed.toC(&xc.arch_x86.msr_relaxed); err != nil { > @@ -1704,6 +1703,7 @@ return fmt.Errorf("converting field ArchX86.MsrRelaxed: > %v", err) > xc.altp2m = C.libxl_altp2m_mode(x.Altp2M) > xc.altp2m_count = C.uint32_t(x.Altp2MCount) > xc.vmtrace_buf_kb = C.int(x.VmtraceBufKb) > +xc.vuart = C.libxl_vuart_type(x.Vuart) > if err := x.Vpmu.toC(&xc.vpmu); err != nil { > return fmt.Errorf("converting field Vpmu: %v", err) > } > diff --git a/tools/golang/xenlight/types.gen.go > b/tools/golang/xenlight/types.gen.go > index 4777f528b52c..2f4153d2510b 100644 > --- a/tools/golang/xenlight/types.gen.go > +++ b/tools/golang/xenlight/types.gen.go > @@ -253,6 +253,7 @@ type VuartType int > const( > VuartTypeUnknown VuartType = 0 > VuartTypeSbsaUart VuartType = 1 > +VuartTypeNs16550 VuartType = 2 > ) > > type VkbBackend int > @@ -596,7 +597,6 @@ Type DomainType > TypeUnion DomainBuildInfoTypeUnion > ArchArm struct { > GicVersion GicVersion > -Vuart VuartType > SveVl SveType > NrSpis uint32 > } > @@ -608,6 +608,7 @@ Altp2MCount uint32 > VmtraceBufKb int > Vpmu Defbool > TrapUnmappedAccesses Defbool > +Vuart VuartType > } > > type DomainBuildInfoTypeUnion interface { > diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c > index 4a19a8d22bdf..f4721b24763c 100644 > --- a/tools/libs/light/libxl_arm.c > +++ b/tools/libs/light/libxl_arm.c > @@ -92,14 +92,26 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc, > uint32_t virtio_mmio_irq = GUEST_VIRTIO_MMIO_SPI_FIRST; > int rc; > > - /* > - * If pl011 vuart is enabled then increment the nr_spis to allow > allocation > - * of SPI VIRQ for pl011. > - */ > - if (d_config->b_info.arch_arm.vuart == LIBXL_VUART_TYPE_SBSA_UART) { > + switch ( d_config->b_info.vuart ) > + { > + case LIBXL_VUART_TYPE_SBSA_UART: > + /* > + * If pl011 vuart is enabled then increment the nr_spis to allow > + * allocation of SPI VIRQ for pl011. > + */ > nr_spis += (GUEST_VPL011_SPI - 32) + 1; > vuart_irq = GUEST_VPL011_SPI; > vuart_enabled = true; > + break; > + > + case LIBXL_VUART_TYPE_NS16550: > + LOG(ERROR, "unsupported UART emulator %d\n", d_config->b_info.vuart); > + abort(); > + break; > + > + case LIBXL_VUART_TYPE_UNKNOWN: > + default: > + break; > } > > for (i = 0; i < d_config->num_disks; i++) { > @@ -1372,7 +1384,7 @@ next_resize: > FDT( make_timer_node(gc, fdt, ainfo, state->clock_frequency) ); > FDT( make_hypervisor_node(gc, fdt, vers) ); > > - if (info->arch_arm.vuart == LIBXL_VUART_TYPE_SBSA_UART) > + if (info->vuart == LIBXL_VUART_TYPE_SBSA_UART) > FDT( make_vpl011_uart_node(gc, fdt, ainfo, dom) ); > > if (info->tee == LIBXL_TEE_TYPE_OPTEE) > @@ -1725,7 +1737,7 @@ int libxl__arch_build_dom_finish(libxl__gc *gc, > { > int rc = 0, ret; > > - if (info->arch_arm.vuart != LIBXL_VUART_TYPE_SBSA_UART) { > + if (info->vuart != LIBXL_VUART_TYPE_SBSA_UART) { > rc = 0; > goto out; > } > diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c > index 4042ae1a8957..cfd7e827867a 100644 > --- a/tools/libs/light/libxl_create.c > +++ b/tools/libs/light/libxl_create.c > @@ -1815,7 +1815,7 @@ static void domcreate_launch_dm(libxl__egc *egc, > libxl__multidev *multidev, > &d_config->vfbs[i]); > } > > - if (d_config->b_info.arch_arm.vuart == LIBXL_VUART_TYPE_SBSA_UART) { > + if (d_config->b_info.vuart == LIBXL_VUART_TYPE_SBSA_UART) { > init_console_info(gc, &vuart, 0); > vuart.backend_domid = state->console_domid; > libxl__device_vuart_add(gc, domid, &vuart, state); > diff --git a/tools/libs/light/libxl_types.idl > b/tools/libs/light/libxl_types.idl > index fe251649f346..fd60c2b26764 100644 > --- a/tools/libs/light/libxl_types.idl > +++ b/tools/libs/light/libxl_types.idl > @@ -276,6 +276,7 @@ libxl_checkpointed_stream = > Enumeration("checkpointed_stream", [ > libxl_vuart_type = Enumeration("vuart_type", [ > (0, "unknown"), > (1, "sbsa_uart"), > + (2, "ns16550"), > ]) > > libxl_vkb_backend = Enumeration("vkb_backend", [ > @@ -722,7 +723,6 @@ libxl_domain_build_info = Struct("domain_build_info",[ > > > ("arch_arm", Struct(None, [("gic_version", libxl_gic_version), > - ("vuart", libxl_vuart_type), > ("sve_vl", libxl_sve_type), > ("nr_spis", uint32, {'init_val': > 'LIBXL_NR_SPIS_DEFAULT'}), > ])), > @@ -739,6 +739,7 @@ libxl_domain_build_info = Struct("domain_build_info",[ > > ("vpmu", libxl_defbool), > ("trap_unmapped_accesses", libxl_defbool), > + ("vuart", libxl_vuart_type), > > ], dir=DIR_IN, > copy_deprecated_fn="libxl__domain_build_info_copy_deprecated", > diff --git a/tools/libs/light/libxl_x86.c b/tools/libs/light/libxl_x86.c > index 60d4e8661c93..0f039ca65a88 100644 > --- a/tools/libs/light/libxl_x86.c > +++ b/tools/libs/light/libxl_x86.c > @@ -2,6 +2,45 @@ > #include "libxl_arch.h" > #include <xen/arch-x86/cpuid.h> > > +static void libxl__arch_domain_vuart_assert( > + libxl__gc *gc, > + libxl_domain_config *d_config, > + struct xen_domctl_createdomain *config) > +{ > + LOG(ERROR, "unsupported UART emulator %d\n", d_config->b_info.vuart); > + abort(); > +} > + > +static void libxl__arch_domain_vuart_unsupported( > + libxl__gc *gc, > + libxl_domain_config *d_config, > + struct xen_domctl_createdomain *config) > +{ > + if ( d_config->b_info.vuart != LIBXL_VUART_TYPE_UNKNOWN ) > + libxl__arch_domain_vuart_assert(gc, d_config, config); > +} > + > +static void libxl__arch_domain_vuart_enable( > + libxl__gc *gc, > + libxl_domain_config *d_config, > + struct xen_domctl_createdomain *config) > +{ > + switch ( d_config->b_info.vuart ) > + { > + case LIBXL_VUART_TYPE_SBSA_UART: > + libxl__arch_domain_vuart_assert(gc, d_config, config); > + break; > + > + case LIBXL_VUART_TYPE_NS16550: > + config->arch.emulation_flags |= XEN_X86_EMU_NS16550; > + break; > + > + case LIBXL_VUART_TYPE_UNKNOWN: > + default: > + break; > + } > +} > + > int libxl__arch_domain_prepare_config(libxl__gc *gc, > libxl_domain_config *d_config, > struct xen_domctl_createdomain *config) > @@ -9,14 +48,17 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc, > switch(d_config->c_info.type) { > case LIBXL_DOMAIN_TYPE_HVM: > config->arch.emulation_flags = (XEN_X86_EMU_ALL & ~XEN_X86_EMU_VPCI); > + libxl__arch_domain_vuart_enable(gc, d_config, config); As mentioned in the previous commit, I don't think this works as expected. You have added XEN_X86_EMU_NS16550 to XEN_X86_EMU_ALL, and hence you need to subtract it from the mask as it's done with VPCI. > if (!libxl_defbool_val(d_config->b_info.u.hvm.pirq)) > config->arch.emulation_flags &= ~XEN_X86_EMU_USE_PIRQ; > break; > case LIBXL_DOMAIN_TYPE_PVH: > config->arch.emulation_flags = XEN_X86_EMU_LAPIC; > + libxl__arch_domain_vuart_unsupported(gc, d_config, config); > break; > case LIBXL_DOMAIN_TYPE_PV: > config->arch.emulation_flags = 0; > + libxl__arch_domain_vuart_unsupported(gc, d_config, config); > break; > default: > abort(); > diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml > index 7e1aabad6cba..4539e78bb283 100644 > --- a/tools/ocaml/libs/xc/xenctrl.ml > +++ b/tools/ocaml/libs/xc/xenctrl.ml > @@ -47,6 +47,7 @@ type x86_arch_emulation_flags = > | X86_EMU_PIT > | X86_EMU_USE_PIRQ > | X86_EMU_VPCI > + | X86_EMU_NS16550 > > type x86_arch_misc_flags = > | X86_MSR_RELAXED > diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli > index f44dba61aeab..66a98180d99b 100644 > --- a/tools/ocaml/libs/xc/xenctrl.mli > +++ b/tools/ocaml/libs/xc/xenctrl.mli > @@ -41,6 +41,7 @@ type x86_arch_emulation_flags = > | X86_EMU_PIT > | X86_EMU_USE_PIRQ > | X86_EMU_VPCI > + | X86_EMU_NS16550 > > type x86_arch_misc_flags = > | X86_MSR_RELAXED > diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c > index 28cdbf07c213..b0d266b5bf63 100644 > --- a/tools/xl/xl_parse.c > +++ b/tools/xl/xl_parse.c > @@ -1498,7 +1498,7 @@ void parse_config_data(const char *config_source, > b_info->max_vcpus = l; > > if (!xlu_cfg_get_string(config, "vuart", &buf, 0)) { > - if (libxl_vuart_type_from_string(buf, &b_info->arch_arm.vuart)) { > + if (libxl_vuart_type_from_string(buf, &b_info->vuart)) { > fprintf(stderr, "ERROR: invalid value \"%s\" for \"vuart\"\n", > buf); > exit(1); > diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c > index 7fd4f7a831dc..6a010a509a60 100644 > --- a/xen/arch/x86/domain.c > +++ b/xen/arch/x86/domain.c > @@ -780,9 +780,10 @@ static bool emulation_flags_ok(const struct domain *d, > uint32_t emflags) > /* HVM domU */ > { > .caps = CAP_HVM | CAP_DOMU, > - .min = X86_EMU_ALL & ~(X86_EMU_VPCI | X86_EMU_USE_PIRQ), > + .min = X86_EMU_ALL & ~(X86_EMU_VPCI | X86_EMU_USE_PIRQ | > + X86_EMU_NS16550), > /* HVM PIRQ feature is user-selectable. */ > - .opt = X86_EMU_USE_PIRQ, > + .opt = X86_EMU_USE_PIRQ | X86_EMU_NS16550, Does this need to be part of the patch that adds X86_EMU_NS16550 into X86_EMU_ALL, as to not break domain creation in the interim? Thanks, Roger.

©2013 Xen Project, A Linux Foundation Collaborative Project. All Rights Reserved.
Linux Foundation is a registered trademark of The Linux Foundation.
Xen Project is a trademark of The Linux Foundation.