From: Gavin Shan <shangw@linux•vnet.ibm.com>
To: Benjamin Herrenschmidt <benh@kernel•crashing.org>
Cc: linuxppc-dev@lists•ozlabs.org, Gavin Shan <shangw@linux•vnet.ibm.com>
Subject: Re: [PATCH 4/7] powerpc/powernv: Patch MSI EOI handler on P8
Date: Thu, 25 Apr 2013 16:08:37 +0800 [thread overview]
Message-ID: <20130425080836.GA27415@shangw.(null)> (raw)
In-Reply-To: <1366836580.2869.16.camel@pasglop>
On Thu, Apr 25, 2013 at 06:49:40AM +1000, Benjamin Herrenschmidt wrote:
>On Wed, 2013-04-24 at 17:37 +0800, Gavin Shan wrote:
>> The EOI handler of MSI/MSI-X interrupts for P8 (PHB3) need additional
>> steps to handle the P/Q bits in IVE before EOIing the corresponding
>> interrupt. The patch changes the EOI handler to cover that.
>
> .../...
>
>> static void pnv_pci_init_ioda_msis(struct pnv_phb *phb)
>> {
>> unsigned int count;
>> @@ -667,6 +681,8 @@ static void pnv_pci_init_ioda_msis(struct pnv_phb *phb)
>> }
>>
>> phb->msi_setup = pnv_pci_ioda_msi_setup;
>> + if (phb->type == PNV_PHB_IODA2)
>> + phb->msi_eoi = pnv_pci_ioda_msi_eoi;
>
>Ouch, another function pointer call in a hot path...
>
Yeah. I've removed it in next version (not send out yet) :-)
>> phb->msi32_support = 1;
>> pr_info(" Allocated bitmap for %d MSIs (base IRQ 0x%x)\n",
>> count, phb->msi_base);
>> diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
>> index a11b5a6..ea6a93d 100644
>> --- a/arch/powerpc/platforms/powernv/pci.c
>> +++ b/arch/powerpc/platforms/powernv/pci.c
>> @@ -115,6 +115,25 @@ static void pnv_teardown_msi_irqs(struct pci_dev *pdev)
>> irq_dispose_mapping(entry->irq);
>> }
>> }
>> +
>> +int pnv_pci_msi_eoi(unsigned int hw_irq)
>> +{
>> + struct pci_controller *hose, *tmp;
>> + struct pnv_phb *phb = NULL;
>> +
>> + list_for_each_entry_safe(hose, tmp, &hose_list, list_node) {
>> + phb = hose->private_data;
>> + if (hw_irq >= phb->msi_base &&
>> + hw_irq < phb->msi_base + phb->msi_bmp.irq_count) {
>> + if (!phb->msi_eoi)
>> + return -EEXIST;
>> + return phb->msi_eoi(phb, hw_irq);
>> + }
>> + }
>> +
>> + /* For LSI interrupts, we needn't do it */
>> + return 0;
>> +}
>
>And a list walk ... that's not right.
>
>Also, you do it for all XICS interrupts, including the non-PCI ones, the
>LSIs, etc... only to figure out that some might not be MSIs later in
>the loop.
>
>Why not instead look at changing the irq_chip for the MSIs ?
>
>IE. When setting up the MSIs for IODA2, use a different irq_chip which
>is a copy of the original one with a different ->eoi callback, which
>does the original xics eoi and then the OPAL stuff ?
>
>You might even be able to use something like container_of to get back
>to the struct phb, no need to iterate them all.
>
Thanks for the detailed explaining, Ben.
I found irq_data hasn't been fully utilized until this moment. I already
have code to start use that. Firstly, "irq_data" is set to the PHB OPAL ID
or invalid value (0xffs) during mapping stage (there, we call irq_set_chip_data()
to trace the PHB OPAL ID or invalid value). Before EOIing the interrupt, we
will check "irq_data" and do special handling on P/Q bits if it has valid value.
With it, the "hot" path should be fast enough and the function pointer (mentioned
above) can be removed.
Thanks,
Gavin
next prev parent reply other threads:[~2013-04-25 8:08 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-04-24 9:37 [PATCH v3 0/7] powerpc/powernv: PHB3 Support Gavin Shan
2013-04-24 9:37 ` [PATCH 1/7] powerpc/powernv: Supports PHB3 Gavin Shan
2013-04-24 9:37 ` [PATCH 2/7] powerpc/powernv: Retrieve IODA2 tables explicitly Gavin Shan
2013-04-24 9:37 ` [PATCH 3/7] powerpc/powernv: Add option CONFIG_POWERNV_MSI Gavin Shan
2013-04-24 9:37 ` [PATCH 4/7] powerpc/powernv: Patch MSI EOI handler on P8 Gavin Shan
2013-04-24 20:49 ` Benjamin Herrenschmidt
2013-04-25 8:08 ` Gavin Shan [this message]
2013-04-25 8:13 ` Gavin Shan
2013-04-25 8:47 ` Benjamin Herrenschmidt
2013-04-25 11:58 ` Gavin Shan
2013-04-24 9:37 ` [PATCH 5/7] powerpc/powernv: TCE invalidation for PHB3 Gavin Shan
2013-04-24 20:52 ` Benjamin Herrenschmidt
2013-04-25 8:39 ` Gavin Shan
2013-04-24 9:37 ` [PATCH 6/7] powerpc/powernv: Build DMA space for PE on PHB3 Gavin Shan
2013-04-24 9:37 ` [PATCH 7/7] powerpc/powernv: Fix invalid IOMMU table Gavin Shan
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='20130425080836.GA27415@shangw.(null)' \
--to=shangw@linux$(echo .)vnet.ibm.com \
--cc=benh@kernel$(echo .)crashing.org \
--cc=linuxppc-dev@lists$(echo .)ozlabs.org \
/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