| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <527C41E9.6040002@oracle.com>
Date: Fri, 08 Nov 2013 09:44:09 +0800
From: Zhenzhong Duan <zhenzhong.duan@...cle.com>
To: Bjorn Helgaas <bhelgaas@...gle.com>,
Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
CC: xen-devel@...ts.xenproject.org,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
Sucheta Chakraborty <sucheta.chakraborty@...gic.com>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>,
"H. Peter Anvin" <hpa@...or.com>, "x86@...nel.org" <x86@...nel.org>
Subject: Re: [PATCH] PCI: Introduce two new MSI infrastructure calls for masking/unmasking.
On 2013-11-07 07:51, Bjorn Helgaas wrote:
> [+cc Thomas, Ingo, Peter, x86 list]
>
> On Wed, Nov 6, 2013 at 2:16 PM, Konrad Rzeszutek Wilk
> <konrad.wilk@...cle.com> wrote:
>> Certain platforms do not allow writes in the MSI-X bars
>> to setup or tear down vector values. To combat against
>> the generic code trying to write to that and either silently
>> being ignored or crashing due to the pagetables being marked r/o
>> this patch introduces a platform over-write.
>>
>> Note that we keep two separate, non-weak, functions
>> default_mask_msi_irqs() and default_mask_msix_irqs() for the
>> behavior of the arch_mask_msi_irqs() and arch_mask_msix_irqs(),
>> as the default behavior is needed by x86 PCI code.
>>
>> For Xen, which does not allow the guest to write to MSI-X
>> tables - as the hypervisor is solely responsible for setting
>> the vector values - we implement two nops.
>>
>> CC: Bjorn Helgaas <bhelgaas@...gle.com>
>> CC: Sucheta Chakraborty <sucheta.chakraborty@...gic.com>
>> CC: Zhenzhong Duan <zhenzhong.duan@...cle.com>
>> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
> I think this is safe, and I'd like to squeeze it into the v3.13 merge
> window next week, since it supersedes three patches Zhenzhong has been
> trying to get in since July [1], and this patch is much simpler to
> understand.
This patch could replace the first two.
I think the third patch of mine is still needed as it does a different
thing.
It optimizes restore path in dom0.
zduan
>
> I *think* this also fixes an actual bug on Xen. Konrad, is there a
> bugzilla or any kind of email problem description that we can include
> here as a reference? I think there's a lost interrupt with qlcnic,
> but I don't know the details or what the failure looks like to a user.
>
> [1] http://lkml.kernel.org/r/51EF44FA.4020903@oracle.com
>
>> ---
>> arch/x86/include/asm/x86_init.h | 3 +++
>> arch/x86/kernel/x86_init.c | 10 ++++++++++
>> arch/x86/pci/xen.c | 13 ++++++++++++-
>> drivers/pci/msi.c | 22 ++++++++++++++++------
>> include/linux/msi.h | 2 ++
>> 5 files changed, 43 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h
>> index 828a156..0f1be11 100644
>> --- a/arch/x86/include/asm/x86_init.h
>> +++ b/arch/x86/include/asm/x86_init.h
>> @@ -172,6 +172,7 @@ 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);
>> @@ -182,6 +183,8 @@ struct x86_msi_ops {
>> void (*teardown_msi_irqs)(struct pci_dev *dev);
>> void (*restore_msi_irqs)(struct pci_dev *dev, int irq);
>> 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 8ce0072..021783b 100644
>> --- a/arch/x86/kernel/x86_init.c
>> +++ b/arch/x86/kernel/x86_init.c
>> @@ -116,6 +116,8 @@ 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 */
>> @@ -138,6 +140,14 @@ void arch_restore_msi_irqs(struct pci_dev *dev, int irq)
>> {
>> x86_msi.restore_msi_irqs(dev, irq);
>> }
>> +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 48e8461..5eee495 100644
>> --- a/arch/x86/pci/xen.c
>> +++ b/arch/x86/pci/xen.c
>> @@ -382,7 +382,14 @@ 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)
>> @@ -406,6 +413,8 @@ 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;
>> #endif
>> return 0;
>> }
>> @@ -485,6 +494,8 @@ 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;
>> #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 d5f90d6..7916699 100644
>> --- a/drivers/pci/msi.c
>> +++ b/drivers/pci/msi.c
>> @@ -185,7 +185,7 @@ static inline __attribute_const__ u32 msi_enabled_mask(u16 control)
>> * reliably as devices without an INTx disable bit will then generate a
>> * level IRQ which will never be cleared.
>> */
>> -static u32 __msi_mask_irq(struct msi_desc *desc, u32 mask, u32 flag)
>> +u32 default_msi_mask_irq(struct msi_desc *desc, u32 mask, u32 flag)
>> {
>> u32 mask_bits = desc->masked;
>>
>> @@ -199,9 +199,14 @@ static u32 __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 = __msi_mask_irq(desc, mask, flag);
>> + desc->masked = arch_msi_mask_irq(desc, mask, flag);
>> }
>>
>> /*
>> @@ -211,7 +216,7 @@ 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.
>> */
>> -static u32 __msix_mask_irq(struct msi_desc *desc, u32 flag)
>> +u32 default_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 +
>> @@ -224,9 +229,14 @@ static u32 __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 = __msix_mask_irq(desc, flag);
>> + desc->masked = arch_msix_mask_irq(desc, flag);
>> }
>>
>> static void msi_set_mask_bit(struct irq_data *data, u32 flag)
>> @@ -902,7 +912,7 @@ void pci_msi_shutdown(struct pci_dev *dev)
>> pci_read_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, &ctrl);
>> mask = msi_capable_mask(ctrl);
>> /* Keep cached state to be restored */
>> - __msi_mask_irq(desc, mask, ~mask);
>> + arch_msi_mask_irq(desc, mask, ~mask);
>>
>> /* Restore dev->irq to its default pin-assertion irq */
>> dev->irq = desc->msi_attrib.default_irq;
>> @@ -998,7 +1008,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 */
>> - __msix_mask_irq(entry, 1);
>> + arch_msix_mask_irq(entry, 1);
>> }
>>
>> msix_set_enable(dev, 0);
>> diff --git a/include/linux/msi.h b/include/linux/msi.h
>> index b17ead8..87cce50 100644
>> --- a/include/linux/msi.h
>> +++ b/include/linux/msi.h
>> @@ -64,6 +64,8 @@ void arch_restore_msi_irqs(struct pci_dev *dev, int irq);
>>
>> void default_teardown_msi_irqs(struct pci_dev *dev);
>> void default_restore_msi_irqs(struct pci_dev *dev, int irq);
>> +u32 default_msi_mask_irq(struct msi_desc *desc, u32 mask, u32 flag);
>> +u32 default_msix_mask_irq(struct msi_desc *desc, u32 flag);
>>
>> struct msi_chip {
>> struct module *owner;
>> --
>> 1.8.3.1
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists