Re: [PATCH v2] PCI: don't allocate resource above 4G for 32-bit BAR

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Feb 20, 2009 at 02:41:43PM +0800, Yu Zhao wrote:
...
> +			/* Memory BAR could be 64-bit */
> +			region.end = (resource_size_t)(u64)-1;
...
> +	pcibios_bus_to_resource(dev, &r, &region);
> +
> +	return r.end;

Still doesn't work.

pcibios_bus_to_resource() typically does

	res->start = region->start + offset;
	res->end = region->end + offset;

so we end up with max = offset - 1, which effectively disables
allocation of 64-bit BARs...

I think that we should drop this min/max approach - it's much more
interesting to consider PCI memory above 4G as yet another type of
address space. This actually makes sense, as even accesses to that
memory are physically different - Dual Address Cycle (DAC) vs. 32-bit
Single Address Cycle (SAC).

So, platform that can deal with 64-bit allocations would set up
an additional root bus resource and mark it with IORESOURCE_MEM64
flag (like Pavel suggested earlier).

The main problem here is how the kernel would detect that hardware can
actually access a DAC memory (I know for a fact that a lot of Intel chipsets
cannot access MMIO >4G, even though subordinate p2p bridges are 64-bit
capable).
On the other hand, there are PCI devices with 64-bit BARs that do not
work properly being placed above 4G boundary. Real life example: some
radeon cards have 64-bit bar for video RAM, but allocating that BAR in
the DAC area basically gives you framebuffer-only mode, hardware
acceleration won't work since the internal GPU is 32-bit...

So moving stuff into MEM64 area should be considered as generally unsafe
operation, and the best default policy is to not enable MEM64 resource
unless we find that BIOS has allocated something there.
At the same time, MEM64 can be easily enabled/disabled based on host
bridge PCI IDs, kernel command line options and so on.

Here is a basic implementation of the above for x86. I think it's
reasonably good starting point for PCI64 work - the next step would
be to teach pci_bus_alloc_resource() about IORESOURCE_MEM64: logic is
similar to prefetch vs non-prefetch case - MEM can hold MEM64 resource,
but not vice versa. And eventually bridge sizing code will be updated
for reasonable 64-bit allocations (it's a non-trivial task, though).

But I think this patch alone should fix cardbus >4G allocations
and similar nonsense.

Signed-off-by: Ivan Kokshaysky <ink@xxxxxxxxxxxxxxxxxxxx>

Ivan.

---
 arch/x86/pci/i386.c    |   51 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/ioport.h |    2 +
 2 files changed, 53 insertions(+), 0 deletions(-)

diff --git a/arch/x86/pci/i386.c b/arch/x86/pci/i386.c
index 5ead808..6e3f22c 100644
--- a/arch/x86/pci/i386.c
+++ b/arch/x86/pci/i386.c
@@ -78,6 +78,47 @@ pcibios_align_resource(void *data, struct resource *res,
 }
 EXPORT_SYMBOL(pcibios_align_resource);
 
+static struct resource mem64 = {
+	.name	= "PCI mem64",
+	.start	= (resource_size_t)(1 << 16) << 16,	/* 4Gb */
+	.end	= -1,
+	.flags	= IORESOURCE_MEM,
+};
+
+static void pcibios_pci64_setup(void)
+{
+	struct resource *r64 = &mem64, *root = &iomem_resource;
+	struct pci_bus *b;
+
+	if (!r64->start)
+		return;		/* 32-bit phys. address, nothing to do */
+
+	if (insert_resource(root, r64)) {
+		printk(KERN_WARNING "PCI: Failed to allocate PCI64 space\n");
+		return;
+	}
+
+	list_for_each_entry(b, &pci_root_buses, node) {
+		/* Is this a "standard" root bus created by pci_create_bus? */
+		if (b->resource[1] != root || b->resource[2])
+			continue;
+		b->resource[2] = r64;	/* create DAC resource */
+	}
+}
+
+static void pcibios_pci64_verify(void)
+{
+	struct pci_bus *b;
+
+	if (mem64.flags & IORESOURCE_MEM64)
+		return;		/* presumably DAC works */
+	list_for_each_entry(b, &pci_root_buses, node) {
+		if (b->resource[2] == &mem64)
+			b->resource[2] = NULL;
+	}
+	printk(KERN_INFO "PCI: allocations above 4G disabled\n");
+}
+
 /*
  *  Handle resources of PCI devices.  If the world were perfect, we could
  *  just allocate all the resource regions and do nothing more.  It isn't.
@@ -137,6 +178,10 @@ static void __init pcibios_allocate_bus_resources(struct list_head *bus_list)
 					 * range.
 					 */
 					r->flags = 0;
+				} else {
+					/* Successful allocation */
+					if (upper_32_bits(r->start))
+						pr->flags |= IORESOURCE_MEM64;
 				}
 			}
 		}
@@ -174,6 +219,10 @@ static void __init pcibios_allocate_resources(int pass)
 					/* We'll assign a new address later */
 					r->end -= r->start;
 					r->start = 0;
+				} else {
+					/* Successful allocation */
+					if (upper_32_bits(r->start))
+						pr->flags |= IORESOURCE_MEM64;
 				}
 			}
 		}
@@ -225,9 +274,11 @@ static int __init pcibios_assign_resources(void)
 void __init pcibios_resource_survey(void)
 {
 	DBG("PCI: Allocating resources\n");
+	pcibios_pci64_setup();
 	pcibios_allocate_bus_resources(&pci_root_buses);
 	pcibios_allocate_resources(0);
 	pcibios_allocate_resources(1);
+	pcibios_pci64_verify();
 
 	e820_reserve_resources_late();
 }
diff --git a/include/linux/ioport.h b/include/linux/ioport.h
index 32e4b2f..30403b3 100644
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -49,6 +49,8 @@ struct resource_list {
 #define IORESOURCE_SIZEALIGN	0x00020000	/* size indicates alignment */
 #define IORESOURCE_STARTALIGN	0x00040000	/* start field is alignment */
 
+#define IORESOURCE_MEM64	0x00080000	/* 64-bit addressing, >4G */
+
 #define IORESOURCE_EXCLUSIVE	0x08000000	/* Userland may not map this resource */
 #define IORESOURCE_DISABLED	0x10000000
 #define IORESOURCE_UNSET	0x20000000
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux