_DIS ACPI PCI Interrupt Links

PATCH In review Len Brown 2 participants 11 messages Dec 19, 2008 – Jan 9, 2009

Subsystems (2)

ACPI PCI

lore

Share & tools

lore

Len Brown

author reviewer

2008-12-19 08:22 UTC #0 lore raw

We've had the reference counting on Links disabled since 2.6.12. Time to try to enable it and re-learn why we disabled it back then:-)

This allows links to get back into the _DISabled state when drivers free IRQs. They may free IRQs at unload time, or at suspend time.

For suspend, this allows us to "act more like boot", in that links start off in _DIS and are programmed as the drivers request them.

irqrouter_resume() will still restore the links that were not in the _DIS state at suspend. (Though I really don't like this routine, and how
it runs AML with irqs disabled)

One opportunity for a bug on resume we may have is that we don't actually force all the links to _DIS before we start programming them as we do at boot time. We can add that if it turns out to be a problem -- but would do it with interrupts on, like we do at boot...

Anyway, this all works for me...
If Rafael comes through with PCI Core code to free IRQ at suspend for all devices, then we may find outselves very close to the state where all the links are disabled at suspend and irqrouter_resume() has nothing to do.

cheers,
-- Len Brown - Intel Open Source Technology Center

Len Brown

author reviewer

2008-12-19 08:22 UTC #1 lore raw

PATCH 1/2 ACPI: update pci_link debug messages

From: Len Brown <[email protected]>

debug message format/content, no functional change

Signed-off-by Len Brown <[email protected]>

---
 drivers/acpi/pci_link.c |   28 ++++++++++++++++++----------
 1 files changed, 18 insertions(+), 10 deletions(-)

drivers/acpi/pci_link.c +18 ~~-10~~

index e52ad91..595b131 100644
@@ -414,7 +414,8 @@ static int acpi_pci_link_set(struct acpi_pci_link *link, int irq)
 		link->irq.active = irq;
 	}
 
-	ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Set IRQ %d\n", link->irq.active));
+	ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Link %s set to IRQ %d\n",
+		acpi_device_bid(link->device), link->irq.active));
 
       end:
 	kfree(resource);

@@ -649,6 +650,9 @@ acpi_pci_link_allocate_irq(acpi_handle handle,
 		return -1;
 	}
 	link->refcnt++;
+	ACPI_DEBUG_PRINT((ACPI_DB_INFO,
+			  "Link %s is referenced %d\n",
+			  acpi_device_bid(link->device), link->refcnt));
 	mutex_unlock(&acpi_link_lock);
 
 	if (triggering)

@@ -657,9 +661,6 @@ acpi_pci_link_allocate_irq(acpi_handle handle,
 		*polarity = link->irq.polarity;
 	if (name)
 		*name = acpi_device_bid(link->device);
-	ACPI_DEBUG_PRINT((ACPI_DB_INFO,
-			  "Link %s is referenced\n",
-			  acpi_device_bid(link->device)));
 	return (link->irq.active);
 }

@@ -705,10 +706,13 @@ int acpi_pci_link_free_irq(acpi_handle handle)
 	link->refcnt--;
 #endif
 	ACPI_DEBUG_PRINT((ACPI_DB_INFO,
-			  "Link %s is dereferenced\n",
-			  acpi_device_bid(link->device)));
+			  "Link %s is dereferenced %d\n",
+			  acpi_device_bid(link->device), link->refcnt));
 
 	if (link->refcnt == 0) {
+		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
+			  "Link %s is disabled\n",
+			  acpi_device_bid(link->device)));
 		acpi_evaluate_object(link->device->handle, "_DIS", NULL, NULL);
 	}
 	mutex_unlock(&acpi_link_lock);

@@ -782,13 +786,17 @@ static int acpi_pci_link_add(struct acpi_device *device)
 	return result;
 }
 
-static int acpi_pci_link_resume(struct acpi_pci_link *link)
+static void acpi_pci_link_resume(struct acpi_pci_link *link)
 {
 
+	ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Link %s resume ref %d act %d ini %d\n",
+		acpi_device_bid(link->device), link->refcnt,
+		link->irq.active, link->irq.initialized));
+
 	if (link->refcnt && link->irq.active && link->irq.initialized)
-		return (acpi_pci_link_set(link, link->irq.active));
-	else
-		return 0;
+		acpi_pci_link_set(link, link->irq.active);
+
+	return;
 }
 
 static int irqrouter_resume(struct sys_device *dev)

Signature · 1 line

1.5.6.5

Len Brown

author reviewer

2008-12-19 08:22 UTC #2 lore raw

PATCH 2/2 ACPI: PCI Interrupt Links -- disable when unused

In reply to Len Brown

From: Len Brown <[email protected]>

We start the system off with disabled links. Here we enable the reference counting
on the links so that when drivers free IRQs the links can return to the disabled state.

Drivers free their IRQ and reference on a link when they unload. They may also do so upon suspend. However, if they don't, irqrouter_resume() will still resume any links that were
not disabled before suspend.

Signed-off-by Len Brown <[email protected]>

---
 drivers/acpi/pci_link.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

drivers/acpi/pci_link.c +3 -1

index 595b131..7d12762 100644
@@ -693,6 +693,7 @@ int acpi_pci_link_free_irq(acpi_handle handle)
 		printk(KERN_ERR PREFIX "Link isn't initialized\n");
 		return -1;
 	}
+<<<<<<< HEAD:drivers/acpi/pci_link.c
 #ifdef	FUTURE_USE
 	/*
 	 * The Link reference count allows us to _DISable an unused link

@@ -703,8 +704,9 @@ int acpi_pci_link_free_irq(acpi_handle handle)
 	 * to prevent duplicate acpi_pci_link_set()
 	 * which would harm some systems
 	 */
+=======
+>>>>>>> 683984a... ACPI: PCI Interrupt Links -- disable when unused:drivers/acpi/pci_link.c
 	link->refcnt--;
-#endif
 	ACPI_DEBUG_PRINT((ACPI_DB_INFO,
 			  "Link %s is dereferenced %d\n",
 			  acpi_device_bid(link->device), link->refcnt));

Signature · 1 line

1.5.6.5

Yinghai Lu

2008-12-19 09:10 UTC #3 lore raw

Re: [PATCH 2/2] ACPI: PCI Interrupt Links -- disable when unused

Quoted 31 lines · Len Brown

From: Len Brown <[email protected]>
We start the system off with disabled links. Here we enable the reference counting
on the links so that when drivers free IRQs the links can return to the disabled state.
Drivers free their IRQ and reference on a link when they unload. They may also do so upon suspend. However, if they don't, irqrouter_resume() will still resume any links that were
not disabled before suspend.
Signed-off-by Len Brown <[email protected]>
---
 drivers/acpi/pci_link.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
drivers/acpi/pci_link.c +1 -0
index 595b131..7d12762 100644
@@ -693,6 +693,7 @@ int acpi_pci_link_free_irq(acpi_handle handle)
               printk(KERN_ERR PREFIX "Link isn't initialized\n");
               return -1;
       }
+<<<<<<< HEAD:drivers/acpi/pci_link.c
 #ifdef FUTURE_USE
       /*
        * The Link reference count allows us to _DISable an unused link

drivers/acpi/pci_link.c int acpi_pci_link_free_irq(acpi_handle handle) · line 707

Quoted 7 lines

drivers/acpi/pci_link.c +2 -1

@@ -703,8 +704,9 @@ int acpi_pci_link_free_irq(acpi_handle handle)
        * to prevent duplicate acpi_pci_link_set()
        * which would harm some systems
        */
+=======
+>>>>>>> 683984a... ACPI: PCI Interrupt Links -- disable when unused:drivers/acpi/pci_link.c
       link->refcnt--;
-#endif

looks like merge conflicts fix.

Len Brown

author reviewer

2008-12-19 09:33 UTC #4 lore raw

Re: [PATCH 2/2] ACPI: PCI Interrupt Links -- disable when unused

From: Len Brown <[email protected]>
Organization: Intel Open Source Technology Center

We start the system off with disabled links. Here we enable the reference counting
on the links so that when drivers free IRQs the links can return to the disabled state.

Signed-off-by Len Brown <[email protected]>

---
Hmmm, look like I confused git-rebase on that one, here it is again...

thanks,
-Len

---
 drivers/acpi/pci_link.c |   11 -----------
 1 files changed, 0 insertions(+), 11 deletions(-)

drivers/acpi/pci_link.c +0 ~~-11~~

index 595b131..0f7722d 100644
@@ -693,18 +693,7 @@ int acpi_pci_link_free_irq(acpi_handle handle)
 		printk(KERN_ERR PREFIX "Link isn't initialized\n");
 		return -1;
 	}
-#ifdef	FUTURE_USE
-	/*
-	 * The Link reference count allows us to _DISable an unused link
-	 * and suspend time, and set it again  on resume.
-	 * However, 2.6.12 still has irq_router.resume
-	 * which blindly restores the link state.
-	 * So we disable the reference count method
-	 * to prevent duplicate acpi_pci_link_set()
-	 * which would harm some systems
-	 */
 	link->refcnt--;
-#endif
 	ACPI_DEBUG_PRINT((ACPI_DB_INFO,
 			  "Link %s is dereferenced %d\n",
 			  acpi_device_bid(link->device), link->refcnt));

Signature · 1 line

1.6.1.rc3.35.gc0ceb

Yinghai Lu

2008-12-19 11:57 UTC #5 lore raw

Re: [PATCH 2/2] ACPI: PCI Interrupt Links -- disable when unused

Quoted 39 lines · Len Brown

From: Len Brown <[email protected]>
Organization: Intel Open Source Technology Center
We start the system off with disabled links. Here we enable the reference counting
on the links so that when drivers free IRQs the links can return to the disabled state.
Drivers free their IRQ and reference on a link when they unload. They may also do so upon suspend. However, if they don't, irqrouter_resume() will still resume any links that were
not disabled before suspend.
Signed-off-by Len Brown <[email protected]>
---
Hmmm, look like I confused git-rebase on that one, here it is again...
thanks,
(quoted diff) +0 -2
-Len
---
 drivers/acpi/pci_link.c |   11 -----------
 1 files changed, 0 insertions(+), 11 deletions(-)
drivers/acpi/pci_link.c +0 -6
index 595b131..0f7722d 100644
@@ -693,18 +693,7 @@ int acpi_pci_link_free_irq(acpi_handle handle)
               printk(KERN_ERR PREFIX "Link isn't initialized\n");
               return -1;
       }
-#ifdef FUTURE_USE
-       /*
-        * The Link reference count allows us to _DISable an unused link
-        * and suspend time, and set it again  on resume.
-        * However, 2.6.12 still has irq_router.resume
-        * which blindly restores the link state.

drivers/acpi/pci_link.c int acpi_pci_link_free_irq(acpi_handle handle)

Quoted 10 lines

drivers/acpi/pci_link.c +0 -6

-        * So we disable the reference count method
-        * to prevent duplicate acpi_pci_link_set()
-        * which would harm some systems
-        */
       link->refcnt--;
-#endif
       ACPI_DEBUG_PRINT((ACPI_DB_INFO,
                         "Link %s is dereferenced %d\n",
                         acpi_device_bid(link->device), link->refcnt));
--

maybe could have one switch in /proc so could not disable that for some kexec path...

Len Brown

author reviewer

2008-12-19 18:39 UTC #6 lore raw

Re: [PATCH 2/2] ACPI: PCI Interrupt Links -- disable when unused

Quoted 23 lines

 drivers/acpi/pci_link.c |   11 -----------
 1 files changed, 0 insertions(+), 11 deletions(-)

drivers/acpi/pci_link.c +0 ~~-10~~

index 595b131..0f7722d 100644
@@ -693,18 +693,7 @@ int acpi_pci_link_free_irq(acpi_handle handle)
               printk(KERN_ERR PREFIX "Link isn't initialized\n");
               return -1;
       }
-#ifdef FUTURE_USE
-       /*
-        * The Link reference count allows us to _DISable an unused link
-        * and suspend time, and set it again  on resume.
-        * However, 2.6.12 still has irq_router.resume
-        * which blindly restores the link state.
-        * So we disable the reference count method
-        * to prevent duplicate acpi_pci_link_set()
-        * which would harm some systems
-        */
       link->refcnt--;

Quoted 8 lines

drivers/acpi/pci_link.c int acpi_pci_link_free_irq(acpi_handle handle)
Quoted 5 lines
drivers/acpi/pci_link.c +0 -2
-#endif
       ACPI_DEBUG_PRINT((ACPI_DB_INFO,
                         "Link %s is dereferenced %d\n",
                         acpi_device_bid(link->device), link->refcnt));
--
maybe could have one switch in /proc so could not disable that for some kexec path...

I fail to comprehend the benefits of kexec,
and the requirements that kexec puts on the kernel, other than:

# CONFIG_KEXEC is not set

-- Len Brown, Intel Open Source Technology Center

Yinghai Lu

2008-12-19 20:21 UTC #7 lore raw

Re: [PATCH 2/2] ACPI: PCI Interrupt Links -- disable when unused

Quoted 9 lines

Quoted 3 lines
maybe could have one switch in /proc so could not disable that for some kexec path...
I fail to comprehend the benefits of kexec,
and the requirements that kexec puts on the kernel, other than:
# CONFIG_KEXEC is not set

for me: it is a tools that i could use to make sure root-cause is in FW, and BIOS guys will not kick back the ball to os team. after modifying table or hw reg in first kernel, and kexec even stock kernel, everything will work well.

Len Brown

author reviewer

2009-01-09 06:55 UTC #8 lore raw

why kexec (was Re: [PATCH 2/2] ACPI: PCI Interrupt Links -- disable when unused)

Quoted 11 lines

Quoted 8 lines
Quoted 2 lines
maybe could have one switch in /proc so could not disable that for some kexec path...
I fail to comprehend the benefits of kexec,
and the requirements that kexec puts on the kernel, other than:
# CONFIG_KEXEC is not set
for me: it is a tools that i could use to make sure root-cause is in FW, and BIOS guys will not kick back the ball to os team. after modifying table or hw reg in first kernel, and kexec even stock kernel, everything will work well.

For ACPI, we already have the ability to override the BIOS tables upon the 1st boot.

I don't know which BIOS guys you refer to,
but when we find a BIOS bug and have access to
the associated BIOS developer, we've never needed to do such a demonstration to convince them they have a bug.

kexec seems like a science project that has a chance of working only under extremely controlled conditions.
I have no problem with that, as long as it is not built into my kernel.

But say kexec is useful to somebody out there -- what are the requirements that kexec puts on the kernel?

-Len

Yinghai Lu

2009-01-09 07:24 UTC #9 lore raw

Re: why kexec (was Re: [PATCH 2/2] ACPI: PCI Interrupt Links -- disable when unused)

Quoted 12 lines · Len Brown

Quoted 11 lines
Quoted 8 lines
Quoted 2 lines
maybe could have one switch in /proc so could not disable that for some kexec path...
I fail to comprehend the benefits of kexec,
and the requirements that kexec puts on the kernel, other than:
# CONFIG_KEXEC is not set
for me: it is a tools that i could use to make sure root-cause is in FW, and BIOS guys will not kick back the ball to os team. after modifying table or hw reg in first kernel, and kexec even stock kernel, everything will work well.

Quoted 7 lines

For ACPI, we already have the ability to override the BIOS tables upon the 1st boot.
I don't know which BIOS guys you refer to,
but when we find a BIOS bug and have access to
the associated BIOS developer, we've never needed to do such a demonstration to convince them they have a bug.

Your BIOS developer must have enough time or have bunch of BIOS engineers.

Quoted 4 lines

kexec seems like a science project that has a chance of working only under extremely controlled conditions.
I have no problem with that, as long as it is not built into my kernel.

could reduce boot time.
or some system has problem with reset...

or linuxbios + tinykernel to kexec final production kernel. here linuxbios only init ram and basic pci resource allocation ...

Quoted 3 lines

But say kexec is useful to somebody out there -- what are the requirements that kexec puts on the kernel?

the support got into the mainline kernel several years ago... from 2.6.16?

Len Brown

author reviewer

2009-01-09 10:30 UTC #10 lore raw

Re: why kexec (was Re: [PATCH 2/2] ACPI: PCI Interrupt Links -- disable when unused)

Quoted 18 lines · Len Brown

Quoted 15 lines
Quoted 11 lines
Quoted 8 lines
Quoted 2 lines
maybe could have one switch in /proc so could not disable that for some kexec path...
I fail to comprehend the benefits of kexec,
and the requirements that kexec puts on the kernel, other than:
# CONFIG_KEXEC is not set
for me: it is a tools that i could use to make sure root-cause is in FW, and BIOS guys will not kick back the ball to os team. after modifying table or hw reg in first kernel, and kexec even stock kernel, everything will work well.
For ACPI, we already have the ability to override the BIOS tables upon the 1st boot.

Quoted 6 lines

Quoted 4 lines
I don't know which BIOS guys you refer to,
but when we find a BIOS bug and have access to
the associated BIOS developer, we've never needed to do such a demonstration to convince them they have a bug.
Your BIOS developer must have enough time or have bunch of BIOS engineers.

Reasonable engineers do not need their own bugs handed to them on a silver platter.

Quoted 5 lines

Quoted 3 lines
kexec seems like a science project that has a chance of working only under extremely controlled conditions.
I have no problem with that, as long as it is not built into my kernel.
could reduce boot time.

Effort would be better spent on reducing boot time w/ kexec.

Quoted 4 lines

or some system has problem with reset...
or linuxbios + tinykernel to kexec final production kernel. here linuxbios only init ram and basic pci resource allocation ...

My understanding is that the linuxbios people realized that creating a BIOS that boots Linux (tiny or not) only to then boot Linux again, was sort of subgenius. So they re-named linuxbios to be coreboot and now they talk about loading a "payload"....

Quoted 4 lines

Quoted 2 lines
But say kexec is useful to somebody out there -- what are the requirements that kexec puts on the kernel?
the support got into the mainline kernel several years ago... from 2.6.16?

I'm afraid I'm not being clear.
This conversation started when you suggested that we should put a hook in to ask ACPI to manipulate Link state for the benefit of kexec.

My question is, what else does kexec need to properly manage the state of the sytem?

I don't think that question has an answer.
I don't think it will ever have an answer.
I think that kexec is a viable concept only under conditions I don't care about.

-Len