2022-11-21 16:12:21

by Thomas Gleixner

[permalink] [raw]
Subject: [patch V2 02/33] genirq/msi: Provide struct msi_parent_ops

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 <[email protected]>
---
V2: Renamed arguments and updated comments (Jason)
---
include/linux/irqdomain.h | 5 +++++
include/linux/msi.h | 21 +++++++++++++++++++++
kernel/irq/msi.c | 41 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 67 insertions(+)

--- 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
@@ -492,6 +492,27 @@ 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 *msi_parent_domain,
+ struct msi_domain_info *msi_child_info);
+};
+
+bool msi_parent_init_dev_msi_info(struct device *dev, struct irq_domain *domain,
+ struct irq_domain *msi_parent_domain,
+ struct msi_domain_info *msi_child_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
@@ -826,6 +826,47 @@ struct irq_domain *msi_create_irq_domain
return domain;
}

+/**
+ * msi_parent_init_dev_msi_info - Delegate initialization of device MSI info down
+ * in the domain hierarchy
+ * @dev: The device for which the domain should be created
+ * @domain: The domain in the hierarchy this op is being called on
+ * @msi_parent_domain: The IRQ_DOMAIN_FLAG_MSI_PARENT domain for the child to
+ * be created
+ * @msi_child_info: The MSI domain info of the IRQ_DOMAIN_FLAG_MSI_DEVICE
+ * domain to be created
+ *
+ * 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 trivial for a simple parent->child relationship, but it gets
+ * interesting with an intermediate domain: root->parent->child. The
+ * intermediate 'parent' can expand the capabilities which the 'root'
+ * 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 domain handle the initialization that's
+ * why there is the @domain and the @msi_parent_domain pointer.
+ */
+bool msi_parent_init_dev_msi_info(struct device *dev, struct irq_domain *domain,
+ struct irq_domain *msi_parent_domain,
+ struct msi_domain_info *msi_child_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, msi_parent_domain,
+ msi_child_info);
+}
+
int msi_domain_prepare_irqs(struct irq_domain *domain, struct device *dev,
int nvec, msi_alloc_info_t *arg)
{



2022-11-23 08:16:25

by Tian, Kevin

[permalink] [raw]
Subject: RE: [patch V2 02/33] genirq/msi: Provide struct msi_parent_ops

> From: Thomas Gleixner <[email protected]>
> Sent: Monday, November 21, 2022 10:38 PM
>
> +/**
> + * msi_parent_init_dev_msi_info - Delegate initialization of device MSI info
> down
> + * in the domain hierarchy
> + * @dev: The device for which the domain should be created
> + * @domain: The domain in the hierarchy this op is being called on
> + * @msi_parent_domain: The IRQ_DOMAIN_FLAG_MSI_PARENT
> domain for the child to
> + * be created
> + * @msi_child_info: The MSI domain info of the
> IRQ_DOMAIN_FLAG_MSI_DEVICE
> + * domain to be created
> + *
> + * 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 trivial for a simple parent->child relationship, but it gets
> + * interesting with an intermediate domain: root->parent->child. The
> + * intermediate 'parent' can expand the capabilities which the 'root'
> + * 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 domain handle the initialization that's
> + * why there is the @domain and the @msi_parent_domain pointer.

This is the part which I don't quite understand (sorry with limited knowledge
in this area).

In concept a hierarchical model has restrictions added up when moving
down to lower layers i.e. presumably the root domain decides the minimal
supported capabilities. In this case there is no need of a real parent pointer
as long as every domain in the stack incrementally adds its restrictions to
info->flags.

I can see why this is required for x86 given that MULTI_MSI is supported
only with IR. and we cannot make vector domain inclusively claiming
MULTI_MSI since it's completely broken when the vector domain becomes
the parent itself, in absence of IR.

Just be curious whether this intermediate-parent-deciding-restrictions
is generic instead of x86 specific, e.g. is it possible to have a 4-layers
hierarchy where the root parent wants to check both two intermediate
parents?

Thanks
Kevin

2022-11-23 12:40:13

by Thomas Gleixner

[permalink] [raw]
Subject: RE: [patch V2 02/33] genirq/msi: Provide struct msi_parent_ops

On Wed, Nov 23 2022 at 07:57, Kevin Tian wrote:
>> From: Thomas Gleixner <[email protected]>
>> + * One solution is to let the root domain handle the initialization that's
>> + * why there is the @domain and the @msi_parent_domain pointer.
>
> This is the part which I don't quite understand (sorry with limited knowledge
> in this area).
>
> In concept a hierarchical model has restrictions added up when moving
> down to lower layers i.e. presumably the root domain decides the minimal
> supported capabilities. In this case there is no need of a real parent pointer
> as long as every domain in the stack incrementally adds its restrictions to
> info->flags.
>
> I can see why this is required for x86 given that MULTI_MSI is supported
> only with IR. and we cannot make vector domain inclusively claiming
> MULTI_MSI since it's completely broken when the vector domain becomes
> the parent itself, in absence of IR.
>
> Just be curious whether this intermediate-parent-deciding-restrictions
> is generic instead of x86 specific, e.g. is it possible to have a 4-layers
> hierarchy where the root parent wants to check both two intermediate
> parents?

Sure. Nothing prevents you from doing so:

dom4:
.init... = dom4_init

dom4_init()
do_stuff()
invoke parent init

dom3:
.init... = parent_init

dom2:
.init... = dom2_init

dom2_init()
do_stuff()
invoke parent init

....

See?

Thanks,

tglx

2022-11-24 01:29:44

by Tian, Kevin

[permalink] [raw]
Subject: RE: [patch V2 02/33] genirq/msi: Provide struct msi_parent_ops

> From: Thomas Gleixner <[email protected]>
> Sent: Wednesday, November 23, 2022 7:29 PM
>
> On Wed, Nov 23 2022 at 07:57, Kevin Tian wrote:
> >> From: Thomas Gleixner <[email protected]>
> >> + * One solution is to let the root domain handle the initialization that's
> >> + * why there is the @domain and the @msi_parent_domain pointer.
> >
> > This is the part which I don't quite understand (sorry with limited
> knowledge
> > in this area).
> >
> > In concept a hierarchical model has restrictions added up when moving
> > down to lower layers i.e. presumably the root domain decides the minimal
> > supported capabilities. In this case there is no need of a real parent pointer
> > as long as every domain in the stack incrementally adds its restrictions to
> > info->flags.
> >
> > I can see why this is required for x86 given that MULTI_MSI is supported
> > only with IR. and we cannot make vector domain inclusively claiming
> > MULTI_MSI since it's completely broken when the vector domain becomes
> > the parent itself, in absence of IR.
> >
> > Just be curious whether this intermediate-parent-deciding-restrictions
> > is generic instead of x86 specific, e.g. is it possible to have a 4-layers
> > hierarchy where the root parent wants to check both two intermediate
> > parents?
>
> Sure. Nothing prevents you from doing so:
>
> dom4:
> .init... = dom4_init
>
> dom4_init()
> do_stuff()
> invoke parent init
>
> dom3:
> .init... = parent_init
>
> dom2:
> .init... = dom2_init
>
> dom2_init()
> do_stuff()
> invoke parent init
>
> ....
>
> See?
>

yes. with the hierarchy being arch specific those dependencies
can be easily figured out.