From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 64D51C43217 for ; Thu, 24 Nov 2022 15:46:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229613AbiKXPqL (ORCPT ); Thu, 24 Nov 2022 10:46:11 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34640 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229448AbiKXPqL (ORCPT ); Thu, 24 Nov 2022 10:46:11 -0500 Received: from ams.source.kernel.org (ams.source.kernel.org [IPv6:2604:1380:4601:e00::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 497D793CF6; Thu, 24 Nov 2022 07:46:09 -0800 (PST) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 0FBD9B82871; Thu, 24 Nov 2022 15:46:08 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 90CE9C433C1; Thu, 24 Nov 2022 15:46:06 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1669304766; bh=nOWuXFcY6tchDx+uGT9fJZYoq2XpD2rR1ixM25MB3DY=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=MP0YU/9PgCIVcADtRQGfecL6nRQ599zr3mxfHTtRGWspodKW6WHFJF6scO049ABGN Z29gf56TBS1iGeP01/tL474w2FW+cStYvnrYfPxRKgHXS0R03ClPHZXZhdFazytxq8 oegc91mbyttVVeftvKxoJ1+T87TW3ouxDL0pNzkpNFNWxFh8TCjWyw4IDnCbIPpk04 e+zvBS++6E9tmUwM2Rq+y3VUAFzQe4wmAqE1qNCPM8mEGkrFt/+YLfL5RvzxF84+jN 6duXktlEkvIqMRj3bTlpbmpj89RPe7zX1tG51FV+zc/G2zw/Tin7ufjS//YfpKmXUB iWk/0J8NFS4Yw== Received: from sofa.misterjones.org ([185.219.108.64] helo=goblin-girl.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1oyEQO-008Pgz-0Y; Thu, 24 Nov 2022 15:46:04 +0000 Date: Thu, 24 Nov 2022 15:46:03 +0000 Message-ID: <86r0xsmk3o.wl-maz@kernel.org> From: Marc Zyngier To: Thomas Gleixner Cc: LKML , x86@kernel.org, Joerg Roedel , Will Deacon , linux-pci@vger.kernel.org, Bjorn Helgaas , Lorenzo Pieralisi , Greg Kroah-Hartman , Jason Gunthorpe , Dave Jiang , Alex Williamson , Kevin Tian , Dan Williams , Logan Gunthorpe , Ashok Raj , Jon Mason , Allen Hubbe Subject: Re: [patch V2 09/21] genirq/msi: Make MSI descriptor iterators device domain aware In-Reply-To: <20221121083326.002153609@linutronix.de> References: <20221121083210.309161925@linutronix.de> <20221121083326.002153609@linutronix.de> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: tglx@linutronix.de, linux-kernel@vger.kernel.org, x86@kernel.org, joro@8bytes.org, will@kernel.org, linux-pci@vger.kernel.org, bhelgaas@google.com, lorenzo.pieralisi@arm.com, gregkh@linuxfoundation.org, jgg@mellanox.com, dave.jiang@intel.com, alex.williamson@redhat.com, kevin.tian@intel.com, dan.j.williams@intel.com, logang@deltatee.com, ashok.raj@intel.com, jdmason@kudzu.us, allenbh@gmail.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org On Mon, 21 Nov 2022 14:36:29 +0000, Thomas Gleixner 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 > --- > 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.