linux-kernel - Re: [PATCH] ALSA: hda-intel - Disable MSI support by default

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <s5hodr7u0ve.wl%tiwai@suse.de>
Date:	Thu, 16 Nov 2006 11:44:37 +0100
From:	Takashi Iwai <tiwai@...e.de>
To:	Jeff Garzik <jeff@...zik.org>
Cc:	"Lu, Yinghai" <yinghai.lu@....com>,
	"Olivier Nicolas" <olivn@...llprod.org>,
	David Miller <davem@...emloft.net>, torvalds@...l.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ALSA: hda-intel - Disable MSI support by default

At Wed, 15 Nov 2006 14:15:45 -0500,
Jeff Garzik wrote:
> 
> ACK the pci_intx() calls, NAK the obviously overweight spinlock changes. 
>   The spinlock changes are completely unnecessary.  Just look at any 
> other (non-ALSA) PCI driver.  Existing "spin_lock()" is fine for both 
> PCI shared irq handlers and MSI irq handlers.
> 
> It sounds like you are trying to work around a reentrancy problem that 
> does not exist.
> 
> Only weird drivers like ps2kbd/mouse or IDE need spin_lock_irqsave(), 
> where separate interrupt sources call the same function.

OK, I revised it, also referring to a similar patch by Yinghai.
I think we can simplify the change like below.
Olivier, could you test this patch, too?

(Here the first chunk (enable_msi=1) is just for testing.
 The patch isn't for merge yet.)


Takashi

---
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index e35cfd3..130ee12 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -55,7 +55,7 @@ static char *model;
 static int position_fix;
 static int probe_mask = -1;
 static int single_cmd;
-static int enable_msi;
+static int enable_msi = 1;
 
 module_param(index, int, 0444);
 MODULE_PARM_DESC(index, "Index value for Intel HD audio interface.");
@@ -1380,7 +1380,8 @@ static int __devinit azx_init_stream(str
 
 static int azx_acquire_irq(struct azx *chip, int do_disconnect)
 {
-	if (request_irq(chip->pci->irq, azx_interrupt, IRQF_DISABLED|IRQF_SHARED,
+	if (request_irq(chip->pci->irq, azx_interrupt,
+			chip->msi ? 0 : IRQF_SHARED,
 			"HDA Intel", chip)) {
 		printk(KERN_ERR "hda-intel: unable to grab IRQ %d, "
 		       "disabling device\n", chip->pci->irq);
@@ -1389,6 +1390,7 @@ static int azx_acquire_irq(struct azx *c
 		return -1;
 	}
 	chip->irq = chip->pci->irq;
+	pci_intx(chi->pci, !chip->msi);
 	return 0;
 }
 
-
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/