Commit 1f80025e authored by Eric W. Biederman's avatar Eric W. Biederman Committed by Linus Torvalds
Browse files

[PATCH] msi: simplify msi sanity checks by adding with generic irq code 



Currently msi.c is doing sanity checks that make certain before an irq is
destroyed it has no more users.

By adding irq_has_action I can perform the test is a generic way, instead of
relying on a msi specific data structure.

By performing the core check in dynamic_irq_cleanup I ensure every user of
dynamic irqs has a test present and we don't free resources that are in use.

In msi.c this allows me to kill the attrib.state member of msi_desc and all of
the assciated code to maintain it.

To keep from freeing data structures when irq cleanup code is called to soon
changing dyanamic_irq_cleanup is insufficient because there are msi specific
data structures that are also not safe to free.

Signed-off-by: default avatarEric W. Biederman <ebiederm@xmission.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Andi Kleen <ak@suse.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Greg KH <greg@kroah.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: default avatarAndrew Morton <akpm@osdl.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@osdl.org>
parent 8b955b0d

drivers/pci/msi.c

+8 −35
Original line number Original line Diff line number Diff line
@@ -188,18 +188,6 @@ static void unmask_MSI_irq(unsigned int irq) 




static unsigned int startup_msi_irq_wo_maskbit(unsigned int irq)

static unsigned int startup_msi_irq_wo_maskbit(unsigned int irq)

{

{

	struct msi_desc *entry;

	unsigned long flags;



	spin_lock_irqsave(&msi_lock, flags);

	entry = msi_desc[irq];

	if (!entry || !entry->dev) {

		spin_unlock_irqrestore(&msi_lock, flags);

		return 0;

	}

	entry->msi_attrib.state = 1;	/* Mark it active */

	spin_unlock_irqrestore(&msi_lock, flags);



	return 0;	/* never anything pending */

	return 0;	/* never anything pending */

}

}
@@ -212,14 +200,6 @@ static unsigned int startup_msi_irq_w_maskbit(unsigned int irq) 




static void shutdown_msi_irq(unsigned int irq)

static void shutdown_msi_irq(unsigned int irq)

{

{

	struct msi_desc *entry;

	unsigned long flags;



	spin_lock_irqsave(&msi_lock, flags);

	entry = msi_desc[irq];

	if (entry && entry->dev)

		entry->msi_attrib.state = 0;	/* Mark it not active */

	spin_unlock_irqrestore(&msi_lock, flags);

}

}





static void end_msi_irq_wo_maskbit(unsigned int irq)

static void end_msi_irq_wo_maskbit(unsigned int irq)
@@ -671,7 +651,6 @@ static int msi_capability_init(struct pci_dev *dev) 
	entry->link.head = irq;

	entry->link.head = irq;

	entry->link.tail = irq;

	entry->link.tail = irq;

	entry->msi_attrib.type = PCI_CAP_ID_MSI;

	entry->msi_attrib.type = PCI_CAP_ID_MSI;

	entry->msi_attrib.state = 0;			/* Mark it not active */

	entry->msi_attrib.is_64 = is_64bit_address(control);

	entry->msi_attrib.is_64 = is_64bit_address(control);

	entry->msi_attrib.entry_nr = 0;

	entry->msi_attrib.entry_nr = 0;

	entry->msi_attrib.maskbit = is_mask_bit_support(control);

	entry->msi_attrib.maskbit = is_mask_bit_support(control);
