From: "Dey, Megha" <megha.dey@intel.com>
To: Marc Zyngier <maz@kernel.org>
Cc: tglx@linutronix.de, linux-kernel@vger.kernel.org,
dave.jiang@intel.com, ashok.raj@intel.com, kevin.tian@intel.com,
dwmw@amazon.co.uk, x86@kernel.org, tony.luck@intel.com,
dan.j.williams@intel.com, jgg@mellanox.com, kvm@vger.kernel.org,
iommu@lists.linux-foundation.org, alex.williamson@redhat.com,
bhelgaas@google.com, linux-pci@vger.kernel.org,
baolu.lu@linux.intel.com, ravi.v.shankar@intel.com
Subject: Re: [Patch V2 13/13] genirq/msi: Provide helpers to return Linux IRQ/dev_msi hw IRQ number
Date: Tue, 30 Mar 2021 07:27:40 +0530 [thread overview]
Message-ID: <64602269-5c82-46ce-b2a0-2586e68e258f@intel.com> (raw)
In-Reply-To: <87r1k2f4w2.wl-maz@kernel.org>
Hi Marc,
On 3/26/2021 6:28 PM, Marc Zyngier wrote:
> On Fri, 26 Mar 2021 01:02:43 +0000,
> "Dey, Megha" <megha.dey@intel.com> wrote:
>> Hi Marc,
>>
>> On 3/25/2021 10:53 AM, Marc Zyngier wrote:
>>> On Fri, 26 Feb 2021 20:11:17 +0000,
>>> Megha Dey <megha.dey@intel.com> wrote:
>>>> From: Dave Jiang <dave.jiang@intel.com>
>>>>
>>>> Add new helpers to get the Linux IRQ number and device specific index
>>>> for given device-relative vector so that the drivers don't need to
>>>> allocate their own arrays to keep track of the vectors and hwirq for
>>>> the multi vector device MSI case.
>>>>
>>>> Reviewed-by: Tony Luck <tony.luck@intel.com>
>>>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>>>> Signed-off-by: Megha Dey <megha.dey@intel.com>
>>>> ---
>>>> include/linux/msi.h | 2 ++
>>>> kernel/irq/msi.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
>>>> 2 files changed, 46 insertions(+)
>>>>
>>>> diff --git a/include/linux/msi.h b/include/linux/msi.h
>>>> index 24abec0..d60a6ba 100644
>>>> --- a/include/linux/msi.h
>>>> +++ b/include/linux/msi.h
>>>> @@ -451,6 +451,8 @@ struct irq_domain *platform_msi_create_irq_domain(struct fwnode_handle *fwnode,
>>>> int platform_msi_domain_alloc_irqs(struct device *dev, unsigned int nvec,
>>>> irq_write_msi_msg_t write_msi_msg);
>>>> void platform_msi_domain_free_irqs(struct device *dev);
>>>> +int msi_irq_vector(struct device *dev, unsigned int nr);
>>>> +int dev_msi_hwirq(struct device *dev, unsigned int nr);
>>>> /* When an MSI domain is used as an intermediate domain */
>>>> int msi_domain_prepare_irqs(struct irq_domain *domain, struct device *dev,
>>>> diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
>>>> index 047b59d..f2a8f55 100644
>>>> --- a/kernel/irq/msi.c
>>>> +++ b/kernel/irq/msi.c
>>>> @@ -581,4 +581,48 @@ struct msi_domain_info *msi_get_domain_info(struct irq_domain *domain)
>>>> return (struct msi_domain_info *)domain->host_data;
>>>> }
>>>> +/**
>>>> + * msi_irq_vector - Get the Linux IRQ number of a device vector
>>>> + * @dev: device to operate on
>>>> + * @nr: device-relative interrupt vector index (0-based).
>>>> + *
>>>> + * Returns the Linux IRQ number of a device vector.
>>>> + */
>>>> +int msi_irq_vector(struct device *dev, unsigned int nr)
>>>> +{
>>>> + struct msi_desc *entry;
>>>> + int i = 0;
>>>> +
>>>> + for_each_msi_entry(entry, dev) {
>>>> + if (i == nr)
>>>> + return entry->irq;
>>>> + i++;
>>> This obviously doesn't work with Multi-MSI, does it?
>> This API is only for devices that support device MSI interrupts. They
>> follow MSI-x format and don't support multi MSI (part of MSI).
>>
>> Not sure if I am missing something here, can you please let me know?
> Nothing in the prototype of the function indicates this limitation,
> nor does the documentation. And I'm not sure why you should exclude
> part of the MSI functionality here. It can't be for performance
> reason, so you might as well make sure this works for all the MSI
> variants:
>
> int msi_irq_vector(struct device *dev, unsigned int nr)
> {
> struct msi_desc *entry;
> int irq, index = 0;
>
> for_each_msi_vector(entry, irq, dev) {
> if (index == nr}
> return irq;
> index++;
> }
>
> return WARN_ON_ONCE(-EINVAL);
> }
Ok, got it!
>>>> + }
>>>> + WARN_ON_ONCE(1);
>>>> + return -EINVAL;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(msi_irq_vector);
>>>> +
>>>> +/**
>>>> + * dev_msi_hwirq - Get the device MSI hw IRQ number of a device vector
>>>> + * @dev: device to operate on
>>>> + * @nr: device-relative interrupt vector index (0-based).
>>>> + *
>>>> + * Return the dev_msi hw IRQ number of a device vector.
>>>> + */
>>>> +int dev_msi_hwirq(struct device *dev, unsigned int nr)
>>>> +{
>>>> + struct msi_desc *entry;
>>>> + int i = 0;
>>>> +
>>>> + for_each_msi_entry(entry, dev) {
>>>> + if (i == nr)
>>>> + return entry->device_msi.hwirq;
>>>> + i++;
>>>> + }
>>>> + WARN_ON_ONCE(1);
>>>> + return -EINVAL;
>>>> +}
> And this helper would be more generally useful if it returned the n-th
> msi_desc entry rather than some obscure field in a substructure.
>
> struct msi_desc *msi_get_nth_desc(struct device *dev, unsigned int nth)
> {
> struct msi_desc *entry = NULL;
> unsigned int i = 0;
>
> for_each_msi_entry(entry, dev) {
> if (i == nth)
> return entry;
>
> i++;
> }
>
> WARN_ON_ONCE(!entry);
> return entry;
> }
>
> You can always wrap it for your particular use case.
Yeah, makes sense.
>
>>>> +EXPORT_SYMBOL_GPL(dev_msi_hwirq);
>>>> +
>>>> #endif /* CONFIG_GENERIC_MSI_IRQ_DOMAIN */
>>> And what uses these helpers?]
>> These helpers are to be used by a driver series(Intel's IDXD driver)
>> which is currently stuck due to VFIO refactoring.
> Then I's suggest you keep the helpers together with the actual user,
> unless this can generally be useful to existing users (exported
> symbols without in-tree users is always a bit odd).
Yeah in the next submission, we will submit this patch series along with
the user driver patch series.
>
> Thanks,
>
> M.
>
prev parent reply other threads:[~2021-03-30 1:59 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-26 20:11 [Patch V2 00/13] Introduce dev-msi and interrupt message store Megha Dey
2021-02-26 20:11 ` [Patch V2 01/13] x86/irq: Add DEV_MSI allocation type Megha Dey
2021-02-26 20:11 ` [Patch V2 02/13] x86/msi: Rename and rework pci_msi_prepare() to cover non-PCI MSI Megha Dey
2021-02-26 20:11 ` [Patch V2 03/13] platform-msi: Provide default irq_chip:: Ack Megha Dey
2021-02-26 20:11 ` [Patch V2 04/13] genirq/proc: Take buslock on affinity write Megha Dey
2021-02-26 20:11 ` [Patch V2 05/13] genirq/msi: Provide and use msi_domain_set_default_info_flags() Megha Dey
2021-02-26 20:11 ` [Patch V2 06/13] platform-msi: Add device MSI infrastructure Megha Dey
2021-02-26 20:11 ` [Patch V2 07/13] irqdomain/msi: Provide msi_alloc/free_store() callbacks Megha Dey
2021-03-25 17:08 ` Marc Zyngier
2021-03-25 18:44 ` Thomas Gleixner
2021-03-26 10:14 ` Marc Zyngier
2021-02-26 20:11 ` [Patch V2 08/13] genirq: Set auxiliary data for an interrupt Megha Dey
2021-03-25 17:23 ` Marc Zyngier
2021-03-25 18:59 ` Thomas Gleixner
2021-03-26 10:32 ` Marc Zyngier
2021-03-26 15:09 ` Thomas Gleixner
2021-02-26 20:11 ` [Patch V2 09/13] iommu/vt-d: Add DEV-MSI support Megha Dey
2021-02-26 20:11 ` [Patch V2 10/13] iommu: Add capability IOMMU_CAP_VIOMMU_HINT Megha Dey
2021-02-26 20:11 ` [Patch V2 11/13] platform-msi: Add platform check for subdevice irq domain Megha Dey
2021-02-26 20:11 ` [Patch V2 12/13] irqchip: Add IMS (Interrupt Message Store) driver Megha Dey
2021-03-25 17:43 ` Marc Zyngier
2021-03-25 19:07 ` Thomas Gleixner
2021-03-26 1:03 ` Dey, Megha
2021-02-26 20:11 ` [Patch V2 13/13] genirq/msi: Provide helpers to return Linux IRQ/dev_msi hw IRQ number Megha Dey
2021-03-25 17:53 ` Marc Zyngier
2021-03-26 1:02 ` Dey, Megha
2021-03-26 12:58 ` Marc Zyngier
2021-03-30 1:57 ` Dey, Megha [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=64602269-5c82-46ce-b2a0-2586e68e258f@intel.com \
--to=megha.dey@intel.com \
--cc=alex.williamson@redhat.com \
--cc=ashok.raj@intel.com \
--cc=baolu.lu@linux.intel.com \
--cc=bhelgaas@google.com \
--cc=dan.j.williams@intel.com \
--cc=dave.jiang@intel.com \
--cc=dwmw@amazon.co.uk \
--cc=iommu@lists.linux-foundation.org \
--cc=jgg@mellanox.com \
--cc=kevin.tian@intel.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=maz@kernel.org \
--cc=ravi.v.shankar@intel.com \
--cc=tglx@linutronix.de \
--cc=tony.luck@intel.com \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).