[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



On Thu, 31 Jul 2025, dmkhn@xxxxxxxxx wrote:
> From: Denis Mukhin <dmukhin@xxxxxxxx> 
> 
> Enable virtual NS16550 for PVH domains in xl.
> 
> {map,unmap}_domain_emuirq_pirq() infrastructure is modified by adding new
> type of interrupt resources 'IRQ_EMU' which means 'emulated device IRQ'
> (similarly to IRQ_MSI_EMU).
> 
> This is necessary to for IOAPIC emulation code to skip IRQ->PIRQ mapping
> (vioapic_hwdom_map_gsi()) when guest OS unmasks vIOAPIC pin corresponding to
> virtual device's IRQ.
> 
> Also, hvm_gsi_eoi() is modified to trigger assertion in hvm_gsi_deassert()
> path for ISA IRQs.
> 
> Signed-off-by: Denis Mukhin <dmukhin@xxxxxxxx>
> ---
> Changes since v3:
> - new patch
> ---
>  tools/libs/light/libxl_x86.c          |  2 +-
>  xen/arch/x86/domain.c                 |  2 ++
>  xen/arch/x86/hvm/vioapic.c            | 10 ++++++++++
>  xen/arch/x86/include/asm/irq.h        |  1 +
>  xen/common/emul/vuart/vuart-ns16550.c | 27 +++++++++++++++++++++++++--
>  xen/drivers/passthrough/x86/hvm.c     |  9 ++++-----
>  6 files changed, 43 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/libs/light/libxl_x86.c b/tools/libs/light/libxl_x86.c
> index 0f039ca65a88..a40647c06cb9 100644
> --- a/tools/libs/light/libxl_x86.c
> +++ b/tools/libs/light/libxl_x86.c
> @@ -54,7 +54,7 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
>          break;
>      case LIBXL_DOMAIN_TYPE_PVH:
>          config->arch.emulation_flags = XEN_X86_EMU_LAPIC;
> -        libxl__arch_domain_vuart_unsupported(gc, d_config, config);
> +        libxl__arch_domain_vuart_enable(gc, d_config, config);
>          break;
>      case LIBXL_DOMAIN_TYPE_PV:
>          config->arch.emulation_flags = 0;
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index 6a010a509a60..39b0c0b199b9 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -769,12 +769,14 @@ static bool emulation_flags_ok(const struct domain *d, 
> uint32_t emflags)
>          {
>              .caps   = CAP_HVM | CAP_HWDOM,
>              .min    = X86_EMU_LAPIC | X86_EMU_IOAPIC | X86_EMU_VPCI,
> +            .opt    = X86_EMU_NS16550,
>          },
>  
>          /* PVH domU */
>          {
>              .caps   = CAP_HVM | CAP_DOMU,
>              .min    = X86_EMU_LAPIC,
> +            .opt    = X86_EMU_NS16550,
>          },
>  
>          /* HVM domU */
> diff --git a/xen/arch/x86/hvm/vioapic.c b/xen/arch/x86/hvm/vioapic.c
> index 7c725f9e471f..86fe3aa4a201 100644
> --- a/xen/arch/x86/hvm/vioapic.c
> +++ b/xen/arch/x86/hvm/vioapic.c
> @@ -177,6 +177,16 @@ static int vioapic_hwdom_map_gsi(unsigned int gsi, 
> unsigned int trig,
>  
>      ASSERT(is_hardware_domain(currd));
>  
> +    /*
> +     * Interrupt is claimed by one of the platform virtual devices (e.g.
> +     * NS16550); do nothing.
> +     */
> +    read_lock(&currd->event_lock);
> +    ret = domain_pirq_to_emuirq(currd, gsi);
> +    read_unlock(&currd->event_lock);
> +    if ( ret != IRQ_UNBOUND )
> +        return 0;
> +
>      /* Interrupt has been unmasked, bind it now. */
>      ret = mp_register_gsi(gsi, trig, pol);
>      if ( ret == -EEXIST )
> diff --git a/xen/arch/x86/include/asm/irq.h b/xen/arch/x86/include/asm/irq.h
> index 8c81f66434a8..731d2bbbb1b4 100644
> --- a/xen/arch/x86/include/asm/irq.h
> +++ b/xen/arch/x86/include/asm/irq.h
> @@ -221,6 +221,7 @@ void cleanup_domain_irq_mapping(struct domain *d);
>  #define IRQ_UNBOUND (-1)
>  #define IRQ_PT      (-2)
>  #define IRQ_MSI_EMU (-3)
> +#define IRQ_EMU     (-4)
>  
>  bool cpu_has_pending_apic_eoi(void);
>  
> diff --git a/xen/common/emul/vuart/vuart-ns16550.c 
> b/xen/common/emul/vuart/vuart-ns16550.c
> index 48bbf58264fe..9ec9aed2c594 100644
> --- a/xen/common/emul/vuart/vuart-ns16550.c
> +++ b/xen/common/emul/vuart/vuart-ns16550.c
> @@ -355,7 +355,9 @@ static void ns16550_irq_assert(const struct vuart_ns16550 
> *vdev)
>      struct domain *d = vdev->owner;
>      int vector;
>  
> -    if ( has_vpic(d) ) /* HVM */
> +    if ( has_vioapic(d) && !has_vpic(d) ) /* PVH */
> +        vector = hvm_ioapic_assert(d, vdev->irq, false);
> +    else if ( has_vpic(d) ) /* HVM */
>          vector = hvm_isa_irq_assert(d, vdev->irq, vioapic_get_vector);
>      else
>          ASSERT_UNREACHABLE();
> @@ -367,7 +369,9 @@ static void ns16550_irq_deassert(const struct 
> vuart_ns16550 *vdev)
>  {
>      struct domain *d = vdev->owner;
>  
> -    if ( has_vpic(d) ) /* HVM */
> +    if ( has_vioapic(d) && !has_vpic(d) ) /* PVH */
> +        hvm_ioapic_deassert(d, vdev->irq);
> +    else if ( has_vpic(d) ) /* HVM */
>          hvm_isa_irq_deassert(d, vdev->irq);
>      else
>          ASSERT_UNREACHABLE();
> @@ -889,6 +893,17 @@ static int cf_check ns16550_init(struct domain *d,
>                  return rc;
>              }
>  
> +            /* Claim virtual IRQ */
> +            write_lock(&d->event_lock);
> +            rc = map_domain_emuirq_pirq(d, r->addr, IRQ_EMU);
> +            write_unlock(&d->event_lock);
> +            if ( rc )
> +            {
> +                pr_err("%s: virtual IRQ#%"PRIu64": cannot claim: %d\n",
> +                        desc->name, r->addr, rc);
> +                return rc;
> +            }
> +
>              vdev->irq = r->addr;
>          }
>          else
> @@ -919,12 +934,20 @@ static int cf_check ns16550_init(struct domain *d,
>  static void cf_check ns16550_deinit(struct domain *d)
>  {
>      struct vuart_ns16550 *vdev = d->arch.hvm.vuart;
> +    int rc;
>  
>      if ( !vdev )
>          return;
>  
>      spin_lock(&vdev->lock);
>  
> +    rc = unmap_domain_pirq_emuirq(vdev->owner, vdev->irq);
> +    if ( rc )
> +    {
> +        pr_err("%s: virtual IRQ#%d: cannot unclaim: %d\n",
> +                vdev->name, vdev->irq, rc);
> +    }

write_lock(&d->event_lock); ?


>      ns16550_fifo_tx_flush(vdev);
>  
>      spin_unlock(&vdev->lock);
> diff --git a/xen/drivers/passthrough/x86/hvm.c 
> b/xen/drivers/passthrough/x86/hvm.c
> index a2ca7e0e570c..22905cd86f95 100644
> --- a/xen/drivers/passthrough/x86/hvm.c
> +++ b/xen/drivers/passthrough/x86/hvm.c
> @@ -924,12 +924,11 @@ static void hvm_gsi_eoi(struct domain *d, unsigned int 
> gsi)
>  {
>      struct pirq *pirq = pirq_info(d, gsi);
>  
> -    /* Check if GSI is actually mapped. */
> -    if ( !pirq_dpci(pirq) )
> -        return;
> -
>      hvm_gsi_deassert(d, gsi);
> -    hvm_pirq_eoi(pirq);
> +
> +    /* Check if GSI is actually mapped. */
> +    if ( pirq_dpci(pirq) )
> +        hvm_pirq_eoi(pirq);
>  }
>  
>  static int cf_check _hvm_dpci_isairq_eoi(
> -- 
> 2.34.1
> 
> 



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.