[PATCH 0/2] ACPI: Ignore Consumer/Producer for QWord/DWord/Word Address Space

[PATCH 0/2] ACPI: Ignore Consumer/Producer for QWord/DWord/Word Address Space

[Bjorn Helgaas]

Per spec, the Consumer/Producer bit is defined only for Extended
Address Space descriptors and should be ignored for QWord/DWord/Word
Address Space descriptors.  My understanding is that this is because
x86 BIOSes didn't use the bit consistently, so it couldn't be relied
upon.  The Extended descriptors were added later and are presumably
more reliable.

Linux currently looks at Consumer/Producer for all Address Space
descriptors, although we don't use the result very much.  The x86 and
ia64 host bridge driver code doesn't use Consumer/Producer to identify
windows; it assumes all descriptors are windows.

We do currently use Consumer/Producer to decide whether to apply _TRA.
The change in these patches is to ignore Consumer/Producer for
QWord/DWord/Word, so we'll apply _TRA from Consumer descriptors where
we didn't before.  Per spec, that should be safe because _TRA is
required to be zero for Consumer resources.

If we want to describe host bridge register space and ECAM space
directly in the PNP0A03 device on ARM64 (and I guess potentially even
on x86 & ia64), I think we need changes like this so the arch code can
use IORESOURCE_WINDOW to tell which descriptors are windows and which
are registers.

This is a subtle area, so please take a look and let me know what you
think.

These patches are based on v4.9-rc1.  It's getting pretty late in the
cycle, but I think we'd really like to get the ARM64 ACPI PCI story
sorted out for the v4.10 merge window.

---

Bjorn Helgaas (2):
      ACPI: Combine acpi_dev_resource_address_space() and acpi_dev_resource_ext_address_space()
      ACPI: Ignore Consumer/Producer for QWord/DWord/Word Address Space


 drivers/acpi/ioapic.c          |    3 -
 drivers/acpi/resource.c        |  130 ++++++++++++++++++----------------------
 drivers/pnp/pnpacpi/rsparser.c |    3 -
 include/linux/acpi.h           |    2 -
 4 files changed, 59 insertions(+), 79 deletions(-)

[PATCH 1/2] ACPI: Combine acpi_dev_resource_address_space() and acpi_dev_resource_ext_address_space()

[Bjorn Helgaas]

Previously acpi_dev_resource_address_space() and
acpi_dev_resource_ext_address_space() were wrappers that called
acpi_decode_space().  We need to distinguish between Word/DWord/QWord
address descriptors and Extended address descriptors, which was impossible
in acpi_decode_space().

Fold the acpi_dev_resource_address_space() and
acpi_dev_resource_ext_address_space() functionality into
acpi_decode_space() and call the result acpi_dev_resource_address_space().

No functional change intended.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/acpi/ioapic.c          |    3 -
 drivers/acpi/resource.c        |  114 ++++++++++++++++------------------------
 drivers/pnp/pnpacpi/rsparser.c |    3 -
 include/linux/acpi.h           |    2 -
 4 files changed, 48 insertions(+), 74 deletions(-)

diff --git a/drivers/acpi/ioapic.c b/drivers/acpi/ioapic.c
index 6d7ce6e..6d91417 100644
--- a/drivers/acpi/ioapic.c
+++ b/drivers/acpi/ioapic.c
@@ -50,8 +50,7 @@ static acpi_status setup_res(struct acpi_resource *acpi_res, void *data)
 		return AE_OK;
 
 	if (!acpi_dev_resource_memory(acpi_res, res)) {
-		if (acpi_dev_resource_address_space(acpi_res, &win) ||
-		    acpi_dev_resource_ext_address_space(acpi_res, &win))
+		if (acpi_dev_resource_address_space(acpi_res, &win))
 			*res = win.res;
 	}
 	if ((res->flags & IORESOURCE_PREFETCH) ||
diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index 56241eb..2732d39e 100644
--- a/drivers/acpi/resource.c
+++ b/drivers/acpi/resource.c
@@ -197,16 +197,54 @@ bool acpi_dev_resource_io(struct acpi_resource *ares, struct resource *res)
 }
 EXPORT_SYMBOL_GPL(acpi_dev_resource_io);
 
