diff mbox series

[V2,30/46] x86/xen: Wrap XEN MSI management into irqdomain

Message ID 20200826112333.622352798@linutronix.de (mailing list archive)
State Not Applicable, archived
Headers show
Series x86, PCI, XEN, genirq ...: Prepare for device MSI | expand

Commit Message

Thomas Gleixner Aug. 26, 2020, 11:16 a.m. UTC
From: Thomas Gleixner <tglx@linutronix.de>

To allow utilizing the irq domain pointer in struct device it is necessary
to make XEN/MSI irq domain compatible.

While the right solution would be to truly convert XEN to irq domains, this
is an exercise which is not possible for mere mortals with limited XENology.

Provide a plain irqdomain wrapper around XEN. While this is blatant
violation of the irqdomain design, it's the only solution for a XEN igorant
person to make progress on the issue which triggered this change.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Juergen Gross <jgross@suse.com>

---
Note: This is completely untested, but it compiles so it must be perfect.
---
 arch/x86/pci/xen.c |   63 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

Comments

David Woodhouse Jan. 15, 2023, 2:12 p.m. UTC | #1
On Wed, 2020-08-26 at 13:16 +0200, Thomas Gleixner wrote:
> From: Thomas Gleixner <tglx@linutronix.de>
> 
> To allow utilizing the irq domain pointer in struct device it is necessary
> to make XEN/MSI irq domain compatible.
> 
> While the right solution would be to truly convert XEN to irq domains, this
> is an exercise which is not possible for mere mortals with limited XENology.
> 
> Provide a plain irqdomain wrapper around XEN. While this is blatant
> violation of the irqdomain design, it's the only solution for a XEN igorant
> person to make progress on the issue which triggered this change.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Acked-by: Juergen Gross <jgross@suse.com>

I think it broke MSI-X support, because xen_pci_msi_domain_info is
lacking a .flags = MSI_FLAGS_PCI_MSIX?

> ---
> Note: This is completely untested, but it compiles so it must be perfect.


I'm working on making it simple for you to test that, by hosting Xen
HVM guests natively in qemu (under KVM¹). 

But I'm absolutely not going to try hacking on both guest and host side
at the same time when I'm trying to ensure compatibility — that way
lies madness.

So for now I'm going to test qemu with older kernels, and maybe someone
(Jürgen}? can test MSI-X to PIRQ support under real Xen?) FWIW if I add
the missing MSI_FLAGS_PCI_MSIX flag then under my qemu I get:

 38:       3180          0  xen-pirq    -msi-x     ens4-rx-0
 39:          0       3610  xen-pirq    -msi-x     ens4-tx-0
 40:          1          0  xen-pirq    -msi-x     ens4

But without the flags I get:

[    8.464212] e1000e 0000:00:04.0 ens4: Failed to initialize MSI interrupts.  Falling back to legacy interrupts.

¹ https://lore.kernel.org/qemu-devel/20230110122042.1562155-1-dwmw2@infradead.org/
David Woodhouse Jan. 15, 2023, 8:27 p.m. UTC | #2
On Sun, 2023-01-15 at 14:12 +0000, David Woodhouse wrote:
> On Wed, 2020-08-26 at 13:16 +0200, Thomas Gleixner wrote:
> > From: Thomas Gleixner <tglx@linutronix.de>
> > 
> > To allow utilizing the irq domain pointer in struct device it is necessary
> > to make XEN/MSI irq domain compatible.
> > 
> > While the right solution would be to truly convert XEN to irq domains, this
> > is an exercise which is not possible for mere mortals with limited XENology.
> > 
> > Provide a plain irqdomain wrapper around XEN. While this is blatant
> > violation of the irqdomain design, it's the only solution for a XEN igorant
> > person to make progress on the issue which triggered this change.
> > 
> > Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> > Acked-by: Juergen Gross <jgross@suse.com>
> 
> I think it broke MSI-X support, because xen_pci_msi_domain_info is
> lacking a .flags = MSI_FLAGS_PCI_MSIX?

