linux-kernel - Re: how to fix acpi_pci_root_remap

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20180817112424.GA31375@e107981-ln.cambridge.arm.com>
Date:   Fri, 17 Aug 2018 12:24:24 +0100
From:   Lorenzo Pieralisi <lorenzo.pieralisi@....com>
To:     "Luck, Tony" <tony.luck@...el.com>
Cc:     Bjorn Helgaas <bhelgaas@...gle.com>, linux-kernel@...r.kernel.org,
        Jayachandran C <jchandra@...adcom.com>,
        Sinan Kaya <okaya@...eaurora.org>,
        Tomasz Nowicki <tn@...ihalf.com>,
        Arnd Bergmann <arnd@...db.de>,
        Boris Brezillon <boris.brezillon@...tlin.com>,
        Miquel Raynal <miquel.raynal@...tlin.com>
Subject: Re: how to fix acpi_pci_root_remap_iospace?

On Thu, Aug 16, 2018 at 01:45:07PM -0700, Luck, Tony wrote:
> Bjorn,
> 
> Back in commit:
> 
>   0a70abb38062 ("PCI/ACPI: Support I/O resources when parsing host bridge resources")
> 
> we added acpi_pci_root_remap_iospace().  On ia64 this was a no-op because ia64
> didn't define PCI_IOBASE, so the entire body of the function was skipped.
> 
> But in the current merge window commit:
> 
>   0bbf47eab469 ("ia64: use asm-generic/io.h")
> 
> ended up defining PCI_IOBASE for us, and now we die horribly
> in early boot with:
> 
>   kernel BUG at lib/ioremap.c:72!
> 
> 
> Is PCI_IOBASE the right thing to check for to decide whether
> acpi_pci_root_remap_iospace() needs to do anything?
> 
> The ugly fix would be:
> 
> 
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index 7433035ded95..de06377de13b 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -741,7 +741,7 @@ static void acpi_pci_root_validate_resources(struct device *dev,
>  static void acpi_pci_root_remap_iospace(struct fwnode_handle *fwnode,
>  			struct resource_entry *entry)
>  {
> -#ifdef PCI_IOBASE
> +#if defined(PCI_IOBASE) && !defined(CONFIG_IA64)
>  	struct resource *res = entry->res;
>  	resource_size_t cpu_addr = res->start;
>  	resource_size_t pci_addr = cpu_addr - entry->offset;
> 
> or we can do some other juggling with defines to get the
> same outcome.
> 
> -Tony

If that gives too much trouble we could move
acpi_pci_root_remap_iospace() to arch/arm64 too (patch compile tested
below) given that at the moment it is generic in theory, just let me
know what's the best course of action.

Lorenzo

-- >8 --
diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
index 0e2ea1c78542..06e27879087b 100644
--- a/arch/arm64/kernel/pci.c
+++ b/arch/arm64/kernel/pci.c
@@ -98,13 +98,46 @@ int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
 	return 0;
 }
 
+static void acpi_pci_root_remap_iospace(struct fwnode_handle *fwnode,
+			struct resource_entry *entry)
+{
+	struct resource *res = entry->res;
+	resource_size_t cpu_addr = res->start;
+	resource_size_t pci_addr = cpu_addr - entry->offset;
+	resource_size_t length = resource_size(res);
+	unsigned long port;
+
+	if (pci_register_io_range(fwnode, cpu_addr, length))
+		goto err;
+
+	port = pci_address_to_pio(cpu_addr);
+	if (port == (unsigned long)-1)
+		goto err;
+
+	res->start = port;
+	res->end = port + length - 1;
+	entry->offset = port - pci_addr;
+
+	if (pci_remap_iospace(res, cpu_addr) < 0)
+		goto err;
+
+	pr_info("Remapped I/O %pa to %pR\n", &cpu_addr, res);
+	return;
+err:
+	res->flags |= IORESOURCE_DISABLED;
+}
+
 static int pci_acpi_root_prepare_resources(struct acpi_pci_root_info *ci)
 {
 	struct resource_entry *entry, *tmp;
+	struct acpi_device *device = ci->bridge;
 	int status;
 
 	status = acpi_pci_probe_root_resources(ci);
 	resource_list_for_each_entry_safe(entry, tmp, &ci->resources) {
+		if (entry->res->flags & IORESOURCE_IO)
+			acpi_pci_root_remap_iospace(&device->fwnode, entry);
+
 		if (!(entry->res->flags & IORESOURCE_WINDOW))
 			resource_list_destroy_entry(entry);
 	}
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 7433035ded95..0f9fb87b6e26 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -738,37 +738,6 @@ static void acpi_pci_root_validate_resources(struct device *dev,
 	}
 }
 
-static void acpi_pci_root_remap_iospace(struct fwnode_handle *fwnode,
-			struct resource_entry *entry)
-{
-#ifdef PCI_IOBASE
-	struct resource *res = entry->res;
-	resource_size_t cpu_addr = res->start;
-	resource_size_t pci_addr = cpu_addr - entry->offset;
-	resource_size_t length = resource_size(res);
-	unsigned long port;
-
-	if (pci_register_io_range(fwnode, cpu_addr, length))
-		goto err;
-
-	port = pci_address_to_pio(cpu_addr);
-	if (port == (unsigned long)-1)
-		goto err;
-
-	res->start = port;
-	res->end = port + length - 1;
-	entry->offset = port - pci_addr;
-
-	if (pci_remap_iospace(res, cpu_addr) < 0)
-		goto err;
-
-	pr_info("Remapped I/O %pa to %pR\n", &cpu_addr, res);
-	return;
-err:
-	res->flags |= IORESOURCE_DISABLED;
-#endif
-}
-
 int acpi_pci_probe_root_resources(struct acpi_pci_root_info *info)
 {
 	int ret;
@@ -789,10 +758,6 @@ int acpi_pci_probe_root_resources(struct acpi_pci_root_info *info)
 			"no IO and memory resources present in _CRS\n");
 	else {
 		resource_list_for_each_entry_safe(entry, tmp, list) {
-			if (entry->res->flags & IORESOURCE_IO)
-				acpi_pci_root_remap_iospace(&device->fwnode,
-						entry);
-
 			if (entry->res->flags & IORESOURCE_DISABLED)
 				resource_list_destroy_entry(entry);
 			else