| Message ID | 1369137900-5748-2-git-send-email-haokexin@gmail.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Commit | 7d4d595dad30328bc6153e235d90f54c918fc127 |
| Delegated to: | Scott Wood |
| Headers | show |
On 05/21/2013 07:04:58 AM, Kevin Hao wrote: > It also seems that we don't support ISA on all the current corenet ds > boards. So picking a primary bus seems useless, remove that function > too. IIRC that was due to some bugs in the PPC PCI code in the absence of any primary bus. fsl_pci_assign_primary() will arbitrarily pick one to be primary if there's no ISA. Have the bugs been fixed? -Scott
On Tue, May 28, 2013 at 05:52:09PM -0500, Scott Wood wrote: > On 05/21/2013 07:04:58 AM, Kevin Hao wrote: > >It also seems that we don't support ISA on all the current corenet ds > >boards. So picking a primary bus seems useless, remove that function > >too. > > IIRC that was due to some bugs in the PPC PCI code in the absence of > any primary bus. Do you know more about these bugs? > fsl_pci_assign_primary() will arbitrarily pick one > to be primary if there's no ISA. Have the bugs been fixed? I know there should be some reason that we put the fsl_pci_assign_primary() here. But frankly I am not sure what bugs this workaround try to fix. For these corenet boards picking one to be primary has no effect to the 64bit kernel. And for 32bit kernel, the only effect of this is that isa_io_base is set to the io virtual base of the primary bus. But the isa_io_base only make sense when we do have a isa bus, so that we can access some well-known io ports directly by using outx/inx. But if we don't have isa bus on the board, the value of isa_io_base should make no sense at all. So we really don't need to pick a fake primary bus. Of course I may miss something, correct me if I am wrong. :-) Thanks, Kevin > > -Scott
On 05/30/2013 05:20:34 AM, Kevin Hao wrote: > On Tue, May 28, 2013 at 05:52:09PM -0500, Scott Wood wrote: > > On 05/21/2013 07:04:58 AM, Kevin Hao wrote: > > >It also seems that we don't support ISA on all the current corenet > ds > > >boards. So picking a primary bus seems useless, remove that > function > > >too. > > > > IIRC that was due to some bugs in the PPC PCI code in the absence of > > any primary bus. > > Do you know more about these bugs? Not off the top of my head -- either search the archives or ask Ben. > > fsl_pci_assign_primary() will arbitrarily pick one > > to be primary if there's no ISA. Have the bugs been fixed? > > I know there should be some reason that we put the > fsl_pci_assign_primary() > here. But frankly I am not sure what bugs this workaround try to fix. > For these > corenet boards picking one to be primary has no effect to the 64bit > kernel. > And for 32bit kernel, the only effect of this is that isa_io_base is > set to the > io virtual base of the primary bus. But the isa_io_base only make > sense when > we do have a isa bus, so that we can access some well-known io ports > directly > by using outx/inx. But if we don't have isa bus on the board, the > value of > isa_io_base should make no sense at all. So we really don't need to > pick a > fake primary bus. Of course I may miss something, correct me if I am > wrong. :-) outx/inx can also be used for PCI I/O BARs. -Scott
On Thu, May 30, 2013 at 01:54:59PM -0500, Scott Wood wrote: > On 05/30/2013 05:20:34 AM, Kevin Hao wrote: > >On Tue, May 28, 2013 at 05:52:09PM -0500, Scott Wood wrote: > >> On 05/21/2013 07:04:58 AM, Kevin Hao wrote: > >> >It also seems that we don't support ISA on all the current > >corenet ds > >> >boards. So picking a primary bus seems useless, remove that > >function > >> >too. > >> > >> IIRC that was due to some bugs in the PPC PCI code in the absence of > >> any primary bus. > > > >Do you know more about these bugs? > > Not off the top of my head -- either search the archives or ask Ben. Hi Ben, Could you shed some light on this issue? Do we really has the restriction that we have to pick one bus controller as primary even there is no ISA bus on the board? I did check the current code and found no code has a requirement for this. I also searched the archives and but found nothing useful. :-( Thanks, Kevin > > >> fsl_pci_assign_primary() will arbitrarily pick one > >> to be primary if there's no ISA. Have the bugs been fixed? > > > >I know there should be some reason that we put the > >fsl_pci_assign_primary() > >here. But frankly I am not sure what bugs this workaround try to > >fix. For these > >corenet boards picking one to be primary has no effect to the > >64bit kernel. > >And for 32bit kernel, the only effect of this is that isa_io_base > >is set to the > >io virtual base of the primary bus. But the isa_io_base only make > >sense when > >we do have a isa bus, so that we can access some well-known io > >ports directly > >by using outx/inx. But if we don't have isa bus on the board, the > >value of > >isa_io_base should make no sense at all. So we really don't need > >to pick a > >fake primary bus. Of course I may miss something, correct me if I > >am wrong. :-) > > outx/inx can also be used for PCI I/O BARs. > > -Scott
On Thu, May 30, 2013 at 01:54:59PM -0500, Scott Wood wrote: > On 05/30/2013 05:20:34 AM, Kevin Hao wrote: > >On Tue, May 28, 2013 at 05:52:09PM -0500, Scott Wood wrote: > >> On 05/21/2013 07:04:58 AM, Kevin Hao wrote: > >> >It also seems that we don't support ISA on all the current > >corenet ds > >> >boards. So picking a primary bus seems useless, remove that > >function > >> >too. > >> > >> IIRC that was due to some bugs in the PPC PCI code in the absence of > >> any primary bus. > > > >Do you know more about these bugs? > > Not off the top of my head -- either search the archives or ask Ben. > > >> fsl_pci_assign_primary() will arbitrarily pick one > >> to be primary if there's no ISA. Have the bugs been fixed? > > > >I know there should be some reason that we put the > >fsl_pci_assign_primary() > >here. But frankly I am not sure what bugs this workaround try to > >fix. For these > >corenet boards picking one to be primary has no effect to the > >64bit kernel. > >And for 32bit kernel, the only effect of this is that isa_io_base > >is set to the > >io virtual base of the primary bus. But the isa_io_base only make > >sense when > >we do have a isa bus, so that we can access some well-known io > >ports directly > >by using outx/inx. But if we don't have isa bus on the board, the > >value of > >isa_io_base should make no sense at all. So we really don't need > >to pick a > >fake primary bus. Of course I may miss something, correct me if I > >am wrong. :-) > > outx/inx can also be used for PCI I/O BARs. Yes, I know there is also PIO. But the value of isa_io_base doesn't have any effect for this. Thanks, Kevin > > -Scott
On Fri, 2013-05-31 at 14:41 +0800, Kevin Hao wrote: > Hi Ben, > > Could you shed some light on this issue? Do we really has the restriction > that we have to pick one bus controller as primary even there is no > ISA bus on the board? I did check the current code and found no code > has a requirement for this. I also searched the archives and but found > nothing useful. :-( You can just pick the first one as primary... The reason we somewhat need a primary is related to how we handle IO space. We ioremap the IO space of all busses and assign the base of the primary one to a global "_IO_BASE". Then any "port" access is an offset from that which means that non-primary can end up having negative offsets. We fix up all resources, which works fine ... unless drivers do stupid casts and the wrap-around fails. The main reason we did that originally is because we still had a slew of x86 originated HW that would access hard wired IO ports, especially on things like CHRP machines, looking for things like 8259 PIC, legacy kbd controllers, UARTs, etc... at fixed IO port numbers. We still support some of these boxes (though I do wonder how long since somebody last booted a Pegasos) so I'm not quite yet keen on getting rid of that stuff... Cheers, Ben.
On Fri, May 31, 2013 at 08:11:20PM +1000, Benjamin Herrenschmidt wrote: > On Fri, 2013-05-31 at 14:41 +0800, Kevin Hao wrote: > > Hi Ben, > > > > Could you shed some light on this issue? Do we really has the restriction > > that we have to pick one bus controller as primary even there is no > > ISA bus on the board? I did check the current code and found no code > > has a requirement for this. I also searched the archives and but found > > nothing useful. :-( > > You can just pick the first one as primary... The reason we somewhat need > a primary is related to how we handle IO space. > > We ioremap the IO space of all busses and assign the base of the primary > one to a global "_IO_BASE". Then any "port" access is an offset from that > which means that non-primary can end up having negative offsets. We fix > up all resources, which works fine ... unless drivers do stupid casts > and the wrap-around fails. > > The main reason we did that originally is because we still had a slew of > x86 originated HW that would access hard wired IO ports, especially on things > like CHRP machines, looking for things like 8259 PIC, legacy kbd controllers, > UARTs, etc... at fixed IO port numbers. > > We still support some of these boxes (though I do wonder how long since > somebody last booted a Pegasos) so I'm not quite yet keen on getting rid > of that stuff... Thanks for the detailed explanation. But I don't mean to drop the support of the primary bus. As you said, this is definitely needed now in order to make some legacy device drivers work. What I want to do is not to pick a primary bus if there is no such ISA devices at all. For example, on a fsl p4080ds board, we would do the following in the current kernel: /* pick up a random host bridge as primary bus */ for_each_matching_node(np, pci_ids) { ... } /* for host bridge 0 */ fsl_add_bridge(pdev, primary = 1) { ... pci_process_bridge_OF_ranges(hose, dev, is_primary = 1); } /* for host bridge 1*/ fsl_add_bridge(pdev, primary = 0) { ... pci_process_bridge_OF_ranges(hose, dev, is_primary = 0); } But there is no ISA bus on this board and we also don't need to support any fixed IO port numbers. So it seems redundant to iterate the device list to pick a host bridge as primary bus. So we can simply change the above to: /* for host bridge 0 */ fsl_add_bridge(pdev, primary = 0) { ... pci_process_bridge_OF_ranges(hose, dev, is_primary = 0); } /* for host bridge 1*/ fsl_add_bridge(pdev, primary = 0) { ... pci_process_bridge_OF_ranges(hose, dev, is_primary = 0); } The effect of this change is that the isa_io_base will be 0 and the IO resource are equal to the virtual address of the IO space. But the IO functions such as outx/inx should work as well. This is why I ask the above question. What do you think about this? Are there any subtle bugs that will be triggered by this? Thanks, Kevin > > Cheers, > Ben. > >
On Fri, May 31, 2013 at 06:27:21PM -0500, Scott Wood wrote: > On 05/31/2013 01:43:49 AM, Kevin Hao wrote: > >On Thu, May 30, 2013 at 01:54:59PM -0500, Scott Wood wrote: > >> On 05/30/2013 05:20:34 AM, Kevin Hao wrote: > >> >On Tue, May 28, 2013 at 05:52:09PM -0500, Scott Wood wrote: > >> >> On 05/21/2013 07:04:58 AM, Kevin Hao wrote: > >> >> >It also seems that we don't support ISA on all the current > >> >corenet ds > >> >> >boards. So picking a primary bus seems useless, remove that > >> >function > >> >> >too. > >> >> > >> >> IIRC that was due to some bugs in the PPC PCI code in the > >absence of > >> >> any primary bus. > >> > > >> >Do you know more about these bugs? > >> > >> Not off the top of my head -- either search the archives or ask Ben. > >> > >> >> fsl_pci_assign_primary() will arbitrarily pick one > >> >> to be primary if there's no ISA. Have the bugs been fixed? > >> > > >> >I know there should be some reason that we put the > >> >fsl_pci_assign_primary() > >> >here. But frankly I am not sure what bugs this workaround try to > >> >fix. For these > >> >corenet boards picking one to be primary has no effect to the > >> >64bit kernel. > >> >And for 32bit kernel, the only effect of this is that isa_io_base > >> >is set to the > >> >io virtual base of the primary bus. But the isa_io_base only make > >> >sense when > >> >we do have a isa bus, so that we can access some well-known io > >> >ports directly > >> >by using outx/inx. But if we don't have isa bus on the board, the > >> >value of > >> >isa_io_base should make no sense at all. So we really don't need > >> >to pick a > >> >fake primary bus. Of course I may miss something, correct me if I > >> >am wrong. :-) > >> > >> outx/inx can also be used for PCI I/O BARs. > > > >Yes, I know there is also PIO. But the value of isa_io_base doesn't > >have any effect for this. > > See this e-mail for some of the issues I was referring to with > isa_io_base being zero: > https://lists.ozlabs.org/pipermail/linuxppc-dev/2012-June/098586.html Thanks for the reference. > > Reading it again I'm not so sure that the problem is so much that we > need a primary, as that somewhat bad things happen on non-primary > buses, such as the possibility of assigning a zero BAR. Some > hardware (including QEMU's PCI emulation) cares about this, though > most doesn't. We only have one PCI bus under QEMU, so when we > started picking an arbitrary bus to be primary, the problem went > away because there was only one bus and therefore there was no > non-primary bus. Sorry, I am not sure what you mean. Do you mean that it will affect the resources assigned to a bridge when it is marked as a primary bus? I searched the code and didn't found anything related to this? Could you give me some hint? :-) Thanks, Kevin > > -Scott
On Sat, 2013-06-01 at 18:59 +0800, Kevin Hao wrote: > The effect of this change is that the isa_io_base will be 0 and the IO > resource are equal to the virtual address of the IO space. But the IO > functions such as outx/inx should work as well. This is why I ask the > above question. What do you think about this? Are there any subtle bugs > that will be triggered by this? I don't see any obvious reason why that wouldn't work but like anything in that area, it needs a bit of testing & hammering to be sure ;-) In fact it would work on pmac32 as well since those generally don't have legacy crap either. So I have no fundamental objection, it just needs testing. My worry is that we need to make sure we don't break old chrp and I don't have any to test with. I'm happy to drop support for stuff nobody uses anymore (we did drop PReP after all and I'm *that* close to drop power3) but as long as somebody is still using a CHRP RS6K or a Pegasos I can't quite drop those... And they do have legacy ISA crap to deal with. Cheers, Ben.
On Sat, Jun 01, 2013 at 09:47:16PM +1000, Benjamin Herrenschmidt wrote: > On Sat, 2013-06-01 at 18:59 +0800, Kevin Hao wrote: > > > The effect of this change is that the isa_io_base will be 0 and the IO > > resource are equal to the virtual address of the IO space. But the IO > > functions such as outx/inx should work as well. This is why I ask the > > above question. What do you think about this? Are there any subtle bugs > > that will be triggered by this? > > I don't see any obvious reason why that wouldn't work but like anything > in that area, it needs a bit of testing & hammering to be sure ;-) Agreed. > > In fact it would work on pmac32 as well since those generally don't have > legacy crap either. > > So I have no fundamental objection, it just needs testing. My worry is > that we need to make sure we don't break old chrp and I don't have any > to test with. Don't worry, my patch just drop the picking of primary bus for several fsl boards. All these changes are in board specific file, so it should have no affect to other boards at all. For the other boards which have the same issue, since this is not emergency, we don't want handle them in a rush. So I would like to merge this patch first and then wait one or two kernel release cycle before handling the others to make sure that we don't break anything. Thanks, Kevin > I'm happy to drop support for stuff nobody uses anymore > (we did drop PReP after all and I'm *that* close to drop power3) but as > long as somebody is still using a CHRP RS6K or a Pegasos I can't quite > drop those... And they do have legacy ISA crap to deal with. > > Cheers, > Ben. > >
On 06/01/2013 07:07:20 PM, Kevin Hao wrote: > On Sat, Jun 01, 2013 at 09:47:16PM +1000, Benjamin Herrenschmidt > wrote: > > On Sat, 2013-06-01 at 18:59 +0800, Kevin Hao wrote: > > > > > The effect of this change is that the isa_io_base will be 0 and > the IO > > > resource are equal to the virtual address of the IO space. But > the IO > > > functions such as outx/inx should work as well. This is why I ask > the > > > above question. What do you think about this? Are there any > subtle bugs > > > that will be triggered by this? > > > > I don't see any obvious reason why that wouldn't work but like > anything > > in that area, it needs a bit of testing & hammering to be sure ;-) > > Agreed. Please include QEMU in your testing, as that was where breakage was observed that caused us to add the default primary. > > In fact it would work on pmac32 as well since those generally don't > have > > legacy crap either. > > > > So I have no fundamental objection, it just needs testing. My worry > is > > that we need to make sure we don't break old chrp and I don't have > any > > to test with. > > Don't worry, my patch just drop the picking of primary bus for several > fsl boards. All these changes are in board specific file, so it should > have no affect to other boards at all. Is anything actually fixed by this? -Scott
On Mon, Jun 03, 2013 at 11:42:03AM -0500, Scott Wood wrote: > On 06/01/2013 07:07:20 PM, Kevin Hao wrote: > >On Sat, Jun 01, 2013 at 09:47:16PM +1000, Benjamin Herrenschmidt > >wrote: > >> On Sat, 2013-06-01 at 18:59 +0800, Kevin Hao wrote: > >> > >> > The effect of this change is that the isa_io_base will be 0 > >and the IO > >> > resource are equal to the virtual address of the IO space. But > >the IO > >> > functions such as outx/inx should work as well. This is why I > >ask the > >> > above question. What do you think about this? Are there any > >subtle bugs > >> > that will be triggered by this? > >> > >> I don't see any obvious reason why that wouldn't work but like > >anything > >> in that area, it needs a bit of testing & hammering to be sure ;-) > > > >Agreed. > > Please include QEMU in your testing, as that was where breakage was > observed that caused us to add the default primary. I tested this on a qemu with the following command: ./qemu-system-ppc -m 1024 -nographic -M ppce500 -kernel ./uImage \ -initrd /root/initrd.gz \ -append "root=/dev/ram rw console=ttyS0,115200 ramdisk_size=0x60000" \ -cpu e500mc -machine dt_compatible=fsl,,P4080DS Before applying my patch: PCI: Probing PCI hardware fsl-pci e0008000.pci: PCI host bridge to bus 0000:00 pci_bus 0000:00: root bus resource [io 0x0000-0xffff] pci_bus 0000:00: root bus resource [mem 0xc0000000-0xdfffffff] pci_bus 0000:00: root bus resource [bus 00-ff] pci_bus 0000:00: busn_res: [bus 00-ff] end is updated to ff pci 0000:00:00.0: [1957:0030] type 01 class 0x060400 pci 0000:00:00.0: reg 10: [mem 0xfff00000-0xffffffff] pci 0000:00:01.0: [1af4:1000] type 00 class 0x020000 pci 0000:00:01.0: reg 10: [io 0x0000-0x001f] pci 0000:00:01.0: reg 14: [mem 0x00000000-0x00000fff] pci 0000:00:00.0: bridge configuration invalid ([bus 00-00]), reconfiguring pci 0000:00:00.0: PCI bridge to [bus 01-ff] pci 0000:00:00.0: bridge window [io 0x0000-0x0fff] pci 0000:00:00.0: bridge window [mem 0x00000000-0x000fffff] pci 0000:00:00.0: bridge window [mem 0x00000000-0x000fffff pref] pci_bus 0000:01: busn_res: [bus 01-ff] end is updated to 01 pci_bus 0000:00: busn_res: [bus 00-ff] end is updated to 01 PCI: Cannot allocate resource region 0 of device 0000:00:00.0, will remap PCI 0000:00 Cannot reserve Legacy IO [io 0x0000-0x0fff] pci 0000:00:00.0: BAR 0: assigned [mem 0xc0000000-0xc00fffff] pci 0000:00:00.0: BAR 8: assigned [mem 0xc0100000-0xc01fffff] pci 0000:00:01.0: BAR 1: assigned [mem 0xc0200000-0xc0200fff] pci 0000:00:01.0: BAR 0: assigned [io 0x1000-0x101f] pci 0000:00:00.0: PCI bridge to [bus 01] pci 0000:00:00.0: bridge window [io 0x0000-0x0fff] pci 0000:00:00.0: bridge window [mem 0xc0100000-0xc01fffff] pci_bus 0000:00: resource 4 [io 0x0000-0xffff] pci_bus 0000:00: resource 5 [mem 0xc0000000-0xdfffffff] pci_bus 0000:01: resource 0 [io 0x0000-0x0fff] pci_bus 0000:01: resource 1 [mem 0xc0100000-0xc01fffff] # lspci -vvv 00:00.0 PCI bridge: Freescale Semiconductor Inc MPC8533E (prog-if 00 [Normal decode]) Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx- Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx- Latency: 0 Region 0: Memory at c0000000 (32-bit, non-prefetchable) [size=1M] Bus: primary=00, secondary=00, subordinate=00, sec-latency=0 I/O behind bridge: 00000000-00000fff Memory behind bridge: 00000000-000fffff Prefetchable memory behind bridge: 00000000-000fffff Secondary status: 66MHz- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- <SERR- <PERR- BridgeCtl: Parity- SERR- NoISA- VGA- MAbort- >Reset- FastB2B- PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn- 00:01.0 Ethernet controller: Red Hat, Inc Virtio network device Subsystem: Red Hat, Inc Device 0001 Control: I/O- Mem- BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx- Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx- Interrupt: pin A routed to IRQ 17 Region 0: I/O ports at 1000 [disabled] [size=32] Region 1: Memory at c0200000 (32-bit, non-prefetchable) [disabled] [size=4K] Capabilities: [40] MSI-X: Enable- Count=3 Masked- Vector table: BAR=1 offset=00000000 PBA: BAR=1 offset=00000800 After applying my patch: PCI: Probing PCI hardware fsl-pci e0008000.pci: PCI host bridge to bus 0000:00 pci_bus 0000:00: root bus resource [io 0xf1020000-0xf102ffff] (bus address [0x0000-0xffff]) pci_bus 0000:00: root bus resource [mem 0xc0000000-0xdfffffff] pci_bus 0000:00: root bus resource [bus 00-ff] pci_bus 0000:00: busn_res: [bus 00-ff] end is updated to ff pci 0000:00:00.0: [1957:0030] type 01 class 0x060400 pci 0000:00:00.0: reg 10: [mem 0xfff00000-0xffffffff] pci 0000:00:01.0: [1af4:1000] type 00 class 0x020000 pci 0000:00:01.0: reg 10: [io 0xf1020000-0xf102001f] pci 0000:00:01.0: reg 14: [mem 0x00000000-0x00000fff] pci 0000:00:00.0: bridge configuration invalid ([bus 00-00]), reconfiguring pci 0000:00:00.0: PCI bridge to [bus 01-ff] pci 0000:00:00.0: bridge window [io 0xf1020000-0xf1020fff] pci 0000:00:00.0: bridge window [mem 0x00000000-0x000fffff] pci 0000:00:00.0: bridge window [mem 0x00000000-0x000fffff pref] pci_bus 0000:01: busn_res: [bus 01-ff] end is updated to 01 pci_bus 0000:00: busn_res: [bus 00-ff] end is updated to 01 PCI: Cannot allocate resource region 0 of device 0000:00:00.0, will remap PCI: Cannot allocate resource region 0 of device 0000:00:01.0, will remap PCI 0000:00 Cannot reserve Legacy IO [io 0xf1020000-0xf1020fff] pci 0000:00:00.0: BAR 0: assigned [mem 0xc0000000-0xc00fffff] pci 0000:00:00.0: BAR 8: assigned [mem 0xc0100000-0xc01fffff] pci 0000:00:01.0: BAR 1: assigned [mem 0xc0200000-0xc0200fff] pci 0000:00:01.0: BAR 0: assigned [io 0xf1021000-0xf102101f] pci 0000:00:00.0: PCI bridge to [bus 01] pci 0000:00:00.0: bridge window [io 0xf1020000-0xf1020fff] pci 0000:00:00.0: bridge window [mem 0xc0100000-0xc01fffff] pci_bus 0000:00: resource 4 [io 0xf1020000-0xf102ffff] pci_bus 0000:00: resource 5 [mem 0xc0000000-0xdfffffff] pci_bus 0000:01: resource 0 [io 0xf1020000-0xf1020fff] pci_bus 0000:01: resource 1 [mem 0xc0100000-0xc01fffff] # lspci -vvv 00:00.0 PCI bridge: Freescale Semiconductor Inc MPC8533E (prog-if 00 [Normal decode]) Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx- Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx- Latency: 0 Region 0: Memory at c0000000 (32-bit, non-prefetchable) [size=1M] Bus: primary=00, secondary=00, subordinate=00, sec-latency=0 I/O behind bridge: 00000000-00000fff Memory behind bridge: 00000000-000fffff Prefetchable memory behind bridge: 00000000-000fffff Secondary status: 66MHz- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- <SERR- <PERR- BridgeCtl: Parity- SERR- NoISA- VGA- MAbort- >Reset- FastB2B- PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn- 00:01.0 Ethernet controller: Red Hat, Inc Virtio network device Subsystem: Red Hat, Inc Device 0001 Control: I/O- Mem- BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx- Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx- Interrupt: pin A routed to IRQ 17 Region 0: I/O ports at 1000 [disabled] [size=32] Region 1: Memory at c0200000 (32-bit, non-prefetchable) [disabled] [size=4K] Capabilities: [40] MSI-X: Enable- Count=3 Masked- Vector table: BAR=1 offset=00000000 PBA: BAR=1 offset=00000800 As you can see, the only difference between these two logs is the io resource address and all the mem and bus address are still the same. > > >> In fact it would work on pmac32 as well since those generally > >don't have > >> legacy crap either. > >> > >> So I have no fundamental objection, it just needs testing. My > >worry is > >> that we need to make sure we don't break old chrp and I don't > >have any > >> to test with. > > > >Don't worry, my patch just drop the picking of primary bus for several > >fsl boards. All these changes are in board specific file, so it should > >have no affect to other boards at all. > > Is anything actually fixed by this? No. As you know the reason we need a primary bus is that we need to support some ISA legacy devices. So it seems a little weird that we have to pick one primary bus even on a board which doesn't has ISA at all. Even though we can't drop the support of the primary bus due to these legacy devices, we had better narrow the scope of using it instead of propagating it to the boards which definitely don't care the primary bus at all. Thanks, Kevin > > -Scott
On Tue, Jul 23, 2013 at 05:31:16PM -0500, Scott Wood wrote: > On 06/06/2013 09:00:20 PM, Kevin Hao wrote: > > Vector table: BAR=1 offset=00000000 <snip> > > PBA: BAR=1 offset=00000800 > > > > > >As you can see, the only difference between these two logs is the > >io resource address and all the mem and bus address are still the > >same. > > I dug a bit deeper into this, and it's making by head hurt. It > seems that we're now getting saved by the host bridge (that for some > reason has the class code of a PCI-to-PCI bridge rather than a host > bridge) having I/O space of 0x1000 bytes[1], which gets allocated at > zero. There have been some changes in the QEMU PCI code since I saw > the problem, including changing the class code of the bridge, so > that's probably why it sort-of works now. We are not just lucky here. Even without the change of the qemu pci code we still can get the correct IO address in the current kernel. The following is the log by running the latest kernel on the qemu which doesn't have the PCI-to-PCI bridge change yet. As you can see we don't pick a primary pci bus in this case and the Ethernet controller still get the I/O ports at 1000. PCI: Probing PCI hardware fsl-pci e0008000.pci: PCI host bridge to bus 0000:00 pci_bus 0000:00: root bus resource [io 0xf1020000-0xf102ffff] (bus address [0x0000-0xffff]) pci_bus 0000:00: root bus resource [mem 0xc0000000-0xdfffffff] pci_bus 0000:00: root bus resource [bus 00-ff] pci_bus 0000:00: busn_res: [bus 00-ff] end is updated to ff pci 0000:00:00.0: [1957:0030] type 00 class 0x0b2000 pci 0000:00:11.0: [1af4:1000] type 00 class 0x020000 pci 0000:00:11.0: reg 0x10: [io 0xf1020000-0xf102001f] pci_bus 0000:00: busn_res: [bus 00-ff] end is updated to 00 pci 0000:00:11.0: BAR 0: assigned [io 0xf1021000-0xf102101f] pci_bus 0000:00: resource 4 [io 0xf1020000-0xf102ffff] pci_bus 0000:00: resource 5 [mem 0xc0000000-0xdfffffff] root@localhost:~# lspci -v 00:00.0 Power PC: Freescale Semiconductor Inc MPC8533E Subsystem: Red Hat, Inc Device 1100 Flags: bus master, fast devsel, latency 0 00:11.0 Ethernet controller: Red Hat, Inc Virtio network device Subsystem: Red Hat, Inc Device 0001 Flags: fast devsel, IRQ 16 I/O ports at 1000 [disabled] [size=32] The reason is that the ppc kernel assume that the BARs starting at 0 is unset and will reassign it later. There is a bug in the previous kernel, so the kernel maybe not work well for qemu in this case. But I think this has been fixed by the commit c5df457f (powerpc/pci: Check the bus address instead of resource address in pcibios_fixup_resources). > > What QEMU is doing does not match what real hardware does, though. > At least on mpc8536 which is similar to mpc8544 (I wasn't able to > quickly get access to a working mpc8544 to test on), the PCI bridge > has class code Processor, rather than bridge of any sort. Thus, on > real hardware we would not get the 0x1000 reservation. This doesn't matter. We can always make sure to reassign these resources starting at 0. I also done a test on the mpc8536ds board. Both the pci and pcie devices work pretty well without picking a primary pci bus on this board. The following is the log. Found FSL PCI host bridge at 0x00000000ffe08000. Firmware bus number: 0->0 PCI host bridge /pci@ffe08000 ranges: MEM 0x0000000080000000..0x000000008fffffff -> 0x0000000080000000 IO 0x00000000ffc00000..0x00000000ffc0ffff -> 0x0000000000000000 /pci@ffe08000: PCICSRBAR @ 0xfff00000 Found FSL PCI host bridge at 0x00000000ffe09000. Firmware bus number: 0->0 PCI host bridge /pcie@ffe09000 ranges: MEM 0x0000000098000000..0x000000009fffffff -> 0x0000000098000000 IO 0x00000000ffc20000..0x00000000ffc2ffff -> 0x0000000000000000 /pcie@ffe09000: PCICSRBAR @ 0xfff00000 Found FSL PCI host bridge at 0x00000000ffe0a000. Firmware bus number: 0->1 PCI host bridge /pcie@ffe0a000 ranges: MEM 0x0000000090000000..0x0000000097ffffff -> 0x0000000090000000 IO 0x00000000ffc10000..0x00000000ffc1ffff -> 0x0000000000000000 /pcie@ffe0a000: PCICSRBAR @ 0xfff00000 Found FSL PCI host bridge at 0x00000000ffe0b000. Firmware bus number: 0->0 PCI host bridge /pcie@ffe0b000 ranges: MEM 0x00000000a0000000..0x00000000bfffffff -> 0x00000000a0000000 IO 0x00000000ffc30000..0x00000000ffc3ffff -> 0x0000000000000000 /pcie@ffe0b000: PCICSRBAR @ 0xfff00000 PCI: Probing PCI hardware fsl-pci ffe08000.pci: PCI host bridge to bus 0000:00 pci_bus 0000:00: root bus resource [io 0xe1020000-0xe102ffff] (bus address [0x0000-0xffff]) pci_bus 0000:00: root bus resource [mem 0x80000000-0x8fffffff] pci_bus 0000:00: root bus resource [bus 00-ff] pci_bus 0000:00: busn_res: [bus 00-ff] end is updated to ff pci 0000:00:00.0: [1957:0050] type 00 class 0x0b2000 pci 0000:00:00.0: reg 0x10: [mem 0xfff00000-0xffffffff] pci 0000:00:00.0: reg 0x14: [mem 0x00000000-0x1fffffff pref] pci 0000:00:11.0: [8086:107c] type 00 class 0x020000 pci 0000:00:11.0: reg 0x10: [mem 0x80000000-0x8001ffff] pci 0000:00:11.0: reg 0x14: [mem 0x80020000-0x8003ffff] pci 0000:00:11.0: reg 0x18: [io 0xe1021000-0xe102103f] pci 0000:00:11.0: reg 0x30: [mem 0x00000000-0x0001ffff pref] pci 0000:00:11.0: PME# supported from D0 D3hot D3cold pci_bus 0000:00: busn_res: [bus 00-ff] end is updated to 00 fsl-pci ffe09000.pcie: PCI host bridge to bus 0001:01 pci_bus 0001:01: root bus resource [io 0xe1040000-0xe104ffff] (bus address [0x0000-0xffff]) pci_bus 0001:01: root bus resource [mem 0x98000000-0x9fffffff] pci_bus 0001:01: root bus resource [bus 01-ff] pci_bus 0001:01: busn_res: [bus 01-ff] end is updated to ff pci 0001:01:00.0: [1957:0050] type 01 class 0x0b2000 pci 0001:01:00.0: ignoring class 0x0b2000 (doesn't match header type 01) pci 0001:01:00.0: supports D1 D2 pci 0001:01:00.0: PME# supported from D0 D1 D2 D3hot D3cold pci 0001:01:00.0: bridge configuration invalid ([bus 00-00]), reconfiguring pci 0001:01:00.0: PCI bridge to [bus 02-ff] pci 0001:01:00.0: bridge window [io 0xe1040000-0xe1040fff] pci 0001:01:00.0: bridge window [mem 0x00000000-0x000fffff] pci 0001:01:00.0: bridge window [mem 0x00000000-0x000fffff 64bit pref] pci_bus 0001:02: busn_res: [bus 02-ff] end is updated to 02 pci_bus 0001:01: busn_res: [bus 01-ff] end is updated to 02 fsl-pci ffe0a000.pcie: PCI host bridge to bus 0002:03 pci_bus 0002:03: root bus resource [io 0xe1060000-0xe106ffff] (bus address [0x0000-0xffff]) pci_bus 0002:03: root bus resource [mem 0x90000000-0x97ffffff] pci_bus 0002:03: root bus resource [bus 03-ff] pci_bus 0002:03: busn_res: [bus 03-ff] end is updated to ff pci 0002:03:00.0: [1957:0050] type 01 class 0x0b2000 pci 0002:03:00.0: ignoring class 0x0b2000 (doesn't match header type 01) pci 0002:03:00.0: supports D1 D2 pci 0002:03:00.0: PME# supported from D0 D1 D2 D3hot D3cold pci 0002:03:00.0: Primary bus is hard wired to 0 pci 0002:03:00.0: bridge configuration invalid ([bus 01-01]), reconfiguring pci 0002:04:00.0: [8086:107d] type 00 class 0x020000 pci 0002:04:00.0: reg 0x10: [mem 0x90000000-0x9001ffff] pci 0002:04:00.0: reg 0x14: [mem 0x90020000-0x9003ffff] pci 0002:04:00.0: reg 0x18: [io 0xe1061000-0xe106101f] pci 0002:04:00.0: PME# supported from D0 D3hot D3cold pci 0002:03:00.0: PCI bridge to [bus 04-ff] pci 0002:03:00.0: bridge window [io 0xe1060000-0xe1060fff] pci 0002:03:00.0: bridge window [mem 0x90000000-0x900fffff] pci_bus 0002:04: busn_res: [bus 04-ff] end is updated to 04 pci_bus 0002:03: busn_res: [bus 03-ff] end is updated to 04 fsl-pci ffe0b000.pcie: PCI host bridge to bus 0003:05 pci_bus 0003:05: root bus resource [io 0xe1080000-0xe108ffff] (bus address [0x0000-0xffff]) pci_bus 0003:05: root bus resource [mem 0xa0000000-0xbfffffff] pci_bus 0003:05: root bus resource [bus 05-ff] pci_bus 0003:05: busn_res: [bus 05-ff] end is updated to ff pci 0003:05:00.0: [1957:0050] type 01 class 0x0b2000 pci 0003:05:00.0: ignoring class 0x0b2000 (doesn't match header type 01) pci 0003:05:00.0: supports D1 D2 pci 0003:05:00.0: PME# supported from D0 D1 D2 D3hot D3cold pci 0003:05:00.0: bridge configuration invalid ([bus 00-00]), reconfiguring pci 0003:05:00.0: PCI bridge to [bus 06-ff] pci 0003:05:00.0: bridge window [io 0xe1080000-0xe1080fff] pci 0003:05:00.0: bridge window [mem 0x00000000-0x000fffff] pci 0003:05:00.0: bridge window [mem 0x00000000-0x000fffff 64bit pref] pci_bus 0003:06: busn_res: [bus 06-ff] end is updated to 06 pci_bus 0003:05: busn_res: [bus 05-ff] end is updated to 06 PCI 0001:01 Cannot reserve Legacy IO [io 0xe1040000-0xe1040fff] PCI 0002:03 Cannot reserve Legacy IO [io 0xe1060000-0xe1060fff] PCI 0003:05 Cannot reserve Legacy IO [io 0xe1080000-0xe1080fff] pci 0000:00:11.0: BAR 6: assigned [mem 0x80040000-0x8005ffff pref] pci 0001:01:00.0: PCI bridge to [bus 02] pci 0001:01:00.0: bridge window [io 0xe1040000-0xe104ffff] pci 0001:01:00.0: bridge window [mem 0x98000000-0x9fffffff] pci 0002:03:00.0: PCI bridge to [bus 04] pci 0002:03:00.0: bridge window [io 0xe1060000-0xe106ffff] pci 0002:03:00.0: bridge window [mem 0x90000000-0x97ffffff] pci 0003:05:00.0: PCI bridge to [bus 06] pci 0003:05:00.0: bridge window [io 0xe1080000-0xe108ffff] pci 0003:05:00.0: bridge window [mem 0xa0000000-0xbfffffff] pci 0001:01:00.0: enabling device (0106 -> 0107) pci 0002:03:00.0: enabling device (0106 -> 0107) pci 0003:05:00.0: enabling device (0106 -> 0107) pci_bus 0000:00: resource 4 [io 0xe1020000-0xe102ffff] pci_bus 0000:00: resource 5 [mem 0x80000000-0x8fffffff] pci_bus 0001:01: resource 4 [io 0xe1040000-0xe104ffff] pci_bus 0001:01: resource 5 [mem 0x98000000-0x9fffffff] pci_bus 0001:02: resource 0 [io 0xe1040000-0xe104ffff] pci_bus 0001:02: resource 1 [mem 0x98000000-0x9fffffff] pci_bus 0002:03: resource 4 [io 0xe1060000-0xe106ffff] pci_bus 0002:03: resource 5 [mem 0x90000000-0x97ffffff] pci_bus 0002:04: resource 0 [io 0xe1060000-0xe106ffff] pci_bus 0002:04: resource 1 [mem 0x90000000-0x97ffffff] pci_bus 0003:05: resource 4 [io 0xe1080000-0xe108ffff] pci_bus 0003:05: resource 5 [mem 0xa0000000-0xbfffffff] pci_bus 0003:06: resource 0 [io 0xe1080000-0xe108ffff] pci_bus 0003:06: resource 1 [mem 0xa0000000-0xbfffffff] root@intel_tunnel_creek-2:/root> lspci -v 0000:00:00.0 Power PC: Freescale Semiconductor Inc MPC8536E (rev 91) Flags: bus master, 66MHz, fast devsel, latency 128 Memory at <ignored> (32-bit, non-prefetchable) Memory at <unassigned> (32-bit, prefetchable) Memory at <unassigned> (64-bit, non-prefetchable) Memory at <unassigned> (64-bit, non-prefetchable) 0000:00:11.0 Ethernet controller: Intel Corporation 82541PI Gigabit Ethernet Controller (rev 05) Subsystem: Intel Corporation PRO/1000 GT Desktop Adapter Flags: bus master, 66MHz, medium devsel, latency 128, IRQ 35 Memory at 80000000 (32-bit, non-prefetchable) [size=128K] Memory at 80020000 (32-bit, non-prefetchable) [size=128K] I/O ports at 1000 [size=64] [virtual] Expansion ROM at 80040000 [disabled] [size=128K] Capabilities: [dc] Power Management version 2 Capabilities: [e4] PCI-X non-bridge device Kernel driver in use: e1000 0001:01:00.0 PCI bridge: Freescale Semiconductor Inc MPC8536E (rev 91) (prog-if 00 [Normal decode]) Flags: bus master, fast devsel, latency 0 Memory at <ignored> (32-bit, non-prefetchable) Bus: primary=00, secondary=02, subordinate=02, sec-latency=0 I/O behind bridge: 00000000-00000fff Memory behind bridge: 98000000-9fffffff Capabilities: [44] Power Management version 2 Capabilities: [4c] Express Root Port (Slot-), MSI 00 Capabilities: [100] Advanced Error Reporting 0002:03:00.0 PCI bridge: Freescale Semiconductor Inc MPC8536E (rev 91) (prog-if 00 [Normal decode]) Flags: bus master, fast devsel, latency 0 Memory at <ignored> (32-bit, non-prefetchable) Bus: primary=00, secondary=04, subordinate=04, sec-latency=0 I/O behind bridge: 00000000-00000fff Memory behind bridge: 90000000-97ffffff Capabilities: [44] Power Management version 2 Capabilities: [4c] Express Root Port (Slot-), MSI 00 Capabilities: [100] Advanced Error Reporting 0002:04:00.0 Ethernet controller: Intel Corporation 82572EI Gigabit Ethernet Controller (Copper) (rev 06) Subsystem: Intel Corporation PRO/1000 PT Server Adapter Flags: bus master, fast devsel, latency 0, IRQ 38 Memory at 90000000 (32-bit, non-prefetchable) [size=128K] Memory at 90020000 (32-bit, non-prefetchable) [size=128K] I/O ports at 1000 [disabled] [size=32] Capabilities: [c8] Power Management version 2 Capabilities: [d0] MSI: Enable+ Count=1/1 Maskable- 64bit+ Capabilities: [e0] Express Endpoint, MSI 00 Capabilities: [100] Advanced Error Reporting Capabilities: [140] Device Serial Number 00-15-17-ff-ff-f0-ad-1f Kernel driver in use: e1000e 0003:05:00.0 PCI bridge: Freescale Semiconductor Inc MPC8536E (rev 91) (prog-if 00 [Normal decode]) Flags: bus master, fast devsel, latency 0 Memory at <ignored> (32-bit, non-prefetchable) Bus: primary=00, secondary=06, subordinate=06, sec-latency=0 I/O behind bridge: 00000000-00000fff Memory behind bridge: a0000000-bfffffff Capabilities: [44] Power Management version 2 Capabilities: [4c] Express Root Port (Slot-), MSI 00 Capabilities: [100] Advanced Error Reporting > What > hardware does seems broken to me (when PCI is configured as a host > rather than as an endpoint), but so does calling a host bridge a > PCI-to-PCI bridge, and so does relying on this mess (not that the > primary-bus hack is much better...). > > Note that in the case of PCIe -- but not PCI -- Linux has a quirk > that relabels the "Processor" on FSL chips as a PCI-to-PCI bridge. > Despite the "Processor" labelling, Freescale PCIe (but not PCI) > devices appear to have a type 1 config space layout in host mode. > I guess this is considered to be the virtual root port. The resource reassign will occur no matter whether it is a pci or pcie. All in all the resource starting at 0 is considered a invalid resource by the ppc kernel and then the kernel will try to reassign it later. So we really don't have a reason to pick up a pci primary bus on a board which doesn't have ISA device at all in the current kernel. Thanks, Kevin > > -Scott > > [1] Really, it's the bridge not having any I/O, but this is how an > I/O base/limit of zero are interpreted.
On 07/25/2013 07:54:53 AM, Kevin Hao wrote: > The reason is that the ppc kernel assume that the BARs starting > at 0 is unset and will reassign it later. There is a bug in the > previous > kernel, so the kernel maybe not work well for qemu in this case. But I > think this has been fixed by the commit c5df457f (powerpc/pci: Check > the > bus address instead of resource address in pcibios_fixup_resources). What ensures that the reassignment will not assign zero? I'm not doubting the result that it does, but I'm having a hard time seeing it in the code. Or are you saying that the resource assignment code will run twice, rather than just saying that we won't trust the firmware when we find zero? -Scott
On Fri, Jul 26, 2013 at 05:43:44PM -0500, Scott Wood wrote: > On 07/25/2013 07:54:53 AM, Kevin Hao wrote: > >The reason is that the ppc kernel assume that the BARs starting > >at 0 is unset and will reassign it later. There is a bug in the > >previous > >kernel, so the kernel maybe not work well for qemu in this case. But I > >think this has been fixed by the commit c5df457f (powerpc/pci: > >Check the > >bus address instead of resource address in pcibios_fixup_resources). > > What ensures that the reassignment will not assign zero? In function pcibios_resource_survey() we will reserve the low IO area before reassignment. Please see the following comments and codes in this function. /* Before we start assigning unassigned resource, we try to reserve * the low IO area and the VGA memory area if they intersect the * bus available resources to avoid allocating things on top of them */ if (!pci_has_flag(PCI_PROBE_ONLY)) { list_for_each_entry(b, &pci_root_buses, node) pcibios_reserve_legacy_regions(b); } Thanks, Kevin > > I'm not doubting the result that it does, but I'm having a hard time > seeing it in the code. > > Or are you saying that the resource assignment code will run twice, > rather than just saying that we won't trust the firmware when we > find zero? > > -Scott
diff --git a/arch/powerpc/platforms/85xx/corenet_ds.c b/arch/powerpc/platforms/85xx/corenet_ds.c index c59c617..aa3690b 100644 --- a/arch/powerpc/platforms/85xx/corenet_ds.c +++ b/arch/powerpc/platforms/85xx/corenet_ds.c @@ -53,12 +53,6 @@ void __init corenet_ds_setup_arch(void) { mpc85xx_smp_init(); -#if defined(CONFIG_PCI) && defined(CONFIG_PPC64) - pci_devs_phb_init(); -#endif - - fsl_pci_assign_primary(); - swiotlb_detect_4g(); pr_info("%s board from Freescale Semiconductor\n", ppc_md.name);
The function pci_devs_phb_init is invoked more earlier than we really probe the pci controller, so it does nothing at all. And we also don't need the pci_dn stuff for the fsl powerpc64 boards, just remove it. It also seems that we don't support ISA on all the current corenet ds boards. So picking a primary bus seems useless, remove that function too. Signed-off-by: Kevin Hao <haokexin@gmail.com> --- arch/powerpc/platforms/85xx/corenet_ds.c | 6 ------ 1 file changed, 6 deletions(-)