From: Mike Qiu <qiudayu@linux•vnet.ibm.com>
To: Michael Ellerman <michael@ellerman•id.au>
Cc: tglx@linutronix•de, linuxppc-dev@lists•ozlabs.org,
linux-kernel@vger•kernel.org
Subject: Re: [PATCH 2/3] irq: Add hw continuous IRQs map to virtual continuous IRQs support
Date: Wed, 06 Mar 2013 13:34:58 +0800 [thread overview]
Message-ID: <5136D582.80101@linux.vnet.ibm.com> (raw)
In-Reply-To: <20130306035443.GB3493@concordia>
[-- Attachment #1: Type: text/plain, Size: 5555 bytes --]
于 2013/3/6 11:54, Michael Ellerman 写道:
> On Tue, Mar 05, 2013 at 03:19:57PM +0800, Mike Qiu wrote:
>> 于 2013/3/5 10:23, Michael Ellerman 写道:
>>> On Tue, Jan 15, 2013 at 03:38:55PM +0800, Mike Qiu wrote:
>>>> Adding a function irq_create_mapping_many() which can associate
>>>> multiple MSIs to a continous irq mapping.
>>>>
>>>> This is needed to enable multiple MSI support for pSeries.
>>>>
>>>> Signed-off-by: Mike Qiu <qiudayu@linux•vnet.ibm.com>
>>>> ---
>>>> include/linux/irq.h | 2 +
>>>> include/linux/irqdomain.h | 3 ++
>>>> kernel/irq/irqdomain.c | 61 +++++++++++++++++++++++++++++++++++++++++++++
>>>> 3 files changed, 66 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/include/linux/irq.h b/include/linux/irq.h
>>>> index 60ef45b..e00a7ec 100644
>>>> --- a/include/linux/irq.h
>>>> +++ b/include/linux/irq.h
>>>> @@ -592,6 +592,8 @@ int __irq_alloc_descs(int irq, unsigned int from, unsigned int cnt, int node,
>>>> #define irq_alloc_desc_from(from, node) \
>>>> irq_alloc_descs(-1, from, 1, node)
>>>> +#define irq_alloc_desc_n(nevc, node) \
>>>> + irq_alloc_descs(-1, 0, nevc, node)
>>> This has been superseeded by irq_alloc_descs_from(), which is the right
>>> way to do it.
>> Yes, but irq_alloc_descs_from() just for 1 irq
> No it's not, look again.
>
> #define irq_alloc_descs_from(from, cnt, node) \
> irq_alloc_descs(-1, from, cnt, node)
Sorry, I see as irq_alloc_desc_from(from, node)
you are right
>
>
>>>> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
>>>> index 96f3a1d..38648e6 100644
>>>> --- a/kernel/irq/irqdomain.c
>>>> +++ b/kernel/irq/irqdomain.c
>>>> @@ -636,6 +636,67 @@ int irq_create_strict_mappings(struct irq_domain *domain, unsigned int irq_base,
>>>> }
>>>> EXPORT_SYMBOL_GPL(irq_create_strict_mappings);
>>>> +/**
>>>> + * irq_create_mapping_many - Map a range of hw IRQs to a range of virtual IRQs
>>>> + * @domain: domain owning the interrupt range
>>>> + * @hwirq_base: beginning of continuous hardware IRQ range
>>>> + * @count: Number of interrupts to map
>>> For multiple-MSI the allocated interrupt numbers must be a power-of-2,
>>> and must be naturally aligned. I don't /think/ that's a requirement for
>>> the virtual numbers, but it's probably best that we do it anyway.
>>>
>>> So this API needs to specify that it will give you back a power-of-2
>>> block that is naturally aligned - otherwise you can't use it for MSI.
>> rtas_call will return the numbers of hardware interrupt, and it
>> should be power-of-2, as this I think do not need to specify
> You're confusing hardware interrupt numbers and virtual interrupt
> numbers. My comment is about irq_create_mapping_many(), which returns
> virtual interrupt numbers.
>
> As I said I don't think there is a requirement that the virtual
> interrupt numbers are also a power-of-2 naturally aligned block, but we
> should allocate them as one anyway, to avoid any issues in future.
But for virtual interrupt numbersit should be a power-of-2 naturally
aligned block, because it must be continuous, as the MSI-HOWTO.txt says:
4.2.2 pci_enable_msi_block
int pci_enable_msi_block(struct pci_dev *dev, int count)
This variation on the above call allows a device driver to request
multiple MSIs. The MSI specification only allows interrupts to be
allocated in powers of two, up to a maximum of 2^5 (32).
If this function returns 0, it has succeeded in allocating at least
as many interrupts as the driver requested
(it may have allocated more in order to satisfy the power-of-two
requirement). In this case, the function enables MSI on this device
and updates dev->irq to be the lowest of the new interrupts
assigned to it. The other interrupts assigned to the device are in
the range dev->irq to dev->irq + count - 1.
See the last line, that means for the virtual interrupts must be a
continuous block.
> And so this API, which returns virtual interrupt numbers, must satisfy
> that specification.
>
>>>> + /* Look for default domain if nececssary */
>>>> + if (!domain)
>>>> + domain = irq_default_domain;
>>>> + if (!domain) {
>>>> + pr_warn("irq_create_mapping called for NULL domain, hwirq=%lx\n"
>>>> + , hwirq_base);
>>>> + WARN_ON(1);
>>>> + return 0;
>>>> + }
>>>> + pr_debug("-> using domain @%p\n", domain);
>>>> +
>>>> + /* For IRQ_DOMAIN_MAP_LEGACY, get the first virtual interrupt number */
>>>> + if (domain->revmap_type == IRQ_DOMAIN_MAP_LEGACY)
>>>> + return irq_domain_legacy_revmap(domain, hwirq_base);
>>> The above doesn't work.
>> Why it doesn't work ?
> Because irq_domain_legacy_revmap() only allocates a single interrupt
> number.
OK, your right.
>>>> + /* Check if mapping already exists */
>>>> + for (i = 0; i < count; i++) {
>>>> + virq = irq_find_mapping(domain, hwirq_base+i);
>>>> + if (virq) {
>>>> + pr_debug("existing mapping on virq %d,"
>>>> + " now dispose it first\n", virq);
>>>> + irq_dispose_mapping(virq);
>>> You might have just disposed of someone elses mapping, we shouldn't do
>>> that. It should be an error to the caller.
>> It's a good question. If the interrupt used for someone elses, why I
>> can apply it from the system?
> I agree, that would be a bug. But disposing of someone elses mapping is
> not OK.
>
>> So it may someone else forget to dispose mapping, and it never be
>> used for others as I have got the interrupt I think.
> Perhaps, but that is a bug that needs to be fixed in the code that
> forgets to dispose of the mapping.
>
> cheers
>
[-- Attachment #2: Type: text/html, Size: 8453 bytes --]
next prev parent reply other threads:[~2013-03-06 5:35 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-01-15 7:38 [PATCH 0/3] Enable multiple MSI feature in pSeries Mike Qiu
2013-01-15 7:38 ` [PATCH 1/3] irq: Set multiple MSI descriptor data for multiple IRQs Mike Qiu
2013-06-05 23:03 ` Grant Likely
2013-01-15 7:38 ` [PATCH 2/3] irq: Add hw continuous IRQs map to virtual continuous IRQs support Mike Qiu
2013-03-05 2:23 ` Michael Ellerman
2013-03-05 7:19 ` Mike Qiu
2013-03-06 3:54 ` Michael Ellerman
2013-03-06 5:34 ` Mike Qiu [this message]
2013-03-06 5:42 ` Michael Ellerman
2013-03-06 7:02 ` Mike Qiu
2013-03-05 2:41 ` Paul Mundt
2013-03-05 7:44 ` Mike Qiu
2013-01-15 7:38 ` [PATCH 3/3] powerpc/pci: Enable pSeries multiple MSI feature Mike Qiu
2013-01-31 2:10 ` [PATCH 0/3] Enable multiple MSI feature in pSeries Mike
2013-02-04 3:23 ` Michael Ellerman
2013-02-04 3:49 ` Mike Qiu
2013-02-04 5:56 ` Michael Ellerman
2013-02-04 6:43 ` Mike Qiu
2013-03-01 3:07 ` Mike
2013-03-01 3:08 ` Mike
2013-03-01 3:54 ` Michael Ellerman
2013-03-04 3:14 ` Mike Qiu
2013-03-05 0:28 ` Michael Ellerman
2013-05-21 14:45 ` Alexander Gordeev
2013-05-22 0:15 ` Benjamin Herrenschmidt
2013-05-22 6:16 ` Mike Qiu
2013-05-22 5:57 ` Mike Qiu
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=5136D582.80101@linux.vnet.ibm.com \
--to=qiudayu@linux$(echo .)vnet.ibm.com \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=linuxppc-dev@lists$(echo .)ozlabs.org \
--cc=michael@ellerman$(echo .)id.au \
--cc=tglx@linutronix$(echo .)de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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