-static bool acpi_decode_space(struct resource_win *win,
-			      struct acpi_resource_address *addr,
-			      struct acpi_address64_attribute *attr)
+/**
+ * acpi_dev_resource_address_space - Extract ACPI address space information.
+ * @ares: Input ACPI resource object.
+ * @win: Output generic resource object.
+ *
+ * Check if the given ACPI resource object represents an address space resource
+ * and if that's the case, use the information in it to populate the generic
+ * resource object pointed to by @win.
+ *
+ * Return:
+ * 1) false with win->res.flags setting to zero: not the expected resource type
+ * 2) false with IORESOURCE_DISABLED in win->res.flags: valid unassigned
+ *    resource
+ * 3) true: valid assigned resource
+ XXX check these return values
+ */
+bool acpi_dev_resource_address_space(struct acpi_resource *ares,
+				     struct resource_win *win)
 {
-	u8 iodec = attr->granularity == 0xfff ? ACPI_DECODE_10 : ACPI_DECODE_16;
-	bool wp = addr->info.mem.write_protect;
-	u64 len = attr->address_length;
-	u64 start, end, offset = 0;
+	struct acpi_resource_address *addr;
+	struct acpi_address64_attribute *attr;
+	struct acpi_resource_extended_address64 *ext_addr;
+	struct acpi_resource_address64 addr64;
+	acpi_status status;
+	u8 iodec;
+	bool wp;
+	u64 len, start, end, offset = 0;
 	struct resource *res = &win->res;
 
+	win->res.flags = 0;
+
+	if (ares->type == ACPI_RESOURCE_TYPE_EXTENDED_ADDRESS64) {
+		ext_addr = &ares->data.ext_address64;
+		addr = (struct acpi_resource_address *)ext_addr;
+		attr = &ext_addr->address;
+	} else {
+		status = acpi_resource_to_address64(ares, &addr64);
+		if (ACPI_FAILURE(status))
+			return false;
+
+		addr = (struct acpi_resource_address *)&addr64;
+		attr = &addr64.address;
+	}
+
+	iodec = attr->granularity == 0xfff ? ACPI_DECODE_10 : ACPI_DECODE_16;
+	wp = addr->info.mem.write_protect;
+	len = attr->address_length;
+
 	/*
 	 * Filter out invalid descriptor according to ACPI Spec 5.0, section
 	 * 6.4.3.5 Address Space Resource Descriptors.
@@ -264,68 +302,9 @@ static bool acpi_decode_space(struct resource_win *win,
 
 	return !(res->flags & IORESOURCE_DISABLED);
 }
-
-/**
- * acpi_dev_resource_address_space - Extract ACPI address space information.
- * @ares: Input ACPI resource object.
- * @win: Output generic resource object.
- *
- * Check if the given ACPI resource object represents an address space resource
- * and if that's the case, use the information in it to populate the generic
- * resource object pointed to by @win.
- *
- * Return:
- * 1) false with win->res.flags setting to zero: not the expected resource type
- * 2) false with IORESOURCE_DISABLED in win->res.flags: valid unassigned
- *    resource
- * 3) true: valid assigned resource
- */
-bool acpi_dev_resource_address_space(struct acpi_resource *ares,
-				     struct resource_win *win)
-{
-	struct acpi_resource_address64 addr;
-
-	win->res.flags = 0;
-	if (ACPI_FAILURE(acpi_resource_to_address64(ares, &addr)))
-		return false;
-
-	return acpi_decode_space(win, (struct acpi_resource_address *)&addr,
-				 &addr.address);
-}
 EXPORT_SYMBOL_GPL(acpi_dev_resource_address_space);
 
 /**
- * acpi_dev_resource_ext_address_space - Extract ACPI address space information.
- * @ares: Input ACPI resource object.
- * @win: Output generic resource object.
- *
- * Check if the given ACPI resource object represents an extended address space
- * resource and if that's the case, use the information in it to populate the
- * generic resource object pointed to by @win.
- *
- * Return:
- * 1) false with win->res.flags setting to zero: not the expected resource type
- * 2) false with IORESOURCE_DISABLED in win->res.flags: valid unassigned
- *    resource
- * 3) true: valid assigned resource
- */
-bool acpi_dev_resource_ext_address_space(struct acpi_resource *ares,
-					 struct resource_win *win)
-{
-	struct acpi_resource_extended_address64 *ext_addr;
-
-	win->res.flags = 0;
-	if (ares->type != ACPI_RESOURCE_TYPE_EXTENDED_ADDRESS64)
-		return false;
-
-	ext_addr = &ares->data.ext_address64;
-
-	return acpi_decode_space(win, (struct acpi_resource_address *)ext_addr,
-				 &ext_addr->address);
-}
-EXPORT_SYMBOL_GPL(acpi_dev_resource_ext_address_space);
-
-/**
  * acpi_dev_irq_flags - Determine IRQ resource flags.
  * @triggering: Triggering type as provided by ACPI.
  * @polarity: Interrupt polarity as provided by ACPI.
@@ -542,8 +521,7 @@ static acpi_status acpi_dev_process_resource(struct acpi_resource *ares,
 
 	if (acpi_dev_resource_memory(ares, res)
 	    || acpi_dev_resource_io(ares, res)
-	    || acpi_dev_resource_address_space(ares, &win)
-	    || acpi_dev_resource_ext_address_space(ares, &win))
+	    || acpi_dev_resource_address_space(ares, &win))
 		return acpi_dev_new_resource_entry(&win, c);
 
 	for (i = 0; acpi_dev_resource_interrupt(ares, i, res); i++) {
diff --git a/drivers/pnp/pnpacpi/rsparser.c b/drivers/pnp/pnpacpi/rsparser.c
index 4b717c6..bc61651 100644
--- a/drivers/pnp/pnpacpi/rsparser.c
+++ b/drivers/pnp/pnpacpi/rsparser.c
@@ -184,8 +184,7 @@ static acpi_status pnpacpi_allocated_resource(struct acpi_resource *res,
 	struct resource *r = &win.res;
 	int i, flags;
 
-	if (acpi_dev_resource_address_space(res, &win)
-	    || acpi_dev_resource_ext_address_space(res, &win)) {
+	if (acpi_dev_resource_address_space(res, &win)) {
 		pnp_add_resource(dev, &win.res);
 		return AE_OK;
 	}
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index ddbeda6..a06a877 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -399,8 +399,6 @@ bool acpi_dev_resource_memory(struct acpi_resource *ares, struct resource *res);
 bool acpi_dev_resource_io(struct acpi_resource *ares, struct resource *res);
 bool acpi_dev_resource_address_space(struct acpi_resource *ares,
 				     struct resource_win *win);
-bool acpi_dev_resource_ext_address_space(struct acpi_resource *ares,
-					 struct resource_win *win);
 unsigned long acpi_dev_irq_flags(u8 triggering, u8 polarity, u8 shareable);
 unsigned int acpi_dev_get_irq_type(int triggering, int polarity);
 bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,

[PATCH 2/2] ACPI: Ignore Consumer/Producer for QWord/DWord/Word Address Space

[Bjorn Helgaas]

Per ACPI spec r6.0, sec 6.4.3.5.1, 2, 3, Bit [0] of General Flags (the
Consumer/Producer bit) should be ignored for QWord/DWord/Word Address Space
descriptors.  The Consumer/Producer bit is defined only for the Extended
Address Space descriptor.

Ignore Consumer/Producer except for Extended Address Space descriptors.

Note that for QWord/DWord/Word descriptors, we previously applied the
translation offset (_TRA) only when the Consumer/Producer bit was set.
This patch changes that: for those descriptors, we ignore Consumer/Producer
and always apply the translation offset.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/acpi/resource.c |   16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index 2732d39e..b45cd8f 100644
--- a/drivers/acpi/resource.c
+++ b/drivers/acpi/resource.c
@@ -261,11 +261,16 @@ bool acpi_dev_resource_address_space(struct acpi_resource *ares,
 	 * primary side. Non-bridge devices must list 0 for all Address
 	 * Translation offset bits.
 	 */
-	if (addr->producer_consumer == ACPI_PRODUCER)
+	if (ares->type == ACPI_RESOURCE_TYPE_EXTENDED_ADDRESS64) {
+		if (addr->producer_consumer == ACPI_PRODUCER)
+			offset = attr->translation_offset;
+		else if (attr->translation_offset)
+			pr_debug("ACPI: translation_offset(%lld) is invalid for non-bridge device.\n",
+				 attr->translation_offset);
+	} else {
 		offset = attr->translation_offset;
-	else if (attr->translation_offset)
-		pr_debug("ACPI: translation_offset(%lld) is invalid for non-bridge device.\n",
-			 attr->translation_offset);
+	}
+
 	start = attr->minimum + offset;
 	end = attr->maximum + offset;
 
@@ -294,7 +299,8 @@ bool acpi_dev_resource_address_space(struct acpi_resource *ares,
 		return false;
 	}
 
-	if (addr->producer_consumer == ACPI_PRODUCER)
+	if (ares->type == ACPI_RESOURCE_TYPE_EXTENDED_ADDRESS64 &&
+	    addr->producer_consumer == ACPI_PRODUCER)
 		res->flags |= IORESOURCE_WINDOW;
 
 	if (addr->info.mem.caching == ACPI_PREFETCHABLE_MEMORY)

[PATCH 2/2] ACPI: Ignore Consumer/Producer for QWord/DWord/Word Address Space

[Lorenzo Pieralisi]

On Tue, Nov 29, 2016 at 12:43:34PM -0600, Bjorn Helgaas wrote:
> Per ACPI spec r6.0, sec 6.4.3.5.1, 2, 3, Bit [0] of General Flags (the
> Consumer/Producer bit) should be ignored for QWord/DWord/Word Address Space
> descriptors.  The Consumer/Producer bit is defined only for the Extended
> Address Space descriptor.
> 
> Ignore Consumer/Producer except for Extended Address Space descriptors.
> 
> Note that for QWord/DWord/Word descriptors, we previously applied the
> translation offset (_TRA) only when the Consumer/Producer bit was set.

"..Consumer/Producer bit was clear" ? If that bit was set:

struct acpi_resource_address->producer_consumer == ACPI_CONSUMER

and we are not applying the _TRA offset in that case, right ?

Thanks,
Lorenzo

> This patch changes that: for those descriptors, we ignore Consumer/Producer
> and always apply the translation offset.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
>  drivers/acpi/resource.c |   16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> index 2732d39e..b45cd8f 100644
> --- a/drivers/acpi/resource.c
> +++ b/drivers/acpi/resource.c
> @@ -261,11 +261,16 @@ bool acpi_dev_resource_address_space(struct acpi_resource *ares,
>  	 * primary side. Non-bridge devices must list 0 for all Address
>  	 * Translation offset bits.
>  	 */
> -	if (addr->producer_consumer == ACPI_PRODUCER)
> +	if (ares->type == ACPI_RESOURCE_TYPE_EXTENDED_ADDRESS64) {
> +		if (addr->producer_consumer == ACPI_PRODUCER)
> +			offset = attr->translation_offset;
> +		else if (attr->translation_offset)
> +			pr_debug("ACPI: translation_offset(%lld) is invalid for non-bridge device.\n",
> +				 attr->translation_offset);
> +	} else {
>  		offset = attr->translation_offset;
> -	else if (attr->translation_offset)
> -		pr_debug("ACPI: translation_offset(%lld) is invalid for non-bridge device.\n",
> -			 attr->translation_offset);
> +	}
> +
>  	start = attr->minimum + offset;
>  	end = attr->maximum + offset;
>  
> @@ -294,7 +299,8 @@ bool acpi_dev_resource_address_space(struct acpi_resource *ares,
>  		return false;
>  	}
>  
> -	if (addr->producer_consumer == ACPI_PRODUCER)
> +	if (ares->type == ACPI_RESOURCE_TYPE_EXTENDED_ADDRESS64 &&
> +	    addr->producer_consumer == ACPI_PRODUCER)
>  		res->flags |= IORESOURCE_WINDOW;
>  
>  	if (addr->info.mem.caching == ACPI_PREFETCHABLE_MEMORY)
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] ACPI: Ignore Consumer/Producer for QWord/DWord/Word Address Space

[Bjorn Helgaas]

On Wed, Nov 30, 2016 at 12:04:25PM +0000, Lorenzo Pieralisi wrote:
> On Tue, Nov 29, 2016 at 12:43:34PM -0600, Bjorn Helgaas wrote:
> > Per ACPI spec r6.0, sec 6.4.3.5.1, 2, 3, Bit [0] of General Flags (the
> > Consumer/Producer bit) should be ignored for QWord/DWord/Word Address Space
> > descriptors.  The Consumer/Producer bit is defined only for the Extended
> > Address Space descriptor.
> > 
> > Ignore Consumer/Producer except for Extended Address Space descriptors.
> > 
> > Note that for QWord/DWord/Word descriptors, we previously applied the
> > translation offset (_TRA) only when the Consumer/Producer bit was set.
> 
> "..Consumer/Producer bit was clear" ? If that bit was set:
> 
> struct acpi_resource_address->producer_consumer == ACPI_CONSUMER
> 
> and we are not applying the _TRA offset in that case, right ?

Right, of course.  How about this instead?

    Note that for QWord/DWord/Word descriptors, we previously applied the
    translation offset (_TRA) only for Producers, i.e., when the Consumer/
    Producer bit was clear.  This patch changes that: for those descriptors,
    we ignore Consumer/Producer and always apply the translation offset.

> > This patch changes that: for those descriptors, we ignore Consumer/Producer
> > and always apply the translation offset.
> > 
> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> > ---
> >  drivers/acpi/resource.c |   16 +++++++++++-----
> >  1 file changed, 11 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> > index 2732d39e..b45cd8f 100644
> > --- a/drivers/acpi/resource.c
> > +++ b/drivers/acpi/resource.c
> > @@ -261,11 +261,16 @@ bool acpi_dev_resource_address_space(struct acpi_resource *ares,
> >  	 * primary side. Non-bridge devices must list 0 for all Address
> >  	 * Translation offset bits.
> >  	 */
> > -	if (addr->producer_consumer == ACPI_PRODUCER)
> > +	if (ares->type == ACPI_RESOURCE_TYPE_EXTENDED_ADDRESS64) {
> > +		if (addr->producer_consumer == ACPI_PRODUCER)
> > +			offset = attr->translation_offset;
> > +		else if (attr->translation_offset)
> > +			pr_debug("ACPI: translation_offset(%lld) is invalid for non-bridge device.\n",
> > +				 attr->translation_offset);
> > +	} else {
> >  		offset = attr->translation_offset;
> > -	else if (attr->translation_offset)
> > -		pr_debug("ACPI: translation_offset(%lld) is invalid for non-bridge device.\n",
> > -			 attr->translation_offset);
> > +	}
> > +
> >  	start = attr->minimum + offset;
> >  	end = attr->maximum + offset;
> >  
> > @@ -294,7 +299,8 @@ bool acpi_dev_resource_address_space(struct acpi_resource *ares,
> >  		return false;
> >  	}
> >  
> > -	if (addr->producer_consumer == ACPI_PRODUCER)
> > +	if (ares->type == ACPI_RESOURCE_TYPE_EXTENDED_ADDRESS64 &&
> > +	    addr->producer_consumer == ACPI_PRODUCER)
> >  		res->flags |= IORESOURCE_WINDOW;
> >  
> >  	if (addr->info.mem.caching == ACPI_PREFETCHABLE_MEMORY)
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> > the body of a message to majordomo at vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/2] ACPI: Ignore Consumer/Producer for QWord/DWord/Word Address Space

[Lorenzo Pieralisi]

On Wed, Nov 30, 2016 at 09:56:17AM -0600, Bjorn Helgaas wrote:
> On Wed, Nov 30, 2016 at 12:04:25PM +0000, Lorenzo Pieralisi wrote:
> > On Tue, Nov 29, 2016 at 12:43:34PM -0600, Bjorn Helgaas wrote:
> > > Per ACPI spec r6.0, sec 6.4.3.5.1, 2, 3, Bit [0] of General Flags (the
> > > Consumer/Producer bit) should be ignored for QWord/DWord/Word Address Space
> > > descriptors.  The Consumer/Producer bit is defined only for the Extended
> > > Address Space descriptor.
> > > 
> > > Ignore Consumer/Producer except for Extended Address Space descriptors.
> > > 
> > > Note that for QWord/DWord/Word descriptors, we previously applied the
> > > translation offset (_TRA) only when the Consumer/Producer bit was set.
> > 
> > "..Consumer/Producer bit was clear" ? If that bit was set:
> > 
> > struct acpi_resource_address->producer_consumer == ACPI_CONSUMER
> > 
> > and we are not applying the _TRA offset in that case, right ?
> 
> Right, of course.  How about this instead?
> 
>     Note that for QWord/DWord/Word descriptors, we previously applied the
>     translation offset (_TRA) only for Producers, i.e., when the Consumer/
>     Producer bit was clear.  This patch changes that: for those descriptors,
>     we ignore Consumer/Producer and always apply the translation offset.

Yes that's a perfect description, thanks a lot !

Lorenzo

> > > This patch changes that: for those descriptors, we ignore Consumer/Producer
> > > and always apply the translation offset.
> > > 
> > > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> > > ---
> > >  drivers/acpi/resource.c |   16 +++++++++++-----
> > >  1 file changed, 11 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> > > index 2732d39e..b45cd8f 100644
> > > --- a/drivers/acpi/resource.c
> > > +++ b/drivers/acpi/resource.c
> > > @@ -261,11 +261,16 @@ bool acpi_dev_resource_address_space(struct acpi_resource *ares,
> > >  	 * primary side. Non-bridge devices must list 0 for all Address
> > >  	 * Translation offset bits.
> > >  	 */
> > > -	if (addr->producer_consumer == ACPI_PRODUCER)
> > > +	if (ares->type == ACPI_RESOURCE_TYPE_EXTENDED_ADDRESS64) {
> > > +		if (addr->producer_consumer == ACPI_PRODUCER)
> > > +			offset = attr->translation_offset;
> > > +		else if (attr->translation_offset)
> > > +			pr_debug("ACPI: translation_offset(%lld) is invalid for non-bridge device.\n",
> > > +				 attr->translation_offset);
> > > +	} else {
> > >  		offset = attr->translation_offset;
> > > -	else if (attr->translation_offset)
> > > -		pr_debug("ACPI: translation_offset(%lld) is invalid for non-bridge device.\n",
> > > -			 attr->translation_offset);
> > > +	}
> > > +
> > >  	start = attr->minimum + offset;
> > >  	end = attr->maximum + offset;
> > >  
> > > @@ -294,7 +299,8 @@ bool acpi_dev_resource_address_space(struct acpi_resource *ares,
> > >  		return false;
> > >  	}
> > >  
> > > -	if (addr->producer_consumer == ACPI_PRODUCER)
> > > +	if (ares->type == ACPI_RESOURCE_TYPE_EXTENDED_ADDRESS64 &&
> > > +	    addr->producer_consumer == ACPI_PRODUCER)
> > >  		res->flags |= IORESOURCE_WINDOW;
> > >  
> > >  	if (addr->info.mem.caching == ACPI_PREFETCHABLE_MEMORY)
> > > 
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> > > the body of a message to majordomo at vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel at lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 0/2] ACPI: Ignore Consumer/Producer for QWord/DWord/Word Address Space

[Rafael J. Wysocki]

Hi,

On Tue, Nov 29, 2016 at 7:43 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> Per spec, the Consumer/Producer bit is defined only for Extended
> Address Space descriptors and should be ignored for QWord/DWord/Word
> Address Space descriptors.  My understanding is that this is because
> x86 BIOSes didn't use the bit consistently, so it couldn't be relied
> upon.  The Extended descriptors were added later and are presumably
> more reliable.
>
> Linux currently looks at Consumer/Producer for all Address Space
> descriptors, although we don't use the result very much.  The x86 and
> ia64 host bridge driver code doesn't use Consumer/Producer to identify
> windows; it assumes all descriptors are windows.
>
> We do currently use Consumer/Producer to decide whether to apply _TRA.
> The change in these patches is to ignore Consumer/Producer for
> QWord/DWord/Word, so we'll apply _TRA from Consumer descriptors where
> we didn't before.  Per spec, that should be safe because _TRA is
> required to be zero for Consumer resources.
>
> If we want to describe host bridge register space and ECAM space
> directly in the PNP0A03 device on ARM64 (and I guess potentially even
> on x86 & ia64), I think we need changes like this so the arch code can
> use IORESOURCE_WINDOW to tell which descriptors are windows and which
> are registers.

It would be good to copy/move the above paragraph to the changelog of
patch [2/2].

The reason for that change is quite unclear otherwise.

> This is a subtle area, so please take a look and let me know what you
> think.
>
> These patches are based on v4.9-rc1.  It's getting pretty late in the
> cycle, but I think we'd really like to get the ARM64 ACPI PCI story
> sorted out for the v4.10 merge window.

So it is better if changes of this sort spend a few weeks in
linux-next before they get merged, just in case they trigger some
obscure issue and need to be rethought.

I'm not against these changes, but I won't be entirely comfortable
with them going straight into the mainline.

Thanks,
Rafael