Re: [PATCH 02/11] PCI: Try to allocate mem64 above 4G at first

Yinghai Lu
2012-05-29T20:40:30Z

[-- Attachment #1: Type: text/plain, Size: 3155 bytes --]

On Tue, May 29, 2012 at 12:23 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Tue, May 29, 2012 at 12:17 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>> On Tue, May 29, 2012 at 10:57 AM, H. Peter Anvin <hpa@zytor.com> wrote:
>>> On 05/29/2012 10:55 AM, Yinghai Lu wrote:
>>>>
>>>> x86 are using 16bits.
>>>>
>>>> some others use 32 bits.
>>>> #define IO_SPACE_LIMIT 0xffffffff
>>>>
>>>> ia64 and sparc are using 64bits.
>>>> #define IO_SPACE_LIMIT               0xffffffffffffffffUL
>>>>
>>>> but pci only support 16bits and 32bits.
>>>>
>>>> maybe later we can add
>>>> PCI_MAX_RESOURCE_16
>>>>
>>>> to handle 16bits and 32bit io ports.
>>>>
>>>
>>> Shouldn't this be dealt by root port apertures?
>>>
>>
>> pci bridge could support 16bits and 32bits io port.
>> but we did not record if 32bits is supported.
>>
>> so during allocating, could have allocated above 64k address to non
>> 32bit bridge.
>>
>> but  x86 is ok, because ioport.end always set to 0xffff.
>> other arches with IO_SPACE_LIMIT with 0xffffffff or
>> 0xffffffffffffffffUL may have problem.
>
> I think current IO_SPACE_LIMIT usage is a little confused.  The
> "ioport_resource.end = IO_SPACE_LIMIT" in kernel/resource.c refers to
> a CPU-side address, not a bus address.  Other uses, e.g., in
> __pci_read_base(), apply it to bus addresses from BARs, which is
> wrong.  Host bridges apply I/O port offsets just like they apply
> memory offsets.  The ia64 IO_SPACE_LIMIT of 0xffffffffffffffffUL means
> there's no restriction on CPU-side I/O port addresses, but any given
> host bridge will translate its I/O port aperture to bus addresses that
> fit in 32 bits.
>
> None of this is really relevant to the question I asked, namely, "why
> Yinghai's patch doesn't limit I/O BAR values to 32 bits?"  That
> constraint is clearly a requirement because I/O BARs are only 32 bits
> wide, but I don't think it needs to be enforced in the code here.  The
> host bridge or upstream P2P bridge apertures should already take care
> of that automatically.  I don't think the 16- or 32-bitness of P2P
> bridge apertures is relevant here, because the I/O resources available
> on the secondary bus already reflect that.
>
> After all that discussion, I think my objection here boils down to
> "you shouldn't change the I/O BAR constraints in a patch that claims
> to allocate 64-bit *memory* BARs above 4GB."
>
> I think the code below is still the clearest way to set the constraints:
>
>   if (res->flags & IORESOURCE_MEM_64) {
>       start = (resource_size_t) (1ULL << 32);
>       end = PCI_MAX_RESOURCE;
>   } else {
>       start = 0;
>       end = PCI_MAX_RESOURCE_32;
>   }
>
> It's not strictly necessary to limit I/O BARs to PCI_MAX_RESOURCE_32
> because host bridge apertures should already enforce that, but I think
> the code above just makes it clearer.

ok, please check the version, that put back PCI_MAX_RESOURCE_32 for io ports.

also RFC to limit for 16 bit ioport handling. only help other arches
that does support 32bit ioports but have bridges only support 16bit io
ports.

Thanks

Yinghai

[-- Attachment #2: allocate_high_at_first_v5.patch --]
[-- Type: application/octet-stream, Size: 2183 bytes --]

Subject: [PATCH] PCI: Try to allocate mem64 above 4G at first

and will fall back to below 4g if it can not find any above 4g.

x86 32bit without X86_PAE support will have bottom set to 0, becuase
resource_size_t is 32bit.

Also for 32bit with resource_size_t 64bit kernel on machine with pae support
we are safe because iomem_resource is limited to 32bit according to
x86_phys_bits.

-v2: update bottom assigning to make it clear for non-pae support machine.
-v3: Bjorn's change:
use MAX_REOURCE instead of -1
use start/end instead of bottom/max
for all arch instead of just x86_64
-v4: updated after PCI_MAX_RESOURCE_32 change.
-v5: restore io handling to use PCI_MAX_RESOURCE_32 as limit.

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

---
drivers/pci/bus.c | 30 +++++++++++++++++++++++-------
1 file changed, 23 insertions(+), 7 deletions(-)

Index: linux-2.6/drivers/pci/bus.c
===================================================================
--- linux-2.6.orig/drivers/pci/bus.c
+++ linux-2.6/drivers/pci/bus.c
@@ -121,14 +121,24 @@ pci_bus_alloc_resource(struct pci_bus *b
{
int i, ret = -ENOMEM;
struct resource *r;
- resource_size_t max = PCI_MAX_RESOURCE;
+ resource_size_t end;
+ resource_size_t start;

type_mask |= IORESOURCE_IO | IORESOURCE_MEM;

- /* don't allocate too high if the pref mem doesn't support 64bit*/
- if (!(res->flags & IORESOURCE_MEM_64))
- max = PCI_MAX_RESOURCE_32;
+ /*
+ * don't allocate too high if the pref mem doesn't support 64bit
+ * also if this is a 64-bit mem resource, try above 4GB first
+ */
+ if (res->flags & IORESOURCE_MEM_64) {
+ start = (resource_size_t)(1ULL<<32);
+ end = PCI_MAX_RESOURCE;
+ } else {
+ start = 0;
+ end = PCI_MAX_RESOURCE_32;
+ }

+again:
pci_bus_for_each_resource(bus, r, i) {
if (!r)
continue;
@@ -145,12 +155,18 @@ pci_bus_alloc_resource(struct pci_bus *b

/* Ok, try it out.. */
ret = allocate_resource(r, res, size,
- r->start ? : min,
- max, align,
+ max(start, r->start ? : min),
+ end, align,
alignf, alignf_data);
if (ret == 0)
- break;
+ return 0;
+ }
+
+ if (start != 0) {
+ start = 0;
+ goto again;
}
+
return ret;
}

[-- Attachment #3: fix_32bit_ioport.patch --]
[-- Type: application/octet-stream, Size: 5224 bytes --]

Subject: [RFC PATCH] PCI: io port 16/32 bits allocation

pci bridge could only support 16bit io port.

some arches are supporting 32bit io ports.

So need to make sure 16bit ioport will be allocated under 64k.
For 32bit ioport, will try to allocate that [64k, 4g), and if it fail
will fall back to [0, 64k).

It is some kind of clone of 64bit mem io handling.

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

---
drivers/pci/bus.c | 20 +++++++++++++++-----
drivers/pci/pci.h | 1 +
drivers/pci/probe.c | 4 +++-
drivers/pci/setup-bus.c | 11 +++++++++--
include/linux/ioport.h | 1 +
5 files changed, 29 insertions(+), 8 deletions(-)

Index: linux-2.6/drivers/pci/bus.c
===================================================================
--- linux-2.6.orig/drivers/pci/bus.c
+++ linux-2.6/drivers/pci/bus.c
@@ -130,12 +130,22 @@ pci_bus_alloc_resource(struct pci_bus *b
* don't allocate too high if the pref mem doesn't support 64bit
* also if this is a 64-bit mem resource, try above 4GB first
*/
- if (res->flags & IORESOURCE_MEM_64) {
- start = (resource_size_t)(1ULL<<32);
- end = PCI_MAX_RESOURCE;
+ if (res->flags & IORESOURCE_MEM) {
+ if (res->flags & IORESOURCE_MEM_64) {
+ start = (resource_size_t)(1ULL<<32);
+ end = PCI_MAX_RESOURCE;
+ } else {
+ start = 0;
+ end = PCI_MAX_RESOURCE_32;
+ }
} else {
- start = 0;
- end = PCI_MAX_RESOURCE_32;
+ if (res->flags & IORESOURCE_IO_32) {
+ start = PCI_MAX_RESOURCE_16 + 1;
+ end = PCI_MAX_RESOURCE_32;
+ } else {
+ start = 0;
+ end = PCI_MAX_RESOURCE_16;
+ }
}

again:
Index: linux-2.6/drivers/pci/pci.h
===================================================================
--- linux-2.6.orig/drivers/pci/pci.h
+++ linux-2.6/drivers/pci/pci.h
@@ -210,6 +210,7 @@ enum pci_bar_type {

#define PCI_MAX_RESOURCE ((resource_size_t)~0)
#define PCI_MAX_RESOURCE_32 ((resource_size_t)0xffffffff)
+#define PCI_MAX_RESOURCE_16 ((resource_size_t)0xffff)

bool pci_bus_read_dev_vendor_id(struct pci_bus *bus, int devfn, u32 *pl,
int crs_timeout);
Index: linux-2.6/include/linux/ioport.h
===================================================================
--- linux-2.6.orig/include/linux/ioport.h
+++ linux-2.6/include/linux/ioport.h
@@ -47,6 +47,7 @@ struct resource {
#define IORESOURCE_MEM_64 0x00100000
#define IORESOURCE_WINDOW 0x00200000 /* forwarded by bridge */
#define IORESOURCE_MUXED 0x00400000 /* Resource is software muxed */
+#define IORESOURCE_IO_32 0x00800000

#define IORESOURCE_EXCLUSIVE 0x08000000 /* Userland may not map this resource */
#define IORESOURCE_DISABLED 0x10000000
Index: linux-2.6/drivers/pci/probe.c
===================================================================
--- linux-2.6.orig/drivers/pci/probe.c
+++ linux-2.6/drivers/pci/probe.c
@@ -130,7 +130,7 @@ static inline unsigned long decode_bar(s

if ((bar & PCI_BASE_ADDRESS_SPACE) == PCI_BASE_ADDRESS_SPACE_IO) {
flags = bar & ~PCI_BASE_ADDRESS_IO_MASK;
- flags |= IORESOURCE_IO;
+ flags |= IORESOURCE_IO | IORESOURCE_IO_32;
return flags;
}

@@ -326,6 +326,8 @@ static void __devinit pci_read_bridge_io

if (base && base <= limit) {
res->flags = (io_base_lo & PCI_IO_RANGE_TYPE_MASK) | IORESOURCE_IO;
+ if ((io_base_lo & PCI_IO_RANGE_TYPE_MASK) == PCI_IO_RANGE_TYPE_32)
+ res->flags |= IORESOURCE_IO_32;
res2.flags = res->flags;
region.start = base;
region.end = limit + 0xfff;
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
@@ -595,8 +595,11 @@ static void pci_bridge_check_ranges(stru
pci_read_config_word(bridge, PCI_IO_BASE, &io);
pci_write_config_word(bridge, PCI_IO_BASE, 0x0);
}
- if (io)
+ if (io) {
b_res[0].flags |= IORESOURCE_IO;
+ if ((io & PCI_IO_RANGE_TYPE_MASK) == PCI_IO_RANGE_TYPE_32)
+ b_res[0].flags |= IORESOURCE_IO_32;
+ }
/* DECchip 21050 pass 2 errata: the bridge may miss an address
disconnect boundary by one PCI data phase.
Workaround: do not use prefetching on this device. */
@@ -710,10 +713,13 @@ static void pbus_size_io(struct pci_bus
struct resource *b_res = find_free_bus_resource(bus, IORESOURCE_IO);
unsigned long size = 0, size0 = 0, size1 = 0;
resource_size_t children_add_size = 0;
+ unsigned int io32_mask = 0;

if (!b_res)
return;

+ io32_mask = b_res->flags & IORESOURCE_IO_32;
+ b_res->flags &= ~IORESOURCE_IO_32;
list_for_each_entry(dev, &bus->devices, bus_list) {
int i;

@@ -731,6 +737,7 @@ static void pbus_size_io(struct pci_bus
else
size1 += r_size;

+ io32_mask &= r->flags & IORESOURCE_IO_32;
if (realloc_head)
children_add_size += get_res_add_size(realloc_head, r);
}
@@ -753,7 +760,7 @@ static void pbus_size_io(struct pci_bus
/* Alignment of the IO window is always 4K */
b_res->start = 4096;
b_res->end = b_res->start + size0 - 1;
- b_res->flags |= IORESOURCE_STARTALIGN;
+ b_res->flags |= IORESOURCE_STARTALIGN | io32_mask;
if (size1 > size0 && realloc_head) {
add_to_list(realloc_head, bus->self, b_res, size1-size0, 4096);
dev_printk(KERN_DEBUG, &bus->self->dev, "bridge window "