Skip to contentSkip to conversation

ACPIPNP and too large IO resources

DISCUSSION Pierre Ossman 4 participants 9 messages Jul 5, 2006 – Aug 5, 2006

Subsystems (2)
ACPI Pnp support
lore
Share & tools
lore

Pierre Ossman

author
#0 lore raw

Hi there!

Commit 1acfb7f2b0d460ee86bdb25ad0679070ec8a5f0d by Bjorn is causing me some grief. Although the patch seems correct, it is triggering another misfeature of the system and I am hoping you have a solution.

Before your patch, the PCI bridge didn't allocate many io ports as they were mislabeled as iomem. But now it puts its dirty paws all over the entire ISA io port address space, effectively disabling PNP.

On my machine it steals the ranges 0x0-0xcf7, 0xcf8-0xcff and 0xd00-0xffff. IOW, the entire range of 0x0-0xffff gets blocked and none of the ISA PNP devices can use ports outside this range.

We can see the same effect in the example given in your commit where only the range 0x3b0-0x3df is left open.

I don't know enough about PNP to determine the problem, but I guess it's the section that checks overlaps with other PNP devices that is somehow wrong. It could also be that everyone keeps coding their DSDTs wrong, but if that's the case then I see little other choice than to be bug compatible.

Rgds
Pierre

#1 lore raw
Quoted 11 lines · Pierre Ossman

Commit 1acfb7f2b0d460ee86bdb25ad0679070ec8a5f0d by Bjorn is causing me some grief. Although the patch seems correct, it is triggering another misfeature of the system and I am hoping you have a solution.

Before your patch, the PCI bridge didn't allocate many io ports as they were mislabeled as iomem. But now it puts its dirty paws all over the entire ISA io port address space, effectively disabling PNP.

On my machine it steals the ranges 0x0-0xcf7, 0xcf8-0xcff and 0xd00-0xffff. IOW, the entire range of 0x0-0xffff gets blocked and none of the ISA PNP devices can use ports outside this range.

Thanks for the report!

It sounds like this might be the same problem as

    http://bugzilla.kernel.org/show_bug.cgi?id=6292

In short, you probably have a bridge device that consumes the entire 0x0-0xffff I/O port range and produces some or all of that range for downstream PNP devices. PNP doesn't know what to do with these windows that are both consumed by the bridge and made available to downstream devices, so it just marks them as being already reserved.

Matthieu Castet wrote a nice patch (attached) that makes PNP just ignore those windows. Can you try it and see whether it fixes the problem you're seeing? This patch is already in -mm, but not yet in mainline. We might need to consider this patch as 2.6.18 material if it resolves your problem. I suspect many people will see the same problem.

If it doesn't fix the problem, could I trouble you to open a report at http://bugzilla.kernel.org and assign it to me? If you could attach the dmesg log and the output of "for i in /sys/bus/pnp/devices/*; do echo $i; cat $i/id; cat $i/resources; cat $i/options; done", that would be useful.

Thanks,
Bjorn

drivers/pnp/pnpacpi/rsparser.c +8 -0
Index: linux-2.6.16/drivers/pnp/pnpacpi/rsparser.c
===================================================================
@@ -170,6 +170,9 @@
 		return;
 	}
 
+    if (p->producer_consumer == ACPI_PRODUCER)
+        return;
+
 	if (p->resource_type == ACPI_MEMORY_RANGE)
 		pnpacpi_parse_allocated_memresource(res_table,
 				p->minimum, p->address_length);

@@ -248,9 +251,14 @@
 		break;
 
 	case ACPI_RESOURCE_TYPE_EXTENDED_ADDRESS64:
+        if (res->data.ext_address64.producer_consumer == ACPI_PRODUCER)
+            return AE_OK;
 		break;
 
 	case ACPI_RESOURCE_TYPE_EXTENDED_IRQ:
