2.6.8.1-mm2 (nvidia breakage)

Hi Michael,

I assume this problem is with the Nvidia binary-only driver? My guess is that the driver doesn't call pci_enable_device() before using pci_dev->irq. I don't have source for that driver, so I can't verify this.

If the driver is from Nvidia, please contact their technical support and request that they fix the driver. I can help explain the issue if necessary.

I also noticed this in your dmesg:

	floppy: controller ACPI FDC0 at I/O 0x3f7-0x3f7 irq 6 dma channel 2
	Using anticipatory io scheduler
	Floppy drive(s): fd0 is 1.44M
	floppy0: no floppy controllers found

I hope that you have the floppy controller disabled in your BIOS, or at least have no floppy drives connected. Can you confirm that? I expected that ACPI would not report the controller at all in such a case, but it looks like yours reports the controller with a zero- length I/O port region. I probably should check for that.

If you do have a floppy drive, it probably doesn't work anymore unless you specify "floppy=no_acpi". If this is the case, we'll have to figure out a quirk or something to make it work.

Thanks for the testing report!

An obvious test would be for someone with an Nvidia to write a little module that does nothing but pci_enable_device it. Load and unload that then see what happens

the original bug report was during module load time, when we're probing our devices via the pci_driver's probe callback. this is well before we hook up interrupts or do anything in our closed source code.

I'm attaching a trimmed down version of our driver that pretty much only does this probe (complete source is included). I don't know if this will reproduce the original bug or not.

Let me know if I can help anymore.

Thanks,
Terence

Thanks, this is enough to show the problem:

	nv_kern_probe(struct pci_dev *dev, ...)
	{
		...
		nv->interrupt_line = dev->irq;
		...
		if (pci_enable_device(dev) != 0)

The driver is looking at dev->irq before calling pci_enable_device(). But dev->irq is not necessarily initialized before pci_enable_device().

I'm not a PCI expert, but I'm not sure you should be looking at all the other dev->resource[] stuff before pci_enable_device() either. Most of the "modern" drivers in the tree seem to do pci_enable_device() very early in the probe() function, i.e., see tg3.c.

Thanks Bjorn,

I'll move that call to pci_enable_device to earlier in the probe call.

but in Kevin's original email, he's hitting an oops within the pci_enable_device call. is that likely due to pci_enable_device being called late?

Thanks,
Terence

Thanks.

I don't seem to have an email from Kevin. Can you forward it to me? The thread I know about started with mail from Michael Geithe that Andrew Morton forwarded to me:

I didn't see any oops there.

Hi Bjorn,

i searched my logs and found that.

Aug 20 18:03:53 stargate kernel: irq 16: nobody cared!
Aug 20 18:03:53 stargate kernel:  [<c0106520>] __report_bad_irq+0x24/0x7b
Aug 20 18:03:53 stargate kernel:  [<c01065f9>] note_interrupt+0x64/0x88
Aug 20 18:03:53 stargate kernel:  [<c010694d>] do_IRQ+0x1af/0x1fb
Aug 20 18:03:53 stargate kernel:  [<c0104890>] common_interrupt+0x18/0x20
Aug 20 18:03:53 stargate kernel: handlers:
Aug 20 18:03:53 stargate kernel: [<f9a11145>] (usb_hcd_irq+0x0/0x5d [usbcore])
Aug 20 18:03:53 stargate kernel: [<f9a11145>] (usb_hcd_irq+0x0/0x5d [usbcore])
Aug 20 18:03:53 stargate kernel: Disabling IRQ #16

Michael Geithe

-

sure, I meant to include this link in the earlier email and forgot:

http://www.uwsg.indiana.edu/hypermail/linux/kernel/0408.1/0127.html

[1] http://www.uwsg.indiana.edu/hypermail/linux/kernel/0408.1/0127.html

Kevin, I suspect the oops you're seeing is due to the problem mentioned here (add_pin_to_irq() is __init but shouldn't be):

    http://groups.google.com/groups?dq=&start=75&hl=en&lr=&ie=UTF-8&group=fa.linux.kernel&c2coff=1&selm=fa.j8igrci.1fgg680%40ifi.uio.no

Can you try Randy's patch without using "pci=routeirq" and see whether it resolves the oops? Of course, the nvidia driver still won't work because it's looking at pci_dev->irq before calling pci_enable_device(), but that's a separate issue.

as Alan pointed out, the video device is bios configured, so may not be hit by this. nonetheless, we've applied a patch along these lines to our internal codebase.

Thanks,
Terence

@@ -1225,6 +1225,7 @@
         {
             nv_state_t *nv = NV_STATE_PTR(&nv_linux_devices[i]);
             release_mem_region(nv->bar.regs.address, nv->bar.regs.size);
+            pci_disable_device(nv_linux_devices[i].dev);
         }
     }
 

@@ -3516,6 +3517,28 @@
         return -1;
     }
 
