lkml.org 
[lkml]   [2022]   [Nov]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [patch V2 08/21] genirq/msi: Add pointers for per device irq domains
    On Mon, 21 Nov 2022 14:36:28 +0000,
    Thomas Gleixner <tglx@linutronix.de> 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@linutronix.de>
    > ---
    > 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@kernel.org>

    M.

    --
    Without deviation from the norm, progress is not possible.
    \
     
     \ /
      Last update: 2022-11-24 15:56    [W:4.381 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site