diff mbox series

[RFC,26/38] x86/xen: Wrap XEN MSI management into irqdomain

Message ID 20200821002947.868727656@linutronix.de
State New
Headers show
Series x86, PCI, XEN, genirq ...: Prepare for device MSI | expand

Commit Message

Thomas Gleixner Aug. 21, 2020, 12:24 a.m. UTC
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>
Cc: linux-pci@vger.kernel.org
Cc: xen-devel@lists.xenproject.org
---
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

Jürgen Groß Aug. 24, 2020, 6:21 a.m. UTC | #1
On 21.08.20 02:24, Thomas Gleixner wrote:
> 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>
> Cc:linux-pci@vger.kernel.org
> Cc:xen-devel@lists.xenproject.org

Acked-by: Juergen Gross <jgross@suse.com>

Looking into https://www.kernel.org/doc/Documentation/IRQ-domain.txt (is
this still valid?) I believe Xen should be able to use the "No Map"
approach, as Xen only ever uses software IRQs (at least those are the
only ones visible to any driver). The (virtualized) hardware interrupts
are Xen events after all.

So maybe morphing Xen into supporting irqdomains in a sane way isn't
that complicated. Maybe I'm missing the main complexities, though.


Juergen

> ---
> Note: This is completely untested, but it compiles so it must be perfect.
> ---
>   arch/x86/pci/xen.c |   63 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 63 insertions(+)
> 
> --- a/arch/x86/pci/xen.c
> +++ b/arch/x86/pci/xen.c
> @@ -406,6 +406,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_initial_domain()) {
> @@ -426,6 +483,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 */
>
Thomas Gleixner Aug. 25, 2020, 7:57 a.m. UTC | #2
On Mon, Aug 24 2020 at 08:21, Jürgen Groß wrote:
> On 21.08.20 02:24, Thomas Gleixner wrote:
>
> Looking into https://www.kernel.org/doc/Documentation/IRQ-domain.txt (is
> this still valid?)

It's halfways correct. Emphasis on halfways.

> I believe Xen should be able to use the "No Map" approach, as Xen only
> ever uses software IRQs (at least those are the only ones visible to
> any driver). The (virtualized) hardware interrupts are Xen events
> after all.
>
> So maybe morphing Xen into supporting irqdomains in a sane way isn't
> that complicated. Maybe I'm missing the main complexities, though.

The wrapper domain I did is pretty much that, but with the extra
functionality required by hierarchical irq domains. So, yes it's
functionally correct, but it's only utilizing the alloc/free interface
and not any of the other mechanisms provided by irqdomains. The latter
should make the overall code simpler but that obviously needs some
thought.

Thanks,

        tglx
diff mbox series

Patch

--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
@@ -406,6 +406,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_initial_domain()) {
@@ -426,6 +483,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 */