All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] PCI: ACPI: IA64: fix IO port generic range check
@ 2016-03-21 11:12 ` Lorenzo Pieralisi
  0 siblings, 0 replies; 10+ messages in thread
From: Lorenzo Pieralisi @ 2016-03-21 11:12 UTC (permalink / raw)
  To: linux-pci, linux-acpi
  Cc: linux-ia64, linux-kernel, Lorenzo Pieralisi, Bjorn Helgaas,
	Hanjun Guo, Jiang Liu, Tony Luck, Tomasz Nowicki, Mark Salter,
	Rafael J. Wysocki

The [0 - 64k] ACPI PCI IO port resource boundary check in:

acpi_dev_ioresource_flags()

is currently applied blindly in the ACPI resource parsing to all
architectures, but only x86 suffers from that IO space limitation.

On arches (ie IA64 and ARM64) where IO space is memory mapped,
the PCI root bridges IO resource windows are firstly initialized from
the _CRS (in acpi_decode_space()) and contain the CPU physical address
at which a root bridge decodes IO space in the CPU physical address
space with the offset value representing the offset required to translate
the PCI bus address into the CPU physical address.

The IO resource windows are then parsed and updated in arch code
before creating and enumerating PCI buses (eg IA64 add_io_space())
to map in an arch specific way the obtained CPU physical address range
to a slice of virtual address space reserved to map PCI IO space,
ending up with PCI bridges resource windows containing IO
resources like the following on a working IA64 configuration:

PCI host bridge to bus 0000:00
pci_bus 0000:00: root bus resource [io  0x1000000-0x100ffff window] (bus
address [0x0000-0xffff])
pci_bus 0000:00: root bus resource [mem 0x000a0000-0x000fffff window]
pci_bus 0000:00: root bus resource [mem 0x80000000-0x8fffffff window]
pci_bus 0000:00: root bus resource [mem 0x80004000000-0x800ffffffff window]
pci_bus 0000:00: root bus resource [bus 00]

This implies that the [0 - 64K] check in acpi_dev_ioresource_flags()
leaves platforms with memory mapped IO space (ie IA64) broken (ie kernel
can't claim IO resources since the host bridge IO resource is disabled
and discarded by ACPI core code, see log on IA64 with missing root bridge
IO resource, silently filtered by current [0 - 64k] check in
acpi_dev_ioresource_flags()):

PCI host bridge to bus 0000:00
pci_bus 0000:00: root bus resource [mem 0x000a0000-0x000fffff window]
pci_bus 0000:00: root bus resource [mem 0x80000000-0x8fffffff window]
pci_bus 0000:00: root bus resource [mem 0x80004000000-0x800ffffffff window]
pci_bus 0000:00: root bus resource [bus 00]

[...]

pci 0000:00:03.0: [1002:515e] type 00 class 0x030000
pci 0000:00:03.0: reg 0x10: [mem 0x80000000-0x87ffffff pref]
pci 0000:00:03.0: reg 0x14: [io  0x1000-0x10ff]
pci 0000:00:03.0: reg 0x18: [mem 0x88020000-0x8802ffff]
pci 0000:00:03.0: reg 0x30: [mem 0x88000000-0x8801ffff pref]
pci 0000:00:03.0: supports D1 D2
pci 0000:00:03.0: can't claim BAR 1 [io  0x1000-0x10ff]: no compatible
bridge window

For this reason, the IO port resources boundaries check in generic ACPI
parsing code should be guarded with a CONFIG_X86 guard so that more arches
(ie ARM64) can benefit from the generic ACPI resources parsing interface
without incurring in unexpected resource filtering, fixing at the same
time current breakage on IA64.

This patch factors out IO ports boundary [0 - 64k] check in generic ACPI
code and makes the IO space check X86 specific to make sure that IO
space resources are usable on other arches too.

Fixes: 3772aea7d6f3 ("ia64/PCI/ACPI: Use common ACPI resource parsing
interface for host bridge")
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Jiang Liu <jiang.liu@linux.intel.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Tomasz Nowicki <tn@semihalf.com>
Cc: Mark Salter <msalter@redhat.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
---
v2 -> v3

- Moved IO resource check to generic ACPI resource code
- Dropped Tested-by tags
- Rebased against v4.5

v2: https://marc.info/?l=linux-acpi&m=145521271330332&w=2

v1 -> v2

- Updated commit log to report missing IO resources
- Fixed function ioport_valid() comment 16k/64k typo

v1: https://marc.info/?l=linux-acpi&m=145432228025354&w=2

 drivers/acpi/resource.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index d02fd53..56241eb 100644
--- a/drivers/acpi/resource.c
+++ b/drivers/acpi/resource.c
@@ -27,8 +27,20 @@
 
 #ifdef CONFIG_X86
 #define valid_IRQ(i) (((i) != 0) && ((i) != 2))
+static inline bool acpi_iospace_resource_valid(struct resource *res)
+{
+	/* On X86 IO space is limited to the [0 - 64K] IO port range */
+	return res->end < 0x10003;
+}
 #else
 #define valid_IRQ(i) (true)
+/*
+ * ACPI IO descriptors on arches other than X86 contain MMIO CPU physical
+ * addresses mapping IO space in CPU physical address space, IO space
+ * resources can be placed anywhere in the 64-bit physical address space.
+ */
+static inline bool
+acpi_iospace_resource_valid(struct resource *res) { return true; }
 #endif
 
 static bool acpi_dev_resource_len_valid(u64 start, u64 end, u64 len, bool io)
@@ -127,7 +139,7 @@ static void acpi_dev_ioresource_flags(struct resource *res, u64 len,
 	if (!acpi_dev_resource_len_valid(res->start, res->end, len, true))
 		res->flags |= IORESOURCE_DISABLED | IORESOURCE_UNSET;
 
-	if (res->end >= 0x10003)
+	if (!acpi_iospace_resource_valid(res))
 		res->flags |= IORESOURCE_DISABLED | IORESOURCE_UNSET;
 
 	if (io_decode == ACPI_DECODE_16)
-- 
2.5.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3] PCI: ACPI: IA64: fix IO port generic range check
@ 2016-03-21 11:12 ` Lorenzo Pieralisi
  0 siblings, 0 replies; 10+ messages in thread
From: Lorenzo Pieralisi @ 2016-03-21 11:12 UTC (permalink / raw)
  To: linux-pci, linux-acpi
  Cc: linux-ia64, linux-kernel, Lorenzo Pieralisi, Bjorn Helgaas,
	Hanjun Guo, Jiang Liu, Tony Luck, Tomasz Nowicki, Mark Salter,
	Rafael J. Wysocki

The [0 - 64k] ACPI PCI IO port resource boundary check in:

acpi_dev_ioresource_flags()

is currently applied blindly in the ACPI resource parsing to all
architectures, but only x86 suffers from that IO space limitation.

On arches (ie IA64 and ARM64) where IO space is memory mapped,
the PCI root bridges IO resource windows are firstly initialized from
the _CRS (in acpi_decode_space()) and contain the CPU physical address
at which a root bridge decodes IO space in the CPU physical address
space with the offset value representing the offset required to translate
the PCI bus address into the CPU physical address.

