On Mon, 21 Nov 2022 14:36:28 +0000,
Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:
>
> With the upcoming per device MSI interrupt domain support it is necessary
> to store the domain pointers per device.
>
> Instead of delegating that storage to device drivers or subsystems create a
> storage array in struct msi_device_data which will also take care of
> tearing down the irq domains when msi_device_data is cleaned up via devres.
>
> The interfaces into the MSI core will be changed from irqdomain pointer
> based interfaces to domain id based interfaces to support multiple MSI
> domains on a single device (e.g. PCI/MSI[-X] and PCI/IMS.
>
> Once the per device domain support is complete the irq domain pointer in
> struct device::msi.domain will not longer contain a pointer to the "global"
> MSI domain. It will contain a pointer to the MSI parent domain instead.
>
> It would be a horrible maze of conditionals to evaluate all over the place
> which domain pointer should be used, i.e. the "global" one in
> device::msi::domain or one from the internal pointer array.
>
> To avoid this evaluate in msi_setup_device_data() whether the irq domain
> which is associated to a device is a "global" or a parent MSI domain. If it
> is global then copy the pointer into the first entry in the irqdomain
> pointer array.
>
> This allows to convert interfaces and implementation to domain ids while
> keeping everything existing working.
>
> Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> ---
> include/linux/msi.h | 3 +++
> include/linux/msi_api.h | 8 ++++++++
> kernel/irq/msi.c | 14 ++++++++++++++
> 3 files changed, 25 insertions(+)
>
> --- a/include/linux/msi.h
> +++ b/include/linux/msi.h
> @@ -77,6 +77,7 @@ struct msi_desc;
> struct pci_dev;
> struct platform_msi_priv_data;
> struct device_attribute;
> +struct irq_domain;
>
> void __get_cached_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
> #ifdef CONFIG_GENERIC_MSI_IRQ
> @@ -180,6 +181,7 @@ enum msi_desc_filter {
> * @mutex: Mutex protecting the MSI descriptor store
> * @__store: Xarray for storing MSI descriptor pointers
> * @__iter_idx: Index to search the next entry for iterators
> + * @__irqdomains: Per device interrupt domains
> */
> struct msi_device_data {
> unsigned long properties;
> @@ -187,6 +189,7 @@ struct msi_device_data {
> struct mutex mutex;
> struct xarray __store;
> unsigned long __iter_idx;
> + struct irq_domain *__irqdomains[MSI_MAX_DEVICE_IRQDOMAINS];
> };
>
> int msi_setup_device_data(struct device *dev);
> --- a/include/linux/msi_api.h
> +++ b/include/linux/msi_api.h
> @@ -10,6 +10,14 @@
>
> struct device;
>
> +/*
> + * Per device interrupt domain related constants.
> + */
> +enum msi_domain_ids {
> + MSI_DEFAULT_DOMAIN,
> + MSI_MAX_DEVICE_IRQDOMAINS,
> +};
> +
> unsigned int msi_get_virq(struct device *dev, unsigned int index);
>
> #endif
> --- a/kernel/irq/msi.c
> +++ b/kernel/irq/msi.c
> @@ -21,6 +21,18 @@
>
> static inline int msi_sysfs_create_group(struct device *dev);
>
> +static inline void msi_setup_default_irqdomain(struct device *dev, struct msi_device_data *md)
Do we really need this to be inline? I'm sure the compiler can figure
it out.
> +{
> + if (!dev->msi.domain)
> + return;
> + /*
> + * If @dev::msi::domain is a global MSI domain, copy the pointer
> + * into the domain array to avoid conditionals all over the place.
> + */
> + if (!irq_domain_is_msi_parent(dev->msi.domain))
> + md->__irqdomains[MSI_DEFAULT_DOMAIN] = dev->msi.domain;
> +}
> +
> /**
> * msi_alloc_desc - Allocate an initialized msi_desc
> * @dev: Pointer to the device for which this is allocated
> @@ -213,6 +225,8 @@ int msi_setup_device_data(struct device
> return ret;
> }
>
> + msi_setup_default_irqdomain(dev, md);
> +
nit: if you move the setup below the msi.data assignment, you could
only pass dev as a parameter. Or pass both and move the assignment in
the function?
> xa_init(&md->__store);
> mutex_init(&md->mutex);
> dev->msi.data = md;
>
>
Irrespective of the above,
Reviewed-by: Marc Zyngier <maz@xxxxxxxxxx>
M.
--
Without deviation from the norm, progress is not possible.
