x86/PCI: pci=use_crs regression fix

PATCH New Bjorn Helgaas 2 participants 3 messages Mar 16, 2010 – Mar 19, 2010

Subsystems (3)

x86 ACPI PCI

lore

Share & tools

lore

Bjorn Helgaas

author

2010-03-16 20:11 UTC #0 lore raw

PATCH 1/1 x86/PCI: pci=use_crs regression fix

This patch fixes some PCI host bridge _CRS parsing problems and may fix Pete Zaitcev's http://bugzilla.kernel.org/show_bug.cgi?id=15533 regression, but I haven't seen any testing results yet.

I'm pretty confident this will fix pci=use_crs on *some* machines even if Pete's problem turns out to be something else, so I think we should just put it in.

---

Bjorn Helgaas (1):
x86/PCI: ignore Consumer/Producer bit and parse additional _CRS resources

 arch/x86/pci/acpi.c |   77 +++++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 71 insertions(+), 6 deletions(-)

Bjorn Helgaas

author

2010-03-16 20:11 UTC #1 lore raw

PATCH 1/1 x86/PCI: ignore Consumer/Producer bit and parse additional _CRS resources

We previously used only resources marked "Producer," i.e., those the bridge forwards downstream. But BIOSes haven't used that bit consistently, so it's useless. For bridge devices, we have to assume *all* ACPI resources are forwarded downstream. Non-Producer bridge resources typically appear in PCI config space and are not described in ACPI.

In addition to ACPI_RESOURCE_TYPE_ADDRESS{16,32,64}, parse these types:

ACPI_RESOURCE_TYPE_IO
ACPI_RESOURCE_TYPE_FIXED_IO
ACPI_RESOURCE_TYPE_MEMORY24
ACPI_RESOURCE_TYPE_MEMORY32
ACPI_RESOURCE_TYPE_FIXED_MEMORY32
ACPI_RESOURCE_TYPE_EXTENDED_ADDRESS64

This basically reimplements what drivers/pnp/pnpacpi/rsparser.c already does, and I plan to take advantage of that, but we're not quite ready to do it yet because it will require converting pci_root.c from an ACPI driver to a PNP driver.

This is a possible fix for http://bugzilla.kernel.org/show_bug.cgi?id=15533
reported by Pete Zaitcev <[email protected]>.

Signed-off-by Bjorn Helgaas <[email protected]>

---

 arch/x86/pci/acpi.c |   77 +++++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 71 insertions(+), 6 deletions(-)

arch/x86/pci/acpi.c +71 -6