The IO resource windows are then parsed and updated in arch code
before creating and enumerating PCI buses (eg IA64 add_io_space())
to map in an arch specific way the obtained CPU physical address range
to a slice of virtual address space reserved to map PCI IO space,
ending up with PCI bridges resource windows containing IO
resources like the following on a working IA64 configuration:

PCI host bridge to bus 0000:00
pci_bus 0000:00: root bus resource [io  0x1000000-0x100ffff window] (bus
address [0x0000-0xffff])
pci_bus 0000:00: root bus resource [mem 0x000a0000-0x000fffff window]
pci_bus 0000:00: root bus resource [mem 0x80000000-0x8fffffff window]
pci_bus 0000:00: root bus resource [mem 0x80004000000-0x800ffffffff window]
pci_bus 0000:00: root bus resource [bus 00]

This implies that the [0 - 64K] check in acpi_dev_ioresource_flags()
leaves platforms with memory mapped IO space (ie IA64) broken (ie kernel
can't claim IO resources since the host bridge IO resource is disabled
and discarded by ACPI core code, see log on IA64 with missing root bridge
IO resource, silently filtered by current [0 - 64k] check in
acpi_dev_ioresource_flags()):

PCI host bridge to bus 0000:00
pci_bus 0000:00: root bus resource [mem 0x000a0000-0x000fffff window]
pci_bus 0000:00: root bus resource [mem 0x80000000-0x8fffffff window]
pci_bus 0000:00: root bus resource [mem 0x80004000000-0x800ffffffff window]
pci_bus 0000:00: root bus resource [bus 00]

[...]

pci 0000:00:03.0: [1002:515e] type 00 class 0x030000
pci 0000:00:03.0: reg 0x10: [mem 0x80000000-0x87ffffff pref]
pci 0000:00:03.0: reg 0x14: [io  0x1000-0x10ff]
pci 0000:00:03.0: reg 0x18: [mem 0x88020000-0x8802ffff]
pci 0000:00:03.0: reg 0x30: [mem 0x88000000-0x8801ffff pref]
pci 0000:00:03.0: supports D1 D2
pci 0000:00:03.0: can't claim BAR 1 [io  0x1000-0x10ff]: no compatible
bridge window

For this reason, the IO port resources boundaries check in generic ACPI
parsing code should be guarded with a CONFIG_X86 guard so that more arches
(ie ARM64) can benefit from the generic ACPI resources parsing interface
without incurring in unexpected resource filtering, fixing at the same
time current breakage on IA64.

This patch factors out IO ports boundary [0 - 64k] check in generic ACPI
code and makes the IO space check X86 specific to make sure that IO
space resources are usable on other arches too.

