[PATCH 1/3] x86/pci: fix -1 calling to e820_all_mapped with mmconfig

[PATCH 1/3] x86/pci: fix -1 calling to e820_all_mapped with mmconfig

[Yinghai Lu]

e820_all_mapped need end is (addr + size) instead of (addr + size - 1)

[Impact: fix -1 offset calling]

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 arch/x86/pci/mmconfig-shared.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Index: linux-2.6/arch/x86/pci/mmconfig-shared.c
===================================================================
--- linux-2.6.orig/arch/x86/pci/mmconfig-shared.c
+++ linux-2.6/arch/x86/pci/mmconfig-shared.c
@@ -375,7 +375,7 @@ static acpi_status __init check_mcfg_res
 		if (!fixmem32)
 			return AE_OK;
 		if ((mcfg_res->start >= fixmem32->address) &&
-		    (mcfg_res->end < (fixmem32->address +
+		    (mcfg_res->end <= (fixmem32->address +
 				      fixmem32->address_length))) {
 			mcfg_res->flags = 1;
 			return AE_CTRL_TERMINATE;
@@ -392,7 +392,7 @@ static acpi_status __init check_mcfg_res
 		return AE_OK;
 
 	if ((mcfg_res->start >= address.minimum) &&
-	    (mcfg_res->end < (address.minimum + address.address_length))) {
+	    (mcfg_res->end <= (address.minimum + address.address_length))) {
 		mcfg_res->flags = 1;
 		return AE_CTRL_TERMINATE;
 	}
@@ -439,7 +439,7 @@ static int __init is_mmconf_reserved(che
 	u64 old_size = size;
 	int valid = 0;
 
-	while (!is_reserved(addr, addr + size - 1, E820_RESERVED)) {
+	while (!is_reserved(addr, addr + size, E820_RESERVED)) {
 		size >>= 1;
 		if (size < (16UL<<20))
 			break;

[PATCH 2/3] pci: don't assume pref memio are 64bit -v2

[Yinghai Lu]

one system with 4g installed ( there is 1g hole)

when 4G installed.
BIOS put ACPI etc need the hole
[    0.000000] BIOS-provided physical RAM map:
[    0.000000]  BIOS-e820: 0000000000000000 - 000000000009bc00 (usable)
[    0.000000]  BIOS-e820: 000000000009bc00 - 00000000000a0000 (reserved)
[    0.000000]  BIOS-e820: 00000000000e3000 - 0000000000100000 (reserved)
[    0.000000]  BIOS-e820: 0000000000100000 - 00000000bffa0000 (usable)
[    0.000000]  BIOS-e820: 00000000bffa0000 - 00000000bffae000 (ACPI data)
[    0.000000]  BIOS-e820: 00000000bffae000 - 00000000bfff0000 (ACPI NVS)
[    0.000000]  BIOS-e820: 00000000bfff0000 - 00000000c0000000 (reserved)
[    0.000000]  BIOS-e820: 00000000fee00000 - 00000000fee01000 (reserved)
[    0.000000]  BIOS-e820: 00000000ffb00000 - 0000000100000000 (reserved)
[    0.000000]  BIOS-e820: 0000000100000000 - 0000000140000000 (usable)
so in kernel resource will be reserved for 0xbffa0000 - 0xbfff0000 for ACPI
0x100000 -  0xbffa0000 for RAM...

and BIOS set
[    0.240007] pci 0000:00:01.0: bridge 64bit mmio pref: [0xbdf00000-0xddefffff]
[    0.237102] pci 0000:01:00.0: reg 10 32bit mmio: [0xc0000000-0xcfffffff]
that is conflict with reserved res. so it can not be reserved Kernel.

then Kernel try to get range from 0x140000000 ( above the RAM, 5G and above 4g)
and set let the bridge to use it, and ATI cards to use it.

but the problem is that ATI only support 32bit ...

we should not assign 64bit range to pci device that only take 32bit pref

try to set PCI_PREF_RANGE_TYPE_64 in 64bit resource of pci_device (besides in pci_bridge),
and make the bus resource only have that bit set when all device under that do support
64bit pref mem
then use that flag to decide the max limit for find/request.

[Impact: do assign wrong range to device that doesn't support it]

v2: fix b_res->flags and logic and passing result.

Reported-and-tested-by: Yannick <yannick.roehlly@free.fr>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 drivers/pci/bus.c       |    8 +++++++-
 drivers/pci/probe.c     |    8 ++++++--
 drivers/pci/setup-bus.c |   40 +++++++++++++++++++++++++++++++---------
 3 files changed, 44 insertions(+), 12 deletions(-)

Index: linux-2.6/drivers/pci/bus.c
===================================================================
--- linux-2.6.orig/drivers/pci/bus.c
+++ linux-2.6/drivers/pci/bus.c
@@ -41,9 +41,15 @@ pci_bus_alloc_resource(struct pci_bus *b
 		void *alignf_data)
 {
 	int i, ret = -ENOMEM;
+	resource_size_t max = -1;
 
 	type_mask |= IORESOURCE_IO | IORESOURCE_MEM;
 
+	/* don't allocate too high if the pref mem doesn't support 64bit*/
+	if ((res->flags & (IORESOURCE_PREFETCH | PCI_PREF_RANGE_TYPE_64)) ==
+	    IORESOURCE_PREFETCH)
+		max = 0xffffffff;
+
 	for (i = 0; i < PCI_BUS_NUM_RESOURCES; i++) {
 		struct resource *r = bus->resource[i];
 		if (!r)
@@ -62,7 +68,7 @@ pci_bus_alloc_resource(struct pci_bus *b
 		/* Ok, try it out.. */
 		ret = allocate_resource(r, res, size,
 					r->start ? : min,
-					-1, align,
+					max, align,
 					alignf, alignf_data);
 		if (ret == 0)
 			break;
Index: linux-2.6/drivers/pci/probe.c
===================================================================
--- linux-2.6.orig/drivers/pci/probe.c
+++ linux-2.6/drivers/pci/probe.c
@@ -193,7 +193,7 @@ int __pci_read_base(struct pci_dev *dev,
 		res->flags |= pci_calc_resource_flags(l) | IORESOURCE_SIZEALIGN;
 		if (type == pci_bar_io) {
 			l &= PCI_BASE_ADDRESS_IO_MASK;
-			mask = PCI_BASE_ADDRESS_IO_MASK & 0xffff;
+			mask = PCI_BASE_ADDRESS_IO_MASK & IO_SPACE_LIMIT;
 		} else {
 			l &= PCI_BASE_ADDRESS_MEM_MASK;
 			mask = (u32)PCI_BASE_ADDRESS_MEM_MASK;
@@ -237,6 +237,9 @@ int __pci_read_base(struct pci_dev *dev,
 			dev_printk(KERN_DEBUG, &dev->dev,
 				"reg %x 64bit mmio: %pR\n", pos, res);
 		}
+
+		if (res->flags & IORESOURCE_PREFETCH)
+			res->flags |= PCI_PREF_RANGE_TYPE_64;
 	} else {
 		sz = pci_size(l, sz, mask);
 
@@ -362,7 +365,8 @@ void __devinit pci_read_bridge_bases(str
 		}
 	}
 	if (base <= limit) {
-		res->flags = (mem_base_lo & PCI_MEMORY_RANGE_TYPE_MASK) | IORESOURCE_MEM | IORESOURCE_PREFETCH;
+		res->flags = (mem_base_lo & PCI_PREF_RANGE_TYPE_MASK) |
+					 IORESOURCE_MEM | IORESOURCE_PREFETCH;
 		res->start = base;
 		res->end = limit + 0xfffff;
 		dev_printk(KERN_DEBUG, &dev->dev, "bridge %sbit mmio pref: %pR\n",
Index: linux-2.6/drivers/pci/setup-bus.c
===================================================================
--- linux-2.6.orig/drivers/pci/setup-bus.c
+++ linux-2.6/drivers/pci/setup-bus.c
@@ -143,6 +143,7 @@ static void pci_setup_bridge(struct pci_
 	struct pci_dev *bridge = bus->self;
 	struct pci_bus_region region;
 	u32 l, bu, lu, io_upper16;
+	int pref_mem64;
 
 	if (pci_is_enabled(bridge))
 		return;
@@ -198,16 +199,22 @@ static void pci_setup_bridge(struct pci_
 	pci_write_config_dword(bridge, PCI_PREF_LIMIT_UPPER32, 0);
 
 	/* Set up PREF base/limit. */
+	pref_mem64 = 0;
 	bu = lu = 0;
 	pcibios_resource_to_bus(bridge, &region, bus->resource[2]);
 	if (bus->resource[2]->flags & IORESOURCE_PREFETCH) {
+		int width = 8;
 		l = (region.start >> 16) & 0xfff0;
 		l |= region.end & 0xfff00000;
-		bu = upper_32_bits(region.start);
-		lu = upper_32_bits(region.end);
-		dev_info(&bridge->dev, "  PREFETCH window: %#016llx-%#016llx\n",
-		    (unsigned long long)region.start,
-		    (unsigned long long)region.end);
+		if (bus->resource[2]->flags & PCI_PREF_RANGE_TYPE_64) {
+			pref_mem64 = 1;
+			bu = upper_32_bits(region.start);
+			lu = upper_32_bits(region.end);
+			width = 16;
+		}
+		dev_info(&bridge->dev, "  PREFETCH window: %#0*llx-%#0*llx\n",
+				width, (unsigned long long)region.start,
+				width, (unsigned long long)region.end);
 	}
 	else {
 		l = 0x0000fff0;
@@ -215,9 +222,11 @@ static void pci_setup_bridge(struct pci_
 	}
 	pci_write_config_dword(bridge, PCI_PREF_MEMORY_BASE, l);
 
-	/* Set the upper 32 bits of PREF base & limit. */
-	pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32, bu);
-	pci_write_config_dword(bridge, PCI_PREF_LIMIT_UPPER32, lu);
+	if (pref_mem64) {
+		/* Set the upper 32 bits of PREF base & limit. */
+		pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32, bu);
+		pci_write_config_dword(bridge, PCI_PREF_LIMIT_UPPER32, lu);
+	}
 
 	pci_write_config_word(bridge, PCI_BRIDGE_CONTROL, bus->bridge_ctl);
 }
@@ -255,8 +264,11 @@ static void pci_bridge_check_ranges(stru
 		pci_read_config_dword(bridge, PCI_PREF_MEMORY_BASE, &pmem);
 		pci_write_config_dword(bridge, PCI_PREF_MEMORY_BASE, 0x0);
 	}
-	if (pmem)
+	if (pmem) {
 		b_res[2].flags |= IORESOURCE_MEM | IORESOURCE_PREFETCH;
+		if ((pmem & PCI_PREF_RANGE_TYPE_MASK) == PCI_PREF_RANGE_TYPE_64)
+			b_res[2].flags |= PCI_PREF_RANGE_TYPE_64;
+	}
 }
 
 /* Helper function for sizing routines: find first available
@@ -336,6 +348,7 @@ static int pbus_size_mem(struct pci_bus
 	resource_size_t aligns[12];	/* Alignments from 1Mb to 2Gb */
 	int order, max_order;
 	struct resource *b_res = find_free_bus_resource(bus, type);
+	unsigned int mem64_mask = 0;
 
 	if (!b_res)
 		return 0;
@@ -344,6 +357,11 @@ static int pbus_size_mem(struct pci_bus
 	max_order = 0;
 	size = 0;
 
+	if (type & IORESOURCE_PREFETCH) {
+		mem64_mask = b_res->flags & PCI_PREF_RANGE_TYPE_64;
+		b_res->flags &= ~PCI_PREF_RANGE_TYPE_64;
+	}
+
 	list_for_each_entry(dev, &bus->devices, bus_list) {
 		int i;
 		
@@ -372,6 +390,8 @@ static int pbus_size_mem(struct pci_bus
 				aligns[order] += align;
 			if (order > max_order)
 				max_order = order;
+			if (r->flags & IORESOURCE_PREFETCH)
+				mem64_mask &= r->flags & PCI_PREF_RANGE_TYPE_64;
 		}
 	}
 
@@ -396,6 +416,8 @@ static int pbus_size_mem(struct pci_bus
 	b_res->start = min_align;
 	b_res->end = size + min_align - 1;
 	b_res->flags |= IORESOURCE_STARTALIGN;
+	if (type & IORESOURCE_PREFETCH)
+		b_res->flags |= mem64_mask;
 	return 1;
 }
 

[PATCH 3/3] pci: don't printout if the bus res size is 0

[Yinghai Lu]

also print out if it is prefetechable mmio

[Impact: cleanup]

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 drivers/pci/setup-bus.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Index: linux-2.6/drivers/pci/setup-bus.c
===================================================================
--- linux-2.6.orig/drivers/pci/setup-bus.c
+++ linux-2.6/drivers/pci/setup-bus.c
@@ -558,11 +558,13 @@ static void pci_bus_dump_res(struct pci_
 
         for (i = 0; i < PCI_BUS_NUM_RESOURCES; i++) {
                 struct resource *res = bus->resource[i];
-                if (!res)
+                if (!res || !res->end)
                         continue;
 
 		dev_printk(KERN_DEBUG, &bus->dev, "resource %d %s %pR\n", i,
-			   (res->flags & IORESOURCE_IO) ? "io: " : "mem:", res);
+			   (res->flags & IORESOURCE_IO) ? "io: " :
+			    ((res->flags & IORESOURCE_PREFETCH)? "pref mem":"mem:"),
+			   res);
         }
 }
 

Re: [PATCH 2/3] pci: don't assume pref memio are 64bit -v2

[Ingo Molnar]

* Yinghai Lu <yinghai@kernel.org> wrote:

> one system with 4g installed ( there is 1g hole)
> 
> when 4G installed.
> BIOS put ACPI etc need the hole
> [    0.000000] BIOS-provided physical RAM map:
> [    0.000000]  BIOS-e820: 0000000000000000 - 000000000009bc00 (usable)
> [    0.000000]  BIOS-e820: 000000000009bc00 - 00000000000a0000 (reserved)
> [    0.000000]  BIOS-e820: 00000000000e3000 - 0000000000100000 (reserved)
> [    0.000000]  BIOS-e820: 0000000000100000 - 00000000bffa0000 (usable)
> [    0.000000]  BIOS-e820: 00000000bffa0000 - 00000000bffae000 (ACPI data)
> [    0.000000]  BIOS-e820: 00000000bffae000 - 00000000bfff0000 (ACPI NVS)
> [    0.000000]  BIOS-e820: 00000000bfff0000 - 00000000c0000000 (reserved)
> [    0.000000]  BIOS-e820: 00000000fee00000 - 00000000fee01000 (reserved)
> [    0.000000]  BIOS-e820: 00000000ffb00000 - 0000000100000000 (reserved)
> [    0.000000]  BIOS-e820: 0000000100000000 - 0000000140000000 (usable)
> so in kernel resource will be reserved for 0xbffa0000 - 0xbfff0000 for ACPI
> 0x100000 -  0xbffa0000 for RAM...

btw., sidenote, it would be nice to enhance the e820 table printout 
with size and hole info as well. I did this manually yesterday in 
another mail:

BIOS-provided physical RAM map:
 BIOS-e820: 0000000000000000 - 000000000009fc00 (usable)       0.639 MB RAM
 BIOS-e820: 000000000009fc00 - 00000000000a0000 (reserved)     0.001 MB
                                                [ hole ]       0.250 MB
 BIOS-e820: 00000000000e0000 - 0000000000100000 (reserved)     0.125 MB
 BIOS-e820: 0000000000100000 - 000000003ed94000 (usable)    1004.5   MB RAM
 BIOS-e820: 000000003ed94000 - 000000003ee4e000 (ACPI NVS)     0.7   MB
 BIOS-e820: 000000003ee4e000 - 000000003fea2000 (usable)      16.3   MB RAM
 BIOS-e820: 000000003fea2000 - 000000003fee9000 (ACPI NVS)     0.3   MB
 BIOS-e820: 000000003fee9000 - 000000003feed000 (usable)       0.15  MB RAM
 BIOS-e820: 000000003feed000 - 000000003feff000 (ACPI data     0.07  MB
 BIOS-e820: 000000003feff000 - 000000003ff00000 (usable)       0.004 MB RAM
                                                [ hole ]       1.0   MB
                                                [ hole ]    3072.0   MB

it would be extremely useful to have that in the printout, to see 
the physical address space layout at a glance. Small holes are easy 
to miss, and e820 entry sizes are hard to judge at a glance.

To make this fit well, i'd suggest to drop the 'BIOS-e820: ' prefix 
- it's redundant.

	Ingo

Re: [PATCH 3/3] pci: don't printout if the bus res size is 0

[Ingo Molnar]

* Yinghai Lu <yinghai@kernel.org> wrote:

> 
> also print out if it is prefetechable mmio
> 
> [Impact: cleanup]
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> 
> ---
>  drivers/pci/setup-bus.c |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> Index: linux-2.6/drivers/pci/setup-bus.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/setup-bus.c
> +++ linux-2.6/drivers/pci/setup-bus.c
> @@ -558,11 +558,13 @@ static void pci_bus_dump_res(struct pci_
>  
>          for (i = 0; i < PCI_BUS_NUM_RESOURCES; i++) {
>                  struct resource *res = bus->resource[i];
> -                if (!res)
> +                if (!res || !res->end)
>                          continue;
>  
>  		dev_printk(KERN_DEBUG, &bus->dev, "resource %d %s %pR\n", i,
> -			   (res->flags & IORESOURCE_IO) ? "io: " : "mem:", res);
> +			   (res->flags & IORESOURCE_IO) ? "io: " :
> +			    ((res->flags & IORESOURCE_PREFETCH)? "pref mem":"mem:"),
> +			   res);

'pref mem' is easily mistaken for 'preferential memory' or something 
similar. Would printing "prefetchable-mem" still be OK?

	Ingo

Re: [PATCH 1/3] x86/pci: fix -1 calling to e820_all_mapped with mmconfig

[Ingo Molnar]

* Yinghai Lu <yinghai@kernel.org> wrote:

> e820_all_mapped need end is (addr + size) instead of (addr + size - 1)
> 
> [Impact: fix -1 offset calling]
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> 
> ---
>  arch/x86/pci/mmconfig-shared.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> Index: linux-2.6/arch/x86/pci/mmconfig-shared.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/pci/mmconfig-shared.c
> +++ linux-2.6/arch/x86/pci/mmconfig-shared.c
> @@ -375,7 +375,7 @@ static acpi_status __init check_mcfg_res
>  		if (!fixmem32)
>  			return AE_OK;
>  		if ((mcfg_res->start >= fixmem32->address) &&
> -		    (mcfg_res->end < (fixmem32->address +
> +		    (mcfg_res->end <= (fixmem32->address +
>  				      fixmem32->address_length))) {
>  			mcfg_res->flags = 1;
>  			return AE_CTRL_TERMINATE;
> @@ -392,7 +392,7 @@ static acpi_status __init check_mcfg_res
>  		return AE_OK;
>  
>  	if ((mcfg_res->start >= address.minimum) &&
> -	    (mcfg_res->end < (address.minimum + address.address_length))) {
> +	    (mcfg_res->end <= (address.minimum + address.address_length))) {
>  		mcfg_res->flags = 1;
>  		return AE_CTRL_TERMINATE;
>  	}
> @@ -439,7 +439,7 @@ static int __init is_mmconf_reserved(che
>  	u64 old_size = size;
>  	int valid = 0;
>  
> -	while (!is_reserved(addr, addr + size - 1, E820_RESERVED)) {
> +	while (!is_reserved(addr, addr + size, E820_RESERVED)) {
>  		size >>= 1;
>  		if (size < (16UL<<20))
>  			break;

good catch. Probably also needed for -stable.

Acked-by: Ingo Molnar <mingo@elte.hu>

	Ingo

Re: [PATCH 2/3] pci: don't assume pref memio are 64bit -v2

[Yinghai Lu]

Ingo Molnar wrote:
> * Yinghai Lu <yinghai@kernel.org> wrote:
> 
>> one system with 4g installed ( there is 1g hole)
>>
>> when 4G installed.
>> BIOS put ACPI etc need the hole
>> [    0.000000] BIOS-provided physical RAM map:
>> [    0.000000]  BIOS-e820: 0000000000000000 - 000000000009bc00 (usable)
>> [    0.000000]  BIOS-e820: 000000000009bc00 - 00000000000a0000 (reserved)
>> [    0.000000]  BIOS-e820: 00000000000e3000 - 0000000000100000 (reserved)
>> [    0.000000]  BIOS-e820: 0000000000100000 - 00000000bffa0000 (usable)
>> [    0.000000]  BIOS-e820: 00000000bffa0000 - 00000000bffae000 (ACPI data)
>> [    0.000000]  BIOS-e820: 00000000bffae000 - 00000000bfff0000 (ACPI NVS)
>> [    0.000000]  BIOS-e820: 00000000bfff0000 - 00000000c0000000 (reserved)
>> [    0.000000]  BIOS-e820: 00000000fee00000 - 00000000fee01000 (reserved)
>> [    0.000000]  BIOS-e820: 00000000ffb00000 - 0000000100000000 (reserved)
>> [    0.000000]  BIOS-e820: 0000000100000000 - 0000000140000000 (usable)
>> so in kernel resource will be reserved for 0xbffa0000 - 0xbfff0000 for ACPI
>> 0x100000 -  0xbffa0000 for RAM...
> 
> btw., sidenote, it would be nice to enhance the e820 table printout 
> with size and hole info as well. I did this manually yesterday in 
> another mail:
> 
> BIOS-provided physical RAM map:
>  BIOS-e820: 0000000000000000 - 000000000009fc00 (usable)       0.639 MB RAM
>  BIOS-e820: 000000000009fc00 - 00000000000a0000 (reserved)     0.001 MB
>                                                 [ hole ]       0.250 MB
>  BIOS-e820: 00000000000e0000 - 0000000000100000 (reserved)     0.125 MB
>  BIOS-e820: 0000000000100000 - 000000003ed94000 (usable)    1004.5   MB RAM
>  BIOS-e820: 000000003ed94000 - 000000003ee4e000 (ACPI NVS)     0.7   MB
>  BIOS-e820: 000000003ee4e000 - 000000003fea2000 (usable)      16.3   MB RAM
>  BIOS-e820: 000000003fea2000 - 000000003fee9000 (ACPI NVS)     0.3   MB
>  BIOS-e820: 000000003fee9000 - 000000003feed000 (usable)       0.15  MB RAM
>  BIOS-e820: 000000003feed000 - 000000003feff000 (ACPI data     0.07  MB
>  BIOS-e820: 000000003feff000 - 000000003ff00000 (usable)       0.004 MB RAM
>                                                 [ hole ]       1.0   MB
>                                                 [ hole ]    3072.0   MB
> 
> it would be extremely useful to have that in the printout, to see 
> the physical address space layout at a glance. Small holes are easy 
> to miss, and e820 entry sizes are hard to judge at a glance.
> 
> To make this fit well, i'd suggest to drop the 'BIOS-e820: ' prefix 
> - it's redundant.

will look at it. and should be another patch for x86.

YH

Re: [PATCH 2/3] pci: don't assume pref memio are 64bit -v2

[Ivan Kokshaysky]

On Sat, Apr 18, 2009 at 01:44:51AM -0700, Yinghai Lu wrote:
> and BIOS set
> [    0.240007] pci 0000:00:01.0: bridge 64bit mmio pref: [0xbdf00000-0xddefffff]

An obvious BIOS bug, the bridge base overlaps the physical low RAM
(0x00000000-0xc0000000). Technically speaking, this nonsense *happens*
to work on Intel hardware, so it seems to be quite common bug nowadays -
BIOS writers get lost in ACPI and other "useful" stuff contradicting
the PCI specs.

...

> +	/* don't allocate too high if the pref mem doesn't support 64bit*/
> +	if ((res->flags & (IORESOURCE_PREFETCH | PCI_PREF_RANGE_TYPE_64)) ==
> +	    IORESOURCE_PREFETCH)
> +		max = 0xffffffff;

This effectively destroys non-x86 64-bit arches. You've been told about
that before, so I'm really surprised to see this "patch" once again.

Categorically NACKed.

P.S. I recall that I had a patch that addressed the issue, and Ingo
made some reasonable comments about it. Will post it tomorrow.

Ivan.

Re: [PATCH 2/3] pci: don't assume pref memio are 64bit -v2

[Ingo Molnar]

* Ivan Kokshaysky <ink@jurassic.park.msu.ru> wrote:

> On Sat, Apr 18, 2009 at 01:44:51AM -0700, Yinghai Lu wrote:
> > and BIOS set
> > [    0.240007] pci 0000:00:01.0: bridge 64bit mmio pref: [0xbdf00000-0xddefffff]
> 
> An obvious BIOS bug, the bridge base overlaps the physical low RAM 
> (0x00000000-0xc0000000). Technically speaking, this nonsense 
> *happens* to work on Intel hardware, so it seems to be quite 
> common bug nowadays - BIOS writers get lost in ACPI and other 
> "useful" stuff contradicting the PCI specs.

it doesnt matter whether we call it a BIOS bug or not.

> ...
> 
> > +	/* don't allocate too high if the pref mem doesn't support 64bit*/
> > +	if ((res->flags & (IORESOURCE_PREFETCH | PCI_PREF_RANGE_TYPE_64)) ==
> > +	    IORESOURCE_PREFETCH)
> > +		max = 0xffffffff;
> 
> This effectively destroys non-x86 64-bit arches. You've been told about
> that before, so I'm really surprised to see this "patch" once again.
> 
> Categorically NACKed.

You can ridicule the patch and can NAK it (and rightfully so, it's 
wrong), but you seem to miss the simple fact that this solves a very 
real problem.

So consider this patch a documentation and analysis of a real 
problem which made Linux work on hardware where it did not work 
before. That's more valuable than 95% of our commits btw.

> P.S. I recall that I had a patch that addressed the issue, and 
> Ingo made some reasonable comments about it. Will post it 
> tomorrow.

That should have been pursued far more agressively.

Thanks,

	Ingo

Re: [PATCH 1/3] x86/pci: fix -1 calling to e820_all_mapped with mmconfig

[Jesse Barnes]

On Sat, 18 Apr 2009 01:43:46 -0700
Yinghai Lu <yinghai@kernel.org> wrote:

> 
> e820_all_mapped need end is (addr + size) instead of (addr + size - 1)
> 
> [Impact: fix -1 offset calling]
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>

Applied, thanks.

-- 
Jesse Barnes, Intel Open Source Technology Center