Reverting old hack

Reverting old hack

[Ralf Baechle]

Below 9f7670e4ddd940d95e48997c2da51614e5fde2cf, an old hack which I
committed in December '07 I think mostly for Cobalt machines.  This is
now getting in the way - in fact the whole loop in
pcibios_fixup_device_resources() may have to go.  So I wonder if this
old hack is still necessary.  Only testing can answer so I'm going to
put a patch to revert this into the -queue tree for 2.6.34.

  Ralf

[MIPS] PCI: Make pcibios_fixup_device_resources ignore legacy resources.

There might be other reasons why a resource might be marked as fixed
such as a PCI UART holding the system console but until we use
IORESOURCE_PCI_FIXED that way also this will work.

Signed-off-by: Ralf Baechle <ralf@linux-mips.org>

diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
index 589b745..6e6981f 100644
--- a/arch/mips/pci/pci.c
+++ b/arch/mips/pci/pci.c
@@ -242,6 +242,8 @@ static void pcibios_fixup_device_resources(struct pci_dev *dev,
 	for (i = 0; i < PCI_NUM_RESOURCES; i++) {
 		if (!dev->resource[i].start)
 			continue;
+		if (dev->resource[i].flags & IORESOURCE_PCI_FIXED)
+			continue;
 		if (dev->resource[i].flags & IORESOURCE_IO)
 			offset = hose->io_offset;
 		else if (dev->resource[i].flags & IORESOURCE_MEM)

Re: Reverting old hack

[Yoichi Yuasa]

Hi Ralf,

On Sat, 20 Feb 2010 12:31:34 +0100
Ralf Baechle <ralf@linux-mips.org> wrote:

> Below 9f7670e4ddd940d95e48997c2da51614e5fde2cf, an old hack which I
> committed in December '07 I think mostly for Cobalt machines.  This is
> now getting in the way - in fact the whole loop in
> pcibios_fixup_device_resources() may have to go.  So I wonder if this
> old hack is still necessary.  Only testing can answer so I'm going to
> put a patch to revert this into the -queue tree for 2.6.34.

It is still necessary for Cobalt.
I got the following IDE resource errors.

pata_via 0000:00:09.1: BAR 0: can't reserve [io  0xf00001f0-0xf00001f7]         
pata_via 0000:00:09.1: failed to request/iomap BARs for port 0 (errno=-16)      
pata_via 0000:00:09.1: BAR 2: can't reserve [io  0xf0000170-0xf0000177]         
pata_via 0000:00:09.1: failed to request/iomap BARs for port 1 (errno=-16)      
pata_via 0000:00:09.1: no available native port 

Yoichi

Re: Reverting old hack

[Bjorn Helgaas]

On Sat, 2010-02-20 at 21:18 +0900, Yoichi Yuasa wrote:
> Hi Ralf,
> 
> On Sat, 20 Feb 2010 12:31:34 +0100
> Ralf Baechle <ralf@linux-mips.org> wrote:
> 
> > Below 9f7670e4ddd940d95e48997c2da51614e5fde2cf, an old hack which I
> > committed in December '07 I think mostly for Cobalt machines.  This is
> > now getting in the way - in fact the whole loop in
> > pcibios_fixup_device_resources() may have to go.  So I wonder if this
> > old hack is still necessary.  Only testing can answer so I'm going to
> > put a patch to revert this into the -queue tree for 2.6.34.
> 
> It is still necessary for Cobalt.
> I got the following IDE resource errors.
> 
> pata_via 0000:00:09.1: BAR 0: can't reserve [io  0xf00001f0-0xf00001f7]         
> pata_via 0000:00:09.1: failed to request/iomap BARs for port 0 (errno=-16)      
> pata_via 0000:00:09.1: BAR 2: can't reserve [io  0xf0000170-0xf0000177]         
> pata_via 0000:00:09.1: failed to request/iomap BARs for port 1 (errno=-16)      
> pata_via 0000:00:09.1: no available native port 

Thanks for trying that out.

I'd like to understand the PCI architecture of Cobalt better.  Would you
mind turning on CONFIG_PCI_DEBUG and posting the dmesg log?

The purpose of IORESOURCE_PCI_FIXED is to say that we shouldn't reassign
the device resources.  But the pcibios_fixup_device_resources() loop
isn't *moving* devices, it's merely converting PCI bus addresses to CPU
addresses.  So I suspect that the IORESOURCE_PCI_FIXED test there is
merely covering up another bug.

For example, maybe 00:09.1 is an internal device that's behind a
different host bridge that does no address conversion, or maybe the host
bridge just handles that device specially.  If something like that is
happening, pcibios_bus_to_resource() should be broken for that device,
and fixing that should allow us to remove the IORESOURCE_PCI_FIXED test.

Bjorn

Re: Reverting old hack

[Yoichi Yuasa]

Hi Bjorn,

On Sat, 20 Feb 2010 07:57:49 -0700
Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:

> On Sat, 2010-02-20 at 21:18 +0900, Yoichi Yuasa wrote:
> > Hi Ralf,
> > 
> > On Sat, 20 Feb 2010 12:31:34 +0100
> > Ralf Baechle <ralf@linux-mips.org> wrote:
> > 
> > > Below 9f7670e4ddd940d95e48997c2da51614e5fde2cf, an old hack which I
> > > committed in December '07 I think mostly for Cobalt machines.  This is
> > > now getting in the way - in fact the whole loop in
> > > pcibios_fixup_device_resources() may have to go.  So I wonder if this
> > > old hack is still necessary.  Only testing can answer so I'm going to
> > > put a patch to revert this into the -queue tree for 2.6.34.
> > 
> > It is still necessary for Cobalt.
> > I got the following IDE resource errors.
> > 
> > pata_via 0000:00:09.1: BAR 0: can't reserve [io  0xf00001f0-0xf00001f7]         
> > pata_via 0000:00:09.1: failed to request/iomap BARs for port 0 (errno=-16)      
> > pata_via 0000:00:09.1: BAR 2: can't reserve [io  0xf0000170-0xf0000177]         
> > pata_via 0000:00:09.1: failed to request/iomap BARs for port 1 (errno=-16)      
> > pata_via 0000:00:09.1: no available native port 
> 
> Thanks for trying that out.
> 
> I'd like to understand the PCI architecture of Cobalt better.  Would you
> mind turning on CONFIG_PCI_DEBUG and posting the dmesg log?

If you want to know what happen, you can see my old e-mail. 

http://marc.info/?l=linux-kernel&m=118792430424186&w=2

> The purpose of IORESOURCE_PCI_FIXED is to say that we shouldn't reassign
> the device resources.  But the pcibios_fixup_device_resources() loop
> isn't *moving* devices, it's merely converting PCI bus addresses to CPU
> addresses.  So I suspect that the IORESOURCE_PCI_FIXED test there is
> merely covering up another bug.

Please look at the following e-mail why this fix was not applied. 

http://marc.info/?l=linux-kernel&m=119690002215135&w=2
 
> For example, maybe 00:09.1 is an internal device that's behind a
> different host bridge that does no address conversion, or maybe the host
> bridge just handles that device specially.  If something like that is
> happening, pcibios_bus_to_resource() should be broken for that device,
> and fixing that should allow us to remove the IORESOURCE_PCI_FIXED test.

Yoichi

Re: Reverting old hack

[Bjorn Helgaas]

On Sunday 21 February 2010 12:45:31 am Yoichi Yuasa wrote:
> > I'd like to understand the PCI architecture of Cobalt better.  Would you
> > mind turning on CONFIG_PCI_DEBUG and posting the dmesg log?
> 
> If you want to know what happen, you can see my old e-mail. 
> 
> http://marc.info/?l=linux-kernel&m=118792430424186&w=2

There's not much detail there.  It would save me a lot of time if
you could collect the complete dmesg log, /proc/iomem, and /proc/ioports.

Thanks,
  Bjorn

Re: Reverting old hack

[Yoichi Yuasa]

Hi Bjorn,

On Mon, 22 Feb 2010 13:55:28 -0700
Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:

> On Sunday 21 February 2010 12:45:31 am Yoichi Yuasa wrote:
> > > I'd like to understand the PCI architecture of Cobalt better.  Would you
> > > mind turning on CONFIG_PCI_DEBUG and posting the dmesg log?
> > 
> > If you want to know what happen, you can see my old e-mail. 
> > 
> > http://marc.info/?l=linux-kernel&m=118792430424186&w=2
> 
> There's not much detail there.  It would save me a lot of time if
> you could collect the complete dmesg log, /proc/iomem, and /proc/ioports.

It cannot boot without old hack.
I just got a console capture.

Yoichi

----
2> execute console=ttyS0,115200 root=/dev/sda2
elf32: 00080000 - 004220ef (800841f0) (ffffffff.80000000)
elf32: 80080000 (80080000) 3694756t + 114764t
net: interface down
Cobalt board ID: 5
pci 0000:00:07.0: BAR 6: assigned [mem 0x12000000-0x1203ffff pref]
pci 0000:00:0c.0: BAR 6: assigned [mem 0x12040000-0x1207ffff pref]
pci 0000:00:0a.0: BAR 0: assigned [mem 0x12080000-0x12080fff]
pci 0000:00:0a.0: BAR 0: set to [mem 0x12080000-0x12080fff] (PCI address [0x12080000-0x12080fff]
pci 0000:00:0a.1: BAR 0: assigned [mem 0x12081000-0x12081fff]
pci 0000:00:0a.1: BAR 0: set to [mem 0x12081000-0x12081fff] (PCI address [0x12081000-0x12081fff]
pci 0000:00:07.0: BAR 1: assigned [mem 0x12082000-0x120823ff]
pci 0000:00:07.0: BAR 1: set to [mem 0x12082000-0x120823ff] (PCI address [0x12082000-0x120823ff]
pci 0000:00:0c.0: BAR 1: assigned [mem 0x12082400-0x120827ff]
pci 0000:00:0c.0: BAR 1: set to [mem 0x12082400-0x120827ff] (PCI address [0x12082400-0x120827ff]
pci 0000:00:0a.2: BAR 0: assigned [mem 0x12082800-0x120828ff]
pci 0000:00:0a.2: BAR 0: set to [mem 0x12082800-0x120828ff] (PCI address [0x12082800-0x120828ff]
pci 0000:00:07.0: BAR 0: assigned [io  0x1000-0x107f]
pci 0000:00:07.0: BAR 0: set to [io  0x1000-0x107f] (PCI address [0x10001000-0x1000107f]
pci 0000:00:0c.0: BAR 0: assigned [io  0x1080-0x10ff]
pci 0000:00:0c.0: BAR 0: set to [io  0x1080-0x10ff] (PCI address [0x10001080-0x100010ff]
pci 0000:00:09.2: BAR 4: assigned [io  0x1400-0x141f]
pci 0000:00:09.2: BAR 4: set to [io  0x1400-0x141f] (PCI address [0x10001400-0x1000141f]
pci 0000:00:09.1: BAR 4: assigned [io  0x1420-0x142f]
pci 0000:00:09.1: BAR 4: set to [io  0x1420-0x142f] (PCI address [0x10001420-0x1000142f]
Switching to clocksource MIPS
NET: Registered protocol family 2
IP route cache hash table entries: 1024 (order: 0, 4096 bytes)
TCP established hash table entries: 4096 (order: 3, 32768 bytes)
TCP bind hash table entries: 4096 (order: 2, 16384 bytes)
TCP: Hash tables configured (established 4096 bind 4096)
TCP reno registered
UDP hash table entries: 256 (order: 0, 4096 bytes)
UDP-Lite hash table entries: 256 (order: 0, 4096 bytes)
NET: Registered protocol family 1
RPC: Registered udp transport module.
RPC: Registered tcp transport module.
RPC: Registered tcp NFSv4.1 backchannel transport module.
pci 0000:00:09.0: Activating ISA DMA hang workarounds
Installing knfsd (copyright (C) 1996 okir@monad.swb.de).
msgmni has been set to 245
io scheduler noop registered
io scheduler deadline registered
io scheduler cfq registered (default)
fb0: Cobalt server LCD frame buffer device
Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
_serial8250.0: ttyS0 at MMIO 0x1c800000 (irq = 21) is a ST16650V2_console [ttyS0] enabled, bootconsole disabled
console [ttyS0] enabled, bootconsole disabled
loop: module loaded
pata_via 0000:00:09.1: BAR 0: can't reserve [io  0xf00001f0-0xf00001f7]
pata_via 0000:00:09.1: failed to request/iomap BARs for port 0 (errno=-16)
pata_via 0000:00:09.1: BAR 2: can't reserve [io  0xf0000170-0xf0000177]
pata_via 0000:00:09.1: failed to request/iomap BARs for port 1 (errno=-16)
pata_via 0000:00:09.1: no available native port
physmap platform flash device: 00080000 at 1fc00000
Found: AMD AM29F040
physmap-flash.0: Found 1 x8 devices at 0x0 in 8-bit bank
number of JEDEC chips: 1
cmdlinepart partition parsing not available
RedBoot partition parsing not available
Using physmap partition information
Creating 1 MTD partitions on "physmap-flash.0":
0x000000000000-0x000000080000 : "firmware"
Linux Tulip driver version 1.1.15 (Feb 27, 2007)
PCI: Enabling device 0000:00:07.0 (0041 -> 0043)
tulip0: Old format EEPROM on 'Cobalt Microserver' board.  Using substitute media control info.
tulip0:  EEPROM default media type Autosense.
tulip0:  Index #0 - Media MII (#11) described by a 21142 MII PHY (3) block.
tulip0:  MII transceiver #1 config 1000 status 7809 advertising 01e1.
eth0: Digital DS21142/43 Tulip rev 65 at Port 0x1000, --:--:--:--:--:--, IRQ 19.
PCI: Enabling device 0000:00:0c.0 (0005 -> 0007)
tulip1: Old format EEPROM on 'Cobalt Microserver' board.  Using substitute media control info.
tulip1:  EEPROM default media type Autosense.
tulip1:  Index #0 - Media MII (#11) described by a 21142 MII PHY (3) block.
tulip1:  MII transceiver #1 config 1000 status 7809 advertising 01e1.
eth1: Digital DS21142/43 Tulip rev 65 at Port 0x1080, --:--:--:--:--:--, IRQ 20.
input: Cobalt buttons as /devices/platform/Cobalt buttons/input/input0
rtc_cmos rtc_cmos: rtc core: registered rtc_cmos as rtc0
rtc0: alarms up to one day, 242 bytes nvram
Registered led device: qube::front
TCP cubic registered
Initializing XFRM netlink socket
NET: Registered protocol family 17
NET: Registered protocol family 15
rtc_cmos rtc_cmos: setting system clock to 2010-02-21 23:19:20 UTC (1266794360)
VFS: Cannot open root device "sda2" or unknown-block(0,0)
Please append a correct "root=" boot option; here are the available partitions:
Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(0,0)

Re: Reverting old hack

[Bjorn Helgaas]

On Monday 22 February 2010 04:51:43 pm Yoichi Yuasa wrote:
> Hi Bjorn,
> 
> On Mon, 22 Feb 2010 13:55:28 -0700
> Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> 
> > On Sunday 21 February 2010 12:45:31 am Yoichi Yuasa wrote:
> > > > I'd like to understand the PCI architecture of Cobalt better.  Would you
> > > > mind turning on CONFIG_PCI_DEBUG and posting the dmesg log?
> > > 
> > > If you want to know what happen, you can see my old e-mail. 
> > > 
> > > http://marc.info/?l=linux-kernel&m=118792430424186&w=2
> > 
> > There's not much detail there.  It would save me a lot of time if
> > you could collect the complete dmesg log, /proc/iomem, and /proc/ioports.
> 
> It cannot boot without old hack.

I know; I meant that the information from a kernel with the old
hack would be useful.  But I think I'm starting to understand anyway.

The Linux I/O port number space is defined here:

    static struct resource cobalt_io_resource = {
        .start  = 0x1000,
        .end    = GT_DEF_PCI0_IO_SIZE - 1,  /* 0x1ffffff */

[As an aside, I'm not sure 0x1000 is the correct start -- for example,
I think Linux I/O port 0x1f0 is forwarded by the host bridge.]

The corresponding PCI I/O port numbers are determined by the PCI
I/O decoder address, so I agree that we need the io_offset to convert
between the Linux port numbers and ports that appear on the PCI bus.

I think the IDE device is a problem because pci_setup_device() fills
in legacy resources with ports 0x1f0-0x1f7, etc.  We expect those
resources to contain PCI bus addresses at this point, but we could
never see those addresses on the Cobalt PCI bus (we would only see
things in the range 0x10000000-0x11ffffff).

When we convert 0x1f0 with pcibios_bus_to_resource() (or with
pcibios_fixup_device_resources() without the IORESOURCE_PCI_FIXED
hack), we get 0x1f0 + 0xf0000000 == 0xf00001f0, when we want 0x1f0
instead.

> pata_via 0000:00:09.1: BAR 0: can't reserve [io  0xf00001f0-0xf00001f7]

I still don't know the best way to fix this, but does this make sense
so far?

Bjorn

Re: Reverting old hack

[Yoichi Yuasa]

On Mon, 22 Feb 2010 17:15:24 -0700
Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:

> On Monday 22 February 2010 04:51:43 pm Yoichi Yuasa wrote:
> > Hi Bjorn,
> > 
> > On Mon, 22 Feb 2010 13:55:28 -0700
> > Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> > 
> > > On Sunday 21 February 2010 12:45:31 am Yoichi Yuasa wrote:
> > > > > I'd like to understand the PCI architecture of Cobalt better.  Would you
> > > > > mind turning on CONFIG_PCI_DEBUG and posting the dmesg log?
> > > > 
> > > > If you want to know what happen, you can see my old e-mail. 
> > > > 
> > > > http://marc.info/?l=linux-kernel&m=118792430424186&w=2
> > > 
> > > There's not much detail there.  It would save me a lot of time if
> > > you could collect the complete dmesg log, /proc/iomem, and /proc/ioports.
> > 
> > It cannot boot without old hack.
> 
> I know; I meant that the information from a kernel with the old
> hack would be useful.  But I think I'm starting to understand anyway.
> 
> The Linux I/O port number space is defined here:
> 
>     static struct resource cobalt_io_resource = {
>         .start  = 0x1000,
>         .end    = GT_DEF_PCI0_IO_SIZE - 1,  /* 0x1ffffff */
> 
> [As an aside, I'm not sure 0x1000 is the correct start -- for example,
> I think Linux I/O port 0x1f0 is forwarded by the host bridge.]

This is the space(0x0-0xfff) for the fixed address devices(PIC, RTC, DMA(just reserved)...). 

$ cat /proc/ioports                                                      
00000000-0000001f : reserved                                                    
00000020-00000021 : pic1                                                        
00000060-0000006f : reserved                                                    
00000070-00000077 : rtc_cmos                                                    
  00000070-00000077 : rtc0                                                      
00000080-0000008f : reserved                                                    
000000a0-000000a1 : pic2                                                        
000000c0-000000df : reserved                                                    
00000170-00000177 : pata_via                                                    
000001f0-000001f7 : pata_via                                                    
00000376-00000376 : pata_via                                                    
000003f6-000003f6 : pata_via                                                    
00001000-01ffffff : PCI I/O                                                     
  00001000-0000107f : 0000:00:07.0                                              
    00001000-0000107f : tulip                                                   
  00001080-000010ff : 0000:00:0c.0                                              
    00001080-000010ff : tulip                                                   
  00001400-0000141f : 0000:00:09.2                                              
  00001420-0000142f : 0000:00:09.1                                              
    00001420-0000142f : pata_via
 
> The corresponding PCI I/O port numbers are determined by the PCI
> I/O decoder address, so I agree that we need the io_offset to convert
> between the Linux port numbers and ports that appear on the PCI bus.
> 
> I think the IDE device is a problem because pci_setup_device() fills
> in legacy resources with ports 0x1f0-0x1f7, etc.  We expect those
> resources to contain PCI bus addresses at this point, but we could
> never see those addresses on the Cobalt PCI bus (we would only see
> things in the range 0x10000000-0x11ffffff).
> 
> When we convert 0x1f0 with pcibios_bus_to_resource() (or with
> pcibios_fixup_device_resources() without the IORESOURCE_PCI_FIXED
> hack), we get 0x1f0 + 0xf0000000 == 0xf00001f0, when we want 0x1f0
> instead.
> 
> > pata_via 0000:00:09.1: BAR 0: can't reserve [io  0xf00001f0-0xf00001f7]
> 
> I still don't know the best way to fix this, but does this make sense
> so far?

That makes sense.

Yoichi

Re: Reverting old hack

[Bjorn Helgaas]

On Sat, 2010-02-20 at 21:18 +0900, Yoichi Yuasa wrote:
> Hi Ralf,
> 
> On Sat, 20 Feb 2010 12:31:34 +0100
> Ralf Baechle <ralf@linux-mips.org> wrote:
> 
> > Below 9f7670e4ddd940d95e48997c2da51614e5fde2cf, an old hack which I
> > committed in December '07 I think mostly for Cobalt machines.  This is
> > now getting in the way - in fact the whole loop in
> > pcibios_fixup_device_resources() may have to go.  So I wonder if this
> > old hack is still necessary.  Only testing can answer so I'm going to
> > put a patch to revert this into the -queue tree for 2.6.34.
> 
> It is still necessary for Cobalt.
> I got the following IDE resource errors.
> 
> pata_via 0000:00:09.1: BAR 0: can't reserve [io  0xf00001f0-0xf00001f7]         
> pata_via 0000:00:09.1: failed to request/iomap BARs for port 0 (errno=-16)      
> pata_via 0000:00:09.1: BAR 2: can't reserve [io  0xf0000170-0xf0000177]         
> pata_via 0000:00:09.1: failed to request/iomap BARs for port 1 (errno=-16)      
> pata_via 0000:00:09.1: no available native port 

I think the problem is that cobalt_io_resource contains PCI bus
addresses when it should contain CPU addresses:

    static struct resource cobalt_io_resource = {
        .start  = 0x1000,
        .end    = GT_DEF_PCI0_IO_SIZE - 1,
        .name   = "PCI I/O",
        .flags  = IORESOURCE_IO,
    };

I think .start and .end need to be adjusted by GT_DEF_PCI0_IO_BASE.

Bjorn

Re: Reverting old hack

[Bjorn Helgaas]

On Sat, 2010-02-20 at 19:57 -0700, Bjorn Helgaas wrote:
> On Sat, 2010-02-20 at 21:18 +0900, Yoichi Yuasa wrote:
> > Hi Ralf,
> > 
> > On Sat, 20 Feb 2010 12:31:34 +0100
> > Ralf Baechle <ralf@linux-mips.org> wrote:
> > 
> > > Below 9f7670e4ddd940d95e48997c2da51614e5fde2cf, an old hack which I
> > > committed in December '07 I think mostly for Cobalt machines.  This is
> > > now getting in the way - in fact the whole loop in
> > > pcibios_fixup_device_resources() may have to go.  So I wonder if this
> > > old hack is still necessary.  Only testing can answer so I'm going to
> > > put a patch to revert this into the -queue tree for 2.6.34.
> > 
> > It is still necessary for Cobalt.
> > I got the following IDE resource errors.
> > 
> > pata_via 0000:00:09.1: BAR 0: can't reserve [io  0xf00001f0-0xf00001f7]         
> > pata_via 0000:00:09.1: failed to request/iomap BARs for port 0 (errno=-16)      
> > pata_via 0000:00:09.1: BAR 2: can't reserve [io  0xf0000170-0xf0000177]         
> > pata_via 0000:00:09.1: failed to request/iomap BARs for port 1 (errno=-16)      
> > pata_via 0000:00:09.1: no available native port 
> 
> I think the problem is that cobalt_io_resource contains PCI bus
> addresses when it should contain CPU addresses:
> 
>     static struct resource cobalt_io_resource = {
>         .start  = 0x1000,
>         .end    = GT_DEF_PCI0_IO_SIZE - 1,
>         .name   = "PCI I/O",
>         .flags  = IORESOURCE_IO,
>     };
> 
> I think .start and .end need to be adjusted by GT_DEF_PCI0_IO_BASE.

Sorry, this was a half-baked answer.  This would probably fix the
reservation conflicts, but of course we'll still have a problem when we
remove the IORESOURCE_PCI_FIXED check.  I'll work on it more and come up
with a patch.

Bjorn

Re: Reverting old hack

[Bjorn Helgaas]

On Sat, 2010-02-20 at 21:18 +0900, Yoichi Yuasa wrote:
> Hi Ralf,
> 
> On Sat, 20 Feb 2010 12:31:34 +0100
> Ralf Baechle <ralf@linux-mips.org> wrote:
> 
> > Below 9f7670e4ddd940d95e48997c2da51614e5fde2cf, an old hack which I
> > committed in December '07 I think mostly for Cobalt machines.  This is
> > now getting in the way - in fact the whole loop in
> > pcibios_fixup_device_resources() may have to go.  So I wonder if this
> > old hack is still necessary.  Only testing can answer so I'm going to
> > put a patch to revert this into the -queue tree for 2.6.34.
> 
> It is still necessary for Cobalt.
> I got the following IDE resource errors.
> 
> pata_via 0000:00:09.1: BAR 0: can't reserve [io  0xf00001f0-0xf00001f7]         
> pata_via 0000:00:09.1: failed to request/iomap BARs for port 0 (errno=-16)      
> pata_via 0000:00:09.1: BAR 2: can't reserve [io  0xf0000170-0xf0000177]         
> pata_via 0000:00:09.1: failed to request/iomap BARs for port 1 (errno=-16)      
> pata_via 0000:00:09.1: no available native port 

I think Cobalt needs something like the patch below, because I think in
your working system, pata_via is using I/O port 0x1f0, not 0xf00001f0.
That means the the port the driver sees in the pci_dev resource is
identical to the port number that appears on the PCI bus, so there is no
io_offset.

There are a few other places that may set non-zero io_offset values:
bcm1480, bcm1480ht. txx9_alloc_pci_controller(), bridge_probe(), and
octeon_pcie_setup().  I don't know whether they have similar issues.



commit 7378269220d477118257d898bec9173743675f5e
Author: Bjorn Helgaas <bjorn.helgaas@hp.com>
Date:   Sat Feb 20 07:52:29 2010 -0700

    [MIPS] remove Cobalt I/O space offset
    
    On Cobalt, "inb(x)" produces an I/O port access to port "x" on the PCI
    bus, which means the io_offset is zero and CPU (resource) addresses are
    identical to PCI bus addresses.  Correcting this means we can remove
    the IORESOURCE_PCI_FIXED check from pcibios_fixup_device_resources().
    
    The io_map_base is used internally by pci_iomap(), inb(), and other I/O
    port access functions to generate an MMIO access to the address that
    produces the desired I/O port PCI transaction.
    
    [Cobalt plat_mem_setup() does this:
      set_io_port_base(CKSEG1ADDR(GT_DEF_PCI0_IO_BASE));
    rather than using cobalt_pci_controller.io_map_base, but the value's
    the same, and I don't know enough to clean that up.]
    
    See http://lkml.org/lkml/2007/7/29/27
    
    Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

diff --git a/arch/mips/cobalt/pci.c b/arch/mips/cobalt/pci.c
index cfce7af..84aa205 100644
--- a/arch/mips/cobalt/pci.c
+++ b/arch/mips/cobalt/pci.c
@@ -34,7 +34,6 @@ static struct pci_controller cobalt_pci_controller = {
 	.pci_ops	= &gt64xxx_pci0_ops,
 	.mem_resource	= &cobalt_mem_resource,
 	.io_resource	= &cobalt_io_resource,
-	.io_offset	= 0 - GT_DEF_PCI0_IO_BASE,
 	.io_map_base	= CKSEG1ADDR(GT_DEF_PCI0_IO_BASE),
 };
 
diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
index f87f5e1..38bc280 100644
--- a/arch/mips/pci/pci.c
+++ b/arch/mips/pci/pci.c
@@ -251,8 +251,6 @@ static void pcibios_fixup_device_resources(struct pci_dev *dev,
 	for (i = 0; i < PCI_NUM_RESOURCES; i++) {
 		if (!dev->resource[i].start)
 			continue;
-		if (dev->resource[i].flags & IORESOURCE_PCI_FIXED)
-			continue;
 		if (dev->resource[i].flags & IORESOURCE_IO)
 			offset = hose->io_offset;
 		else if (dev->resource[i].flags & IORESOURCE_MEM)

Re: Reverting old hack

[Yoichi Yuasa]

Hi Bjorn,

On Sun, 21 Feb 2010 22:07:37 -0700
Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:

> On Sat, 2010-02-20 at 21:18 +0900, Yoichi Yuasa wrote:
> > Hi Ralf,
> > 
> > On Sat, 20 Feb 2010 12:31:34 +0100
> > Ralf Baechle <ralf@linux-mips.org> wrote:
> > 
> > > Below 9f7670e4ddd940d95e48997c2da51614e5fde2cf, an old hack which I
> > > committed in December '07 I think mostly for Cobalt machines.  This is
> > > now getting in the way - in fact the whole loop in
> > > pcibios_fixup_device_resources() may have to go.  So I wonder if this
> > > old hack is still necessary.  Only testing can answer so I'm going to
> > > put a patch to revert this into the -queue tree for 2.6.34.
> > 
> > It is still necessary for Cobalt.
> > I got the following IDE resource errors.
> > 
> > pata_via 0000:00:09.1: BAR 0: can't reserve [io  0xf00001f0-0xf00001f7]         
> > pata_via 0000:00:09.1: failed to request/iomap BARs for port 0 (errno=-16)      
> > pata_via 0000:00:09.1: BAR 2: can't reserve [io  0xf0000170-0xf0000177]         
> > pata_via 0000:00:09.1: failed to request/iomap BARs for port 1 (errno=-16)      
> > pata_via 0000:00:09.1: no available native port 
> 
> I think Cobalt needs something like the patch below, because I think in
> your working system, pata_via is using I/O port 0x1f0, not 0xf00001f0.
> That means the the port the driver sees in the pci_dev resource is
> identical to the port number that appears on the PCI bus, so there is no
> io_offset.
> 
> There are a few other places that may set non-zero io_offset values:
> bcm1480, bcm1480ht. txx9_alloc_pci_controller(), bridge_probe(), and
> octeon_pcie_setup().  I don't know whether they have similar issues.
> 
> 
> 
> commit 7378269220d477118257d898bec9173743675f5e
> Author: Bjorn Helgaas <bjorn.helgaas@hp.com>
> Date:   Sat Feb 20 07:52:29 2010 -0700
> 
>     [MIPS] remove Cobalt I/O space offset
>     
>     On Cobalt, "inb(x)" produces an I/O port access to port "x" on the PCI
>     bus, which means the io_offset is zero and CPU (resource) addresses are
>     identical to PCI bus addresses.  Correcting this means we can remove
>     the IORESOURCE_PCI_FIXED check from pcibios_fixup_device_resources().
>     
>     The io_map_base is used internally by pci_iomap(), inb(), and other I/O
>     port access functions to generate an MMIO access to the address that
>     produces the desired I/O port PCI transaction.
>     
>     [Cobalt plat_mem_setup() does this:
>       set_io_port_base(CKSEG1ADDR(GT_DEF_PCI0_IO_BASE));
>     rather than using cobalt_pci_controller.io_map_base, but the value's
>     the same, and I don't know enough to clean that up.]
>     
>     See http://lkml.org/lkml/2007/7/29/27
>     
>     Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
> 
> diff --git a/arch/mips/cobalt/pci.c b/arch/mips/cobalt/pci.c
> index cfce7af..84aa205 100644
> --- a/arch/mips/cobalt/pci.c
> +++ b/arch/mips/cobalt/pci.c
> @@ -34,7 +34,6 @@ static struct pci_controller cobalt_pci_controller = {
>  	.pci_ops	= &gt64xxx_pci0_ops,
>  	.mem_resource	= &cobalt_mem_resource,
>  	.io_resource	= &cobalt_io_resource,
> -	.io_offset	= 0 - GT_DEF_PCI0_IO_BASE,
>  	.io_map_base	= CKSEG1ADDR(GT_DEF_PCI0_IO_BASE),
>  };

io_offset is necessary for DEC tulip on Cobalt. 
It doesn't work when this patch is applied. 

Yoichi

Re: Reverting old hack

[Ralf Baechle]

On Sun, Feb 21, 2010 at 10:07:37PM -0700, Bjorn Helgaas wrote:

> > > Below 9f7670e4ddd940d95e48997c2da51614e5fde2cf, an old hack which I
> > > committed in December '07 I think mostly for Cobalt machines.  This is
> > > now getting in the way - in fact the whole loop in
> > > pcibios_fixup_device_resources() may have to go.  So I wonder if this
> > > old hack is still necessary.  Only testing can answer so I'm going to
> > > put a patch to revert this into the -queue tree for 2.6.34.
> > 
> > It is still necessary for Cobalt.
> > I got the following IDE resource errors.
> > 
> > pata_via 0000:00:09.1: BAR 0: can't reserve [io  0xf00001f0-0xf00001f7]         
> > pata_via 0000:00:09.1: failed to request/iomap BARs for port 0 (errno=-16)      
> > pata_via 0000:00:09.1: BAR 2: can't reserve [io  0xf0000170-0xf0000177]         
> > pata_via 0000:00:09.1: failed to request/iomap BARs for port 1 (errno=-16)      
> > pata_via 0000:00:09.1: no available native port 
> 
> I think Cobalt needs something like the patch below, because I think in
> your working system, pata_via is using I/O port 0x1f0, not 0xf00001f0.
> That means the the port the driver sees in the pci_dev resource is
> identical to the port number that appears on the PCI bus, so there is no
> io_offset.
> 
> There are a few other places that may set non-zero io_offset values:
> bcm1480, bcm1480ht. txx9_alloc_pci_controller(), bridge_probe(), and
> octeon_pcie_setup().  I don't know whether they have similar issues.

It's a while since I last looked into this but here's how things afair
are working on a MIPS-based Cobalt system.

The system is based on a MIPS processor and a GT-64111 system controller.
Addresses within a certain CPU address range are passed to the PCI bus as
I/O cycles without address cycles.  Since memory is starting at CPU address
zero (and has to because of the processors used), that address window has
to get mapped somewhere else.  So a CPU access to some virtual address gets
translated to physical address 0xf00001f0.  The GT-64111 passes this to the
PCI bus as I/O port address 0xf00001f0.  Finally the VT82C586 chip which
only decodes the low 16 bits drops treats this as an I/O port space address
0x1f0.

  Ralf

Re: Reverting old hack

[Bjorn Helgaas]

On Monday 22 February 2010 06:28:30 am Ralf Baechle wrote:
> It's a while since I last looked into this but here's how things afair
> are working on a MIPS-based Cobalt system.
> 
> The system is based on a MIPS processor and a GT-64111 system controller.
> Addresses within a certain CPU address range are passed to the PCI bus as
> I/O cycles without address cycles.  Since memory is starting at CPU address
> zero (and has to because of the processors used), that address window has
> to get mapped somewhere else.  So a CPU access to some virtual address gets
> translated to physical address 0xf00001f0.  The GT-64111 passes this to the
> PCI bus as I/O port address 0xf00001f0.  Finally the VT82C586 chip which
> only decodes the low 16 bits drops treats this as an I/O port space address
> 0x1f0.

Yoichi, can you try the patch below?  I think this is basically the
approach Ben suggested long ago:
    http://marc.info/?l=linux-kernel&m=119733290624544&w=2

Thanks,
  Bjorn


diff --git a/arch/mips/pci/fixup-cobalt.c b/arch/mips/pci/fixup-cobalt.c
index 9553b14..7579551 100644
--- a/arch/mips/pci/fixup-cobalt.c
+++ b/arch/mips/pci/fixup-cobalt.c
@@ -51,6 +51,67 @@ static void qube_raq_galileo_early_fixup(struct pci_dev *dev)
 DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_MARVELL, PCI_DEVICE_ID_MARVELL_GT64111,
 	 qube_raq_galileo_early_fixup);
 
+static void __devinit cobalt_legacy_ide_resource_fixup(struct pci_dev *dev,
+						       struct resource *res)
+{
+	struct pci_controller *hose = (struct pci_controller *)dev->sysdata;
+	unsigned long offset = hose->io_offset;
+	struct resource orig = *res;
+
+	if (!(res->flags & IORESOURCE_IO) ||
+	    !(res->flags & IORESOURCE_PCI_FIXED))
+		return;
+
+	res->start -= offset;
+	res->end -= offset;
+	dev_printk(KERN_DEBUG, &dev->dev, "converted legacy %pR to bus %pR\n",
+		   &orig, res);
+}
+
+static void __devinit cobalt_legacy_ide_fixup(struct pci_dev *dev)
+{
+	u32 class;
+	u8 progif;
+
+	/*
+	 * If the IDE controller is in legacy mode, pci_setup_device() fills in
+	 * the resources with the legacy addresses that normally appear on the
+	 * PCI bus, just as if we had read them from a BAR.
+	 *
+	 * However, with the GT-64111, those legacy addresses, e.g., 0x1f0,
+	 * will never appear on the PCI bus because it converts memory accesses
+	 * in the PCI I/O region (which is never at address zero) into I/O port
+	 * accesses with no address translation.
+	 *
+	 * For example, if GT_DEF_PCI0_IO_BASE is 0x10000000, a load or store
+	 * to physical address 0x100001f0 will become a PCI access to I/O port
+	 * 0x100001f0.  There's no way to generate an access to I/O port 0x1f0,
+	 * but the VT82C586 IDE controller does respond at 0x100001f0 because
+	 * it only decodes the low 16 bits of the address.
+	 *
+	 * When this quirk runs, the pci_dev resources should contain bus
+	 * addresses, not Linux I/O port numbers, so convert legacy addresses
+	 * like 0x1f0 to bus addresses like 0x100001f0.  Later, we'll convert
+	 * them back with pcibios_fixup_bus() or pcibios_bus_to_resource().
+	 */
+	class = dev->class >> 8;
+	if (class != PCI_CLASS_STORAGE_IDE)
+		return;
+
+	pci_read_config_byte(dev, PCI_CLASS_PROG, &progif);
+	if ((progif & 1) == 0) {
+		cobalt_legacy_ide_resource_fixup(dev, &dev->resource[0]);
+		cobalt_legacy_ide_resource_fixup(dev, &dev->resource[1]);
+	}
+	if ((progif & 4) == 0) {
+		cobalt_legacy_ide_resource_fixup(dev, &dev->resource[2]);
+		cobalt_legacy_ide_resource_fixup(dev, &dev->resource[3]);
+	}
+}
+
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C586_1,
+	  cobalt_legacy_ide_fixup);
+
 static void qube_raq_via_bmIDE_fixup(struct pci_dev *dev)
 {
 	unsigned short cfgword;
diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
index f87f5e1..38bc280 100644
--- a/arch/mips/pci/pci.c
+++ b/arch/mips/pci/pci.c
@@ -251,8 +251,6 @@ static void pcibios_fixup_device_resources(struct pci_dev *dev,
 	for (i = 0; i < PCI_NUM_RESOURCES; i++) {
 		if (!dev->resource[i].start)
 			continue;
-		if (dev->resource[i].flags & IORESOURCE_PCI_FIXED)
-			continue;
 		if (dev->resource[i].flags & IORESOURCE_IO)
 			offset = hose->io_offset;
 		else if (dev->resource[i].flags & IORESOURCE_MEM)

Re: Reverting old hack

[Yoichi Yuasa]

On Tue, 23 Feb 2010 16:01:14 -0700
Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:

> On Monday 22 February 2010 06:28:30 am Ralf Baechle wrote:
> > It's a while since I last looked into this but here's how things afair
> > are working on a MIPS-based Cobalt system.
> > 
> > The system is based on a MIPS processor and a GT-64111 system controller.
> > Addresses within a certain CPU address range are passed to the PCI bus as
> > I/O cycles without address cycles.  Since memory is starting at CPU address
> > zero (and has to because of the processors used), that address window has
> > to get mapped somewhere else.  So a CPU access to some virtual address gets
> > translated to physical address 0xf00001f0.  The GT-64111 passes this to the
> > PCI bus as I/O port address 0xf00001f0.  Finally the VT82C586 chip which
> > only decodes the low 16 bits drops treats this as an I/O port space address
> > 0x1f0.
> 
> Yoichi, can you try the patch below?  I think this is basically the
> approach Ben suggested long ago:
>     http://marc.info/?l=linux-kernel&m=119733290624544&w=2

It works fine with 2.6.34 queue tree.
pci.c change is already committed by Ralf.

Thanks,

Yoichi

Re: Reverting old hack

[Ralf Baechle]

On Wed, Feb 24, 2010 at 09:03:33AM +0900, Yoichi Yuasa wrote:

> > approach Ben suggested long ago:
> >     http://marc.info/?l=linux-kernel&m=119733290624544&w=2
> 
> It works fine with 2.6.34 queue tree.
> pci.c change is already committed by Ralf.

Which I just dropped from queue.  To keep the tree bisectable removal of
the old hack and adding the fixup should be done in the same patch so I'd
go for Bjorn's patch.

There is another somewhat theoretical correctness issue.  Because the
VIA SuperIO chip only decodes 24 bits of address space but port address
space currently being configured as 32MB there is the theoretical
possibility of I/O port addresses that alias with legacy addresses getting
allocated.

The complicated solution is to reserve all address range that potencially
could cause such aliases.  But with the PCI spec limiting port allocations
for devices to a maximum of 256 bytes 16MB of port address space already is
way more than one would ever expect to be used so I suggest to just limit
the port address space to 16MB.

Could you test the patch below?

  Ralf

Signed-off-by: Ralf Baechle <ralf@linux-mips.org>

 arch/mips/cobalt/pci.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/mips/cobalt/pci.c b/arch/mips/cobalt/pci.c
index cfce7af..85ec9cc 100644
--- a/arch/mips/cobalt/pci.c
+++ b/arch/mips/cobalt/pci.c
@@ -25,7 +25,7 @@ static struct resource cobalt_mem_resource = {
 
 static struct resource cobalt_io_resource = {
 	.start	= 0x1000,
-	.end	= GT_DEF_PCI0_IO_SIZE - 1,
+	.end	= 0xffffffUL,
 	.name	= "PCI I/O",
 	.flags	= IORESOURCE_IO,
 };

Re: Reverting old hack

[Bjorn Helgaas]

On Wednesday 24 February 2010 09:41:00 am Ralf Baechle wrote:
> On Wed, Feb 24, 2010 at 09:03:33AM +0900, Yoichi Yuasa wrote:
> 
> > > approach Ben suggested long ago:
> > >     http://marc.info/?l=linux-kernel&m=119733290624544&w=2
> > 
> > It works fine with 2.6.34 queue tree.
> > pci.c change is already committed by Ralf.
> 
> Which I just dropped from queue.  To keep the tree bisectable removal of
> the old hack and adding the fixup should be done in the same patch so I'd
> go for Bjorn's patch.

Right, thanks.

> There is another somewhat theoretical correctness issue.  Because the
> VIA SuperIO chip only decodes 24 bits of address space but port address
> space currently being configured as 32MB there is the theoretical
> possibility of I/O port addresses that alias with legacy addresses getting
> allocated.

Does this mean my comment:

+        * but the VT82C586 IDE controller does respond at 0x100001f0 because
+        * it only decodes the low 16 bits of the address.

should say "24 bits" instead of "16 bits"?

Bjorn

Re: Reverting old hack

[Ralf Baechle]

On Wed, Feb 24, 2010 at 09:59:56AM -0700, Bjorn Helgaas wrote:

> > There is another somewhat theoretical correctness issue.  Because the
> > VIA SuperIO chip only decodes 24 bits of address space but port address
> > space currently being configured as 32MB there is the theoretical
> > possibility of I/O port addresses that alias with legacy addresses getting
> > allocated.
> 
> Does this mean my comment:
> 
> +        * but the VT82C586 IDE controller does respond at 0x100001f0 because
> +        * it only decodes the low 16 bits of the address.
> 
> should say "24 bits" instead of "16 bits"?

Yes, afair.

  Ralf

Re: Reverting old hack

[Benjamin Herrenschmidt]

On Wed, 2010-02-24 at 17:41 +0100, Ralf Baechle wrote:
> 
> The complicated solution is to reserve all address range that potencially
> could cause such aliases.  But with the PCI spec limiting port allocations
> for devices to a maximum of 256 bytes 16MB of port address space already is
> way more than one would ever expect to be used so I suggest to just limit
> the port address space to 16MB.
> 
> Could you test the patch below?

On PPC I set the top level IO resource to no more than 1M, actually even
as small as 64K on some bridges. There's no point doing more, x86 ony
have 64K of IO space anyways :-)

Cheers,
Ben.

Re: Reverting old hack

[Yoichi Yuasa]

On Wed, 24 Feb 2010 17:41:00 +0100
Ralf Baechle <ralf@linux-mips.org> wrote:

> On Wed, Feb 24, 2010 at 09:03:33AM +0900, Yoichi Yuasa wrote:
> 
> > > approach Ben suggested long ago:
> > >     http://marc.info/?l=linux-kernel&m=119733290624544&w=2
> > 
> > It works fine with 2.6.34 queue tree.
> > pci.c change is already committed by Ralf.
> 
> Which I just dropped from queue.  To keep the tree bisectable removal of
> the old hack and adding the fixup should be done in the same patch so I'd
> go for Bjorn's patch.
> 
> There is another somewhat theoretical correctness issue.  Because the
> VIA SuperIO chip only decodes 24 bits of address space but port address
> space currently being configured as 32MB there is the theoretical
> possibility of I/O port addresses that alias with legacy addresses getting
> allocated.
> 
> The complicated solution is to reserve all address range that potencially
> could cause such aliases.  But with the PCI spec limiting port allocations
> for devices to a maximum of 256 bytes 16MB of port address space already is
> way more than one would ever expect to be used so I suggest to just limit
> the port address space to 16MB.
> 
> Could you test the patch below?

It has no problem.

$ cat /proc/ioports                                                      
00000000-0000001f : reserved                                                    
00000020-00000021 : pic1                                                        
00000060-0000006f : reserved                                                    
00000070-00000077 : rtc_cmos                                                    
  00000070-00000077 : rtc0                                                      
00000080-0000008f : reserved                                                    
000000a0-000000a1 : pic2                                                        
000000c0-000000df : reserved                                                    
00000170-00000177 : pata_via                                                    
000001f0-000001f7 : pata_via                                                    
00000376-00000376 : pata_via                                                    
000003f6-000003f6 : pata_via                                                    
00001000-00ffffff : PCI I/O                                                     
  00001000-0000107f : 0000:00:07.0                                              
    00001000-0000107f : tulip                                                   
  00001080-000010ff : 0000:00:0c.0                                              
    00001080-000010ff : tulip                                                   
  00001400-0000141f : 0000:00:09.2                                              
  00001420-0000142f : 0000:00:09.1                                              
    00001420-0000142f : pata_via

Yoichi

Re: Reverting old hack

[Ralf Baechle]

On Thu, Feb 25, 2010 at 05:39:56PM +0900, Yoichi Yuasa wrote:

> 
> It has no problem.

Thanks, so I've queued it for 2.6.34.

  Ralf

Re: Reverting old hack

[Ralf Baechle]

On Tue, Feb 23, 2010 at 04:01:14PM -0700, Bjorn Helgaas wrote:

> Yoichi, can you try the patch below?  I think this is basically the
> approach Ben suggested long ago:
>     http://marc.info/?l=linux-kernel&m=119733290624544&w=2

No Signed-off-by ...

  Ralf

Re: Reverting old hack

[Bjorn Helgaas]

On Wednesday 24 February 2010 09:13:41 am Ralf Baechle wrote:
> On Tue, Feb 23, 2010 at 04:01:14PM -0700, Bjorn Helgaas wrote:
> 
> > Yoichi, can you try the patch below?  I think this is basically the
> > approach Ben suggested long ago:
> >     http://marc.info/?l=linux-kernel&m=119733290624544&w=2
> 
> No Signed-off-by ...

Yep, I'll update it with a proper changelog and S-O-B.  I want to
look at the other platforms that have non-zero io_offsets; they
might need similar quirks.

Bjorn