* Re: driver skip pci_set_master, fix it? No. [not found] <5344251D.7040805@pobox.com> @ 2014-04-08 18:27 ` Bjorn Helgaas 2014-04-08 21:18 ` Mark Lord 0 siblings, 1 reply; 9+ messages in thread From: Bjorn Helgaas @ 2014-04-08 18:27 UTC (permalink / raw) To: Mark Lord Cc: linux-kernel@vger.kernel.org, Yinghai Lu, Theodore Ts'o, linux-pci@vger.kernel.org, Benjamin Herrenschmidt [+cc Ben, linux-pci] On Tue, Apr 8, 2014 at 10:34 AM, Mark Lord <mlord@pobox.com> wrote: > I am working a couple of drivers for chips that perform extensive bus-mastering ops. > These including full SRIOV support, and allow assigning virtual functions to virtual machines, etc. > > One thing the driver (still in development) does for safety, > is defer the call to pci_set_master() until *after* it has mapped > the MMIO space of the chips, so it can reset/flush the DMA engines > before giving them permission to scribble over host RAM. > > But a recent patch to the kernel has removed this from the driver's control. > The core PCI now does pci_set_master() immediately on pci_enable_device(). I assume you're talking about the one added by cf3e1feba7f9 ("PCI: Workaround missing pci_set_master in pci drivers"), but as far as I can tell, it only calls pci_set_master() for *bridge* devices. What am I missing? Is pci_set_master() being called for your endpoint? What path is that? Bjorn ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: driver skip pci_set_master, fix it? No. 2014-04-08 18:27 ` driver skip pci_set_master, fix it? No Bjorn Helgaas @ 2014-04-08 21:18 ` Mark Lord 2014-04-09 2:51 ` Benjamin Herrenschmidt 0 siblings, 1 reply; 9+ messages in thread From: Mark Lord @ 2014-04-08 21:18 UTC (permalink / raw) To: Bjorn Helgaas Cc: linux-kernel@vger.kernel.org, Yinghai Lu, Theodore Ts'o, linux-pci@vger.kernel.org, Benjamin Herrenschmidt On 14-04-08 02:27 PM, Bjorn Helgaas wrote: > [+cc Ben, linux-pci] > > On Tue, Apr 8, 2014 at 10:34 AM, Mark Lord <mlord@pobox.com> wrote: >> I am working a couple of drivers for chips that perform extensive bus-mastering ops. >> These including full SRIOV support, and allow assigning virtual functions to virtual machines, etc. >> >> One thing the driver (still in development) does for safety, >> is defer the call to pci_set_master() until *after* it has mapped >> the MMIO space of the chips, so it can reset/flush the DMA engines >> before giving them permission to scribble over host RAM. >> >> But a recent patch to the kernel has removed this from the driver's control. >> The core PCI now does pci_set_master() immediately on pci_enable_device(). > > I assume you're talking about the one added by cf3e1feba7f9 ("PCI: > Workaround missing pci_set_master in pci drivers"), but as far as I > can tell, it only calls pci_set_master() for *bridge* devices. What > am I missing? Is pci_set_master() being called for your endpoint? > What path is that? Yes, it is being called during execution of the _probe() function in my driver, as evidenced by the annoying (and wrong) message it produces. Next time I've got the hardware at hand, I'll put a "dump_stack()" into there to see the exact calling path. -- Mark Lord Real-Time Remedies Inc. mlord@pobox.com ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: driver skip pci_set_master, fix it? No. 2014-04-08 21:18 ` Mark Lord @ 2014-04-09 2:51 ` Benjamin Herrenschmidt 2014-04-09 13:08 ` Mark Lord 0 siblings, 1 reply; 9+ messages in thread From: Benjamin Herrenschmidt @ 2014-04-09 2:51 UTC (permalink / raw) To: Mark Lord Cc: Bjorn Helgaas, linux-kernel@vger.kernel.org, Yinghai Lu, Theodore Ts'o, linux-pci@vger.kernel.org On Tue, 2014-04-08 at 17:18 -0400, Mark Lord wrote: > > I assume you're talking about the one added by cf3e1feba7f9 ("PCI: > > Workaround missing pci_set_master in pci drivers"), but as far as I > > can tell, it only calls pci_set_master() for *bridge* devices. What > > am I missing? Is pci_set_master() being called for your endpoint? > > What path is that? > > Yes, it is being called during execution of the _probe() function in my driver, > as evidenced by the annoying (and wrong) message it produces. > > Next time I've got the hardware at hand, I'll put a "dump_stack()" into there > to see the exact calling path. Note that one of the reason we want to do it early on bridges is that without it, we may also not get the PCIe error messages. Cheers, Ben. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: driver skip pci_set_master, fix it? No. 2014-04-09 2:51 ` Benjamin Herrenschmidt @ 2014-04-09 13:08 ` Mark Lord 2014-04-09 14:12 ` Mark Lord 0 siblings, 1 reply; 9+ messages in thread From: Mark Lord @ 2014-04-09 13:08 UTC (permalink / raw) To: Benjamin Herrenschmidt Cc: Bjorn Helgaas, linux-kernel@vger.kernel.org, Yinghai Lu, Theodore Ts'o, linux-pci@vger.kernel.org On 14-04-08 10:51 PM, Benjamin Herrenschmidt wrote: > On Tue, 2014-04-08 at 17:18 -0400, Mark Lord wrote: >>> I assume you're talking about the one added by cf3e1feba7f9 ("PCI: >>> Workaround missing pci_set_master in pci drivers"), but as far as I >>> can tell, it only calls pci_set_master() for *bridge* devices. What >>> am I missing? Is pci_set_master() being called for your endpoint? >>> What path is that? >> >> Yes, it is being called during execution of the _probe() function in my driver, >> as evidenced by the annoying (and wrong) message it produces. >> >> Next time I've got the hardware at hand, I'll put a "dump_stack()" into there >> to see the exact calling path. > > Note that one of the reason we want to do it early on bridges is that without it, > we may also not get the PCIe error messages. Sure, for bridges. I'll get a stack trace later today, but what I suspect is happening is that this multi-function card is being treated by the PCI layers as a "bridge" for purposes of the multiple virtual functions it implements. We will probably need to distinguish this kind of device from real bridges here. -- Mark Lord Real-Time Remedies Inc. mlord@pobox.com ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: driver skip pci_set_master, fix it? No. 2014-04-09 13:08 ` Mark Lord @ 2014-04-09 14:12 ` Mark Lord 2014-04-09 14:18 ` Mark Lord 0 siblings, 1 reply; 9+ messages in thread From: Mark Lord @ 2014-04-09 14:12 UTC (permalink / raw) To: Benjamin Herrenschmidt Cc: Bjorn Helgaas, linux-kernel@vger.kernel.org, Yinghai Lu, Theodore Ts'o, linux-pci@vger.kernel.org On 14-04-09 09:08 AM, Mark Lord wrote: > On 14-04-08 10:51 PM, Benjamin Herrenschmidt wrote: >> On Tue, 2014-04-08 at 17:18 -0400, Mark Lord wrote: >>>> I assume you're talking about the one added by cf3e1feba7f9 ("PCI: >>>> Workaround missing pci_set_master in pci drivers"), but as far as I >>>> can tell, it only calls pci_set_master() for *bridge* devices. What >>>> am I missing? Is pci_set_master() being called for your endpoint? >>>> What path is that? >>> >>> Yes, it is being called during execution of the _probe() function in my driver, >>> as evidenced by the annoying (and wrong) message it produces. >>> >>> Next time I've got the hardware at hand, I'll put a "dump_stack()" into there >>> to see the exact calling path. >> >> Note that one of the reason we want to do it early on bridges is that without it, >> we may also not get the PCIe error messages. > > Sure, for bridges. > > I'll get a stack trace later today, but what I suspect is happening > is that this multi-function card is being treated by the PCI layers > as a "bridge" for purposes of the multiple virtual functions it implements. > > We will probably need to distinguish this kind of device from real bridges here. Here's the call trace, all the way back to k7_probe(), the driver's PCI "probe" function, and beyond: [ 30.481454] k7: loading driver version 0.80 [ 30.485561] pcieport 0000:00:1c.0: driver skip pci_set_master, fix it! [ 30.485580] CPU: 2 PID: 4401 Comm: insmod Tainted: G O 3.12.14 #3 [ 30.485583] Hardware name: Supermicro X9SCI/X9SCA/X9SCI/X9SCA, BIOS 2.0b 09/17/2012 [ 30.485590] 0000000000000300 ffff88041c11b9b8 ffffffff8156c40b 0000000000000000 [ 30.485598] ffff88041d2b7000 ffff88041c11b9d8 ffffffff812dc493 0000000000000300 [ 30.485603] ffff88041d399000 ffff88041c11ba08 ffffffff812dc50d 0000000000001000 [ 30.485607] Call Trace: [ 30.485616] [<ffffffff8156c40b>] dump_stack+0x4f/0x84 [ 30.485622] [<ffffffff812dc493>] pci_enable_bridge+0x93/0xa0 [ 30.485627] [<ffffffff812dc50d>] pci_enable_device_flags+0x6d/0xe0 [ 30.485631] [<ffffffff812dc58e>] pci_enable_device+0xe/0x10 [ 30.485641] [<ffffffffa0469c0d>] k7_enable_device+0x3d/0xa30 [k7] [ 30.485649] [<ffffffffa0462d72>] ? k7_devmem_alloc+0x32/0x140 [k7] [ 30.485654] [<ffffffff81572ab6>] ? _raw_spin_lock+0x16/0x40 [ 30.485658] [<ffffffff81572721>] ? _raw_spin_unlock+0x11/0x40 [ 30.485666] [<ffffffffa046aee8>] k7_probe+0x458/0x630 [k7] [ 30.485682] [<ffffffff812de3d6>] local_pci_probe+0x46/0x80 [ 30.485696] [<ffffffff812de6f1>] pci_device_probe+0x101/0x110 [ 30.485702] [<ffffffff813941d6>] driver_probe_device+0x76/0x240 [ 30.485705] [<ffffffff8139443b>] __driver_attach+0x9b/0xa0 [ 30.485709] [<ffffffff813943a0>] ? driver_probe_device+0x240/0x240 [ 30.485713] [<ffffffff81392385>] bus_for_each_dev+0x55/0x90 [ 30.485717] [<ffffffff81393ce9>] driver_attach+0x19/0x20 [ 30.485720] [<ffffffff81393814>] bus_add_driver+0x104/0x290 [ 30.485724] [<ffffffff81394abf>] driver_register+0x5f/0xf0 [ 30.485728] [<ffffffff812dd3f6>] __pci_register_driver+0x46/0x50 [ 30.485736] [<ffffffffa024c16e>] k7_init+0x16e/0x1000 [k7] [ 30.485746] [<ffffffffa024c000>] ? 0xffffffffa024bfff [ 30.485765] [<ffffffff81000302>] do_one_initcall+0x112/0x160 [ 30.485779] [<ffffffff81038143>] ? set_memory_nx+0x43/0x50 [ 30.485785] [<ffffffff810abbe1>] load_module+0x1e51/0x2480 [ 30.485789] [<ffffffff810a8b10>] ? show_initstate+0x50/0x50 [ 30.485794] [<ffffffff810ac2ae>] SyS_init_module+0x9e/0xc0 [ 30.485799] [<ffffffff8157389b>] tracesys+0xdd/0xe ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: driver skip pci_set_master, fix it? No. 2014-04-09 14:12 ` Mark Lord @ 2014-04-09 14:18 ` Mark Lord 2014-04-09 15:52 ` Bjorn Helgaas 0 siblings, 1 reply; 9+ messages in thread From: Mark Lord @ 2014-04-09 14:18 UTC (permalink / raw) To: Benjamin Herrenschmidt Cc: Bjorn Helgaas, linux-kernel@vger.kernel.org, Yinghai Lu, Theodore Ts'o, linux-pci@vger.kernel.org On 14-04-09 10:12 AM, Mark Lord wrote: > On 14-04-09 09:08 AM, Mark Lord wrote: >> On 14-04-08 10:51 PM, Benjamin Herrenschmidt wrote: >>> On Tue, 2014-04-08 at 17:18 -0400, Mark Lord wrote: >>>>> I assume you're talking about the one added by cf3e1feba7f9 ("PCI: >>>>> Workaround missing pci_set_master in pci drivers"), but as far as I >>>>> can tell, it only calls pci_set_master() for *bridge* devices. What >>>>> am I missing? Is pci_set_master() being called for your endpoint? >>>>> What path is that? >>>> >>>> Yes, it is being called during execution of the _probe() function in my driver, >>>> as evidenced by the annoying (and wrong) message it produces. >>>> >>>> Next time I've got the hardware at hand, I'll put a "dump_stack()" into there >>>> to see the exact calling path. >>> >>> Note that one of the reason we want to do it early on bridges is that without it, >>> we may also not get the PCIe error messages. >> >> Sure, for bridges. >> >> I'll get a stack trace later today, but what I suspect is happening >> is that this multi-function card is being treated by the PCI layers >> as a "bridge" for purposes of the multiple virtual functions it implements. >> >> We will probably need to distinguish this kind of device from real bridges here. > > Here's the call trace, all the way back to k7_probe(), > the driver's PCI "probe" function, and beyond: > > [ 30.481454] k7: loading driver version 0.80 > [ 30.485561] pcieport 0000:00:1c.0: driver skip pci_set_master, fix it! > [ 30.485580] CPU: 2 PID: 4401 Comm: insmod Tainted: G O 3.12.14 #3 > [ 30.485583] Hardware name: Supermicro X9SCI/X9SCA/X9SCI/X9SCA, BIOS 2.0b 09/17/2012 > [ 30.485590] 0000000000000300 ffff88041c11b9b8 ffffffff8156c40b 0000000000000000 > [ 30.485598] ffff88041d2b7000 ffff88041c11b9d8 ffffffff812dc493 0000000000000300 > [ 30.485603] ffff88041d399000 ffff88041c11ba08 ffffffff812dc50d 0000000000001000 > [ 30.485607] Call Trace: > [ 30.485616] [<ffffffff8156c40b>] dump_stack+0x4f/0x84 > [ 30.485622] [<ffffffff812dc493>] pci_enable_bridge+0x93/0xa0 > [ 30.485627] [<ffffffff812dc50d>] pci_enable_device_flags+0x6d/0xe0 > [ 30.485631] [<ffffffff812dc58e>] pci_enable_device+0xe/0x10 > [ 30.485641] [<ffffffffa0469c0d>] k7_enable_device+0x3d/0xa30 [k7] > [ 30.485649] [<ffffffffa0462d72>] ? k7_devmem_alloc+0x32/0x140 [k7] > [ 30.485654] [<ffffffff81572ab6>] ? _raw_spin_lock+0x16/0x40 > [ 30.485658] [<ffffffff81572721>] ? _raw_spin_unlock+0x11/0x40 > [ 30.485666] [<ffffffffa046aee8>] k7_probe+0x458/0x630 [k7] > > [ 30.485682] [<ffffffff812de3d6>] local_pci_probe+0x46/0x80 > [ 30.485696] [<ffffffff812de6f1>] pci_device_probe+0x101/0x110 > [ 30.485702] [<ffffffff813941d6>] driver_probe_device+0x76/0x240 > [ 30.485705] [<ffffffff8139443b>] __driver_attach+0x9b/0xa0 > [ 30.485709] [<ffffffff813943a0>] ? driver_probe_device+0x240/0x240 > [ 30.485713] [<ffffffff81392385>] bus_for_each_dev+0x55/0x90 > [ 30.485717] [<ffffffff81393ce9>] driver_attach+0x19/0x20 > [ 30.485720] [<ffffffff81393814>] bus_add_driver+0x104/0x290 > [ 30.485724] [<ffffffff81394abf>] driver_register+0x5f/0xf0 > [ 30.485728] [<ffffffff812dd3f6>] __pci_register_driver+0x46/0x50 > [ 30.485736] [<ffffffffa024c16e>] k7_init+0x16e/0x1000 [k7] > [ 30.485746] [<ffffffffa024c000>] ? 0xffffffffa024bfff > [ 30.485765] [<ffffffff81000302>] do_one_initcall+0x112/0x160 > [ 30.485779] [<ffffffff81038143>] ? set_memory_nx+0x43/0x50 > [ 30.485785] [<ffffffff810abbe1>] load_module+0x1e51/0x2480 > [ 30.485789] [<ffffffff810a8b10>] ? show_initstate+0x50/0x50 > [ 30.485794] [<ffffffff810ac2ae>] SyS_init_module+0x9e/0xc0 > [ 30.485799] [<ffffffff8157389b>] tracesys+0xdd/0xe > The e1000e network driver is suffering from this as well in 3.12.14. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: driver skip pci_set_master, fix it? No. 2014-04-09 14:18 ` Mark Lord @ 2014-04-09 15:52 ` Bjorn Helgaas 2014-04-09 16:40 ` Mark Lord 0 siblings, 1 reply; 9+ messages in thread From: Bjorn Helgaas @ 2014-04-09 15:52 UTC (permalink / raw) To: Mark Lord Cc: Benjamin Herrenschmidt, linux-kernel@vger.kernel.org, Yinghai Lu, Theodore Ts'o, linux-pci@vger.kernel.org On Wed, Apr 9, 2014 at 8:18 AM, Mark Lord <mlord@pobox.com> wrote: > On 14-04-09 10:12 AM, Mark Lord wrote: >> On 14-04-09 09:08 AM, Mark Lord wrote: >>> On 14-04-08 10:51 PM, Benjamin Herrenschmidt wrote: >>>> On Tue, 2014-04-08 at 17:18 -0400, Mark Lord wrote: >>>>>> I assume you're talking about the one added by cf3e1feba7f9 ("PCI: >>>>>> Workaround missing pci_set_master in pci drivers"), but as far as I >>>>>> can tell, it only calls pci_set_master() for *bridge* devices. What >>>>>> am I missing? Is pci_set_master() being called for your endpoint? >>>>>> What path is that? >>>>> >>>>> Yes, it is being called during execution of the _probe() function in my driver, >>>>> as evidenced by the annoying (and wrong) message it produces. >>>>> >>>>> Next time I've got the hardware at hand, I'll put a "dump_stack()" into there >>>>> to see the exact calling path. >>>> >>>> Note that one of the reason we want to do it early on bridges is that without it, >>>> we may also not get the PCIe error messages. >>> >>> Sure, for bridges. >>> >>> I'll get a stack trace later today, but what I suspect is happening >>> is that this multi-function card is being treated by the PCI layers >>> as a "bridge" for purposes of the multiple virtual functions it implements. >>> >>> We will probably need to distinguish this kind of device from real bridges here. >> >> Here's the call trace, all the way back to k7_probe(), >> the driver's PCI "probe" function, and beyond: >> >> [ 30.481454] k7: loading driver version 0.80 >> [ 30.485561] pcieport 0000:00:1c.0: driver skip pci_set_master, fix it! This message says we're enabling bus mastering for a PCIe Root Port, which I think is the expected behavior and shouldn't cause trouble for your device (correct me if I'm wrong). I don't know the system topology, but I'm guessing the k7 device is below that Root Port. We might be enabling bus mastering for the k7 device, too, but that's not what this message is about, and we'd have to look at the k7 command register to know for sure whether we did anything to it. >> [ 30.485580] CPU: 2 PID: 4401 Comm: insmod Tainted: G O 3.12.14 #3 >> [ 30.485583] Hardware name: Supermicro X9SCI/X9SCA/X9SCI/X9SCA, BIOS 2.0b 09/17/2012 >> [ 30.485590] 0000000000000300 ffff88041c11b9b8 ffffffff8156c40b 0000000000000000 >> [ 30.485598] ffff88041d2b7000 ffff88041c11b9d8 ffffffff812dc493 0000000000000300 >> [ 30.485603] ffff88041d399000 ffff88041c11ba08 ffffffff812dc50d 0000000000001000 >> [ 30.485607] Call Trace: >> [ 30.485616] [<ffffffff8156c40b>] dump_stack+0x4f/0x84 >> [ 30.485622] [<ffffffff812dc493>] pci_enable_bridge+0x93/0xa0 >> [ 30.485627] [<ffffffff812dc50d>] pci_enable_device_flags+0x6d/0xe0 >> [ 30.485631] [<ffffffff812dc58e>] pci_enable_device+0xe/0x10 >> [ 30.485641] [<ffffffffa0469c0d>] k7_enable_device+0x3d/0xa30 [k7] >> [ 30.485649] [<ffffffffa0462d72>] ? k7_devmem_alloc+0x32/0x140 [k7] >> [ 30.485654] [<ffffffff81572ab6>] ? _raw_spin_lock+0x16/0x40 >> [ 30.485658] [<ffffffff81572721>] ? _raw_spin_unlock+0x11/0x40 >> [ 30.485666] [<ffffffffa046aee8>] k7_probe+0x458/0x630 [k7] >> >> [ 30.485682] [<ffffffff812de3d6>] local_pci_probe+0x46/0x80 >> [ 30.485696] [<ffffffff812de6f1>] pci_device_probe+0x101/0x110 >> [ 30.485702] [<ffffffff813941d6>] driver_probe_device+0x76/0x240 >> [ 30.485705] [<ffffffff8139443b>] __driver_attach+0x9b/0xa0 >> [ 30.485709] [<ffffffff813943a0>] ? driver_probe_device+0x240/0x240 >> [ 30.485713] [<ffffffff81392385>] bus_for_each_dev+0x55/0x90 >> [ 30.485717] [<ffffffff81393ce9>] driver_attach+0x19/0x20 >> [ 30.485720] [<ffffffff81393814>] bus_add_driver+0x104/0x290 >> [ 30.485724] [<ffffffff81394abf>] driver_register+0x5f/0xf0 >> [ 30.485728] [<ffffffff812dd3f6>] __pci_register_driver+0x46/0x50 >> [ 30.485736] [<ffffffffa024c16e>] k7_init+0x16e/0x1000 [k7] >> [ 30.485746] [<ffffffffa024c000>] ? 0xffffffffa024bfff >> [ 30.485765] [<ffffffff81000302>] do_one_initcall+0x112/0x160 >> [ 30.485779] [<ffffffff81038143>] ? set_memory_nx+0x43/0x50 >> [ 30.485785] [<ffffffff810abbe1>] load_module+0x1e51/0x2480 >> [ 30.485789] [<ffffffff810a8b10>] ? show_initstate+0x50/0x50 >> [ 30.485794] [<ffffffff810ac2ae>] SyS_init_module+0x9e/0xc0 >> [ 30.485799] [<ffffffff8157389b>] tracesys+0xdd/0xe >> > > The e1000e network driver is suffering from this as well in 3.12.14. I'll look at this more closely, in 3.12.14 in particular (I was looking at 3.14 before). Can you collect "lspci -vv" output for one or both of these systems (the whole system, not just the device in question)? Maybe you could read the PCI command register after the pci_enable_device() and verify that bus mastering is actually being enabled when you didn't expect it? Bjorn ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: driver skip pci_set_master, fix it? No. 2014-04-09 15:52 ` Bjorn Helgaas @ 2014-04-09 16:40 ` Mark Lord 2014-04-09 17:26 ` Bjorn Helgaas 0 siblings, 1 reply; 9+ messages in thread From: Mark Lord @ 2014-04-09 16:40 UTC (permalink / raw) To: Bjorn Helgaas Cc: Benjamin Herrenschmidt, linux-kernel@vger.kernel.org, Yinghai Lu, Theodore Ts'o, linux-pci@vger.kernel.org On 14-04-09 11:52 AM, Bjorn Helgaas wrote: > On Wed, Apr 9, 2014 at 8:18 AM, Mark Lord <mlord@pobox.com> wrote: >> On 14-04-09 10:12 AM, Mark Lord wrote: >>> On 14-04-09 09:08 AM, Mark Lord wrote: >>>> On 14-04-08 10:51 PM, Benjamin Herrenschmidt wrote: >>>>> On Tue, 2014-04-08 at 17:18 -0400, Mark Lord wrote: >>>>>>> I assume you're talking about the one added by cf3e1feba7f9 ("PCI: >>>>>>> Workaround missing pci_set_master in pci drivers"), but as far as I >>>>>>> can tell, it only calls pci_set_master() for *bridge* devices. What >>>>>>> am I missing? Is pci_set_master() being called for your endpoint? >>>>>>> What path is that? >>>>>> >>>>>> Yes, it is being called during execution of the _probe() function in my driver, >>>>>> as evidenced by the annoying (and wrong) message it produces. >>>>>> >>>>>> Next time I've got the hardware at hand, I'll put a "dump_stack()" into there >>>>>> to see the exact calling path. >>>>> >>>>> Note that one of the reason we want to do it early on bridges is that without it, >>>>> we may also not get the PCIe error messages. >>>> >>>> Sure, for bridges. >>>> >>>> I'll get a stack trace later today, but what I suspect is happening >>>> is that this multi-function card is being treated by the PCI layers >>>> as a "bridge" for purposes of the multiple virtual functions it implements. >>>> >>>> We will probably need to distinguish this kind of device from real bridges here. >>> >>> Here's the call trace, all the way back to k7_probe(), >>> the driver's PCI "probe" function, and beyond: >>> >>> [ 30.481454] k7: loading driver version 0.80 >>> [ 30.485561] pcieport 0000:00:1c.0: driver skip pci_set_master, fix it! > > This message says we're enabling bus mastering for a PCIe Root Port, > which I think is the expected behavior and shouldn't cause trouble for > your device (correct me if I'm wrong). > > I don't know the system topology, but I'm guessing the k7 device is > below that Root Port. We might be enabling bus mastering for the k7 > device, too, but that's not what this message is about, and we'd have > to look at the k7 command register to know for sure whether we did > anything to it. > >>> [ 30.485580] CPU: 2 PID: 4401 Comm: insmod Tainted: G O 3.12.14 #3 >>> [ 30.485583] Hardware name: Supermicro X9SCI/X9SCA/X9SCI/X9SCA, BIOS 2.0b 09/17/2012 >>> [ 30.485590] 0000000000000300 ffff88041c11b9b8 ffffffff8156c40b 0000000000000000 >>> [ 30.485598] ffff88041d2b7000 ffff88041c11b9d8 ffffffff812dc493 0000000000000300 >>> [ 30.485603] ffff88041d399000 ffff88041c11ba08 ffffffff812dc50d 0000000000001000 >>> [ 30.485607] Call Trace: >>> [ 30.485616] [<ffffffff8156c40b>] dump_stack+0x4f/0x84 >>> [ 30.485622] [<ffffffff812dc493>] pci_enable_bridge+0x93/0xa0 >>> [ 30.485627] [<ffffffff812dc50d>] pci_enable_device_flags+0x6d/0xe0 >>> [ 30.485631] [<ffffffff812dc58e>] pci_enable_device+0xe/0x10 >>> [ 30.485641] [<ffffffffa0469c0d>] k7_enable_device+0x3d/0xa30 [k7] >>> [ 30.485649] [<ffffffffa0462d72>] ? k7_devmem_alloc+0x32/0x140 [k7] >>> [ 30.485654] [<ffffffff81572ab6>] ? _raw_spin_lock+0x16/0x40 >>> [ 30.485658] [<ffffffff81572721>] ? _raw_spin_unlock+0x11/0x40 >>> [ 30.485666] [<ffffffffa046aee8>] k7_probe+0x458/0x630 [k7] ... >> The e1000e network driver is suffering from this as well in 3.12.14. > > I'll look at this more closely, in 3.12.14 in particular (I was > looking at 3.14 before). Can you collect "lspci -vv" output for one > or both of these systems (the whole system, not just the device in > question)? > > Maybe you could read the PCI command register after the > pci_enable_device() and verify that bus mastering is actually being > enabled when you didn't expect it? I've checked the master bit now in my own driver, and you are right -- it is still 0 after pci_enable_device(). So that message is complaining about the root port driver, not my driver or the e1000e driver. Confusing at first. Whoever added the message ought to have taken care of the root ports already. So a fix may still be needed for that. To confirm this, here are the messages and the tree view: pcieport 0000:00:1c.4: driver skip pci_set_master, fix it! pcieport 0000:00:1c.5: driver skip pci_set_master, fix it! pcieport 0000:00:1c.0: driver skip pci_set_master, fix it! lspci -t -[0000:00]-+-00.0 +-01.0-[01]--+-00.0 | \-00.1 +-16.0 +-16.1 +-1a.0 +-1c.0-[02]--+-00.0 (these are part of my "k7" device) | +-00.1 | +-00.2 | +-00.3 | +-00.4 | +-00.5 | +-00.6 | \-00.7 +-1c.4-[03]----00.0 (e1000e) +-1c.5-[04]----00.0 (e1000e) +-1d.0 +-1e.0-[05]----03.0 +-1f.0 +-1f.2 \-1f.3 Here is the simple lspci view. The device I am working with has not yet been announced/released, so I have to hide the identification for now (NDA). The driver is being developed for GPL licensing/distribution though. 00:00.0 Host bridge: Intel Corporation Xeon E3-1200 Processor Family DRAM Controller (rev 09) 00:01.0 PCI bridge: Intel Corporation Xeon E3-1200/2nd Generation Core Processor Family PCI Express Root Port (rev 09) 00:16.0 Communication controller: Intel Corporation 6 Series/C200 Series Chipset Family MEI Controller #1 (rev 04) 00:16.1 Communication controller: Intel Corporation 6 Series/C200 Series Chipset Family MEI Controller #2 (rev 04) 00:1a.0 USB controller: Intel Corporation 6 Series/C200 Series Chipset Family USB Enhanced Host Controller #2 (rev 05) 00:1c.0 PCI bridge: Intel Corporation 6 Series/C200 Series Chipset Family PCI Express Root Port 1 (rev b5) 00:1c.4 PCI bridge: Intel Corporation 6 Series/C200 Series Chipset Family PCI Express Root Port 5 (rev b5) 00:1c.5 PCI bridge: Intel Corporation 6 Series/C200 Series Chipset Family PCI Express Root Port 6 (rev b5) 00:1d.0 USB controller: Intel Corporation 6 Series/C200 Series Chipset Family USB Enhanced Host Controller #1 (rev 05) 00:1e.0 PCI bridge: Intel Corporation 82801 PCI Bridge (rev a5) 00:1f.0 ISA bridge: Intel Corporation C204 Chipset Family LPC Controller (rev 05) 00:1f.2 SATA controller: Intel Corporation 6 Series/C200 Series Chipset Family SATA AHCI Controller (rev 05) 00:1f.3 SMBus: Intel Corporation 6 Series/C200 Series Chipset Family SMBus Controller (rev 05) 01:00.0 VGA compatible controller: Advanced Micro Devices [AMD] nee ATI RV380 [Radeon X600 (PCIE)] 01:00.1 Display controller: Advanced Micro Devices [AMD] nee ATI RV380 [Radeon X600] 02:00.0 Network... 02:00.1 Network... 02:00.2 Network... 02:00.3 Network... 02:00.4 Network... 02:00.5 Network... 02:00.6 Network... 02:00.7 Network... 03:00.0 Ethernet controller: Intel Corporation 82574L Gigabit Network Connection 04:00.0 Ethernet controller: Intel Corporation 82574L Gigabit Network Connection 05:03.0 VGA compatible controller: Matrox Electronics Systems Ltd. MGA G200eW WPCM450 (rev 0a) ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: driver skip pci_set_master, fix it? No. 2014-04-09 16:40 ` Mark Lord @ 2014-04-09 17:26 ` Bjorn Helgaas 0 siblings, 0 replies; 9+ messages in thread From: Bjorn Helgaas @ 2014-04-09 17:26 UTC (permalink / raw) To: Mark Lord Cc: Benjamin Herrenschmidt, linux-kernel@vger.kernel.org, Yinghai Lu, Theodore Ts'o, linux-pci@vger.kernel.org On Wed, Apr 9, 2014 at 10:40 AM, Mark Lord <mlord@pobox.com> wrote: > On 14-04-09 11:52 AM, Bjorn Helgaas wrote: >> On Wed, Apr 9, 2014 at 8:18 AM, Mark Lord <mlord@pobox.com> wrote: >>> On 14-04-09 10:12 AM, Mark Lord wrote: >>>> On 14-04-09 09:08 AM, Mark Lord wrote: >>>>> On 14-04-08 10:51 PM, Benjamin Herrenschmidt wrote: >>>>>> On Tue, 2014-04-08 at 17:18 -0400, Mark Lord wrote: >>>>>>>> I assume you're talking about the one added by cf3e1feba7f9 ("PCI: >>>>>>>> Workaround missing pci_set_master in pci drivers"), but as far as I >>>>>>>> can tell, it only calls pci_set_master() for *bridge* devices. What >>>>>>>> am I missing? Is pci_set_master() being called for your endpoint? >>>>>>>> What path is that? >>>>>>> >>>>>>> Yes, it is being called during execution of the _probe() function in my driver, >>>>>>> as evidenced by the annoying (and wrong) message it produces. >>>>>>> >>>>>>> Next time I've got the hardware at hand, I'll put a "dump_stack()" into there >>>>>>> to see the exact calling path. >>>>>> >>>>>> Note that one of the reason we want to do it early on bridges is that without it, >>>>>> we may also not get the PCIe error messages. >>>>> >>>>> Sure, for bridges. >>>>> >>>>> I'll get a stack trace later today, but what I suspect is happening >>>>> is that this multi-function card is being treated by the PCI layers >>>>> as a "bridge" for purposes of the multiple virtual functions it implements. >>>>> >>>>> We will probably need to distinguish this kind of device from real bridges here. >>>> >>>> Here's the call trace, all the way back to k7_probe(), >>>> the driver's PCI "probe" function, and beyond: >>>> >>>> [ 30.481454] k7: loading driver version 0.80 >>>> [ 30.485561] pcieport 0000:00:1c.0: driver skip pci_set_master, fix it! >> >> This message says we're enabling bus mastering for a PCIe Root Port, >> which I think is the expected behavior and shouldn't cause trouble for >> your device (correct me if I'm wrong). >> >> I don't know the system topology, but I'm guessing the k7 device is >> below that Root Port. We might be enabling bus mastering for the k7 >> device, too, but that's not what this message is about, and we'd have >> to look at the k7 command register to know for sure whether we did >> anything to it. >> >>>> [ 30.485580] CPU: 2 PID: 4401 Comm: insmod Tainted: G O 3.12.14 #3 >>>> [ 30.485583] Hardware name: Supermicro X9SCI/X9SCA/X9SCI/X9SCA, BIOS 2.0b 09/17/2012 >>>> [ 30.485590] 0000000000000300 ffff88041c11b9b8 ffffffff8156c40b 0000000000000000 >>>> [ 30.485598] ffff88041d2b7000 ffff88041c11b9d8 ffffffff812dc493 0000000000000300 >>>> [ 30.485603] ffff88041d399000 ffff88041c11ba08 ffffffff812dc50d 0000000000001000 >>>> [ 30.485607] Call Trace: >>>> [ 30.485616] [<ffffffff8156c40b>] dump_stack+0x4f/0x84 >>>> [ 30.485622] [<ffffffff812dc493>] pci_enable_bridge+0x93/0xa0 >>>> [ 30.485627] [<ffffffff812dc50d>] pci_enable_device_flags+0x6d/0xe0 >>>> [ 30.485631] [<ffffffff812dc58e>] pci_enable_device+0xe/0x10 >>>> [ 30.485641] [<ffffffffa0469c0d>] k7_enable_device+0x3d/0xa30 [k7] >>>> [ 30.485649] [<ffffffffa0462d72>] ? k7_devmem_alloc+0x32/0x140 [k7] >>>> [ 30.485654] [<ffffffff81572ab6>] ? _raw_spin_lock+0x16/0x40 >>>> [ 30.485658] [<ffffffff81572721>] ? _raw_spin_unlock+0x11/0x40 >>>> [ 30.485666] [<ffffffffa046aee8>] k7_probe+0x458/0x630 [k7] > ... >>> The e1000e network driver is suffering from this as well in 3.12.14. >> >> I'll look at this more closely, in 3.12.14 in particular (I was >> looking at 3.14 before). Can you collect "lspci -vv" output for one >> or both of these systems (the whole system, not just the device in >> question)? >> >> Maybe you could read the PCI command register after the >> pci_enable_device() and verify that bus mastering is actually being >> enabled when you didn't expect it? > > I've checked the master bit now in my own driver, > and you are right -- it is still 0 after pci_enable_device(). > > So that message is complaining about the root port driver, > not my driver or the e1000e driver. Confusing at first. > > Whoever added the message ought to have taken care of the > root ports already. So a fix may still be needed for that. Definitely confusing. That message has already been removed by fbeeb822f6f4 ("PCI: Drop warning about drivers that don't use pci_set_master()"). I'm sorry it wasn't removed soon enough to save you the confusion :) So I think your concern is resolved. Let me know if it's not and I'll look into it more. Thanks, Bjorn ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2014-04-09 17:26 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <5344251D.7040805@pobox.com>
2014-04-08 18:27 ` driver skip pci_set_master, fix it? No Bjorn Helgaas
2014-04-08 21:18 ` Mark Lord
2014-04-09 2:51 ` Benjamin Herrenschmidt
2014-04-09 13:08 ` Mark Lord
2014-04-09 14:12 ` Mark Lord
2014-04-09 14:18 ` Mark Lord
2014-04-09 15:52 ` Bjorn Helgaas
2014-04-09 16:40 ` Mark Lord
2014-04-09 17:26 ` Bjorn Helgaas
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).