From: Yijing Wang <wangyijing@huawei.com>
To: Bjorn Helgaas <bhelgaas@google.com>
Cc: linux-mips@linux-mips.org, linux-ia64@vger.kernel.org,
linux-pci@vger.kernel.org, Bharat.Bhushan@freescale.com,
Thierry Reding <thierry.reding@gmail.com>,
sparclinux@vger.kernel.org, linux-arch@vger.kernel.org,
linux-s390@vger.kernel.org, Russell King <linux@arm.linux.org.uk>,
Joerg Roedel <joro@8bytes.org>,
x86@kernel.org, Sebastian Ott <sebott@linux.vnet.ibm.com>,
xen-devel@lists.xenproject.org, arnab.basu@freescale.com,
Liviu Dudau <liviu@dudau.co.uk>, Arnd Bergmann <arnd@arndb.de>,
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
Chris Metcalf <cmetcalf@tilera.com>,
Thomas Gleixner <tglx@linutronix.de>,
linux-arm-kernel@lists.infradead.org,
Thomas Petazzoni <thomas.petazzoni@free-electrons.com>,
Xinwei Hu <huxinwei@huawei.com>, Tony Luck <tony.luck@intel.com>,
Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>,
linux-kernel@vger.kernel.org, Ralf Baechle <ralf@linux-mips.org>,
iommu@lists.linux-foundation.org,
David Vrabel <david.vrabel@citrix.com>,
Wuyun <wuyun.wu@huawei.com>,
linuxppc-dev@lists.ozlabs.org,
"David S. Miller" <davem@davemloft.net>,
Lucas Stach <l.stach@pengutronix.de>
Subject: Re: [PATCH v3 02/27] x86/xen/MSI: Eliminate arch_msix_mask_irq() and arch_msi_mask_irq()
Date: Thu, 23 Oct 2014 12:44:24 +0800 [thread overview]
Message-ID: <544887A8.5090708@huawei.com> (raw)
In-Reply-To: <20141023042535.GA11770@google.com>
On 2014/10/23 12:25, Bjorn Helgaas wrote:
> On Wed, Oct 15, 2014 at 11:06:50AM +0800, Yijing Wang wrote:
>> Commit 0e4ccb1505a9 ("PCI: Add x86_msi.msi_mask_irq() and msix_mask_irq()")
>> introduced two __weak arch functions arch_msix_mask_irq() and
>> arch_msi_mask_irq() to work around a bug when running xen in x86.
>> These two functions made msi code more complex. This patch reverts
>> the commit and introduces a global flag to control msi mask action to
>> avoid the bug. The patch is also preparation for using MSI chip framework
>> instead of weak arch MSI functions in all platforms. Keep default_msi_mask_irq()
>> and default_msix_mask_irq() in linux/msi.h to make s390 MSI code compile
>> happy, they will be removed in the later patch.
>
> This patch is basically a revert of 0e4ccb1505a9 intermingled with the
> addition of the new pci_msi_ignore_mask technique.
>
> Can you please split this into two patches:
>
> - Add the pci_msi_ignore_mask stuff (alongside the existing way)
> - Revert 0e4ccb1505a9
>
> I think that will make it easier to see what's going on.
OK, I will do it, thanks.
Thanks!
Yijing.
>
>> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
>> CC: David Vrabel <david.vrabel@citrix.com>
>> CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> ---
>> Hi David and Konrad,
>> I dropped the Acked-by and tested-by, because this version has a
>> lot changes compared to last. So, I guess you may want to check it again.
>> ---
>> arch/x86/include/asm/x86_init.h | 3 ---
>> arch/x86/kernel/x86_init.c | 10 ----------
>> arch/x86/pci/xen.c | 14 ++------------
>> drivers/pci/msi.c | 29 ++++++++++++-----------------
>> include/linux/msi.h | 8 ++++++--
>> 5 files changed, 20 insertions(+), 44 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h
>> index e45e4da..f58a9c7 100644
>> --- a/arch/x86/include/asm/x86_init.h
>> +++ b/arch/x86/include/asm/x86_init.h
>> @@ -172,7 +172,6 @@ struct x86_platform_ops {
>>
>> struct pci_dev;
>> struct msi_msg;
>> -struct msi_desc;
>>
>> struct x86_msi_ops {
>> int (*setup_msi_irqs)(struct pci_dev *dev, int nvec, int type);
>> @@ -183,8 +182,6 @@ struct x86_msi_ops {
>> void (*teardown_msi_irqs)(struct pci_dev *dev);
>> void (*restore_msi_irqs)(struct pci_dev *dev);
>> int (*setup_hpet_msi)(unsigned int irq, unsigned int id);
>> - u32 (*msi_mask_irq)(struct msi_desc *desc, u32 mask, u32 flag);
>> - u32 (*msix_mask_irq)(struct msi_desc *desc, u32 flag);
>> };
>>
>> struct IO_APIC_route_entry;
>> diff --git a/arch/x86/kernel/x86_init.c b/arch/x86/kernel/x86_init.c
>> index e48b674..234b072 100644
>> --- a/arch/x86/kernel/x86_init.c
>> +++ b/arch/x86/kernel/x86_init.c
>> @@ -116,8 +116,6 @@ struct x86_msi_ops x86_msi = {
>> .teardown_msi_irqs = default_teardown_msi_irqs,
>> .restore_msi_irqs = default_restore_msi_irqs,
>> .setup_hpet_msi = default_setup_hpet_msi,
>> - .msi_mask_irq = default_msi_mask_irq,
>> - .msix_mask_irq = default_msix_mask_irq,
>> };
>>
>> /* MSI arch specific hooks */
>> @@ -140,14 +138,6 @@ void arch_restore_msi_irqs(struct pci_dev *dev)
>> {
>> x86_msi.restore_msi_irqs(dev);
>> }
>> -u32 arch_msi_mask_irq(struct msi_desc *desc, u32 mask, u32 flag)
>> -{
>> - return x86_msi.msi_mask_irq(desc, mask, flag);
>> -}
>> -u32 arch_msix_mask_irq(struct msi_desc *desc, u32 flag)
>> -{
>> - return x86_msi.msix_mask_irq(desc, flag);
>> -}
>> #endif
>>
>> struct x86_io_apic_ops x86_io_apic_ops = {
>> diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
>> index 093f5f4..e5b8b78 100644
>> --- a/arch/x86/pci/xen.c
>> +++ b/arch/x86/pci/xen.c
>> @@ -394,14 +394,6 @@ static void xen_teardown_msi_irq(unsigned int irq)
>> {
>> xen_destroy_irq(irq);
>> }
>> -static u32 xen_nop_msi_mask_irq(struct msi_desc *desc, u32 mask, u32 flag)
>> -{
>> - return 0;
>> -}
>> -static u32 xen_nop_msix_mask_irq(struct msi_desc *desc, u32 flag)
>> -{
>> - return 0;
>> -}
>> #endif
>>
>> int __init pci_xen_init(void)
>> @@ -425,8 +417,7 @@ int __init pci_xen_init(void)
>> x86_msi.setup_msi_irqs = xen_setup_msi_irqs;
>> x86_msi.teardown_msi_irq = xen_teardown_msi_irq;
>> x86_msi.teardown_msi_irqs = xen_teardown_msi_irqs;
>> - x86_msi.msi_mask_irq = xen_nop_msi_mask_irq;
>> - x86_msi.msix_mask_irq = xen_nop_msix_mask_irq;
>> + pci_msi_ignore_mask = 1;
>> #endif
>> return 0;
>> }
>> @@ -506,8 +497,7 @@ int __init pci_xen_initial_domain(void)
>> x86_msi.setup_msi_irqs = xen_initdom_setup_msi_irqs;
>> x86_msi.teardown_msi_irq = xen_teardown_msi_irq;
>> x86_msi.restore_msi_irqs = xen_initdom_restore_msi_irqs;
>> - x86_msi.msi_mask_irq = xen_nop_msi_mask_irq;
>> - x86_msi.msix_mask_irq = xen_nop_msix_mask_irq;
>> + pci_msi_ignore_mask = 1;
>> #endif
>> xen_setup_acpi_sci();
>> __acpi_register_gsi = acpi_register_gsi_xen;
>> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
>> index ecb92a5..22e413c 100644
>> --- a/drivers/pci/msi.c
>> +++ b/drivers/pci/msi.c
>> @@ -23,6 +23,7 @@
>> #include "pci.h"
>>
>> static int pci_msi_enable = 1;
>> +int pci_msi_ignore_mask;
>>
>> #define msix_table_size(flags) ((flags & PCI_MSIX_FLAGS_QSIZE) + 1)
>>
>> @@ -162,11 +163,11 @@ static inline __attribute_const__ u32 msi_mask(unsigned x)
>> * reliably as devices without an INTx disable bit will then generate a
>> * level IRQ which will never be cleared.
>> */
>> -u32 default_msi_mask_irq(struct msi_desc *desc, u32 mask, u32 flag)
>> +u32 __msi_mask_irq(struct msi_desc *desc, u32 mask, u32 flag)
>> {
>> u32 mask_bits = desc->masked;
>>
>> - if (!desc->msi_attrib.maskbit)
>> + if (pci_msi_ignore_mask || !desc->msi_attrib.maskbit)
>> return 0;
>>
>> mask_bits &= ~mask;
>> @@ -176,14 +177,9 @@ u32 default_msi_mask_irq(struct msi_desc *desc, u32 mask, u32 flag)
>> return mask_bits;
>> }
>>
>> -__weak u32 arch_msi_mask_irq(struct msi_desc *desc, u32 mask, u32 flag)
>> -{
>> - return default_msi_mask_irq(desc, mask, flag);
>> -}
>> -
>> static void msi_mask_irq(struct msi_desc *desc, u32 mask, u32 flag)
>> {
>> - desc->masked = arch_msi_mask_irq(desc, mask, flag);
>> + desc->masked = __msi_mask_irq(desc, mask, flag);
>> }
>>
>> /*
>> @@ -193,11 +189,15 @@ static void msi_mask_irq(struct msi_desc *desc, u32 mask, u32 flag)
>> * file. This saves a few milliseconds when initialising devices with lots
>> * of MSI-X interrupts.
>> */
>> -u32 default_msix_mask_irq(struct msi_desc *desc, u32 flag)
>> +u32 __msix_mask_irq(struct msi_desc *desc, u32 flag)
>> {
>> u32 mask_bits = desc->masked;
>> unsigned offset = desc->msi_attrib.entry_nr * PCI_MSIX_ENTRY_SIZE +
>> PCI_MSIX_ENTRY_VECTOR_CTRL;
>> +
>> + if (pci_msi_ignore_mask)
>> + return 0;
>> +
>> mask_bits &= ~PCI_MSIX_ENTRY_CTRL_MASKBIT;
>> if (flag)
>> mask_bits |= PCI_MSIX_ENTRY_CTRL_MASKBIT;
>> @@ -206,14 +206,9 @@ u32 default_msix_mask_irq(struct msi_desc *desc, u32 flag)
>> return mask_bits;
>> }
>>
>> -__weak u32 arch_msix_mask_irq(struct msi_desc *desc, u32 flag)
>> -{
>> - return default_msix_mask_irq(desc, flag);
>> -}
>> -
>> static void msix_mask_irq(struct msi_desc *desc, u32 flag)
>> {
>> - desc->masked = arch_msix_mask_irq(desc, flag);
>> + desc->masked = __msix_mask_irq(desc, flag);
>> }
>>
>> static void msi_set_mask_bit(struct irq_data *data, u32 flag)
>> @@ -866,7 +861,7 @@ void pci_msi_shutdown(struct pci_dev *dev)
>> /* Return the device with MSI unmasked as initial states */
>> mask = msi_mask(desc->msi_attrib.multi_cap);
>> /* Keep cached state to be restored */
>> - arch_msi_mask_irq(desc, mask, ~mask);
>> + __msi_mask_irq(desc, mask, ~mask);
>>
>> /* Restore dev->irq to its default pin-assertion irq */
>> dev->irq = desc->msi_attrib.default_irq;
>> @@ -964,7 +959,7 @@ void pci_msix_shutdown(struct pci_dev *dev)
>> /* Return the device with MSI-X masked as initial states */
>> list_for_each_entry(entry, &dev->msi_list, list) {
>> /* Keep cached states to be restored */
>> - arch_msix_mask_irq(entry, 1);
>> + __msix_mask_irq(entry, 1);
>> }
>>
>> msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_ENABLE, 0);
>> diff --git a/include/linux/msi.h b/include/linux/msi.h
>> index 44f4746..9ac1e3b 100644
>> --- a/include/linux/msi.h
>> +++ b/include/linux/msi.h
>> @@ -10,6 +10,8 @@ struct msi_msg {
>> u32 data; /* 16 bits of msi message data */
>> };
>>
>> +extern int pci_msi_ignore_mask;
>> +
>> /* Helper functions */
>> struct irq_data;
>> struct msi_desc;
>> @@ -21,6 +23,8 @@ void __write_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
>> void read_msi_msg(unsigned int irq, struct msi_msg *msg);
>> void get_cached_msi_msg(unsigned int irq, struct msi_msg *msg);
>> void write_msi_msg(unsigned int irq, struct msi_msg *msg);
>> +u32 __msix_mask_irq(struct msi_desc *desc, u32 flag);
>> +u32 __msi_mask_irq(struct msi_desc *desc, u32 mask, u32 flag);
>>
>> struct msi_desc {
>> struct {
>> @@ -61,8 +65,8 @@ void arch_restore_msi_irqs(struct pci_dev *dev);
>>
>> void default_teardown_msi_irqs(struct pci_dev *dev);
>> void default_restore_msi_irqs(struct pci_dev *dev);
>> -u32 default_msi_mask_irq(struct msi_desc *desc, u32 mask, u32 flag);
>> -u32 default_msix_mask_irq(struct msi_desc *desc, u32 flag);
>> +#define default_msi_mask_irq __msi_mask_irq
>> +#define default_msix_mask_irq __msix_mask_irq
>>
>> struct msi_chip {
>> struct module *owner;
>> --
>> 1.7.1
>>
>
> .
>
--
Thanks!
Yijing
next prev parent reply other threads:[~2014-10-23 4:45 UTC|newest]
Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-10-15 3:06 [PATCH v3 00/27] Use MSI chip framework to configure MSI/MSI-X in all platforms Yijing Wang
2014-10-15 3:06 ` [PATCH v3 01/27] MSI: Remove the redundant irq_set_chip_data() Yijing Wang
2014-10-15 3:06 ` [PATCH v3 02/27] x86/xen/MSI: Eliminate arch_msix_mask_irq() and arch_msi_mask_irq() Yijing Wang
2014-10-23 4:25 ` Bjorn Helgaas
2014-10-23 4:44 ` Yijing Wang [this message]
2014-10-15 3:06 ` [PATCH v3 03/27] s390/MSI: Use __msi_mask_irq() instead of default_msi_mask_irq() Yijing Wang
2014-10-15 3:06 ` [PATCH v3 04/27] arm/MSI: Save MSI chip in pci_sys_data Yijing Wang
2014-10-20 17:21 ` Lorenzo Pieralisi
2014-10-21 1:32 ` Yijing Wang
2014-10-23 5:35 ` Bjorn Helgaas
2014-10-23 6:32 ` Yijing Wang
2014-10-15 3:06 ` [PATCH v3 05/27] PCI: tegra: Save msi " Yijing Wang
2014-10-23 5:18 ` Bjorn Helgaas
2014-10-23 6:23 ` Yijing Wang
2014-10-15 3:06 ` [PATCH v3 06/27] PCI: designware: " Yijing Wang
2014-10-15 3:06 ` [PATCH v3 07/27] PCI: rcar: " Yijing Wang
2014-10-15 3:06 ` [PATCH v3 08/27] PCI: mvebu: " Yijing Wang
2014-10-15 3:06 ` [PATCH v3 09/27] arm/PCI: Clean unused pcibios_add_bus() and pcibios_remove_bus() Yijing Wang
2014-10-23 5:38 ` Bjorn Helgaas
2014-10-23 6:39 ` Yijing Wang
2014-10-15 3:06 ` [PATCH v3 10/27] PCI/MSI: Remove useless bus->msi assignment Yijing Wang
2014-10-23 5:41 ` Bjorn Helgaas
2014-10-23 6:40 ` Yijing Wang
2014-10-15 3:06 ` [PATCH v3 11/27] PCI/MSI: Refactor struct msi_chip to make it become more common Yijing Wang
2014-10-15 3:07 ` [PATCH v3 12/27] x86/MSI: Use MSI chip framework to configure MSI/MSI-X irq Yijing Wang
2014-10-15 3:07 ` [PATCH v3 13/27] x86/xen/MSI: " Yijing Wang
2014-10-15 3:07 ` [PATCH v3 14/27] Irq_remapping/MSI: " Yijing Wang
2014-10-15 3:07 ` [PATCH v3 15/27] x86/MSI: Remove unused MSI weak arch functions Yijing Wang
2014-10-15 3:07 ` [PATCH v3 16/27] Mips/MSI: Save msi chip in pci sysdata Yijing Wang
2014-10-25 13:04 ` Ralf Baechle
2014-10-27 1:06 ` Yijing Wang
2014-10-15 3:07 ` [PATCH v3 17/27] MIPS/Octeon/MSI: Use MSI chip framework to configure MSI/MSI-X irq Yijing Wang
2014-10-15 3:07 ` [PATCH v3 18/27] MIPS/Xlp: Remove the dead function destroy_irq() to fix build error Yijing Wang
2014-10-25 13:11 ` Ralf Baechle
2014-10-15 3:07 ` [PATCH v3 19/27] MIPS/Xlp/MSI: Use MSI chip framework to configure MSI/MSI-X irq Yijing Wang
2014-10-15 3:07 ` [PATCH v3 20/27] MIPS/Xlr/MSI: " Yijing Wang
2014-10-15 3:07 ` [PATCH v3 21/27] Powerpc/MSI: " Yijing Wang
2014-10-17 7:23 ` Michael Ellerman
2014-10-17 7:42 ` Yijing Wang
2014-10-15 3:07 ` [PATCH v3 22/27] s390/MSI: " Yijing Wang
2014-10-16 12:13 ` Sebastian Ott
2014-10-17 1:04 ` Yijing Wang
2014-10-15 3:07 ` [PATCH v3 23/27] arm/iop13xx/MSI: " Yijing Wang
2014-10-15 3:07 ` [PATCH v3 24/27] IA64/MSI: " Yijing Wang
2014-10-22 23:53 ` Bjorn Helgaas
2014-10-23 1:17 ` Yijing Wang
2014-10-15 3:07 ` [PATCH v3 25/27] Sparc/MSI: " Yijing Wang
2014-10-15 2:36 ` David Miller
2014-10-15 3:07 ` [PATCH v3 26/27] tile/MSI: " Yijing Wang
2014-10-15 3:07 ` [PATCH v3 27/27] PCI/MSI: Clean up unused MSI arch functions Yijing Wang
2014-10-23 5:43 ` [PATCH v3 00/27] Use MSI chip framework to configure MSI/MSI-X in all platforms Bjorn Helgaas
2014-10-23 7:45 ` Yijing Wang
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=544887A8.5090708@huawei.com \
--to=wangyijing@huawei.com \
--cc=Bharat.Bhushan@freescale.com \
--cc=arnab.basu@freescale.com \
--cc=arnd@arndb.de \
--cc=bhelgaas@google.com \
--cc=cmetcalf@tilera.com \
--cc=davem@davemloft.net \
--cc=david.vrabel@citrix.com \
--cc=huxinwei@huawei.com \
--cc=iommu@lists.linux-foundation.org \
--cc=joro@8bytes.org \
--cc=konrad.wilk@oracle.com \
--cc=l.stach@pengutronix.de \
--cc=linux-arch@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-ia64@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mips@linux-mips.org \
--cc=linux-pci@vger.kernel.org \
--cc=linux-s390@vger.kernel.org \
--cc=linux@arm.linux.org.uk \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=liviu@dudau.co.uk \
--cc=ralf@linux-mips.org \
--cc=sebott@linux.vnet.ibm.com \
--cc=sergei.shtylyov@cogentembedded.com \
--cc=sparclinux@vger.kernel.org \
--cc=tglx@linutronix.de \
--cc=thierry.reding@gmail.com \
--cc=thomas.petazzoni@free-electrons.com \
--cc=tony.luck@intel.com \
--cc=wuyun.wu@huawei.com \
--cc=x86@kernel.org \
--cc=xen-devel@lists.xenproject.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).