Re: [PATCH v6 03/15] emul/ns16x50: implement emulator stub

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 5 Sep 2025, dmukhin@xxxxxxx wrote:
> From: Denis Mukhin <dmukhin@xxxxxxxx> 
> 
> The change is the first on the way on introducing minimally functional
> NS16550-compatible UART emulator.
> 
> Define UART state and a set of emulated registers.
> 
> Implement alloc/free vUART hooks.
> 
> Stub out I/O port handler.
> 
> Add initialization of the NS16x50-compatible UART emulator state machine.
> 
> Plumb debug logging.
> 
> Signed-off-by: Denis Mukhin <dmukhin@xxxxxxxx>
> ---
> Changes since v5:
> - v5 feedback
> - Link to v5: 
> https://lore.kernel.org/xen-devel/20250828235409.2835815-4-dmukhin@xxxxxxxx/
> ---
>  xen/arch/x86/hvm/hvm.c          |  21 ++
>  xen/common/emul/vuart/Kconfig   |  19 ++
>  xen/common/emul/vuart/Makefile  |   1 +
>  xen/common/emul/vuart/ns16x50.c | 366 ++++++++++++++++++++++++++++++++
>  4 files changed, 407 insertions(+)
>  create mode 100644 xen/common/emul/vuart/ns16x50.c
> 
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index 23bd7f078a1d..91c971f11e14 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -28,6 +28,7 @@
>  #include <xen/softirq.h>
>  #include <xen/trace.h>
>  #include <xen/vm_event.h>
> +#include <xen/vuart.h>
>  #include <xen/vpci.h>
>  #include <xen/wait.h>
>  #include <xen/warning.h>
> @@ -689,6 +690,22 @@ int hvm_domain_initialise(struct domain *d,
>      if ( rc != 0 )
>          goto fail1;
>  
> +    /* Limit NS16550 emulator for dom0 only for now. */
> +    if ( IS_ENABLED(CONFIG_VUART_NS16X50) && is_hardware_domain(d) )
> +    {
> +        struct vuart_info info = {
> +            .name       = "COM2",
> +            .compatible = "ns16550",
> +            .base_addr  = 0x2f8,
> +            .size       = 8,
> +            .irq        = 3,
> +        };
> +
> +        rc = vuart_init(d, &info);
> +        if ( rc )
> +            goto out_vioapic_deinit;
> +    }
> +
>      stdvga_init(d);
>  
>      rtc_init(d);
> @@ -712,6 +729,8 @@ int hvm_domain_initialise(struct domain *d,
>      return 0;
>  
>   fail2:
> +    vuart_deinit(d);
> + out_vioapic_deinit:
>      vioapic_deinit(d);
>   fail1:
>      if ( is_hardware_domain(d) )
> @@ -774,6 +793,8 @@ void hvm_domain_destroy(struct domain *d)
>      if ( hvm_funcs.domain_destroy )
>          alternative_vcall(hvm_funcs.domain_destroy, d);
>  
> +    vuart_deinit(d);
> +
>      vioapic_deinit(d);
>  
>      XFREE(d->arch.hvm.pl_time);
> diff --git a/xen/common/emul/vuart/Kconfig b/xen/common/emul/vuart/Kconfig
> index ce1b976b7da7..a27d7ca135af 100644
> --- a/xen/common/emul/vuart/Kconfig
> +++ b/xen/common/emul/vuart/Kconfig
> @@ -3,4 +3,23 @@ config VUART_FRAMEWORK
>  
>  menu "UART Emulation"
>  
> +config VUART_NS16X50
> +     bool "NS16550-compatible UART Emulator" if EXPERT
> +     depends on X86 && HVM
> +     select VUART_FRAMEWORK
> +     default n
> +     help
> +       In-hypervisor NS16x50 UART emulation.
> +
> +       Only legacy PC COM2 port is emulated.
> +
> +       This is strictly for testing purposes (such as early HVM guest 
> console),
> +       and not appropriate for use in production.
> +
> +config VUART_NS16X50_DEBUG
> +     bool "NS16550-compatible UART Emulator Debugging"
> +     depends on VUART_NS16X50 && DEBUG
> +     help
> +       Enable development debugging.

There is a question about adding the kconfig option early in the series.
I think it would be best to add it as last patch


>  endmenu