diff mbox series

[02/33] genirq/msi: Provide struct msi_parent_ops

Message ID 20221111135205.368911521@linutronix.de
State New
Headers show
Series genirq, PCI/MSI: Support for per device MSI and PCI/IMS - Part 3 implementation | expand

Commit Message

Thomas Gleixner Nov. 11, 2022, 1:58 p.m. UTC
MSI parent domains must have some control over the MSI domains which are
built on top. On domain creation they need to fill in e.g. architecture
specific chip callbacks or msi domain ops to make the outermost domain
parent agnostic which is obviously required for architecture independence
etc.

The structure contains:

    1) A bitfield which exposes the supported functional features. This
       allows to check for features and is also used in the initialization
       callback to mask out unsupported features when the actual domain
       implementation requests a broader range, e.g. on x86 PCI multi-MSI
       is only supported by remapping domains but not by the underlying
       vector domain. The PCI/MSI code can then always request multi-MSI
       support, but the resulting feature set after creation might not
       have it set.

    2) An optional string prefix which is put in front of domain and chip
       names during creation of the MSI domain. That allows to keep the
       naming schemes e.g. on x86 where PCI-MSI domains have a IR- prefix
       when interrupt remapping is enabled.

    3) An initialization callback to sanity check the domain info of
       the to be created MSI domain, to restrict features and to
       apply changes in MSI ops and interrupt chip callbacks to
       accomodate to the particular MSI parent implementation and/or
       the underlying hierarchy.

Add a conveniance function to delegate the initialization from the
MSI parent domain to an underlying domain in the hierarchy.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/irqdomain.h |    5 +++++
 include/linux/msi.h       |   20 ++++++++++++++++++++
 kernel/irq/msi.c          |   36 ++++++++++++++++++++++++++++++++++++
 3 files changed, 61 insertions(+)

Comments

Jason Gunthorpe Nov. 16, 2022, 6:57 p.m. UTC | #1
On Fri, Nov 11, 2022 at 02:58:14PM +0100, Thomas Gleixner wrote:
> +/**
> + * msi_parent_init_dev_msi_info - Delegate initialization of device MSI info to parent domain
> + * @dev:		The device for which the domain should be created
> + * @domain:		The domain which delegates
> + * @real_parent:	The real parent domain of the to be initialized MSI domain
> + * @info:		The MSI domain info to initialize
> + *
> + * Return: true on success, false otherwise
> + *
> + * This is the most complex problem of per device MSI domains and the
> + * underlying interrupt domain hierarchy:
> + *
> + * The device domain to be initialized requests the broadest feature set
> + * possible and the underlying domain hierarchy puts restrictions on it.
> + *
> + * That's working perfectly fine for a strict parent->device model, but it
> + * falls apart with a root_parent->real_parent->device chain because the

This language hurt my brain :)

> +bool msi_parent_init_dev_msi_info(struct device *dev, struct irq_domain *domain,
> +				  struct irq_domain *real_parent, struct msi_domain_info *info)

'real_parent' is global IRQ_DOMAIN_FLAG_MSI_PARENT of the dev for
which we are constructing a msi_domain_info to create a child aka
IRQ_DOMAIN_FLAG_MSI_DEVICE?

'domain' is the current step in the hierarchy as we walk up the ops
pointers?

Maybe:

@child_info: The MSI domain info of the IRQ_DOMAIN_FLAG_MSI_DEVICE
             domain to be created
@parent_domain: The IRQ_DOMAIN_FLAG_MSI_PARENT domain for the child to
                be created
@domain: The domain in the hierarchy this op is being called on

And perhaps it would be a bit clearer to put the parent_domain inside
the msi_domain_info, which is basically acting as an argument bundle
for a future allocation call?

Jason
Thomas Gleixner Nov. 17, 2022, 3:58 p.m. UTC | #2
On Wed, Nov 16 2022 at 14:57, Jason Gunthorpe wrote:
> On Fri, Nov 11, 2022 at 02:58:14PM +0100, Thomas Gleixner wrote:
>> + * This is the most complex problem of per device MSI domains and the
>> + * underlying interrupt domain hierarchy:
>> + *
>> + * The device domain to be initialized requests the broadest feature set
>> + * possible and the underlying domain hierarchy puts restrictions on it.
>> + *
>> + * That's working perfectly fine for a strict parent->device model, but it
>> + * falls apart with a root_parent->real_parent->device chain because the
>
> This language hurt my brain :)

IKR

>> +bool msi_parent_init_dev_msi_info(struct device *dev, struct irq_domain *domain,
>> +				  struct irq_domain *real_parent, struct msi_domain_info *info)
>
> 'real_parent' is global IRQ_DOMAIN_FLAG_MSI_PARENT of the dev for
> which we are constructing a msi_domain_info to create a child aka
> IRQ_DOMAIN_FLAG_MSI_DEVICE?
>
> 'domain' is the current step in the hierarchy as we walk up the ops
> pointers?

Yes.

> Maybe:
>
> @child_info: The MSI domain info of the IRQ_DOMAIN_FLAG_MSI_DEVICE
>              domain to be created
> @parent_domain: The IRQ_DOMAIN_FLAG_MSI_PARENT domain for the child to
>                 be created
> @domain: The domain in the hierarchy this op is being called on

Definitely better.

> And perhaps it would be a bit clearer to put the parent_domain inside
> the msi_domain_info, which is basically acting as an argument bundle
> for a future allocation call?

Maybe. Let me try.
Thomas Gleixner Nov. 18, 2022, 1:52 p.m. UTC | #3
On Thu, Nov 17 2022 at 16:58, Thomas Gleixner wrote:
> On Wed, Nov 16 2022 at 14:57, Jason Gunthorpe wrote:
>
>> And perhaps it would be a bit clearer to put the parent_domain inside
>> the msi_domain_info, which is basically acting as an argument bundle
>> for a future allocation call?
>
> Maybe. Let me try.

No. That's redundant storage because the domain creation stores the
parent domain in irqdomain::parent which is what the hierarchy code
uses. That code does not know about msi_domain_info.

Thanks,

        tglx
diff mbox series

Patch

--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -46,6 +46,7 @@  struct irq_desc;
 struct cpumask;
 struct seq_file;
 struct irq_affinity_desc;
+struct msi_parent_ops;
 
 #define IRQ_DOMAIN_IRQ_SPEC_PARAMS 16
 
@@ -134,6 +135,7 @@  struct irq_domain_chip_generic;
  * @pm_dev:	Pointer to a device that can be utilized for power management
  *		purposes related to the irq domain.
  * @parent:	Pointer to parent irq_domain to support hierarchy irq_domains
+ * @msi_parent_ops: Pointer to MSI parent domain methods for per device domain init
  *
  * Revmap data, used internally by the irq domain code:
  * @revmap_size:	Size of the linear map table @revmap[]
@@ -157,6 +159,9 @@  struct irq_domain {
 #ifdef	CONFIG_IRQ_DOMAIN_HIERARCHY
 	struct irq_domain		*parent;
 #endif
+#ifdef CONFIG_GENERIC_MSI_IRQ
+	const struct msi_parent_ops	*msi_parent_ops;
+#endif
 
 	/* reverse map data. The linear map gets appended to the irq_domain */
 	irq_hw_number_t			hwirq_max;
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -488,6 +488,26 @@  enum {
 
 };
 
