linux-kernel - Re: [Bug] PCI: Enable INTx if BIOS left them disabled

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: <CAE9FiQVNFqTw+92qy8zcwG1haiSZzU+KE3oh4YnjNc_aE6KUnA@mail.gmail.com>
Date:	Fri, 7 Mar 2014 11:39:48 -0800
From:	Yinghai Lu <yinghai@...nel.org>
To:	Bjorn Helgaas <bhelgaas@...gle.com>
Cc:	Andreas Noever <andreas.noever@...il.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
	Rajat Jain <rajatxjain@...il.com>
Subject: Re: [Bug] PCI: Enable INTx if BIOS left them disabled - triggers
 during rescan

On Fri, Mar 7, 2014 at 8:45 AM, Bjorn Helgaas <bhelgaas@...gle.com> wrote:
>
> I opened a bugzilla report at https://bugzilla.kernel.org/show_bug.cgi?id=71691
>
> It seems like clearing DisINTx has some effect on MSI.  I don't see
> anything in the spec that would suggest this (I'm looking at the PCIe
> r3.0 spec, sec 7.5.1.1).
>
> Can somebody point out a connection between DisINTx and MSI?  If not,
> maybe we'll need some sort of quirk to deal with this.

I had different impression: if you disable INTx in some chipset, MSI will not
work anymore.

so we have

static void pci_intx_for_msi(struct pci_dev *dev, int enable)
{
        if (!(dev->dev_flags & PCI_DEV_FLAGS_MSI_INTX_DISABLE_BUG))
                pci_intx(dev, enable);
}

and have quirks for ati and broadcom chip to set that FLAG.

regarding the regression: i would suggest move out
do_pci_enable_intx() from re-enable path.

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 5a24cb3..92718c9 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1190,11 +1190,22 @@ int __weak pcibios_enable_device(struct
pci_dev *dev, int bars)
     return pci_enable_resources(dev, bars);
 }

+static void do_pci_enable_intx(struct pci_dev *dev)
+{
+    u8 pin;
+    pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin);
+    if (pin) {
+        pci_read_config_word(dev, PCI_COMMAND, &cmd);
+        if (cmd & PCI_COMMAND_INTX_DISABLE)
+            pci_write_config_word(dev, PCI_COMMAND,
+                          cmd & ~PCI_COMMAND_INTX_DISABLE);
+    }
+}
+
 static int do_pci_enable_device(struct pci_dev *dev, int bars)
 {
     int err;
     u16 cmd;
-    u8 pin;

     err = pci_set_power_state(dev, PCI_D0);
     if (err < 0 && err != -EIO)
@@ -1204,14 +1215,6 @@ static int do_pci_enable_device(struct pci_dev
*dev, int bars)
         return err;
     pci_fixup_device(pci_fixup_enable, dev);

-    pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin);
-    if (pin) {
-        pci_read_config_word(dev, PCI_COMMAND, &cmd);
-        if (cmd & PCI_COMMAND_INTX_DISABLE)
-            pci_write_config_word(dev, PCI_COMMAND,
-                          cmd & ~PCI_COMMAND_INTX_DISABLE);
-    }
-
     return 0;
 }

@@ -1287,6 +1290,8 @@ static int pci_enable_device_flags(struct
pci_dev *dev, unsigned long flags)
     err = do_pci_enable_device(dev, bars);
     if (err < 0)
         atomic_dec(&dev->enable_cnt);
+    else
+        do_pci_enable_intx(dev);
     return err;
 }

View attachment "do_not_enable_intx_again.patch" of type "text/x-patch" (1413 bytes)