iommu.lists.linux-foundation.org archive mirror
 help / color / mirror / Atom feed
From: Marc Zyngier <maz@kernel.org>
To: "Dey, Megha" <megha.dey@intel.com>
Cc: alex.williamson@redhat.com, kevin.tian@intel.com,
	tony.luck@intel.com, dave.jiang@intel.com, ashok.raj@intel.com,
	kvm@vger.kernel.org, ravi.v.shankar@intel.com,
	linux-pci@vger.kernel.org, x86@kernel.org,
	linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org,
	jgg@mellanox.com, bhelgaas@google.com, tglx@linutronix.de,
	dan.j.williams@intel.com, dwmw@amazon.co.uk
Subject: Re: [Patch V2 13/13] genirq/msi: Provide helpers to return Linux IRQ/dev_msi hw IRQ number
Date: Fri, 26 Mar 2021 12:58:37 +0000	[thread overview]
Message-ID: <87r1k2f4w2.wl-maz@kernel.org> (raw)
In-Reply-To: <5bed6fea-32e1-d909-0a5c-439d0f0a7dfe@intel.com>

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);
}

>
> > 
> >> +	}
> >> +	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.

> >> +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).

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

  reply	other threads:[~2021-03-26 12:58 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 [this message]
2021-03-30  1:57         ` Dey, Megha

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=87r1k2f4w2.wl-maz@kernel.org \
    --to=maz@kernel.org \
    --cc=alex.williamson@redhat.com \
    --cc=ashok.raj@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=megha.dey@intel.com \
    --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).