+        if (res->data.extended_irq.producer_consumer == ACPI_PRODUCER)
+            return AE_OK;
+
 		for (i = 0; i < res->data.extended_irq.interrupt_count; i++) {
 			pnpacpi_parse_allocated_irqresource(res_table,
 				res->data.extended_irq.interrupts[i],
author
#2 lore raw
Quoted 2 lines · Bjorn Helgaas

Thanks for the report!

Np. Gave me an excuse to try out git bisect. ;)

Quoted 10 lines

It sounds like this might be the same problem as

    http://bugzilla.kernel.org/show_bug.cgi?id=6292

In short, you probably have a bridge device that consumes the entire 0x0-0xffff I/O port range and produces some or all of that range for downstream PNP devices. PNP doesn't know what to do with these windows that are both consumed by the bridge and made available to downstream devices, so it just marks them as being already reserved.

Ah, that explains things.

Quoted 7 lines

Matthieu Castet wrote a nice patch (attached) that makes PNP just ignore those windows. Can you try it and see whether it fixes the problem you're seeing? This patch is already in -mm, but not yet in mainline. We might need to consider this patch as 2.6.18 material if it resolves your problem. I suspect many people will see the same problem.

The patch works nicely and removes all memory and io regions for the PCI bridge but for the range 0xcf8-0xcff.

Thanks
Pierre

#3 lore raw
Quoted 20 lines · Pierre Ossman
Quoted 9 lines · Bjorn Helgaas

It sounds like this might be the same problem as

    http://bugzilla.kernel.org/show_bug.cgi?id=6292

In short, you probably have a bridge device that consumes the entire 0x0-0xffff I/O port range and produces some or all of that range for downstream PNP devices. PNP doesn't know what to do with these windows that are both consumed by the bridge and made available to downstream devices, so it just marks them as being already reserved.

Ah, that explains things.

Quoted 6 lines

Matthieu Castet wrote a nice patch (attached) that makes PNP just ignore those windows. Can you try it and see whether it fixes the problem you're seeing? This patch is already in -mm, but not yet in mainline. We might need to consider this patch as 2.6.18 material if it resolves your problem. I suspect many people will see the same problem.

Quoted 2 lines

The patch works nicely and removes all memory and io regions for the PCI bridge but for the range 0xcf8-0xcff.

Andrew, I think we should try again to push pnpacpi-reject-acpi_producer-resources.patch to the mainline.

Pierre's report (starts here: http://lkml.org/lkml/2006/7/5/20)
is another instance of http://bugzilla.kernel.org/show_bug.cgi?id=6292.

I suspect that many PNP devices are broken in 2.6.17 because of this problem. Probably the only reason we haven't seen more reports is that PNPACPI isn't turned on by default. (Maybe we should do that in -mm?)

You recently proposed pushing it:

    http://marc.theaimsgroup.com/?l=linux-acpi&m=115119275408021&w=2

Len initially nacked it, but I think the outcome of the discussion is that Shaohua doesn't object to this patch. He probably would still like to blacklist PNP0A03, but that's an additional step we don't have to take at the same time.

Bjorn

#4 lore raw
Quoted 36 lines · Bjorn Helgaas
Quoted 20 lines · Pierre Ossman
Quoted 9 lines · Bjorn Helgaas

It sounds like this might be the same problem as

    http://bugzilla.kernel.org/show_bug.cgi?id=6292

In short, you probably have a bridge device that consumes the entire 0x0-0xffff I/O port range and produces some or all of that range for downstream PNP devices. PNP doesn't know what to do with these windows that are both consumed by the bridge and made available to downstream devices, so it just marks them as being already reserved.

Ah, that explains things.

Quoted 6 lines

Matthieu Castet wrote a nice patch (attached) that makes PNP just ignore those windows. Can you try it and see whether it fixes the problem you're seeing? This patch is already in -mm, but not yet in mainline. We might need to consider this patch as 2.6.18 material if it resolves your problem. I suspect many people will see the same problem.

Quoted 2 lines

The patch works nicely and removes all memory and io regions for the PCI bridge but for the range 0xcf8-0xcff.

Andrew, I think we should try again to push pnpacpi-reject-acpi_producer-resources.patch to the mainline.

Pierre's report (starts here: http://lkml.org/lkml/2006/7/5/20)
is another instance of http://bugzilla.kernel.org/show_bug.cgi?id=6292.

I suspect that many PNP devices are broken in 2.6.17 because of this problem. Probably the only reason we haven't seen more reports is that PNPACPI isn't turned on by default. (Maybe we should do that in -mm?)

Quoted 7 lines

You recently proposed pushing it:

    http://marc.theaimsgroup.com/?l=linux-acpi&m=115119275408021&w=2

Len initially nacked it, but I think the outcome of the discussion is that Shaohua doesn't object to this patch. He probably would still like to blacklist PNP0A03, but that's an additional step we don't have to take at the same time.

OK, well let's please push this up the priority list and work out what want to do. If Len's now OK with merging it then I think all lights are green?

pnpacpi-reject-acpi_producer-resources.patch:

From: matthieu castet <[email protected]>

A patch in -mm kernel correct the parsing of "address resources" of pnpacpi. Before we assumed it was memory only, but it could be also IO.

But this change show an hidden bug : some resources could be producer type that are not handled by pnp layer. So we should ignore the producer resources.

This patch fixes bug 6292 (http://bugzilla.kernel.org/show_bug.cgi?id=6292).
Some devices like PNP0A03 have 0xd00-0xffff and 0x0-0xcf7 as IO producer resources.

Before correcting "address resources" parsing, it was seen as memory and was harmless, because nobody tried to reserve this memory range as it should be IO.

With the correction it become IO resources, and make failed all others device that want to register IO in this range and use pnp layer (like a ISA sound card).

The solution is to ignore producer resources

Signed-off-by Matthieu CASTET <[email protected]>
Signed-off-by Uwe Bugla <[email protected]>
Cc Bjorn Helgaas <[email protected]>
Cc Adam Belay <[email protected]>
Cc "Brown, Len" <[email protected]>

akpm: previously nacked, as per comment #26. But am hanging onto it until the thing gets fixed for real.

Signed-off-by Andrew Morton <[email protected]>

---

 drivers/pnp/pnpacpi/rsparser.c |    8 ++++++++
 1 file changed, 8 insertions(+)
drivers/pnp/pnpacpi/rsparser.c renamed from drivers/pnp/pnpacpi/rsparser.c~pnpacpi-reject-acpi_producer-resources +8 -0
diff -puN drivers/pnp/pnpacpi/rsparser.c~pnpacpi-reject-acpi_producer-resources drivers/pnp/pnpacpi/rsparser.c
@@ -173,6 +173,9 @@ pnpacpi_parse_allocated_address_space(st
 		return;
 	}
 
+	if (p->producer_consumer == ACPI_PRODUCER)
+		return;
+
 	if (p->resource_type == ACPI_MEMORY_RANGE)
 		pnpacpi_parse_allocated_memresource(res_table,
 				p->minimum, p->address_length);

@@ -252,9 +255,14 @@ static acpi_status pnpacpi_allocated_res
 		break;
 
 	case ACPI_RESOURCE_TYPE_EXTENDED_ADDRESS64:
+		if (res->data.ext_address64.producer_consumer == ACPI_PRODUCER)
+			return AE_OK;
 		break;
 
 	case ACPI_RESOURCE_TYPE_EXTENDED_IRQ:
+		if (res->data.extended_irq.producer_consumer == ACPI_PRODUCER)
+			return AE_OK;
+
 		for (i = 0; i < res->data.extended_irq.interrupt_count; i++) {
 			pnpacpi_parse_allocated_irqresource(res_table,
 				res->data.extended_irq.interrupts[i],

_

#5 lore raw
Quoted 10 lines · Andrew Morton
Quoted 6 lines · Bjorn Helgaas

You recently proposed pushing it:

    http://marc.theaimsgroup.com/?l=linux-acpi&m=115119275408021&w=2

Len initially nacked it, but I think the outcome of the discussion is that Shaohua doesn't object to this patch. He probably would still like to blacklist PNP0A03, but that's an additional step we don't have to take at the same time.

OK, well let's please push this up the priority list and work out what want to do. If Len's now OK with merging it then I think all lights are green?

My opinion is that we don't need to blacklist PNP0A03 unless we discover a problem.

Len, Shaohua, can you ack this?

Quoted 75 lines

pnpacpi-reject-acpi_producer-resources.patch:

From: matthieu castet <[email protected]>

A patch in -mm kernel correct the parsing of "address resources" of pnpacpi. Before we assumed it was memory only, but it could be also IO.

But this change show an hidden bug : some resources could be producer type that are not handled by pnp layer. So we should ignore the producer resources.

This patch fixes bug 6292 (http://bugzilla.kernel.org/show_bug.cgi?id=6292).
Some devices like PNP0A03 have 0xd00-0xffff and 0x0-0xcf7 as IO producer resources.

Before correcting "address resources" parsing, it was seen as memory and was harmless, because nobody tried to reserve this memory range as it should be IO.

With the correction it become IO resources, and make failed all others device that want to register IO in this range and use pnp layer (like a ISA sound card).

The solution is to ignore producer resources

Signed-off-by Matthieu CASTET <[email protected]>
Signed-off-by Uwe Bugla <[email protected]>
Cc Bjorn Helgaas <[email protected]>
Cc Adam Belay <[email protected]>
Cc "Brown, Len" <[email protected]>

akpm: previously nacked, as per comment #26. But am hanging onto it until the thing gets fixed for real.

Signed-off-by Andrew Morton <[email protected]>

---

 drivers/pnp/pnpacpi/rsparser.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff -puN drivers/pnp/pnpacpi/rsparser.c~pnpacpi-reject-acpi_producer-resources drivers/pnp/pnpacpi/rsparser.c

drivers/pnp/pnpacpi/rsparser.c renamed from drivers/pnp/pnpacpi/rsparser.c~pnpacpi-reject-acpi_producer-resources +8 -0
@@ -173,6 +173,9 @@ pnpacpi_parse_allocated_address_space(st
 		return;
 	}
 
+	if (p->producer_consumer == ACPI_PRODUCER)
+		return;
+
 	if (p->resource_type == ACPI_MEMORY_RANGE)
 		pnpacpi_parse_allocated_memresource(res_table,
 				p->minimum, p->address_length);

@@ -252,9 +255,14 @@ static acpi_status pnpacpi_allocated_res
 		break;
 
 	case ACPI_RESOURCE_TYPE_EXTENDED_ADDRESS64:
+		if (res->data.ext_address64.producer_consumer == ACPI_PRODUCER)
+			return AE_OK;
 		break;
 
 	case ACPI_RESOURCE_TYPE_EXTENDED_IRQ:
+		if (res->data.extended_irq.producer_consumer == ACPI_PRODUCER)
+			return AE_OK;
+
 		for (i = 0; i < res->data.extended_irq.interrupt_count; i++) {
 			pnpacpi_parse_allocated_irqresource(res_table,
 				res->data.extended_irq.interrupts[i],

_

-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#6 lore raw
Quoted 12 lines · Bjorn Helgaas
Quoted 3 lines · Andrew Morton
Quoted 2 lines · Bjorn Helgaas

You recently proposed pushing it:

http://marc.theaimsgroup.com/?l=linux-acpi&m=115119275408021&w=2

Quoted 1 line
Quoted 1 line

Len initially nacked it, but I think the outcome of the

discussion

Quoted 5 lines
Quoted 3 lines

is that Shaohua doesn't object to this patch. He probably would still like to blacklist PNP0A03, but that's an additional step we don't have to take at the same time.

OK, well let's please push this up the priority list and work out

Quoted 8 lines

what want

Quoted 1 line

to do. If Len's now OK with merging it then I think all lights are

green?

My opinion is that we don't need to blacklist PNP0A03 unless we discover a problem.

Len, Shaohua, can you ack this?

Sure. Please merge the patch Bjorn pointed out.

Thanks,
Shaohua

Quoted 95 lines
Quoted 6 lines

pnpacpi-reject-acpi_producer-resources.patch:

From: matthieu castet <[email protected]>

A patch in -mm kernel correct the parsing of "address resources" of

pnpacpi.

Quoted 3 lines

Before we assumed it was memory only, but it could be also IO.

But this change show an hidden bug : some resources could be

producer type

Quoted 1 line

that are not handled by pnp layer. So we should ignore the

producer

Quoted 3 lines

resources.

This patch fixes bug 6292

(http://bugzilla.kernel.org/show_bug.cgi?id=6292).

Quoted 1 line

Some devices like PNP0A03 have 0xd00-0xffff and 0x0-0xcf7 as IO

producer

Quoted 3 lines

resources.

Before correcting "address resources" parsing, it was seen as memory

and was

Quoted 1 line

harmless, because nobody tried to reserve this memory range as it

should be

Quoted 3 lines

IO.

With the correction it become IO resources, and make failed all

others device

Quoted 1 line

that want to register IO in this range and use pnp layer (like a ISA

sound

Quoted 11 lines

card).

The solution is to ignore producer resources

Signed-off-by Matthieu CASTET <[email protected]>
Signed-off-by Uwe Bugla <[email protected]>
Cc Bjorn Helgaas <[email protected]>
Cc Adam Belay <[email protected]>
Cc "Brown, Len" <[email protected]>

akpm: previously nacked, as per comment #26. But am hanging onto it

until the

Quoted 9 lines

thing gets fixed for real.

Signed-off-by Andrew Morton <[email protected]>

---

 drivers/pnp/pnpacpi/rsparser.c |    8 ++++++++ 
 1 file changed, 8 insertions(+)

diff -puN

drivers/pnp/pnpacpi/rsparser.c~pnpacpi-reject-acpi_producer-resources drivers/pnp/pnpacpi/rsparser.c

Quoted 1 line

---

a/drivers/pnp/pnpacpi/rsparser.c~pnpacpi-reject-acpi_producer-resources

Quoted 12 lines

+++ a/drivers/pnp/pnpacpi/rsparser.c

drivers/pnp/pnpacpi/rsparser.c renamed from drivers/pnp/pnpacpi/rsparser.c~pnpacpi-reject-acpi_producer-resources +3 -0
@@ -173,6 +173,9 @@ pnpacpi_parse_allocated_address_space(st 
              return; 
      } 
 
+     if (p->producer_consumer == ACPI_PRODUCER) 
+             return; 
+ 
      if (p->resource_type == ACPI_MEMORY_RANGE) 
              pnpacpi_parse_allocated_memresource(res_table, 
                              p->minimum, p->address_length);

drivers/pnp/pnpacpi/rsparser.c static acpi_status pnpacpi_allocated_res · line 258

Quoted 4 lines
drivers/pnp/pnpacpi/rsparser.c renamed from drivers/pnp/pnpacpi/rsparser.c~pnpacpi-reject-acpi_producer-resources +1 -0
@@ -252,9 +255,14 @@ static acpi_status pnpacpi_allocated_res 
              break; 
 
      case ACPI_RESOURCE_TYPE_EXTENDED_ADDRESS64: 
+             if (res->data.ext_address64.producer_consumer ==

ACPI_PRODUCER)

drivers/pnp/pnpacpi/rsparser.c static acpi_status pnpacpi_allocated_res

Quoted 5 lines
drivers/pnp/pnpacpi/rsparser.c renamed from drivers/pnp/pnpacpi/rsparser.c~pnpacpi-reject-acpi_producer-resources +2 -0
+                     return AE_OK; 
              break; 
 
      case ACPI_RESOURCE_TYPE_EXTENDED_IRQ: 
+             if (res->data.extended_irq.producer_consumer ==

ACPI_PRODUCER)

drivers/pnp/pnpacpi/rsparser.c static acpi_status pnpacpi_allocated_res

Quoted 3 lines
drivers/pnp/pnpacpi/rsparser.c renamed from drivers/pnp/pnpacpi/rsparser.c~pnpacpi-reject-acpi_producer-resources +2 -0
+                     return AE_OK; 
+ 
              for (i = 0; i <

res->data.extended_irq.interrupt_count; i++) {

Quoted 1 line

pnpacpi_parse_allocated_irqresource(res_table,

Quoted 5 lines
                              res->data.extended_irq.interrupts[i],

_

-
To unsubscribe from this list: send the line "unsubscribe

linux-acpi" in

Quoted 3 lines

the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#7 lore raw
Quoted 2 lines · Shaohua Li

Sure. Please merge the patch Bjorn pointed out.

So? What's the status of this? Still not in Linus' tree.

Rgds
Pierre

#8 lore raw
Quoted 5 lines · On Sat, 05 Aug 2006 02:19:46 +0200 Pierre Ossman <[email protected]> wrote:
Quoted 2 lines · Shaohua Li

Sure. Please merge the patch Bjorn pointed out.

So? What's the status of this? Still not in Linus' tree.

We have an ack from Shaohua. I'll merge it.