On Mon, Oct 27, 2014 at 10:44:36AM +0800, Yijing Wang wrote:
> Commit 0e4ccb1505a9 ("PCI: Add x86_msi.msi_mask_irq() and msix_mask_irq()")
> fixed MSI mask bug which may cause kernel crash. But the commit
> made MSI code complex. Introduce a new global flag "pci_msi_ignore_mask"
> to ignore MSI/MSI-X to fix this issue, it's a cleaner solution.
> And the commit 0e4ccb1505a9 will be reverted in the later patch.
The 0e4ccb1505a9 changelog says Xen guests can't write to MSI-X BARs.
But it makes mask_irq a no-op for both MSI-X and MSI. The MSI mask bit is
in config space, not in memory space. So why does mask_irq need to be a
no-op for MSI as well? Are Xen guests prohibited from writing to config
space, too? (It's fine if they are; it's just that the changelog
specifically mentioned MSI-X memory space tables, and it didn't mention
config space at all.)
And I *assume* there's some Xen mechanism that accomplishes the mask_irq in
a different way, since the actual mask_irq interface does nothing? (This
is really a question for 0e4ccb1505a9, since I don't think this particular
patch changes anything in that respect.)
> Signed-off-by: Yijing Wang <wangyijing@xxxxxxxxxx>
> CC: David Vrabel <david.vrabel@xxxxxxxxxx>
> CC: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> CC: xen-devel@xxxxxxxxxxxxxxxxxxxx
> ---
> arch/x86/pci/xen.c | 2 ++
> drivers/pci/msi.c | 7 ++++++-
> include/linux/msi.h | 1 +
> 3 files changed, 9 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
> index 093f5f4..5ef62ed 100644
> --- a/arch/x86/pci/xen.c
> +++ b/arch/x86/pci/xen.c
> @@ -427,6 +427,7 @@ int __init pci_xen_init(void)
> 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;
> }
> @@ -508,6 +509,7 @@ int __init pci_xen_initial_domain(void)
> 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 38511d9..ecb5f54 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)
>
> @@ -166,7 +167,7 @@ u32 default_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;
> @@ -198,6 +199,10 @@ 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 +
> 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;
> diff --git a/include/linux/msi.h b/include/linux/msi.h
> index 44f4746..86dc501 100644
> --- a/include/linux/msi.h
> +++ b/include/linux/msi.h
> @@ -10,6 +10,7 @@ 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;
> --
> 1.7.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