index d255ce8..e0f45c2 100644
@@ -65,14 +65,79 @@ resource_to_addr(struct acpi_resource *resource,
 			struct acpi_resource_address64 *addr)
 {
 	acpi_status status;
+	struct acpi_resource_io *io;
+	struct acpi_resource_fixed_io *fixed_io;
+	struct acpi_resource_memory24 *memory24;
+	struct acpi_resource_memory32 *memory32;
+	struct acpi_resource_fixed_memory32 *fixed_memory32;
+	struct acpi_resource_extended_address64 *ext_addr64;
+
+	memset(addr, 0, sizeof(*addr));
+
+	switch (resource->type) {
+	case ACPI_RESOURCE_TYPE_IO:
+		io = &resource->data.io;
+		addr->resource_type = ACPI_IO_RANGE;
+		addr->minimum = io->minimum;
+		addr->address_length = io->address_length;
+		return AE_OK;
+
+	case ACPI_RESOURCE_TYPE_FIXED_IO:
+		fixed_io = &resource->data.fixed_io;
+		addr->resource_type = ACPI_IO_RANGE;
+		addr->minimum = fixed_io->address;
+		addr->address_length = fixed_io->address_length;
+		return AE_OK;
+
+	case ACPI_RESOURCE_TYPE_MEMORY24:
+		memory24 = &resource->data.memory24;
+		addr->resource_type = ACPI_MEMORY_RANGE;
+		addr->minimum = memory24->minimum;
+		addr->address_length = memory24->address_length;
+		return AE_OK;
+
+	case ACPI_RESOURCE_TYPE_MEMORY32:
+		memory32 = &resource->data.memory32;
+		addr->resource_type = ACPI_MEMORY_RANGE;
+		addr->minimum = memory32->minimum;
+		addr->address_length = memory32->address_length;
+		return AE_OK;
 
-	status = acpi_resource_to_address64(resource, addr);
-	if (ACPI_SUCCESS(status) &&
-	    (addr->resource_type == ACPI_MEMORY_RANGE ||
-	    addr->resource_type == ACPI_IO_RANGE) &&
-	    addr->address_length > 0 &&
-	    addr->producer_consumer == ACPI_PRODUCER) {
+	case ACPI_RESOURCE_TYPE_FIXED_MEMORY32:
+		fixed_memory32 = &resource->data.fixed_memory32;
+		addr->resource_type = ACPI_MEMORY_RANGE;
+		addr->minimum = fixed_memory32->address;
+		addr->address_length = fixed_memory32->address_length;
 		return AE_OK;
+
+	case ACPI_RESOURCE_TYPE_ADDRESS16:
+	case ACPI_RESOURCE_TYPE_ADDRESS32:
+	case ACPI_RESOURCE_TYPE_ADDRESS64:
+		status = acpi_resource_to_address64(resource, addr);
+		if (ACPI_SUCCESS(status) &&
+		    (addr->resource_type == ACPI_MEMORY_RANGE ||
+		     addr->resource_type == ACPI_IO_RANGE) &&
+		    addr->address_length > 0) {
+			return AE_OK;
+		}
+		break;
+
+	case ACPI_RESOURCE_TYPE_EXTENDED_ADDRESS64:
+		ext_addr64 = &resource->data.ext_address64;
+		if ((ext_addr64->resource_type == ACPI_MEMORY_RANGE ||
+		     ext_addr64->resource_type == ACPI_IO_RANGE) &&
+		    ext_addr64->address_length > 0) {
+			addr->resource_type = ext_addr64->resource_type;
+			addr->minimum = ext_addr64->minimum;
+			addr->address_length = ext_addr64->address_length;
+			addr->translation_offset =
+					ext_addr64->translation_offset;
+			if (ext_addr64->resource_type == ACPI_MEMORY_RANGE)
+				addr->info.mem.caching =
+						ext_addr64->info.mem.caching;
+			return AE_OK;
+		}
+		break;
 	}
 	return AE_ERROR;
 }

Jesse Barnes

Applied

2010-03-19 20:38 UTC #2 lore raw

Re: [PATCH] x86/PCI: ignore Consumer/Producer bit and parse additional _CRS resources

Quoted 21 lines · On Tue, 16 Mar 2010 14:11:34 -0600 Bjorn Helgaas <[email protected]> wrote:

We previously used only resources marked "Producer," i.e., those the bridge forwards downstream. But BIOSes haven't used that bit consistently, so it's useless. For bridge devices, we have to assume *all* ACPI resources are forwarded downstream. Non-Producer bridge resources typically appear in PCI config space and are not described in ACPI.
In addition to ACPI_RESOURCE_TYPE_ADDRESS{16,32,64}, parse these types:
ACPI_RESOURCE_TYPE_IO
ACPI_RESOURCE_TYPE_FIXED_IO
ACPI_RESOURCE_TYPE_MEMORY24
ACPI_RESOURCE_TYPE_MEMORY32
ACPI_RESOURCE_TYPE_FIXED_MEMORY32
ACPI_RESOURCE_TYPE_EXTENDED_ADDRESS64
This basically reimplements what drivers/pnp/pnpacpi/rsparser.c already does, and I plan to take advantage of that, but we're not quite ready to do it yet because it will require converting pci_root.c from an ACPI driver to a PNP driver.

Quoted 6 lines

This is a possible fix for http://bugzilla.kernel.org/show_bug.cgi?id=15533
reported by Pete Zaitcev <[email protected]>.
Signed-off-by Bjorn Helgaas <[email protected]>
---

Applied to my for-linus tree, thanks.

Signature · 1 line

Jesse Barnes, Intel Open Source Technology Center