Hm, I think it only actually *broke* with commit 99f3d27976 ("PCI/MSI:
Reject MSI-X early") from November last year. So 6.1 was OK and we have
time to fix it in 6.2.

Confirmed on real Xen at least that a Fedora 37 install with a 6.0.7
kernel works fine, then on upgrading to Rawhide's 6.2-rc3 it dies with 

[   41.498694] ena 0000:00:03.0 (unnamed net_device) (uninitialized): Failed to enable MSI-X. irq_cnt -524
[   41.498705] ena 0000:00:03.0: Can not reserve msix vectors
[   41.498712] ena 0000:00:03.0: Failed to enable and set the admin interrupts

> > ---
> > Note: This is completely untested, but it compiles so it must be perfect.
> 
> 
> I'm working on making it simple for you to test that, by hosting Xen
> HVM guests natively in qemu (under KVM¹). 
> 
> But I'm absolutely not going to try hacking on both guest and host side
> at the same time when I'm trying to ensure compatibility — that way
> lies madness.
> 
> So for now I'm going to test qemu with older kernels, and maybe someone
> (Jürgen}? can test MSI-X to PIRQ support under real Xen?) FWIW if I add
> the missing MSI_FLAGS_PCI_MSIX flag then under my qemu I get:
> 
>  38:       3180          0  xen-pirq    -msi-x     ens4-rx-0
>  39:          0       3610  xen-pirq    -msi-x     ens4-tx-0
>  40:          1          0  xen-pirq    -msi-x     ens4
> 
> But without the flags I get:
> 
> [    8.464212] e1000e 0000:00:04.0 ens4: Failed to initialize MSI interrupts.  Falling back to legacy interrupts.
> 
> ¹ https://lore.kernel.org/qemu-devel/20230110122042.1562155-1-dwmw2@infradead.org/
diff mbox series

Patch

--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
@@ -407,6 +407,63 @@  static void xen_teardown_msi_irq(unsigne
 	WARN_ON_ONCE(1);
 }
 
+static int xen_msi_domain_alloc_irqs(struct irq_domain *domain,
+				     struct device *dev,  int nvec)
+{
+	int type;
+
+	if (WARN_ON_ONCE(!dev_is_pci(dev)))
+		return -EINVAL;
+
+	if (first_msi_entry(dev)->msi_attrib.is_msix)
+		type = PCI_CAP_ID_MSIX;
+	else
+		type = PCI_CAP_ID_MSI;
+
+	return x86_msi.setup_msi_irqs(to_pci_dev(dev), nvec, type);
+}
+
+static void xen_msi_domain_free_irqs(struct irq_domain *domain,
+				     struct device *dev)
+{
+	if (WARN_ON_ONCE(!dev_is_pci(dev)))
+		return;
+
+	x86_msi.teardown_msi_irqs(to_pci_dev(dev));
+}
+
+static struct msi_domain_ops xen_pci_msi_domain_ops = {
+	.domain_alloc_irqs	= xen_msi_domain_alloc_irqs,
+	.domain_free_irqs	= xen_msi_domain_free_irqs,
+};
+
+static struct msi_domain_info xen_pci_msi_domain_info = {
+	.ops			= &xen_pci_msi_domain_ops,
+};
+
+/*
+ * This irq domain is a blatant violation of the irq domain design, but
+ * distangling XEN into real irq domains is not a job for mere mortals with
+ * limited XENology. But it's the least dangerous way for a mere mortal to
+ * get rid of the arch_*_msi_irqs() hackery in order to store the irq
+ * domain pointer in struct device. This irq domain wrappery allows to do
+ * that without breaking XEN terminally.
+ */
+static __init struct irq_domain *xen_create_pci_msi_domain(void)
+{
+	struct irq_domain *d = NULL;
+	struct fwnode_handle *fn;
+
+	fn = irq_domain_alloc_named_fwnode("XEN-MSI");
+	if (fn)
+		d = msi_create_irq_domain(fn, &xen_pci_msi_domain_info, NULL);
+
+	/* FIXME: No idea how to survive if this fails */
+	BUG_ON(!d);
+
+	return d;
+}
+
 static __init void xen_setup_pci_msi(void)
 {
 	if (xen_pv_domain()) {
@@ -427,6 +484,12 @@  static __init void xen_setup_pci_msi(voi
 	}
 
 	x86_msi.teardown_msi_irq = xen_teardown_msi_irq;
+
+	/*
+	 * Override the PCI/MSI irq domain init function. No point
+	 * in allocating the native domain and never use it.
+	 */
+	x86_init.irqs.create_pci_msi_domain = xen_create_pci_msi_domain;
 }
 
 #else /* CONFIG_PCI_MSI */