On Thu, 28 Aug 2025, dmuk...@xen.org wrote: > From: Denis Mukhin <dmuk...@ford.com> > > Make sure that NS16550 emulator does not share virtual device IRQ with the > physical one. This is needed for enabling NS16550 emulator for PVH hwdom > (dom0). > > To do that, move per-domain interrupt rangeset allocation before arch-specific > code. Add irqs_setup_access() to setup the initial rangeset. > > Signed-off-by: Denis Mukhin <dmuk...@ford.com> > --- > Changes since v4: > - new patch > - Link to original patch in v4: > https://lore.kernel.org/xen-devel/20250731192130.3948419-4-dmuk...@ford.com/ > --- > xen/arch/x86/dom0_build.c | 1 - > xen/arch/x86/hvm/dom0_build.c | 7 +++++++ > xen/arch/x86/include/asm/irq.h | 2 ++ > xen/arch/x86/irq.c | 8 ++++++++ > xen/arch/x86/pv/dom0_build.c | 3 +++ > xen/common/domain.c | 8 ++++++-- > xen/common/emul/vuart/ns16x50.c | 9 +++++++++ > 7 files changed, 35 insertions(+), 3 deletions(-) > > diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c > index 26202b33345c..9dc87efbf3e8 100644 > --- a/xen/arch/x86/dom0_build.c > +++ b/xen/arch/x86/dom0_build.c > @@ -442,7 +442,6 @@ int __init dom0_setup_permissions(struct domain *d) > > rc |= iomem_permit_access(d, 0UL, > PFN_DOWN(1UL << domain_max_paddr_bits(d)) - 1); > - rc |= irqs_permit_access(d, 1, nr_irqs_gsi - 1); > > /* Local APIC. */ > if ( mp_lapic_addr != 0 ) > diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c > index 5551f9044836..245a42dec9aa 100644 > --- a/xen/arch/x86/hvm/dom0_build.c > +++ b/xen/arch/x86/hvm/dom0_build.c > @@ -1348,6 +1348,13 @@ int __init dom0_construct_pvh(const struct boot_domain > *bd) > */ > pvh_setup_mmcfg(d); > > + rc = irqs_setup_access(d); > + if ( rc ) > + { > + printk("%pd unable to setup IRQ rangeset: %d\n", d, rc); > + return rc; > + } > + > /* > * Setup permissions early so that calls to add MMIO regions to the > * p2m as part of vPCI setup don't fail due to permission checks. > diff --git a/xen/arch/x86/include/asm/irq.h b/xen/arch/x86/include/asm/irq.h > index 8c81f66434a8..8bffec3bbfee 100644 > --- a/xen/arch/x86/include/asm/irq.h > +++ b/xen/arch/x86/include/asm/irq.h > @@ -231,4 +231,6 @@ int allocate_and_map_gsi_pirq(struct domain *d, int > index, int *pirq_p); > int allocate_and_map_msi_pirq(struct domain *d, int index, int *pirq_p, > int type, struct msi_info *msi); > > +int irqs_setup_access(struct domain *d); > + > #endif /* _ASM_HW_IRQ_H */ > diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c > index 556134f85aa0..079277be719d 100644 > --- a/xen/arch/x86/irq.c > +++ b/xen/arch/x86/irq.c > @@ -3046,3 +3046,11 @@ int allocate_and_map_msi_pirq(struct domain *d, int > index, int *pirq_p, > > return ret; > } > + > +int irqs_setup_access(struct domain *d) > +{ > + if ( is_hardware_domain(d) ) > + return irqs_permit_access(d, 1, nr_irqs_gsi - 1); > + > + return 0; > +} > diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c > index 2b8b4d869ee7..1a092b802833 100644 > --- a/xen/arch/x86/pv/dom0_build.c > +++ b/xen/arch/x86/pv/dom0_build.c > @@ -1037,6 +1037,9 @@ static int __init dom0_construct(const struct > boot_domain *bd) > rc = ioports_setup_access(d); > BUG_ON(rc != 0); > > + rc = irqs_setup_access(d); > + BUG_ON(rc != 0); > + > rc = dom0_setup_permissions(d); > BUG_ON(rc != 0); > > diff --git a/xen/common/domain.c b/xen/common/domain.c > index 104e917f07e3..eb83e3198f37 100644 > --- a/xen/common/domain.c > +++ b/xen/common/domain.c > @@ -950,6 +950,11 @@ struct domain *domain_create(domid_t domid, > radix_tree_init(&d->pirq_tree); > #endif > > + err = -ENOMEM; > + d->irq_caps = rangeset_new(d, "Interrupts", 0); > + if ( !d->irq_caps ) > + goto fail; > + > if ( (err = arch_domain_create(d, config, flags)) != 0 ) > goto fail; > init_status |= INIT_arch; > @@ -959,8 +964,7 @@ struct domain *domain_create(domid_t domid, > > err = -ENOMEM; > d->iomem_caps = rangeset_new(d, "I/O Memory", RANGESETF_prettyprint_hex); > - d->irq_caps = rangeset_new(d, "Interrupts", 0); > - if ( !d->iomem_caps || !d->irq_caps ) > + if ( !d->iomem_caps ) > goto fail; > > if ( (err = xsm_domain_create(XSM_HOOK, d, config->ssidref)) != 0 ) > diff --git a/xen/common/emul/vuart/ns16x50.c b/xen/common/emul/vuart/ns16x50.c > index 8860f25ffdeb..aea38304b60c 100644 > --- a/xen/common/emul/vuart/ns16x50.c > +++ b/xen/common/emul/vuart/ns16x50.c > @@ -794,6 +794,15 @@ static int ns16x50_init(void *arg) > return rc; > } > > + /* Disallow sharing physical IRQ */ > + rc = irq_deny_access(d, info->irq); > + if ( rc ) > + { > + ns16x50_err(info, "virtual IRQ#%d: conflict w/ physical IRQ: %d\n", > + info->irq, rc); > + return rc; > + }
Also this one I wonder if it could be in hvm_domain_initialise, it would make more sense to keep the irq_deny_access there, compared to here which is supposed to be a generic emulator > /* NB: report 115200 baud rate. */ > vdev->regs[NS16X50_REGS_NUM + UART_DLL] = divisor & 0xff; > vdev->regs[NS16X50_REGS_NUM + UART_DLM] = (divisor >> 8) & 0xff; > -- > 2.51.0 >
