Re: [PATCH 1/3] x86/xen: Introduce a global flag to fix the MSI mask bug

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2014/11/11 8:04, Bjorn Helgaas wrote:
> 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.)

Yes, it's another history problem, maybe Konrad know the detail.

> 
>> 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
>>
> 
> .
> 


-- 
Thanks!
Yijing

--
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




[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux