[PATCH, RFC] adjust legacy IDE resource setting

[PATCH, RFC] adjust legacy IDE resource setting

[Jan Beulich]

The change to force legacy mode IDE channels' resources to fixed
non-zero values confuses (at least some versions of) X, because the
values reported by the kernel and those readable from PCI config space
aren't consistent anymore. Therefore, this patch arranges for the
respective BARs to also get updated if possible.

Signed-off-by: Jan Beulich <jbeulich@novell.com>

--- linux-2.6.20/drivers/pci/probe.c	2007-02-04 19:44:54.000000000 +0100
+++ 2.6.20-pci-ide-legacy/drivers/pci/probe.c	2007-02-14 15:54:58.000000000 +0100
@@ -639,7 +639,29 @@ static void pci_read_irq(struct pci_dev 
 	dev->irq = irq;
 }
 
-#define LEGACY_IO_RESOURCE	(IORESOURCE_IO | IORESOURCE_PCI_FIXED)
+static void change_legacy_io_resource(struct pci_dev * dev, unsigned index,
+                                      unsigned start, unsigned end)
+{
+	unsigned base = start & PCI_BASE_ADDRESS_IO_MASK;
+	unsigned len = (end | ~PCI_BASE_ADDRESS_IO_MASK) - base + 1;
+
+	if (!(pci_resource_flags(dev, index) & IORESOURCE_IO))
+		printk(KERN_WARNING "%s: cannot adjust BAR%u (not I/O)\n",
+		       pci_name(dev), index);
+	else if (pci_resource_len(dev, index) != len)
+		printk(KERN_WARNING "%s: cannot adjust BAR%u (size %04X)\n",
+		       pci_name(dev), index, (unsigned)pci_resource_len(dev, index));
+	else {
+		printk(KERN_INFO "%s: trying to change BAR%u from %04X to %04X\n",
+		       pci_name(dev), index,
+		       (unsigned)pci_resource_start(dev, index), base);
+		pci_write_config_dword(dev, PCI_BASE_ADDRESS_0 + index * 4, base);
+	}
+	pci_resource_start(dev, index) = start;
+	pci_resource_end(dev, index)   = end;
+	pci_resource_flags(dev, index) =
+		IORESOURCE_IO | IORESOURCE_PCI_FIXED | PCI_BASE_ADDRESS_SPACE_IO;
+}
 
 /**
  * pci_setup_device - fill in class and map information of a device
@@ -692,20 +714,12 @@ static int pci_setup_device(struct pci_d
 			u8 progif;
 			pci_read_config_byte(dev, PCI_CLASS_PROG, &progif);
 			if ((progif & 1) == 0) {
-				dev->resource[0].start = 0x1F0;
-				dev->resource[0].end = 0x1F7;
-				dev->resource[0].flags = LEGACY_IO_RESOURCE;
-				dev->resource[1].start = 0x3F6;
-				dev->resource[1].end = 0x3F6;
-				dev->resource[1].flags = LEGACY_IO_RESOURCE;
+				change_legacy_io_resource(dev, 0, 0x1F0, 0x1F7);
+				change_legacy_io_resource(dev, 1, 0x3F6, 0x3F6);
 			}
 			if ((progif & 4) == 0) {
-				dev->resource[2].start = 0x170;
-				dev->resource[2].end = 0x177;
-				dev->resource[2].flags = LEGACY_IO_RESOURCE;
-				dev->resource[3].start = 0x376;
-				dev->resource[3].end = 0x376;
-				dev->resource[3].flags = LEGACY_IO_RESOURCE;
+				change_legacy_io_resource(dev, 2, 0x170, 0x177);
+				change_legacy_io_resource(dev, 3, 0x376, 0x376);
 			}
 		}
 		break;

Re: [PATCH, RFC] adjust legacy IDE resource setting

[Alan]

On Wed, 14 Feb 2007 15:05:24 +0000
"Jan Beulich" <jbeulich@novell.com> wrote:

> The change to force legacy mode IDE channels' resources to fixed
> non-zero values confuses (at least some versions of) X, because the
> values reported by the kernel and those readable from PCI config space
> aren't consistent anymore. Therefore, this patch arranges for the
> respective BARs to also get updated if possible.

If X is getting confused fix X. Those BARs are *undefined* in legacy
mode. The value in them is undefined, the results that end up there if
you do write to them are undefined too. If X believes those BAR values
blindly it'll do the wrong thing in some freaky cases.

Which specific versions of X are problematic ?

As to the implementation:
	start and end as passed are the real I/O values so you don't need
to mask them that I can see.

I've no fundamental problem with writing the BAR values back to avoid
confusing some apparently broken X, but I'd like to know what X, what
circumstances and that X is also getting fixed.

Alan

Re: [PATCH, RFC] adjust legacy IDE resource setting

[Jan Beulich]

>>> Alan <alan@lxorguk.ukuu.org.uk> 14.02.07 16:40 >>>
>On Wed, 14 Feb 2007 15:05:24 +0000
>"Jan Beulich" <jbeulich@novell.com> wrote:
>
>> The change to force legacy mode IDE channels' resources to fixed
>> non-zero values confuses (at least some versions of) X, because the
>> values reported by the kernel and those readable from PCI config space
>> aren't consistent anymore. Therefore, this patch arranges for the
>> respective BARs to also get updated if possible.
>
>If X is getting confused fix X. Those BARs are *undefined* in legacy
>mode. The value in them is undefined, the results that end up there if
>you do write to them are undefined too. If X believes those BAR values
>blindly it'll do the wrong thing in some freaky cases.
>
>Which specific versions of X are problematic ?

The one I ran into problems with is reporting

X Window System Version 6.9.0
Release Date: 21 December 2005

(used in SLES10, the specific package version is xorg-x11-6.9.0-50.14)

>As to the implementation:
>	start and end as passed are the real I/O values so you don't need
>to mask them that I can see.

The masking is done primarily to (a) calculate the correct length (from a BAR's
perspective), as I don't want to write the BAR if its length doesn't match the
expectation, and (b) to properly report the new value in the printk.

>I've no fundamental problem with writing the BAR values back to avoid
>confusing some apparently broken X, but I'd like to know what X, what
>circumstances and that X is also getting fixed.

Of course I also opened a bug against X, as I too think it's doing something
wrong here.

Jan

Re: [PATCH, RFC] adjust legacy IDE resource setting

[Alan]

> The masking is done primarily to (a) calculate the correct length (from a BAR's
> perspective), as I don't want to write the BAR if its length doesn't match the
> expectation, and (b) to properly report the new value in the printk.

Ok I guess you have to do something like that since you can't properly
encode BAR 1 and BAR 3.

> Of course I also opened a bug against X, as I too think it's doing something
> wrong here.

If you can add a comment about why it is done (X problem) then it looks
fine to me.

Alan

Re: [PATCH, RFC] adjust legacy IDE resource setting

[Petri Kaukasoina]

On Wed, Feb 14, 2007 at 03:05:24PM +0000, Jan Beulich wrote:
> The change to force legacy mode IDE channels' resources to fixed
> non-zero values confuses (at least some versions of) X, because the
> values reported by the kernel and those readable from PCI config space
> aren't consistent anymore. Therefore, this patch arranges for the
> respective BARs to also get updated if possible.
> 
> Signed-off-by: Jan Beulich <jbeulich@novell.com>
> 
> --- linux-2.6.20/drivers/pci/probe.c	2007-02-04 19:44:54.000000000 +0100
> +++ 2.6.20-pci-ide-legacy/drivers/pci/probe.c	2007-02-14 15:54:58.000000000 +0100

...

On Wed, Feb 14, 2007 at 04:09:36PM +0000, Alan wrote:
> > Of course I also opened a bug against X, as I too think it's doing something
> > wrong here.
> 
> If you can add a comment about why it is done (X problem) then it looks
> fine to me.

This would be nice in 2.6.20-stable, too.