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 09/21] genirq/msi: Make MSI descriptor iterators device domain aware
    On Mon, 21 Nov 2022 14:36:29 +0000,
    Thomas Gleixner <tglx@linutronix.de> wrote:
    >
    > To support multiple MSI interrupt domains per device it is necessary to
    > segment the xarray MSI descriptor storage. Each domain gets up to
    > MSI_MAX_INDEX entries.
    >
    > Change the iterators so they operate with domain ids and take the domain
    > offsets into account.
    >
    > The publicly available iterators which are mostly used in legacy
    > implementations and the PCI/MSI core default to MSI_DEFAULT_DOMAIN (0)
    > which is the id for the existing "global" domains.
    >
    > No functional change.
    >
    > Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
    > ---
    > V2: Fix the off by one so the index space is including MSI_MAX_INDEX (Kevin)
    > ---
    > include/linux/msi.h | 45 +++++++++++++++++++++++++++++++++++++++++----
    > kernel/irq/msi.c | 43 +++++++++++++++++++++++++++++++++++--------
    > 2 files changed, 76 insertions(+), 12 deletions(-)
    >
    > --- a/include/linux/msi.h
    > +++ b/include/linux/msi.h
    > @@ -181,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
    > + * @__iter_max: Index to limit the search
    > * @__irqdomains: Per device interrupt domains
    > */
    > struct msi_device_data {
    > @@ -189,6 +190,7 @@ struct msi_device_data {
    > struct mutex mutex;
    > struct xarray __store;
    > unsigned long __iter_idx;
    > + unsigned long __iter_max;
    > struct irq_domain *__irqdomains[MSI_MAX_DEVICE_IRQDOMAINS];
    > };
    >
    > @@ -197,14 +199,34 @@ int msi_setup_device_data(struct device
    > void msi_lock_descs(struct device *dev);
    > void msi_unlock_descs(struct device *dev);
    >
    > -struct msi_desc *msi_first_desc(struct device *dev, enum msi_desc_filter filter);
    > +struct msi_desc *msi_domain_first_desc(struct device *dev, unsigned int domid,
    > + enum msi_desc_filter filter);
    > +
    > +/**
    > + * msi_first_desc - Get the first MSI descriptor of the default irqdomain
    > + * @dev: Device to operate on
    > + * @filter: Descriptor state filter
    > + *
    > + * Must be called with the MSI descriptor mutex held, i.e. msi_lock_descs()
    > + * must be invoked before the call.
    > + *
    > + * Return: Pointer to the first MSI descriptor matching the search
    > + * criteria, NULL if none found.
    > + */
    > +static inline struct msi_desc *msi_first_desc(struct device *dev,
    > + enum msi_desc_filter filter)
    > +{
    > + return msi_domain_first_desc(dev, MSI_DEFAULT_DOMAIN, filter);
    > +}
    > +
    > struct msi_desc *msi_next_desc(struct device *dev, enum msi_desc_filter filter);
    >
    > /**
    > - * msi_for_each_desc - Iterate the MSI descriptors
    > + * msi_domain_for_each_desc - Iterate the MSI descriptors in a specific domain
    > *
    > * @desc: struct msi_desc pointer used as iterator
    > * @dev: struct device pointer - device to iterate
    > + * @domid: The id of the interrupt domain which should be walked.
    > * @filter: Filter for descriptor selection
    > *
    > * Notes:
    > @@ -212,10 +234,25 @@ struct msi_desc *msi_next_desc(struct de
    > * pair.
    > * - It is safe to remove a retrieved MSI descriptor in the loop.
    > */
    > -#define msi_for_each_desc(desc, dev, filter) \
    > - for ((desc) = msi_first_desc((dev), (filter)); (desc); \
    > +#define msi_domain_for_each_desc(desc, dev, domid, filter) \
    > + for ((desc) = msi_domain_first_desc((dev), (domid), (filter)); (desc); \
    > (desc) = msi_next_desc((dev), (filter)))
    >
    > +/**
    > + * msi_for_each_desc - Iterate the MSI descriptors in the default irqdomain
    > + *
    > + * @desc: struct msi_desc pointer used as iterator
    > + * @dev: struct device pointer - device to iterate
    > + * @filter: Filter for descriptor selection
    > + *
    > + * Notes:
    > + * - The loop must be protected with a msi_lock_descs()/msi_unlock_descs()
    > + * pair.
    > + * - It is safe to remove a retrieved MSI descriptor in the loop.
    > + */
    > +#define msi_for_each_desc(desc, dev, filter) \
    > + msi_domain_for_each_desc((desc), (dev), MSI_DEFAULT_DOMAIN, (filter))
    > +
    > #define msi_desc_to_dev(desc) ((desc)->dev)
    >
    > #ifdef CONFIG_IRQ_MSI_IOMMU
    > --- a/kernel/irq/msi.c
    > +++ b/kernel/irq/msi.c
    > @@ -21,6 +21,10 @@
    >
    > static inline int msi_sysfs_create_group(struct device *dev);
    >
    > +/* Invalid XA index which is outside of any searchable range */
    > +#define MSI_XA_MAX_INDEX (ULONG_MAX - 1)
    > +#define MSI_XA_DOMAIN_SIZE (MSI_MAX_INDEX + 1)
    > +
    > static inline void msi_setup_default_irqdomain(struct device *dev, struct msi_device_data *md)
    > {
    > if (!dev->msi.domain)
    > @@ -33,6 +37,20 @@ static inline void msi_setup_default_irq
    > md->__irqdomains[MSI_DEFAULT_DOMAIN] = dev->msi.domain;
    > }
    >
    > +static int msi_get_domain_base_index(struct device *dev, unsigned int domid)
    > +{
    > + lockdep_assert_held(&dev->msi.data->mutex);
    > +
    > + if (WARN_ON_ONCE(domid >= MSI_MAX_DEVICE_IRQDOMAINS))
    > + return -ENODEV;
    > +
    > + if (WARN_ON_ONCE(!dev->msi.data->__irqdomains[domid]))
    > + return -ENODEV;
    > +
    > + return domid * MSI_XA_DOMAIN_SIZE;
    > +}

    So what I understand of this is that we split the index space into
    segments, one per msi_domain_ids, MSI_XA_DOMAIN_SIZE apart.

    Why didn't you decide to go all the way and have one xarray per
    irqdomain? It's not that big a structure, and it would make the whole
    thing a bit more straightforward.

    Or do you anticipate cases where you'd walk the __store xarray across
    irqdomains?

    Thanks,

    M.

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