+/**
+ * struct msi_parent_ops - MSI parent domain callbacks and configuration info
+ *
+ * @supported_flags:	Required: The supported MSI flags of the parent domain
+ * @prefix:		Optional: Prefix for the domain and chip name
+ * @init_dev_msi_info:	Required: Callback for MSI parent domains to setup parent
+ *			domain specific domain flags, domain ops and interrupt chip
+ *			callbacks when a per device domain is created.
+ */
+struct msi_parent_ops {
+	u32		supported_flags;
+	const char	*prefix;
+	bool		(*init_dev_msi_info)(struct device *dev, struct irq_domain *domain,
+					     struct irq_domain *real_parent,
+					     struct msi_domain_info *info);
+};
+
+bool msi_parent_init_dev_msi_info(struct device *dev, struct irq_domain *domain,
+				  struct irq_domain *real_parent, struct msi_domain_info *info);
+
 int msi_domain_set_affinity(struct irq_data *data, const struct cpumask *mask,
 			    bool force);
 
--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -825,6 +825,42 @@  struct irq_domain *msi_create_irq_domain
 	return domain;
 }
 
+/**
+ * msi_parent_init_dev_msi_info - Delegate initialization of device MSI info to parent domain
+ * @dev:		The device for which the domain should be created
+ * @domain:		The domain which delegates
+ * @real_parent:	The real parent domain of the to be initialized MSI domain
+ * @info:		The MSI domain info to initialize
+ *
+ * Return: true on success, false otherwise
+ *
+ * This is the most complex problem of per device MSI domains and the
+ * underlying interrupt domain hierarchy:
+ *
+ * The device domain to be initialized requests the broadest feature set
+ * possible and the underlying domain hierarchy puts restrictions on it.
+ *
+ * That's working perfectly fine for a strict parent->device model, but it
+ * falls apart with a root_parent->real_parent->device chain because the
+ * intermediate 'real parent' can expand the capabilities which the
+ * 'root_parent' domain is providing. So that creates a classic hen and egg
+ * problem: Which entity is doing the restrictions/expansions?
+ *
+ * One solution is to let the root parent domain handle the initialization
+ * that's why there is the @domain and the @real_parent pointer.
+ */
+bool msi_parent_init_dev_msi_info(struct device *dev, struct irq_domain *domain,
+				  struct irq_domain *real_parent, struct msi_domain_info *info)
+{
+	struct irq_domain *parent = domain->parent;
+
+	if (WARN_ON_ONCE(!parent || !parent->msi_parent_ops ||
+			 !parent->msi_parent_ops->init_dev_msi_info))
+		return false;
+
+	return parent->msi_parent_ops->init_dev_msi_info(dev, parent, real_parent, info);
+}
+
 int msi_domain_prepare_irqs(struct irq_domain *domain, struct device *dev,
 			    int nvec, msi_alloc_info_t *arg)
 {