Hi Denis, Thank you for the patch.
On Tue, Sep 9, 2025 at 12:12 AM <[email protected]> wrote: > > From: Denis Mukhin <[email protected]> > > 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 <[email protected]> > --- > Changes since v6: > - n/a > --- > 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 775c33928585..edf76b02e1a1 100644 > --- a/xen/common/domain.c > +++ b/xen/common/domain.c > @@ -952,6 +952,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; > @@ -961,8 +966,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 ea34c3ae598a..6bd58ba5540b 100644 > --- a/xen/common/emul/vuart/ns16x50.c > +++ b/xen/common/emul/vuart/ns16x50.c > @@ -797,6 +797,15 @@ static int ns16x50_init(void *arg) > return rc; > } > > + /* Disallow sharing physical IRQ */ Should this be undone on teardown and error paths? Best regards, Mykola > + 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; > + } > + > /* NB: report 115200 baud rate. */ > vdev->regs[NS16X50_REGS_NUM + UART_DLL] = divisor & UINT8_MAX; > vdev->regs[NS16X50_REGS_NUM + UART_DLM] = (divisor >> 8) & UINT8_MAX; > -- > 2.51.0 > >