Fixes: 3772aea7d6f3 ("ia64/PCI/ACPI: Use common ACPI resource parsing
interface for host bridge")
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Jiang Liu <jiang.liu@linux.intel.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Tomasz Nowicki <tn@semihalf.com>
Cc: Mark Salter <msalter@redhat.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
---
v2 -> v3

- Moved IO resource check to generic ACPI resource code
- Dropped Tested-by tags
- Rebased against v4.5

v2: https://marc.info/?l=linux-acpi&m\x145521271330332&w=2

v1 -> v2

- Updated commit log to report missing IO resources
- Fixed function ioport_valid() comment 16k/64k typo

v1: https://marc.info/?l=linux-acpi&m\x145432228025354&w=2

 drivers/acpi/resource.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index d02fd53..56241eb 100644
--- a/drivers/acpi/resource.c
+++ b/drivers/acpi/resource.c
@@ -27,8 +27,20 @@
 
 #ifdef CONFIG_X86
 #define valid_IRQ(i) (((i) != 0) && ((i) != 2))
+static inline bool acpi_iospace_resource_valid(struct resource *res)
+{
+	/* On X86 IO space is limited to the [0 - 64K] IO port range */
+	return res->end < 0x10003;
+}
 #else
 #define valid_IRQ(i) (true)
+/*
+ * ACPI IO descriptors on arches other than X86 contain MMIO CPU physical
+ * addresses mapping IO space in CPU physical address space, IO space
+ * resources can be placed anywhere in the 64-bit physical address space.
+ */
+static inline bool
+acpi_iospace_resource_valid(struct resource *res) { return true; }
 #endif
 
 static bool acpi_dev_resource_len_valid(u64 start, u64 end, u64 len, bool io)
@@ -127,7 +139,7 @@ static void acpi_dev_ioresource_flags(struct resource *res, u64 len,
 	if (!acpi_dev_resource_len_valid(res->start, res->end, len, true))
 		res->flags |= IORESOURCE_DISABLED | IORESOURCE_UNSET;
 
-	if (res->end >= 0x10003)
+	if (!acpi_iospace_resource_valid(res))
 		res->flags |= IORESOURCE_DISABLED | IORESOURCE_UNSET;
 
 	if (io_decode = ACPI_DECODE_16)
-- 
2.5.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] PCI: ACPI: IA64: fix IO port generic range check
  2016-03-21 11:12 ` Lorenzo Pieralisi
@ 2016-03-21 12:42   ` Rafael J. Wysocki
  -1 siblings, 0 replies; 10+ messages in thread
From: Rafael J. Wysocki @ 2016-03-21 12:42 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Bjorn Helgaas
  Cc: Linux PCI, ACPI Devel Maling List, linux-ia64,
	Linux Kernel Mailing List, Hanjun Guo, Jiang Liu, Tony Luck,
	Tomasz Nowicki, Mark Salter, Rafael J. Wysocki

On Mon, Mar 21, 2016 at 12:12 PM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> The [0 - 64k] ACPI PCI IO port resource boundary check in:
>
> acpi_dev_ioresource_flags()
>
> is currently applied blindly in the ACPI resource parsing to all
> architectures, but only x86 suffers from that IO space limitation.
>
> On arches (ie IA64 and ARM64) where IO space is memory mapped,
> the PCI root bridges IO resource windows are firstly initialized from
> the _CRS (in acpi_decode_space()) and contain the CPU physical address
> at which a root bridge decodes IO space in the CPU physical address
> space with the offset value representing the offset required to translate
> the PCI bus address into the CPU physical address.
>
> The IO resource windows are then parsed and updated in arch code
> before creating and enumerating PCI buses (eg IA64 add_io_space())
> to map in an arch specific way the obtained CPU physical address range
> to a slice of virtual address space reserved to map PCI IO space,
> ending up with PCI bridges resource windows containing IO
> resources like the following on a working IA64 configuration:
>
> PCI host bridge to bus 0000:00
> pci_bus 0000:00: root bus resource [io  0x1000000-0x100ffff window] (bus
> address [0x0000-0xffff])
> pci_bus 0000:00: root bus resource [mem 0x000a0000-0x000fffff window]
> pci_bus 0000:00: root bus resource [mem 0x80000000-0x8fffffff window]
> pci_bus 0000:00: root bus resource [mem 0x80004000000-0x800ffffffff window]
> pci_bus 0000:00: root bus resource [bus 00]
>
> This implies that the [0 - 64K] check in acpi_dev_ioresource_flags()
> leaves platforms with memory mapped IO space (ie IA64) broken (ie kernel
> can't claim IO resources since the host bridge IO resource is disabled
> and discarded by ACPI core code, see log on IA64 with missing root bridge
> IO resource, silently filtered by current [0 - 64k] check in
> acpi_dev_ioresource_flags()):
>
> PCI host bridge to bus 0000:00
> pci_bus 0000:00: root bus resource [mem 0x000a0000-0x000fffff window]
> pci_bus 0000:00: root bus resource [mem 0x80000000-0x8fffffff window]
> pci_bus 0000:00: root bus resource [mem 0x80004000000-0x800ffffffff window]
> pci_bus 0000:00: root bus resource [bus 00]
>
> [...]
>
> pci 0000:00:03.0: [1002:515e] type 00 class 0x030000
> pci 0000:00:03.0: reg 0x10: [mem 0x80000000-0x87ffffff pref]
> pci 0000:00:03.0: reg 0x14: [io  0x1000-0x10ff]
> pci 0000:00:03.0: reg 0x18: [mem 0x88020000-0x8802ffff]
> pci 0000:00:03.0: reg 0x30: [mem 0x88000000-0x8801ffff pref]
> pci 0000:00:03.0: supports D1 D2
> pci 0000:00:03.0: can't claim BAR 1 [io  0x1000-0x10ff]: no compatible
> bridge window
>
> For this reason, the IO port resources boundaries check in generic ACPI
> parsing code should be guarded with a CONFIG_X86 guard so that more arches
> (ie ARM64) can benefit from the generic ACPI resources parsing interface
> without incurring in unexpected resource filtering, fixing at the same
> time current breakage on IA64.
>
> This patch factors out IO ports boundary [0 - 64k] check in generic ACPI
> code and makes the IO space check X86 specific to make sure that IO
> space resources are usable on other arches too.
>
> Fixes: 3772aea7d6f3 ("ia64/PCI/ACPI: Use common ACPI resource parsing
> interface for host bridge")
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Hanjun Guo <hanjun.guo@linaro.org>
> Cc: Jiang Liu <jiang.liu@linux.intel.com>
> Cc: Tony Luck <tony.luck@intel.com>
> Cc: Tomasz Nowicki <tn@semihalf.com>
> Cc: Mark Salter <msalter@redhat.com>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> ---
> v2 -> v3
>
> - Moved IO resource check to generic ACPI resource code
> - Dropped Tested-by tags
> - Rebased against v4.5
>
> v2: https://marc.info/?l=linux-acpi&m=145521271330332&w=2
>
> v1 -> v2
>
> - Updated commit log to report missing IO resources
> - Fixed function ioport_valid() comment 16k/64k typo
>
> v1: https://marc.info/?l=linux-acpi&m=145432228025354&w=2
>
>  drivers/acpi/resource.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> index d02fd53..56241eb 100644
> --- a/drivers/acpi/resource.c
> +++ b/drivers/acpi/resource.c
> @@ -27,8 +27,20 @@
>
>  #ifdef CONFIG_X86
>  #define valid_IRQ(i) (((i) != 0) && ((i) != 2))
> +static inline bool acpi_iospace_resource_valid(struct resource *res)
> +{
> +       /* On X86 IO space is limited to the [0 - 64K] IO port range */
> +       return res->end < 0x10003;
> +}
>  #else
>  #define valid_IRQ(i) (true)
> +/*
> + * ACPI IO descriptors on arches other than X86 contain MMIO CPU physical
> + * addresses mapping IO space in CPU physical address space, IO space
> + * resources can be placed anywhere in the 64-bit physical address space.
> + */
> +static inline bool
> +acpi_iospace_resource_valid(struct resource *res) { return true; }
>  #endif
>
>  static bool acpi_dev_resource_len_valid(u64 start, u64 end, u64 len, bool io)
> @@ -127,7 +139,7 @@ static void acpi_dev_ioresource_flags(struct resource *res, u64 len,
>         if (!acpi_dev_resource_len_valid(res->start, res->end, len, true))
>                 res->flags |= IORESOURCE_DISABLED | IORESOURCE_UNSET;
>
> -       if (res->end >= 0x10003)
> +       if (!acpi_iospace_resource_valid(res))
>                 res->flags |= IORESOURCE_DISABLED | IORESOURCE_UNSET;
>
>         if (io_decode == ACPI_DECODE_16)
> --

This is fine by me.

Bjorn?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] PCI: ACPI: IA64: fix IO port generic range check
@ 2016-03-21 12:42   ` Rafael J. Wysocki
  0 siblings, 0 replies; 10+ messages in thread
From: Rafael J. Wysocki @ 2016-03-21 12:42 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Bjorn Helgaas
  Cc: Linux PCI, ACPI Devel Maling List, linux-ia64,
	Linux Kernel Mailing List, Hanjun Guo, Jiang Liu, Tony Luck,
	Tomasz Nowicki, Mark Salter, Rafael J. Wysocki

On Mon, Mar 21, 2016 at 12:12 PM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> The [0 - 64k] ACPI PCI IO port resource boundary check in:
>
> acpi_dev_ioresource_flags()
>
> is currently applied blindly in the ACPI resource parsing to all
> architectures, but only x86 suffers from that IO space limitation.
>
> On arches (ie IA64 and ARM64) where IO space is memory mapped,
> the PCI root bridges IO resource windows are firstly initialized from
> the _CRS (in acpi_decode_space()) and contain the CPU physical address
> at which a root bridge decodes IO space in the CPU physical address
> space with the offset value representing the offset required to translate
> the PCI bus address into the CPU physical address.
>
> The IO resource windows are then parsed and updated in arch code
> before creating and enumerating PCI buses (eg IA64 add_io_space())
> to map in an arch specific way the obtained CPU physical address range
> to a slice of virtual address space reserved to map PCI IO space,
> ending up with PCI bridges resource windows containing IO
> resources like the following on a working IA64 configuration:
>
> PCI host bridge to bus 0000:00
> pci_bus 0000:00: root bus resource [io  0x1000000-0x100ffff window] (bus
> address [0x0000-0xffff])
> pci_bus 0000:00: root bus resource [mem 0x000a0000-0x000fffff window]
> pci_bus 0000:00: root bus resource [mem 0x80000000-0x8fffffff window]
> pci_bus 0000:00: root bus resource [mem 0x80004000000-0x800ffffffff window]
> pci_bus 0000:00: root bus resource [bus 00]
>
> This implies that the [0 - 64K] check in acpi_dev_ioresource_flags()
> leaves platforms with memory mapped IO space (ie IA64) broken (ie kernel
> can't claim IO resources since the host bridge IO resource is disabled
> and discarded by ACPI core code, see log on IA64 with missing root bridge
> IO resource, silently filtered by current [0 - 64k] check in
> acpi_dev_ioresource_flags()):
>
> PCI host bridge to bus 0000:00
> pci_bus 0000:00: root bus resource [mem 0x000a0000-0x000fffff window]
> pci_bus 0000:00: root bus resource [mem 0x80000000-0x8fffffff window]
> pci_bus 0000:00: root bus resource [mem 0x80004000000-0x800ffffffff window]
> pci_bus 0000:00: root bus resource [bus 00]
>
> [...]
>
> pci 0000:00:03.0: [1002:515e] type 00 class 0x030000
> pci 0000:00:03.0: reg 0x10: [mem 0x80000000-0x87ffffff pref]
> pci 0000:00:03.0: reg 0x14: [io  0x1000-0x10ff]
> pci 0000:00:03.0: reg 0x18: [mem 0x88020000-0x8802ffff]
> pci 0000:00:03.0: reg 0x30: [mem 0x88000000-0x8801ffff pref]
> pci 0000:00:03.0: supports D1 D2
> pci 0000:00:03.0: can't claim BAR 1 [io  0x1000-0x10ff]: no compatible
> bridge window
>
> For this reason, the IO port resources boundaries check in generic ACPI
> parsing code should be guarded with a CONFIG_X86 guard so that more arches
> (ie ARM64) can benefit from the generic ACPI resources parsing interface
> without incurring in unexpected resource filtering, fixing at the same
> time current breakage on IA64.
>
> This patch factors out IO ports boundary [0 - 64k] check in generic ACPI
> code and makes the IO space check X86 specific to make sure that IO
> space resources are usable on other arches too.
>
> Fixes: 3772aea7d6f3 ("ia64/PCI/ACPI: Use common ACPI resource parsing
> interface for host bridge")
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Hanjun Guo <hanjun.guo@linaro.org>
> Cc: Jiang Liu <jiang.liu@linux.intel.com>
> Cc: Tony Luck <tony.luck@intel.com>
> Cc: Tomasz Nowicki <tn@semihalf.com>
> Cc: Mark Salter <msalter@redhat.com>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> ---
> v2 -> v3
>
> - Moved IO resource check to generic ACPI resource code
> - Dropped Tested-by tags
> - Rebased against v4.5
>
> v2: https://marc.info/?l=linux-acpi&m\x145521271330332&w=2
>
> v1 -> v2
>
> - Updated commit log to report missing IO resources
> - Fixed function ioport_valid() comment 16k/64k typo
>
> v1: https://marc.info/?l=linux-acpi&m\x145432228025354&w=2
>
>  drivers/acpi/resource.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> index d02fd53..56241eb 100644
> --- a/drivers/acpi/resource.c
> +++ b/drivers/acpi/resource.c
> @@ -27,8 +27,20 @@
>
>  #ifdef CONFIG_X86
>  #define valid_IRQ(i) (((i) != 0) && ((i) != 2))
> +static inline bool acpi_iospace_resource_valid(struct resource *res)
> +{
> +       /* On X86 IO space is limited to the [0 - 64K] IO port range */
> +       return res->end < 0x10003;
> +}
>  #else
>  #define valid_IRQ(i) (true)
> +/*
> + * ACPI IO descriptors on arches other than X86 contain MMIO CPU physical
> + * addresses mapping IO space in CPU physical address space, IO space
> + * resources can be placed anywhere in the 64-bit physical address space.
> + */
> +static inline bool
> +acpi_iospace_resource_valid(struct resource *res) { return true; }
>  #endif
>
>  static bool acpi_dev_resource_len_valid(u64 start, u64 end, u64 len, bool io)
> @@ -127,7 +139,7 @@ static void acpi_dev_ioresource_flags(struct resource *res, u64 len,
>         if (!acpi_dev_resource_len_valid(res->start, res->end, len, true))
>                 res->flags |= IORESOURCE_DISABLED | IORESOURCE_UNSET;
>
> -       if (res->end >= 0x10003)
> +       if (!acpi_iospace_resource_valid(res))
>                 res->flags |= IORESOURCE_DISABLED | IORESOURCE_UNSET;
>
>         if (io_decode = ACPI_DECODE_16)
> --

This is fine by me.

Bjorn?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] PCI: ACPI: IA64: fix IO port generic range check
  2016-03-21 12:42   ` Rafael J. Wysocki
@ 2016-03-22 13:02     ` Bjorn Helgaas
  -1 siblings, 0 replies; 10+ messages in thread
From: Bjorn Helgaas @ 2016-03-22 13:02 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Lorenzo Pieralisi, Bjorn Helgaas, Linux PCI,
	ACPI Devel Maling List, linux-ia64, Linux Kernel Mailing List,
	Hanjun Guo, Jiang Liu, Tony Luck, Tomasz Nowicki, Mark Salter,
	Rafael J. Wysocki

On Mon, Mar 21, 2016 at 01:42:01PM +0100, Rafael J. Wysocki wrote:
> On Mon, Mar 21, 2016 at 12:12 PM, Lorenzo Pieralisi
> <lorenzo.pieralisi@arm.com> wrote:
> > The [0 - 64k] ACPI PCI IO port resource boundary check in:
> >
> > acpi_dev_ioresource_flags()
> >
> > is currently applied blindly in the ACPI resource parsing to all
> > architectures, but only x86 suffers from that IO space limitation.
> >
> > On arches (ie IA64 and ARM64) where IO space is memory mapped,
> > the PCI root bridges IO resource windows are firstly initialized from
> > the _CRS (in acpi_decode_space()) and contain the CPU physical address
> > at which a root bridge decodes IO space in the CPU physical address
> > space with the offset value representing the offset required to translate
> > the PCI bus address into the CPU physical address.
> >
> > The IO resource windows are then parsed and updated in arch code
> > before creating and enumerating PCI buses (eg IA64 add_io_space())
> > to map in an arch specific way the obtained CPU physical address range
> > to a slice of virtual address space reserved to map PCI IO space,
> > ending up with PCI bridges resource windows containing IO
> > resources like the following on a working IA64 configuration:
> >
> > PCI host bridge to bus 0000:00
> > pci_bus 0000:00: root bus resource [io  0x1000000-0x100ffff window] (bus
> > address [0x0000-0xffff])
> > pci_bus 0000:00: root bus resource [mem 0x000a0000-0x000fffff window]
> > pci_bus 0000:00: root bus resource [mem 0x80000000-0x8fffffff window]
> > pci_bus 0000:00: root bus resource [mem 0x80004000000-0x800ffffffff window]
> > pci_bus 0000:00: root bus resource [bus 00]
> >
> > This implies that the [0 - 64K] check in acpi_dev_ioresource_flags()
> > leaves platforms with memory mapped IO space (ie IA64) broken (ie kernel
> > can't claim IO resources since the host bridge IO resource is disabled
> > and discarded by ACPI core code, see log on IA64 with missing root bridge
> > IO resource, silently filtered by current [0 - 64k] check in
> > acpi_dev_ioresource_flags()):
> >
> > PCI host bridge to bus 0000:00
> > pci_bus 0000:00: root bus resource [mem 0x000a0000-0x000fffff window]
> > pci_bus 0000:00: root bus resource [mem 0x80000000-0x8fffffff window]
> > pci_bus 0000:00: root bus resource [mem 0x80004000000-0x800ffffffff window]
> > pci_bus 0000:00: root bus resource [bus 00]
> >
> > [...]
> >
> > pci 0000:00:03.0: [1002:515e] type 00 class 0x030000
> > pci 0000:00:03.0: reg 0x10: [mem 0x80000000-0x87ffffff pref]
> > pci 0000:00:03.0: reg 0x14: [io  0x1000-0x10ff]
> > pci 0000:00:03.0: reg 0x18: [mem 0x88020000-0x8802ffff]
> > pci 0000:00:03.0: reg 0x30: [mem 0x88000000-0x8801ffff pref]
> > pci 0000:00:03.0: supports D1 D2
> > pci 0000:00:03.0: can't claim BAR 1 [io  0x1000-0x10ff]: no compatible
> > bridge window
> >
> > For this reason, the IO port resources boundaries check in generic ACPI
> > parsing code should be guarded with a CONFIG_X86 guard so that more arches
> > (ie ARM64) can benefit from the generic ACPI resources parsing interface
> > without incurring in unexpected resource filtering, fixing at the same
> > time current breakage on IA64.
> >
> > This patch factors out IO ports boundary [0 - 64k] check in generic ACPI
> > code and makes the IO space check X86 specific to make sure that IO
> > space resources are usable on other arches too.
> >
> > Fixes: 3772aea7d6f3 ("ia64/PCI/ACPI: Use common ACPI resource parsing
> > interface for host bridge")
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: Hanjun Guo <hanjun.guo@linaro.org>
> > Cc: Jiang Liu <jiang.liu@linux.intel.com>
> > Cc: Tony Luck <tony.luck@intel.com>
> > Cc: Tomasz Nowicki <tn@semihalf.com>
> > Cc: Mark Salter <msalter@redhat.com>
> > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> > ---
> > v2 -> v3
> >
> > - Moved IO resource check to generic ACPI resource code
> > - Dropped Tested-by tags
> > - Rebased against v4.5
> >
> > v2: https://marc.info/?l=linux-acpi&m=145521271330332&w=2
> >
> > v1 -> v2
> >
> > - Updated commit log to report missing IO resources
> > - Fixed function ioport_valid() comment 16k/64k typo
> >
> > v1: https://marc.info/?l=linux-acpi&m=145432228025354&w=2
> >
> >  drivers/acpi/resource.c | 14 +++++++++++++-
> >  1 file changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> > index d02fd53..56241eb 100644
> > --- a/drivers/acpi/resource.c
> > +++ b/drivers/acpi/resource.c
> > @@ -27,8 +27,20 @@
> >
> >  #ifdef CONFIG_X86
> >  #define valid_IRQ(i) (((i) != 0) && ((i) != 2))
> > +static inline bool acpi_iospace_resource_valid(struct resource *res)
> > +{
> > +       /* On X86 IO space is limited to the [0 - 64K] IO port range */
> > +       return res->end < 0x10003;
> > +}
> >  #else
> >  #define valid_IRQ(i) (true)
> > +/*
> > + * ACPI IO descriptors on arches other than X86 contain MMIO CPU physical
> > + * addresses mapping IO space in CPU physical address space, IO space
> > + * resources can be placed anywhere in the 64-bit physical address space.
> > + */
> > +static inline bool
> > +acpi_iospace_resource_valid(struct resource *res) { return true; }
> >  #endif
> >
> >  static bool acpi_dev_resource_len_valid(u64 start, u64 end, u64 len, bool io)
> > @@ -127,7 +139,7 @@ static void acpi_dev_ioresource_flags(struct resource *res, u64 len,
> >         if (!acpi_dev_resource_len_valid(res->start, res->end, len, true))
> >                 res->flags |= IORESOURCE_DISABLED | IORESOURCE_UNSET;
> >
> > -       if (res->end >= 0x10003)
> > +       if (!acpi_iospace_resource_valid(res))
> >                 res->flags |= IORESOURCE_DISABLED | IORESOURCE_UNSET;
> >
> >         if (io_decode == ACPI_DECODE_16)
> > --
> 
> This is fine by me.
> 
> Bjorn?

Looks good to me.

Bjorn

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] PCI: ACPI: IA64: fix IO port generic range check
@ 2016-03-22 13:02     ` Bjorn Helgaas
  0 siblings, 0 replies; 10+ messages in thread
From: Bjorn Helgaas @ 2016-03-22 13:02 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Lorenzo Pieralisi, Bjorn Helgaas, Linux PCI,
	ACPI Devel Maling List, linux-ia64, Linux Kernel Mailing List,
	Hanjun Guo, Jiang Liu, Tony Luck, Tomasz Nowicki, Mark Salter,
	Rafael J. Wysocki

On Mon, Mar 21, 2016 at 01:42:01PM +0100, Rafael J. Wysocki wrote:
> On Mon, Mar 21, 2016 at 12:12 PM, Lorenzo Pieralisi
> <lorenzo.pieralisi@arm.com> wrote:
> > The [0 - 64k] ACPI PCI IO port resource boundary check in:
> >
> > acpi_dev_ioresource_flags()
> >
> > is currently applied blindly in the ACPI resource parsing to all
> > architectures, but only x86 suffers from that IO space limitation.
> >
> > On arches (ie IA64 and ARM64) where IO space is memory mapped,
> > the PCI root bridges IO resource windows are firstly initialized from
> > the _CRS (in acpi_decode_space()) and contain the CPU physical address
> > at which a root bridge decodes IO space in the CPU physical address
> > space with the offset value representing the offset required to translate
> > the PCI bus address into the CPU physical address.
> >
> > The IO resource windows are then parsed and updated in arch code
> > before creating and enumerating PCI buses (eg IA64 add_io_space())
> > to map in an arch specific way the obtained CPU physical address range
> > to a slice of virtual address space reserved to map PCI IO space,
> > ending up with PCI bridges resource windows containing IO
> > resources like the following on a working IA64 configuration:
> >
> > PCI host bridge to bus 0000:00
> > pci_bus 0000:00: root bus resource [io  0x1000000-0x100ffff window] (bus
> > address [0x0000-0xffff])
> > pci_bus 0000:00: root bus resource [mem 0x000a0000-0x000fffff window]
> > pci_bus 0000:00: root bus resource [mem 0x80000000-0x8fffffff window]
> > pci_bus 0000:00: root bus resource [mem 0x80004000000-0x800ffffffff window]
> > pci_bus 0000:00: root bus resource [bus 00]
> >
> > This implies that the [0 - 64K] check in acpi_dev_ioresource_flags()
> > leaves platforms with memory mapped IO space (ie IA64) broken (ie kernel
> > can't claim IO resources since the host bridge IO resource is disabled
> > and discarded by ACPI core code, see log on IA64 with missing root bridge
> > IO resource, silently filtered by current [0 - 64k] check in
> > acpi_dev_ioresource_flags()):
> >
> > PCI host bridge to bus 0000:00
> > pci_bus 0000:00: root bus resource [mem 0x000a0000-0x000fffff window]
> > pci_bus 0000:00: root bus resource [mem 0x80000000-0x8fffffff window]
> > pci_bus 0000:00: root bus resource [mem 0x80004000000-0x800ffffffff window]
> > pci_bus 0000:00: root bus resource [bus 00]
> >
> > [...]
> >
> > pci 0000:00:03.0: [1002:515e] type 00 class 0x030000
> > pci 0000:00:03.0: reg 0x10: [mem 0x80000000-0x87ffffff pref]
> > pci 0000:00:03.0: reg 0x14: [io  0x1000-0x10ff]
> > pci 0000:00:03.0: reg 0x18: [mem 0x88020000-0x8802ffff]
> > pci 0000:00:03.0: reg 0x30: [mem 0x88000000-0x8801ffff pref]
> > pci 0000:00:03.0: supports D1 D2
> > pci 0000:00:03.0: can't claim BAR 1 [io  0x1000-0x10ff]: no compatible
> > bridge window
> >
> > For this reason, the IO port resources boundaries check in generic ACPI
> > parsing code should be guarded with a CONFIG_X86 guard so that more arches
> > (ie ARM64) can benefit from the generic ACPI resources parsing interface
> > without incurring in unexpected resource filtering, fixing at the same
> > time current breakage on IA64.
> >
> > This patch factors out IO ports boundary [0 - 64k] check in generic ACPI
> > code and makes the IO space check X86 specific to make sure that IO
> > space resources are usable on other arches too.
> >
> > Fixes: 3772aea7d6f3 ("ia64/PCI/ACPI: Use common ACPI resource parsing
> > interface for host bridge")
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: Hanjun Guo <hanjun.guo@linaro.org>
> > Cc: Jiang Liu <jiang.liu@linux.intel.com>
> > Cc: Tony Luck <tony.luck@intel.com>
> > Cc: Tomasz Nowicki <tn@semihalf.com>
> > Cc: Mark Salter <msalter@redhat.com>
> > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> > ---
> > v2 -> v3
> >
> > - Moved IO resource check to generic ACPI resource code
> > - Dropped Tested-by tags
> > - Rebased against v4.5
> >
> > v2: https://marc.info/?l=linux-acpi&m\x145521271330332&w=2
> >
> > v1 -> v2
> >
> > - Updated commit log to report missing IO resources
> > - Fixed function ioport_valid() comment 16k/64k typo
> >
> > v1: https://marc.info/?l=linux-acpi&m\x145432228025354&w=2
> >
> >  drivers/acpi/resource.c | 14 +++++++++++++-
> >  1 file changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> > index d02fd53..56241eb 100644
> > --- a/drivers/acpi/resource.c
> > +++ b/drivers/acpi/resource.c
> > @@ -27,8 +27,20 @@
> >
> >  #ifdef CONFIG_X86
> >  #define valid_IRQ(i) (((i) != 0) && ((i) != 2))
> > +static inline bool acpi_iospace_resource_valid(struct resource *res)
> > +{
> > +       /* On X86 IO space is limited to the [0 - 64K] IO port range */
> > +       return res->end < 0x10003;
> > +}
> >  #else
> >  #define valid_IRQ(i) (true)
> > +/*
> > + * ACPI IO descriptors on arches other than X86 contain MMIO CPU physical
> > + * addresses mapping IO space in CPU physical address space, IO space
> > + * resources can be placed anywhere in the 64-bit physical address space.
> > + */
> > +static inline bool
> > +acpi_iospace_resource_valid(struct resource *res) { return true; }
> >  #endif
> >
> >  static bool acpi_dev_resource_len_valid(u64 start, u64 end, u64 len, bool io)
> > @@ -127,7 +139,7 @@ static void acpi_dev_ioresource_flags(struct resource *res, u64 len,
> >         if (!acpi_dev_resource_len_valid(res->start, res->end, len, true))
> >                 res->flags |= IORESOURCE_DISABLED | IORESOURCE_UNSET;
> >
> > -       if (res->end >= 0x10003)
> > +       if (!acpi_iospace_resource_valid(res))
> >                 res->flags |= IORESOURCE_DISABLED | IORESOURCE_UNSET;
> >
> >         if (io_decode = ACPI_DECODE_16)
> > --
> 
> This is fine by me.
> 
> Bjorn?

Looks good to me.

Bjorn

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] PCI: ACPI: IA64: fix IO port generic range check
  2016-03-22 13:02     ` Bjorn Helgaas
@ 2016-03-22 14:42       ` Rafael J. Wysocki
  -1 siblings, 0 replies; 10+ messages in thread
From: Rafael J. Wysocki @ 2016-03-22 14:42 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Rafael J. Wysocki, Lorenzo Pieralisi, Bjorn Helgaas, Linux PCI,
	ACPI Devel Maling List, linux-ia64, Linux Kernel Mailing List,
	Hanjun Guo, Jiang Liu, Tony Luck, Tomasz Nowicki, Mark Salter

On Tuesday, March 22, 2016 08:02:19 AM Bjorn Helgaas wrote:
> On Mon, Mar 21, 2016 at 01:42:01PM +0100, Rafael J. Wysocki wrote:
> > On Mon, Mar 21, 2016 at 12:12 PM, Lorenzo Pieralisi

[cut]

> > >
> > > Fixes: 3772aea7d6f3 ("ia64/PCI/ACPI: Use common ACPI resource parsing
> > > interface for host bridge")
> > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > > Cc: Hanjun Guo <hanjun.guo@linaro.org>
> > > Cc: Jiang Liu <jiang.liu@linux.intel.com>
> > > Cc: Tony Luck <tony.luck@intel.com>
> > > Cc: Tomasz Nowicki <tn@semihalf.com>
> > > Cc: Mark Salter <msalter@redhat.com>
> > > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> > > ---
> > > v2 -> v3
> > >
> > > - Moved IO resource check to generic ACPI resource code
> > > - Dropped Tested-by tags
> > > - Rebased against v4.5
> > >
> > > v2: https://marc.info/?l=linux-acpi&m=145521271330332&w=2
> > >
> > > v1 -> v2
> > >
> > > - Updated commit log to report missing IO resources
> > > - Fixed function ioport_valid() comment 16k/64k typo
> > >
> > > v1: https://marc.info/?l=linux-acpi&m=145432228025354&w=2
> > >
> > >  drivers/acpi/resource.c | 14 +++++++++++++-
> > >  1 file changed, 13 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> > > index d02fd53..56241eb 100644
> > > --- a/drivers/acpi/resource.c
> > > +++ b/drivers/acpi/resource.c
> > > @@ -27,8 +27,20 @@
> > >
> > >  #ifdef CONFIG_X86
> > >  #define valid_IRQ(i) (((i) != 0) && ((i) != 2))
> > > +static inline bool acpi_iospace_resource_valid(struct resource *res)
> > > +{
> > > +       /* On X86 IO space is limited to the [0 - 64K] IO port range */
> > > +       return res->end < 0x10003;
> > > +}
> > >  #else
> > >  #define valid_IRQ(i) (true)
> > > +/*
> > > + * ACPI IO descriptors on arches other than X86 contain MMIO CPU physical
> > > + * addresses mapping IO space in CPU physical address space, IO space
> > > + * resources can be placed anywhere in the 64-bit physical address space.
> > > + */
> > > +static inline bool
> > > +acpi_iospace_resource_valid(struct resource *res) { return true; }
> > >  #endif
> > >
> > >  static bool acpi_dev_resource_len_valid(u64 start, u64 end, u64 len, bool io)
> > > @@ -127,7 +139,7 @@ static void acpi_dev_ioresource_flags(struct resource *res, u64 len,
> > >         if (!acpi_dev_resource_len_valid(res->start, res->end, len, true))
> > >                 res->flags |= IORESOURCE_DISABLED | IORESOURCE_UNSET;
> > >
> > > -       if (res->end >= 0x10003)
> > > +       if (!acpi_iospace_resource_valid(res))
> > >                 res->flags |= IORESOURCE_DISABLED | IORESOURCE_UNSET;
> > >
> > >         if (io_decode == ACPI_DECODE_16)
> > > --
> > 
> > This is fine by me.
> > 
> > Bjorn?
> 
> Looks good to me.

OK, thanks!

May I take this as an ACK? :-)


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] PCI: ACPI: IA64: fix IO port generic range check
@ 2016-03-22 14:42       ` Rafael J. Wysocki
  0 siblings, 0 replies; 10+ messages in thread
From: Rafael J. Wysocki @ 2016-03-22 14:42 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Rafael J. Wysocki, Lorenzo Pieralisi, Bjorn Helgaas, Linux PCI,
	ACPI Devel Maling List, linux-ia64, Linux Kernel Mailing List,
	Hanjun Guo, Jiang Liu, Tony Luck, Tomasz Nowicki, Mark Salter

On Tuesday, March 22, 2016 08:02:19 AM Bjorn Helgaas wrote:
> On Mon, Mar 21, 2016 at 01:42:01PM +0100, Rafael J. Wysocki wrote:
> > On Mon, Mar 21, 2016 at 12:12 PM, Lorenzo Pieralisi

[cut]

> > >
> > > Fixes: 3772aea7d6f3 ("ia64/PCI/ACPI: Use common ACPI resource parsing
> > > interface for host bridge")
> > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > > Cc: Hanjun Guo <hanjun.guo@linaro.org>
> > > Cc: Jiang Liu <jiang.liu@linux.intel.com>
> > > Cc: Tony Luck <tony.luck@intel.com>
> > > Cc: Tomasz Nowicki <tn@semihalf.com>
> > > Cc: Mark Salter <msalter@redhat.com>
> > > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> > > ---
> > > v2 -> v3
> > >
> > > - Moved IO resource check to generic ACPI resource code
> > > - Dropped Tested-by tags
> > > - Rebased against v4.5
> > >
> > > v2: https://marc.info/?l=linux-acpi&m\x145521271330332&w=2
> > >
> > > v1 -> v2
> > >
> > > - Updated commit log to report missing IO resources
> > > - Fixed function ioport_valid() comment 16k/64k typo
> > >
> > > v1: https://marc.info/?l=linux-acpi&m\x145432228025354&w=2
> > >
> > >  drivers/acpi/resource.c | 14 +++++++++++++-
> > >  1 file changed, 13 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> > > index d02fd53..56241eb 100644
> > > --- a/drivers/acpi/resource.c
> > > +++ b/drivers/acpi/resource.c
> > > @@ -27,8 +27,20 @@
> > >
> > >  #ifdef CONFIG_X86
> > >  #define valid_IRQ(i) (((i) != 0) && ((i) != 2))
> > > +static inline bool acpi_iospace_resource_valid(struct resource *res)
> > > +{
> > > +       /* On X86 IO space is limited to the [0 - 64K] IO port range */
> > > +       return res->end < 0x10003;
> > > +}
> > >  #else
> > >  #define valid_IRQ(i) (true)
> > > +/*
> > > + * ACPI IO descriptors on arches other than X86 contain MMIO CPU physical
> > > + * addresses mapping IO space in CPU physical address space, IO space
> > > + * resources can be placed anywhere in the 64-bit physical address space.
> > > + */
> > > +static inline bool
> > > +acpi_iospace_resource_valid(struct resource *res) { return true; }
> > >  #endif
> > >
> > >  static bool acpi_dev_resource_len_valid(u64 start, u64 end, u64 len, bool io)
> > > @@ -127,7 +139,7 @@ static void acpi_dev_ioresource_flags(struct resource *res, u64 len,
> > >         if (!acpi_dev_resource_len_valid(res->start, res->end, len, true))
> > >                 res->flags |= IORESOURCE_DISABLED | IORESOURCE_UNSET;
> > >
> > > -       if (res->end >= 0x10003)
> > > +       if (!acpi_iospace_resource_valid(res))
> > >                 res->flags |= IORESOURCE_DISABLED | IORESOURCE_UNSET;
> > >
> > >         if (io_decode = ACPI_DECODE_16)
> > > --
> > 
> > This is fine by me.
> > 
> > Bjorn?
> 
> Looks good to me.

OK, thanks!

May I take this as an ACK? :-)


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] PCI: ACPI: IA64: fix IO port generic range check
  2016-03-21 11:12 ` Lorenzo Pieralisi
@ 2016-03-23  8:35   ` Hanjun Guo
  -1 siblings, 0 replies; 10+ messages in thread
From: Hanjun Guo @ 2016-03-23  8:35 UTC (permalink / raw)
  To: Lorenzo Pieralisi, linux-pci, linux-acpi
  Cc: linux-ia64, linux-kernel, Bjorn Helgaas, Jiang Liu, Tony Luck,
	Tomasz Nowicki, Mark Salter, Rafael J. Wysocki

On 2016/3/21 19:12, Lorenzo Pieralisi wrote:
> The [0 - 64k] ACPI PCI IO port resource boundary check in:
>
> acpi_dev_ioresource_flags()
>
> is currently applied blindly in the ACPI resource parsing to all
> architectures, but only x86 suffers from that IO space limitation.
>
> On arches (ie IA64 and ARM64) where IO space is memory mapped,
> the PCI root bridges IO resource windows are firstly initialized from
> the _CRS (in acpi_decode_space()) and contain the CPU physical address
> at which a root bridge decodes IO space in the CPU physical address
> space with the offset value representing the offset required to translate
> the PCI bus address into the CPU physical address.
>
> The IO resource windows are then parsed and updated in arch code
> before creating and enumerating PCI buses (eg IA64 add_io_space())
> to map in an arch specific way the obtained CPU physical address range
> to a slice of virtual address space reserved to map PCI IO space,
> ending up with PCI bridges resource windows containing IO
> resources like the following on a working IA64 configuration:
>
> PCI host bridge to bus 0000:00
> pci_bus 0000:00: root bus resource [io  0x1000000-0x100ffff window] (bus
> address [0x0000-0xffff])
> pci_bus 0000:00: root bus resource [mem 0x000a0000-0x000fffff window]
> pci_bus 0000:00: root bus resource [mem 0x80000000-0x8fffffff window]
> pci_bus 0000:00: root bus resource [mem 0x80004000000-0x800ffffffff window]
> pci_bus 0000:00: root bus resource [bus 00]
>
> This implies that the [0 - 64K] check in acpi_dev_ioresource_flags()
> leaves platforms with memory mapped IO space (ie IA64) broken (ie kernel
> can't claim IO resources since the host bridge IO resource is disabled
> and discarded by ACPI core code, see log on IA64 with missing root bridge
> IO resource, silently filtered by current [0 - 64k] check in
> acpi_dev_ioresource_flags()):
>
> PCI host bridge to bus 0000:00
> pci_bus 0000:00: root bus resource [mem 0x000a0000-0x000fffff window]
> pci_bus 0000:00: root bus resource [mem 0x80000000-0x8fffffff window]
> pci_bus 0000:00: root bus resource [mem 0x80004000000-0x800ffffffff window]
> pci_bus 0000:00: root bus resource [bus 00]
>
> [...]
>
> pci 0000:00:03.0: [1002:515e] type 00 class 0x030000
> pci 0000:00:03.0: reg 0x10: [mem 0x80000000-0x87ffffff pref]
> pci 0000:00:03.0: reg 0x14: [io  0x1000-0x10ff]
> pci 0000:00:03.0: reg 0x18: [mem 0x88020000-0x8802ffff]
> pci 0000:00:03.0: reg 0x30: [mem 0x88000000-0x8801ffff pref]
> pci 0000:00:03.0: supports D1 D2
> pci 0000:00:03.0: can't claim BAR 1 [io  0x1000-0x10ff]: no compatible
> bridge window
>
> For this reason, the IO port resources boundaries check in generic ACPI
> parsing code should be guarded with a CONFIG_X86 guard so that more arches
> (ie ARM64) can benefit from the generic ACPI resources parsing interface
> without incurring in unexpected resource filtering, fixing at the same
> time current breakage on IA64.
>
> This patch factors out IO ports boundary [0 - 64k] check in generic ACPI
> code and makes the IO space check X86 specific to make sure that IO
> space resources are usable on other arches too.
>
> Fixes: 3772aea7d6f3 ("ia64/PCI/ACPI: Use common ACPI resource parsing
> interface for host bridge")
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Hanjun Guo <hanjun.guo@linaro.org>
> Cc: Jiang Liu <jiang.liu@linux.intel.com>
> Cc: Tony Luck <tony.luck@intel.com>
> Cc: Tomasz Nowicki <tn@semihalf.com>
> Cc: Mark Salter <msalter@redhat.com>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> ---
> v2 -> v3
>
> - Moved IO resource check to generic ACPI resource code
> - Dropped Tested-by tags
> - Rebased against v4.5

Add this patch on top of Linus's latest tree, and test it
on a HP rx2660 IA64 machine, the machine boot with NIC
working properly, and no regressions in boot log.

Tested-by: Hanjun Guo <hanjun.guo@linaro.org>

Thanks
Hanjun

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] PCI: ACPI: IA64: fix IO port generic range check
@ 2016-03-23  8:35   ` Hanjun Guo
  0 siblings, 0 replies; 10+ messages in thread
From: Hanjun Guo @ 2016-03-23  8:35 UTC (permalink / raw)
  To: Lorenzo Pieralisi, linux-pci, linux-acpi
  Cc: linux-ia64, linux-kernel, Bjorn Helgaas, Jiang Liu, Tony Luck,
	Tomasz Nowicki, Mark Salter, Rafael J. Wysocki

On 2016/3/21 19:12, Lorenzo Pieralisi wrote:
> The [0 - 64k] ACPI PCI IO port resource boundary check in:
>
> acpi_dev_ioresource_flags()
>
> is currently applied blindly in the ACPI resource parsing to all
> architectures, but only x86 suffers from that IO space limitation.
>
> On arches (ie IA64 and ARM64) where IO space is memory mapped,
> the PCI root bridges IO resource windows are firstly initialized from
> the _CRS (in acpi_decode_space()) and contain the CPU physical address
> at which a root bridge decodes IO space in the CPU physical address
> space with the offset value representing the offset required to translate
> the PCI bus address into the CPU physical address.
>
> The IO resource windows are then parsed and updated in arch code
> before creating and enumerating PCI buses (eg IA64 add_io_space())
> to map in an arch specific way the obtained CPU physical address range
> to a slice of virtual address space reserved to map PCI IO space,
> ending up with PCI bridges resource windows containing IO
> resources like the following on a working IA64 configuration:
>
> PCI host bridge to bus 0000:00
> pci_bus 0000:00: root bus resource [io  0x1000000-0x100ffff window] (bus
> address [0x0000-0xffff])
> pci_bus 0000:00: root bus resource [mem 0x000a0000-0x000fffff window]
> pci_bus 0000:00: root bus resource [mem 0x80000000-0x8fffffff window]
> pci_bus 0000:00: root bus resource [mem 0x80004000000-0x800ffffffff window]
> pci_bus 0000:00: root bus resource [bus 00]
>
> This implies that the [0 - 64K] check in acpi_dev_ioresource_flags()
> leaves platforms with memory mapped IO space (ie IA64) broken (ie kernel
> can't claim IO resources since the host bridge IO resource is disabled
> and discarded by ACPI core code, see log on IA64 with missing root bridge
> IO resource, silently filtered by current [0 - 64k] check in
> acpi_dev_ioresource_flags()):
>
> PCI host bridge to bus 0000:00
> pci_bus 0000:00: root bus resource [mem 0x000a0000-0x000fffff window]
> pci_bus 0000:00: root bus resource [mem 0x80000000-0x8fffffff window]
> pci_bus 0000:00: root bus resource [mem 0x80004000000-0x800ffffffff window]
> pci_bus 0000:00: root bus resource [bus 00]
>
> [...]
>
> pci 0000:00:03.0: [1002:515e] type 00 class 0x030000
> pci 0000:00:03.0: reg 0x10: [mem 0x80000000-0x87ffffff pref]
> pci 0000:00:03.0: reg 0x14: [io  0x1000-0x10ff]
> pci 0000:00:03.0: reg 0x18: [mem 0x88020000-0x8802ffff]
> pci 0000:00:03.0: reg 0x30: [mem 0x88000000-0x8801ffff pref]
> pci 0000:00:03.0: supports D1 D2
> pci 0000:00:03.0: can't claim BAR 1 [io  0x1000-0x10ff]: no compatible
> bridge window
>
> For this reason, the IO port resources boundaries check in generic ACPI
> parsing code should be guarded with a CONFIG_X86 guard so that more arches
> (ie ARM64) can benefit from the generic ACPI resources parsing interface
> without incurring in unexpected resource filtering, fixing at the same
> time current breakage on IA64.
>
> This patch factors out IO ports boundary [0 - 64k] check in generic ACPI
> code and makes the IO space check X86 specific to make sure that IO
> space resources are usable on other arches too.
>
> Fixes: 3772aea7d6f3 ("ia64/PCI/ACPI: Use common ACPI resource parsing
> interface for host bridge")
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Hanjun Guo <hanjun.guo@linaro.org>
> Cc: Jiang Liu <jiang.liu@linux.intel.com>
> Cc: Tony Luck <tony.luck@intel.com>
> Cc: Tomasz Nowicki <tn@semihalf.com>
> Cc: Mark Salter <msalter@redhat.com>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> ---
> v2 -> v3
>
> - Moved IO resource check to generic ACPI resource code
> - Dropped Tested-by tags
> - Rebased against v4.5

Add this patch on top of Linus's latest tree, and test it
on a HP rx2660 IA64 machine, the machine boot with NIC
working properly, and no regressions in boot log.

Tested-by: Hanjun Guo <hanjun.guo@linaro.org>

Thanks
Hanjun

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2016-03-23  8:35 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-21 11:12 [PATCH v3] PCI: ACPI: IA64: fix IO port generic range check Lorenzo Pieralisi
2016-03-21 11:12 ` Lorenzo Pieralisi
2016-03-21 12:42 ` Rafael J. Wysocki
2016-03-21 12:42   ` Rafael J. Wysocki
2016-03-22 13:02   ` Bjorn Helgaas
2016-03-22 13:02     ` Bjorn Helgaas
2016-03-22 14:42     ` Rafael J. Wysocki
2016-03-22 14:42       ` Rafael J. Wysocki
2016-03-23  8:35 ` Hanjun Guo
2016-03-23  8:35   ` Hanjun Guo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.