+    // enable io, mem, and bus-mastering in pci config space
+    if (pci_enable_device(dev) != 0)
+    {
+        nv_printf(NV_DBG_ERRORS,
+            "NVRM: pci_enable_device failed, aborting\n");
+        return -1;
+    }
+
+    // request ownership of our bars
+    // keeps other drivers from banging our registers.
+    // only do this for registers, as vesafb requests our framebuffer and will
+    // keep us from working properly
+    if (!request_mem_region(dev->resource[0].start,
+                            dev->resource[0].end - dev->resource[0].start + 1,
+                            "nvidia"))
+    {
+        nv_printf(NV_DBG_ERRORS,
+            "NVRM: pci_request_regions failed, aborting\n");
+        goto err_disable_dev;
+    }
+    pci_set_master(dev);
+
     /* initialize bus-dependent config state */
     nvl = &nv_linux_devices[num_nv_devices];
     nv  = NV_STATE_PTR(nvl);

@@ -3545,7 +3568,7 @@
         nv_printf(NV_DBG_ERRORS, "NVRM: Please check your BIOS settings.         \n");
         nv_printf(NV_DBG_ERRORS, "NVRM: [Plug & Play OS   ] should be set to NO  \n");
         nv_printf(NV_DBG_ERRORS, "NVRM: [Assign IRQ to VGA] should be set to YES \n");
-        return -1;
+        goto err_zero_dev;
     }
 
     /* sanity check the IO apertures */

@@ -3569,39 +3592,9 @@
                 nv->bar.fb.address, nv->bar.fb.size);
         }
 
-        /* Clear out the data */
-        os_mem_set(nvl, 0, sizeof(nv_linux_state_t));
-
-        return -1;
-    }
-
-    // request ownership of our bars
-    // keeps other drivers from banging our registers.
-    // only do this for registers, as vesafb requests our framebuffer and will
-    // keep us from working properly
-    if (!request_mem_region(nv->bar.regs.address, nv->bar.regs.size, "nvidia"))
-    {
-        nv_printf(NV_DBG_ERRORS,
-            "NVRM: pci_request_regions failed, aborting\n");
-
-        /* Clear out the data */
-        os_mem_set(nvl, 0, sizeof(nv_linux_state_t));
-
-        return -1;
+        goto err_zero_dev;
     }
 
-    // enable io, mem, and bus-mastering in pci config space
-    if (pci_enable_device(dev) != 0)
-    {
-        nv_printf(NV_DBG_ERRORS,
-            "NVRM: pci_enable_device failed, aborting\n");
-
-        pci_release_regions(dev);
-        os_mem_set(nvl, 0, sizeof(nv_linux_state_t));
-
-        return -1;
-    }
-    pci_set_master(nvl->dev);
 
 #if defined(NV_BUILD_NV_PAT_SUPPORT)
     if (nvos_find_pci_express_capability(nvl->dev))

@@ -3618,13 +3611,7 @@
     if (nv->bar.regs.map == NULL)
     {
         nv_printf(NV_DBG_ERRORS, "NVRM: failed to map registers!!\n");
-
-        pci_release_regions(dev);
-
-        /* Clear out the data */
-        os_mem_set(nvl, 0, sizeof(nv_linux_state_t));
-
-        return -1;
+        goto err_zero_dev;
     }
     nv->flags |= NV_FLAG_MAP_REGS_EARLY;
 #endif

@@ -3641,6 +3628,15 @@
     num_nv_devices++;
 
     return 0;
+
+err_zero_dev:
+    os_mem_set(nvl, 0, sizeof(nv_linux_state_t));
+    release_mem_region(dev->resource[0].start,
+                       dev->resource[0].end - dev->resource[0].start + 1);
+
+err_disable_dev:
+    pci_disable_device(dev);
+    return -1;
 }
 
 int NV_API_CALL nv_no_incoherent_mappings

To be pedantically clear about this, looking at pci_dev->irq before calling pci_enable_device() is *guaranteed* to fail, regardless of what the BIOS does. So nvidia users will have to use "pci=routeirq" until there's a new version of the nvidia driver.

Alan was specifically referring to the BAR configuration that may be done by the BIOS.

I'm assuming your patch makes the driver call pci_enable_device() before using either irq or BAR information. That's the best way to future-proof the driver.

ah, ok. thanks for clarifying.

yes, that's exactly what we've done.

Thanks,
Terence

Bjorn> To be pedantically clear about this, looking at Bjorn> pci_dev->irq before calling pci_enable_device() is Bjorn> *guaranteed* to fail, regardless of what the BIOS does. So Bjorn> nvidia users will have to use "pci=routeirq" until there's Bjorn> a new version of the nvidia driver.

Terence, correct me if I'm wrong, but the change to add pci_enable_device() goes in the part of the nvidia driver that has source available. So users can apply this patch themselves even without another Nvidia release.

- Roland

correct.

Thanks,
Terence

Should just pass it along to minion.de ...

Martin Schlemmer

Indeed. pci_enable_device may assign the actual BARs as well as the interrupt lines to the device. I suspect its harmless here because the base video device is BIOS configured.