@@ -744,7 +723,6 @@ static int msix_capability_init(struct pci_dev *dev, 
 		j = entries[i].entry;

 		j = entries[i].entry;

 		entries[i].vector = irq;

 		entries[i].vector = irq;

		entry->msi_attrib.type = PCI_CAP_ID_MSIX;

		entry->msi_attrib.type = PCI_CAP_ID_MSIX;

 		entry->msi_attrib.state = 0;		/* Mark it not active */

		entry->msi_attrib.is_64 = 1;

		entry->msi_attrib.is_64 = 1;

		entry->msi_attrib.entry_nr = j;

		entry->msi_attrib.entry_nr = j;

		entry->msi_attrib.maskbit = 1;

		entry->msi_attrib.maskbit = 1;
@@ -897,12 +875,12 @@ void pci_disable_msi(struct pci_dev* dev) 
		spin_unlock_irqrestore(&msi_lock, flags);

		spin_unlock_irqrestore(&msi_lock, flags);

		return;

		return;

	}

	}

	if (entry->msi_attrib.state) {

	if (irq_has_action(dev->irq)) {

		spin_unlock_irqrestore(&msi_lock, flags);

		spin_unlock_irqrestore(&msi_lock, flags);

		printk(KERN_WARNING "PCI: %s: pci_disable_msi() called without "

		printk(KERN_WARNING "PCI: %s: pci_disable_msi() called without "

		       "free_irq() on MSI irq %d\n",

		       "free_irq() on MSI irq %d\n",

		       pci_name(dev), dev->irq);

		       pci_name(dev), dev->irq);

		BUG_ON(entry->msi_attrib.state > 0);

		BUG_ON(irq_has_action(dev->irq));

	} else {

	} else {

		default_irq = entry->msi_attrib.default_irq;

		default_irq = entry->msi_attrib.default_irq;

		spin_unlock_irqrestore(&msi_lock, flags);

		spin_unlock_irqrestore(&msi_lock, flags);
@@ -1035,17 +1013,16 @@ void pci_disable_msix(struct pci_dev* dev) 




	temp = dev->irq;

	temp = dev->irq;

	if (!msi_lookup_irq(dev, PCI_CAP_ID_MSIX)) {

	if (!msi_lookup_irq(dev, PCI_CAP_ID_MSIX)) {

		int state, irq, head, tail = 0, warning = 0;

		int irq, head, tail = 0, warning = 0;

		unsigned long flags;

		unsigned long flags;





		irq = head = dev->irq;

		irq = head = dev->irq;

		dev->irq = temp;			/* Restore pin IRQ */

		dev->irq = temp;			/* Restore pin IRQ */

		while (head != tail) {

		while (head != tail) {

			spin_lock_irqsave(&msi_lock, flags);

			spin_lock_irqsave(&msi_lock, flags);

			state = msi_desc[irq]->msi_attrib.state;

			tail = msi_desc[irq]->link.tail;

			tail = msi_desc[irq]->link.tail;

			spin_unlock_irqrestore(&msi_lock, flags);

			spin_unlock_irqrestore(&msi_lock, flags);

			if (state)

			if (irq_has_action(irq))

				warning = 1;

				warning = 1;

			else if (irq != head)	/* Release MSI-X irq */

			else if (irq != head)	/* Release MSI-X irq */

				msi_free_irq(dev, irq);

				msi_free_irq(dev, irq);
@@ -1072,7 +1049,7 @@ void pci_disable_msix(struct pci_dev* dev) 
 **/

 **/

void msi_remove_pci_irq_vectors(struct pci_dev* dev)

void msi_remove_pci_irq_vectors(struct pci_dev* dev)

{

{

	int state, pos, temp;

	int pos, temp;

	unsigned long flags;

	unsigned long flags;





	if (!pci_msi_enable || !dev)

	if (!pci_msi_enable || !dev)
@@ -1081,14 +1058,11 @@ void msi_remove_pci_irq_vectors(struct pci_dev* dev) 
	temp = dev->irq;		/* Save IOAPIC IRQ */

	temp = dev->irq;		/* Save IOAPIC IRQ */

	pos = pci_find_capability(dev, PCI_CAP_ID_MSI);

	pos = pci_find_capability(dev, PCI_CAP_ID_MSI);

	if (pos > 0 && !msi_lookup_irq(dev, PCI_CAP_ID_MSI)) {

	if (pos > 0 && !msi_lookup_irq(dev, PCI_CAP_ID_MSI)) {

		spin_lock_irqsave(&msi_lock, flags);

		if (irq_has_action(dev->irq)) {

		state = msi_desc[dev->irq]->msi_attrib.state;

		spin_unlock_irqrestore(&msi_lock, flags);

		if (state) {

			printk(KERN_WARNING "PCI: %s: msi_remove_pci_irq_vectors() "

			printk(KERN_WARNING "PCI: %s: msi_remove_pci_irq_vectors() "

			       "called without free_irq() on MSI irq %d\n",

			       "called without free_irq() on MSI irq %d\n",

			       pci_name(dev), dev->irq);

			       pci_name(dev), dev->irq);

			BUG_ON(state > 0);

			BUG_ON(irq_has_action(dev->irq));

		} else /* Release MSI irq assigned to this device */

		} else /* Release MSI irq assigned to this device */

			msi_free_irq(dev, dev->irq);

			msi_free_irq(dev, dev->irq);

		dev->irq = temp;		/* Restore IOAPIC IRQ */

		dev->irq = temp;		/* Restore IOAPIC IRQ */
@@ -1101,11 +1075,10 @@ void msi_remove_pci_irq_vectors(struct pci_dev* dev) 
		irq = head = dev->irq;

		irq = head = dev->irq;

		while (head != tail) {

		while (head != tail) {

			spin_lock_irqsave(&msi_lock, flags);

			spin_lock_irqsave(&msi_lock, flags);

			state = msi_desc[irq]->msi_attrib.state;

			tail = msi_desc[irq]->link.tail;

			tail = msi_desc[irq]->link.tail;

			base = msi_desc[irq]->mask_base;

			base = msi_desc[irq]->mask_base;

			spin_unlock_irqrestore(&msi_lock, flags);

			spin_unlock_irqrestore(&msi_lock, flags);

			if (state)

			if (irq_has_action(irq))

				warning = 1;

				warning = 1;

			else if (irq != head) /* Release MSI-X irq */

			else if (irq != head) /* Release MSI-X irq */

				msi_free_irq(dev, irq);

				msi_free_irq(dev, irq);

drivers/pci/msi.h

+1 −1
Original line number Original line Diff line number Diff line
@@ -53,7 +53,7 @@ struct msi_desc { 
	struct {

	struct {

		__u8	type	: 5; 	/* {0: unused, 5h:MSI, 11h:MSI-X} */

		__u8	type	: 5; 	/* {0: unused, 5h:MSI, 11h:MSI-X} */

		__u8	maskbit	: 1; 	/* mask-pending bit supported ?   */

		__u8	maskbit	: 1; 	/* mask-pending bit supported ?   */

		__u8	state	: 1; 	/* {0: free, 1: busy}		  */

		__u8	unused	: 1;

		__u8	is_64	: 1;	/* Address size: 0=32bit 1=64bit  */

		__u8	is_64	: 1;	/* Address size: 0=32bit 1=64bit  */

		__u8	pos;	 	/* Location of the msi capability */

		__u8	pos;	 	/* Location of the msi capability */

		__u16	entry_nr;    	/* specific enabled entry 	  */

		__u16	entry_nr;    	/* specific enabled entry 	  */

include/linux/irq.h

+7 −0
Original line number Original line Diff line number Diff line
@@ -372,6 +372,13 @@ set_irq_chained_handler(unsigned int irq, 
extern int create_irq(void);

extern int create_irq(void);

extern void destroy_irq(unsigned int irq);

extern void destroy_irq(unsigned int irq);





/* Test to see if a driver has successfully requested an irq */

static inline int irq_has_action(unsigned int irq)

{

	struct irq_desc *desc = irq_desc + irq;

	return desc->action != NULL;

}



/* Dynamic irq helper functions */

/* Dynamic irq helper functions */

extern void dynamic_irq_init(unsigned int irq);

extern void dynamic_irq_init(unsigned int irq);

extern void dynamic_irq_cleanup(unsigned int irq);

extern void dynamic_irq_cleanup(unsigned int irq);

kernel/irq/chip.c

+7 −0
Original line number Original line Diff line number Diff line
@@ -67,6 +67,13 @@ void dynamic_irq_cleanup(unsigned int irq) 




	desc = irq_desc + irq;

	desc = irq_desc + irq;

	spin_lock_irqsave(&desc->lock, flags);

	spin_lock_irqsave(&desc->lock, flags);

	if (desc->action) {

		spin_unlock_irqrestore(&desc->lock, flags);

		printk(KERN_ERR "Destroying IRQ%d without calling free_irq\n",

			irq);

		WARN_ON(1);

		return;

	}

	desc->handle_irq = handle_bad_irq;

	desc->handle_irq = handle_bad_irq;

	desc->chip = &no_irq_chip;

	desc->chip = &no_irq_chip;

	spin_unlock_irqrestore(&desc->lock, flags);

	spin_unlock_irqrestore(&desc->lock, flags);

CRA Git | Maintained and supported by SUSTech CRA and CCSE