linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] MSI: check for BIOS assigned addresses
@ 2009-03-06 15:11 Daniel Yeisley
  2009-03-06 16:24 ` Ingo Molnar
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Daniel Yeisley @ 2009-03-06 15:11 UTC (permalink / raw)
  To: linux-kernel; +Cc: akpm

When creating MSI addresses the kernel makes them up and writes them to
the hardware without first checking for what the system BIOS may have
assigned.  The patch below checks for already assigned values, and
leaves them alone if they exist. 


Signed-off-by: Dan Yeisley <dan.yeisley@unisys.com>

---
diff -Naurp linux-2.6.29-rc5/arch/x86/kernel/io_apic.c linux-2.6.29-rc5-dpy/arch/x86/kernel/io_apic.c
--- linux-2.6.29-rc5/arch/x86/kernel/io_apic.c	2009-02-13 18:31:30.000000000 -0500
+++ linux-2.6.29-rc5-dpy/arch/x86/kernel/io_apic.c	2009-03-04 08:25:30.000000000 -0500
@@ -3258,6 +3258,7 @@ static int msi_compose_msg(struct pci_de
 	struct irq_cfg *cfg;
 	int err;
 	unsigned dest;
+	struct irq_desc *desc = irq_to_desc(irq);
 
 	cfg = irq_cfg(irq);
 	err = assign_irq_vector(irq, cfg, TARGET_CPUS);
@@ -3265,6 +3266,8 @@ static int msi_compose_msg(struct pci_de
 		return err;
 
 	dest = cpu_mask_to_apicid_and(cfg->domain, TARGET_CPUS);
+	
+	read_msi_msg_desc(desc, msg);
 
 #ifdef CONFIG_INTR_REMAP
 	if (irq_remapped(irq)) {
@@ -3295,17 +3298,18 @@ static int msi_compose_msg(struct pci_de
 	} else
 #endif
 	{
-		msg->address_hi = MSI_ADDR_BASE_HI;
-		msg->address_lo =
-			MSI_ADDR_BASE_LO |
-			((INT_DEST_MODE == 0) ?
-				MSI_ADDR_DEST_MODE_PHYSICAL:
-				MSI_ADDR_DEST_MODE_LOGICAL) |
-			((INT_DELIVERY_MODE != dest_LowestPrio) ?
-				MSI_ADDR_REDIRECTION_CPU:
-				MSI_ADDR_REDIRECTION_LOWPRI) |
-			MSI_ADDR_DEST_ID(dest);
-
+		if (!msg->address_hi && !msg->address_lo) {
+			msg->address_hi = MSI_ADDR_BASE_HI;
+			msg->address_lo = 
+				MSI_ADDR_BASE_LO |
+				((INT_DEST_MODE == 0) ?
+					MSI_ADDR_DEST_MODE_PHYSICAL:
+					MSI_ADDR_DEST_MODE_LOGICAL) |
+				((INT_DELIVERY_MODE != dest_LowestPrio) ?
+					MSI_ADDR_REDIRECTION_CPU:
+					MSI_ADDR_REDIRECTION_LOWPRI) |
+				MSI_ADDR_DEST_ID(dest); 
+		}
 		msg->data =
 			MSI_DATA_TRIGGER_EDGE |
 			MSI_DATA_LEVEL_ASSERT |
@@ -3440,11 +3444,13 @@ static int setup_msi_irq(struct pci_dev 
 	int ret;
 	struct msi_msg msg;
 
+	set_irq_msi(irq, msidesc);
 	ret = msi_compose_msg(dev, irq, &msg);
-	if (ret < 0)
+	if (ret < 0) {
+		set_irq_msi(irq, NULL);
 		return ret;
+	}
 
-	set_irq_msi(irq, msidesc);
 	write_msi_msg(irq, &msg);
 
 #ifdef CONFIG_INTR_REMAP



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

* Re: [PATCH] MSI: check for BIOS assigned addresses
  2009-03-06 16:24 ` Ingo Molnar
@ 2009-03-06 15:56   ` Daniel Yeisley
  0 siblings, 0 replies; 11+ messages in thread
From: Daniel Yeisley @ 2009-03-06 15:56 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Yinghai Lu, H. Peter Anvin, Thomas Gleixner, linux-kernel, akpm

ACPI hotplug doesn't work.

Dan


On Fri, 2009-03-06 at 17:24 +0100, Ingo Molnar wrote:
> * Daniel Yeisley <dan.yeisley@unisys.com> wrote:
> 
> > When creating MSI addresses the kernel makes them up and 
> > writes them to the hardware without first checking for what 
> > the system BIOS may have assigned.  The patch below checks for 
> > already assigned values, and leaves them alone if they exist.
> > 
> > Signed-off-by: Dan Yeisley <dan.yeisley@unisys.com>
> 
> What happens without this fix - do you get non-working cards? 
> Boot hang? Or a suboptimal layout?
> 
> 	Ingo


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

* Re: [PATCH] MSI: check for BIOS assigned addresses
  2009-03-06 15:11 [PATCH] MSI: check for BIOS assigned addresses Daniel Yeisley
@ 2009-03-06 16:24 ` Ingo Molnar
  2009-03-06 15:56   ` Daniel Yeisley
  2009-03-06 18:09 ` [tip:x86/urgent] x86: " Daniel Yeisley
  2009-03-06 18:36 ` [tip:x86/apic] " Daniel Yeisley
  2 siblings, 1 reply; 11+ messages in thread
From: Ingo Molnar @ 2009-03-06 16:24 UTC (permalink / raw)
  To: Daniel Yeisley, Yinghai Lu, H. Peter Anvin, Thomas Gleixner
  Cc: linux-kernel, akpm


* Daniel Yeisley <dan.yeisley@unisys.com> wrote:

> When creating MSI addresses the kernel makes them up and 
> writes them to the hardware without first checking for what 
> the system BIOS may have assigned.  The patch below checks for 
> already assigned values, and leaves them alone if they exist.
> 
> Signed-off-by: Dan Yeisley <dan.yeisley@unisys.com>

What happens without this fix - do you get non-working cards? 
Boot hang? Or a suboptimal layout?

	Ingo

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

* [tip:x86/urgent] x86: MSI: check for BIOS assigned addresses
  2009-03-06 15:11 [PATCH] MSI: check for BIOS assigned addresses Daniel Yeisley
  2009-03-06 16:24 ` Ingo Molnar
@ 2009-03-06 18:09 ` Daniel Yeisley
  2009-03-06 18:36 ` [tip:x86/apic] " Daniel Yeisley
  2 siblings, 0 replies; 11+ messages in thread
From: Daniel Yeisley @ 2009-03-06 18:09 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, yinghai, dan.yeisley, tglx, mingo

Commit-ID:  805700617032d9df9bb247ee20571b17ccae28a8
Gitweb:     http://git.kernel.org/tip/805700617032d9df9bb247ee20571b17ccae28a8
Author:     "Daniel Yeisley" <dan.yeisley@unisys.com>
AuthorDate: Fri, 6 Mar 2009 10:11:49 -0500
Commit:     Ingo Molnar <mingo@elte.hu>
CommitDate: Fri, 6 Mar 2009 19:04:37 +0100

x86: MSI: check for BIOS assigned addresses

Impact: fix ACPI hotplug on certain systems

When creating MSI addresses the kernel makes them up and writes them to
the hardware without first checking for what the system BIOS may have
assigned.  The patch below checks for already assigned values, and
leaves them alone if they exist.

Signed-off-by: Dan Yeisley <dan.yeisley@unisys.com>
Cc: Yinghai Lu <yinghai@kernel.org>
LKML-Reference: <1236352309.31604.2.camel@localhost.localdomain>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 arch/x86/kernel/io_apic.c |   32 +++++++++++++++++++-------------
 1 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kernel/io_apic.c b/arch/x86/kernel/io_apic.c
index bc7ac4d..416e5e3 100644
--- a/arch/x86/kernel/io_apic.c
+++ b/arch/x86/kernel/io_apic.c
@@ -3258,6 +3258,7 @@ static int msi_compose_msg(struct pci_dev *pdev, unsigned int irq, struct msi_ms
 	struct irq_cfg *cfg;
 	int err;
 	unsigned dest;
+	struct irq_desc *desc = irq_to_desc(irq);
 
 	cfg = irq_cfg(irq);
 	err = assign_irq_vector(irq, cfg, TARGET_CPUS);
@@ -3266,6 +3267,8 @@ static int msi_compose_msg(struct pci_dev *pdev, unsigned int irq, struct msi_ms
 
 	dest = cpu_mask_to_apicid_and(cfg->domain, TARGET_CPUS);
 
+	read_msi_msg_desc(desc, msg);
+
 #ifdef CONFIG_INTR_REMAP
 	if (irq_remapped(irq)) {
 		struct irte irte;
@@ -3295,17 +3298,18 @@ static int msi_compose_msg(struct pci_dev *pdev, unsigned int irq, struct msi_ms
 	} else
 #endif
 	{
-		msg->address_hi = MSI_ADDR_BASE_HI;
-		msg->address_lo =
-			MSI_ADDR_BASE_LO |
-			((INT_DEST_MODE == 0) ?
-				MSI_ADDR_DEST_MODE_PHYSICAL:
-				MSI_ADDR_DEST_MODE_LOGICAL) |
-			((INT_DELIVERY_MODE != dest_LowestPrio) ?
-				MSI_ADDR_REDIRECTION_CPU:
-				MSI_ADDR_REDIRECTION_LOWPRI) |
-			MSI_ADDR_DEST_ID(dest);
-
+		if (!msg->address_hi && !msg->address_lo) {
+			msg->address_hi = MSI_ADDR_BASE_HI;
+			msg->address_lo =
+				MSI_ADDR_BASE_LO |
+				((INT_DEST_MODE == 0) ?
+					MSI_ADDR_DEST_MODE_PHYSICAL :
+					MSI_ADDR_DEST_MODE_LOGICAL) |
+				((INT_DELIVERY_MODE != dest_LowestPrio) ?
+					MSI_ADDR_REDIRECTION_CPU :
+					MSI_ADDR_REDIRECTION_LOWPRI) |
+				MSI_ADDR_DEST_ID(dest);
+		}
 		msg->data =
 			MSI_DATA_TRIGGER_EDGE |
 			MSI_DATA_LEVEL_ASSERT |
@@ -3440,11 +3444,13 @@ static int setup_msi_irq(struct pci_dev *dev, struct msi_desc *msidesc, int irq)
 	int ret;
 	struct msi_msg msg;
 
+	set_irq_msi(irq, msidesc);
 	ret = msi_compose_msg(dev, irq, &msg);
-	if (ret < 0)
+	if (ret < 0) {
+		set_irq_msi(irq, NULL);
 		return ret;
+	}
 
-	set_irq_msi(irq, msidesc);
 	write_msi_msg(irq, &msg);
 
 #ifdef CONFIG_INTR_REMAP

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

* Re: [tip:x86/apic] x86: MSI: check for BIOS assigned addresses
  2009-03-06 18:45   ` Ingo Molnar
@ 2009-03-06 18:28     ` Daniel Yeisley
  2009-03-06 19:13       ` Ingo Molnar
  0 siblings, 1 reply; 11+ messages in thread
From: Daniel Yeisley @ 2009-03-06 18:28 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: mingo, hpa, linux-kernel, yinghai, stable, tglx, linux-tip-commits

On Fri, 2009-03-06 at 19:45 +0100, Ingo Molnar wrote:
> * Daniel Yeisley <dan.yeisley@unisys.com> wrote:
> 
> > Commit-ID:  affd49d36e2e2df4cbb3e07a57db901c23823b6a
> > Gitweb:     http://git.kernel.org/tip/affd49d36e2e2df4cbb3e07a57db901c23823b6a
> > Author:     "Daniel Yeisley" <dan.yeisley@unisys.com>
> > AuthorDate: Fri, 6 Mar 2009 10:11:49 -0500
> > Commit:     Ingo Molnar <mingo@elte.hu>
> > CommitDate: Fri, 6 Mar 2009 19:32:44 +0100
> > 
> > x86: MSI: check for BIOS assigned addresses
> > 
> > Impact: fix ACPI hotplug on certain systems
> 
> Note, this commit means that i moved the commit over from 
> tip:x86/urgent into tip:x86/apic - i.e. with a 2.6.30 merge 
> date. When exactly was this regression introduced? I think this 
> code never really accepted the MSI settings from the firmware, 
> so changing that behavior might affect a lot of systems.
> 
> So i'm uneasy about applying this fix this late in the .29 
> cycle, but marked it for -stable backport so it could hit 
> 2.6.29.1 if it stays problem-free.
> 
> 	Ingo

I initially noticed the problem on a 2.6.18 kernel.  ACPI hotplugging
didn't work unless I booted with pci=nomsi.  I don't have a problem with
waiting until 2.6.30 to make sure it is thoroughly tested.

Dan


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

* [tip:x86/apic] x86: MSI: check for BIOS assigned addresses
  2009-03-06 15:11 [PATCH] MSI: check for BIOS assigned addresses Daniel Yeisley
  2009-03-06 16:24 ` Ingo Molnar
  2009-03-06 18:09 ` [tip:x86/urgent] x86: " Daniel Yeisley
@ 2009-03-06 18:36 ` Daniel Yeisley
  2009-03-06 18:45   ` Ingo Molnar
  2 siblings, 1 reply; 11+ messages in thread
From: Daniel Yeisley @ 2009-03-06 18:36 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, yinghai, stable, dan.yeisley, tglx, mingo

Commit-ID:  affd49d36e2e2df4cbb3e07a57db901c23823b6a
Gitweb:     http://git.kernel.org/tip/affd49d36e2e2df4cbb3e07a57db901c23823b6a
Author:     "Daniel Yeisley" <dan.yeisley@unisys.com>
AuthorDate: Fri, 6 Mar 2009 10:11:49 -0500
Commit:     Ingo Molnar <mingo@elte.hu>
CommitDate: Fri, 6 Mar 2009 19:32:44 +0100

x86: MSI: check for BIOS assigned addresses

Impact: fix ACPI hotplug on certain systems

When creating MSI addresses the kernel makes them up and writes them to
the hardware without first checking for what the system BIOS may have
assigned.  The patch below checks for already assigned values, and
leaves them alone if they exist.

Signed-off-by: Dan Yeisley <dan.yeisley@unisys.com>
Cc: Yinghai Lu <yinghai@kernel.org>
Cc: <stable@kernel.org>
LKML-Reference: <1236352309.31604.2.camel@localhost.localdomain>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 arch/x86/kernel/apic/io_apic.c |   32 +++++++++++++++++++-------------
 1 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 00e6071..60484e1 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -3254,6 +3254,7 @@ static int msi_compose_msg(struct pci_dev *pdev, unsigned int irq, struct msi_ms
 	struct irq_cfg *cfg;
 	int err;
 	unsigned dest;
+	struct irq_desc *desc = irq_to_desc(irq);
 
 	if (disable_apic)
 		return -ENXIO;
@@ -3265,6 +3266,8 @@ static int msi_compose_msg(struct pci_dev *pdev, unsigned int irq, struct msi_ms
 
 	dest = apic->cpu_mask_to_apicid_and(cfg->domain, apic->target_cpus());
 
+	read_msi_msg_desc(desc, msg);
+
 #ifdef CONFIG_INTR_REMAP
 	if (irq_remapped(irq)) {
 		struct irte irte;
@@ -3294,17 +3297,18 @@ static int msi_compose_msg(struct pci_dev *pdev, unsigned int irq, struct msi_ms
 	} else
 #endif
 	{
-		msg->address_hi = MSI_ADDR_BASE_HI;
-		msg->address_lo =
-			MSI_ADDR_BASE_LO |
-			((apic->irq_dest_mode == 0) ?
-				MSI_ADDR_DEST_MODE_PHYSICAL:
-				MSI_ADDR_DEST_MODE_LOGICAL) |
-			((apic->irq_delivery_mode != dest_LowestPrio) ?
-				MSI_ADDR_REDIRECTION_CPU:
-				MSI_ADDR_REDIRECTION_LOWPRI) |
-			MSI_ADDR_DEST_ID(dest);
-
+		if (!msg->address_hi && !msg->address_lo) {
+			msg->address_hi = MSI_ADDR_BASE_HI;
+			msg->address_lo =
+				MSI_ADDR_BASE_LO |
+				((apic->irq_dest_mode == 0) ?
+					MSI_ADDR_DEST_MODE_PHYSICAL :
+					MSI_ADDR_DEST_MODE_LOGICAL) |
+				((apic->irq_delivery_mode != dest_LowestPrio) ?
+					MSI_ADDR_REDIRECTION_CPU :
+					MSI_ADDR_REDIRECTION_LOWPRI) |
+				MSI_ADDR_DEST_ID(dest);
+		}
 		msg->data =
 			MSI_DATA_TRIGGER_EDGE |
 			MSI_DATA_LEVEL_ASSERT |
@@ -3439,11 +3443,13 @@ static int setup_msi_irq(struct pci_dev *dev, struct msi_desc *msidesc, int irq)
 	int ret;
 	struct msi_msg msg;
 
+	set_irq_msi(irq, msidesc);
 	ret = msi_compose_msg(dev, irq, &msg);
-	if (ret < 0)
+	if (ret < 0) {
+		set_irq_msi(irq, NULL);
 		return ret;
+	}
 
-	set_irq_msi(irq, msidesc);
 	write_msi_msg(irq, &msg);
 
 #ifdef CONFIG_INTR_REMAP

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

* Re: [tip:x86/apic] x86: MSI: check for BIOS assigned addresses
  2009-03-06 18:36 ` [tip:x86/apic] " Daniel Yeisley
@ 2009-03-06 18:45   ` Ingo Molnar
  2009-03-06 18:28     ` Daniel Yeisley
  0 siblings, 1 reply; 11+ messages in thread
From: Ingo Molnar @ 2009-03-06 18:45 UTC (permalink / raw)
  To: mingo, hpa, linux-kernel, yinghai, stable, dan.yeisley, tglx
  Cc: linux-tip-commits


* Daniel Yeisley <dan.yeisley@unisys.com> wrote:

> Commit-ID:  affd49d36e2e2df4cbb3e07a57db901c23823b6a
> Gitweb:     http://git.kernel.org/tip/affd49d36e2e2df4cbb3e07a57db901c23823b6a
> Author:     "Daniel Yeisley" <dan.yeisley@unisys.com>
> AuthorDate: Fri, 6 Mar 2009 10:11:49 -0500
> Commit:     Ingo Molnar <mingo@elte.hu>
> CommitDate: Fri, 6 Mar 2009 19:32:44 +0100
> 
> x86: MSI: check for BIOS assigned addresses
> 
> Impact: fix ACPI hotplug on certain systems

Note, this commit means that i moved the commit over from 
tip:x86/urgent into tip:x86/apic - i.e. with a 2.6.30 merge 
date. When exactly was this regression introduced? I think this 
code never really accepted the MSI settings from the firmware, 
so changing that behavior might affect a lot of systems.

So i'm uneasy about applying this fix this late in the .29 
cycle, but marked it for -stable backport so it could hit 
2.6.29.1 if it stays problem-free.

	Ingo

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

* Re: [tip:x86/apic] x86: MSI: check for BIOS assigned addresses
  2009-03-06 18:28     ` Daniel Yeisley
@ 2009-03-06 19:13       ` Ingo Molnar
  2009-03-07  1:06         ` Yinghai Lu
  0 siblings, 1 reply; 11+ messages in thread
From: Ingo Molnar @ 2009-03-06 19:13 UTC (permalink / raw)
  To: Daniel Yeisley
  Cc: mingo, hpa, linux-kernel, yinghai, stable, tglx, linux-tip-commits


* Daniel Yeisley <dan.yeisley@unisys.com> wrote:

> On Fri, 2009-03-06 at 19:45 +0100, Ingo Molnar wrote:
> > * Daniel Yeisley <dan.yeisley@unisys.com> wrote:
> > 
> > > Commit-ID:  affd49d36e2e2df4cbb3e07a57db901c23823b6a
> > > Gitweb:     http://git.kernel.org/tip/affd49d36e2e2df4cbb3e07a57db901c23823b6a
> > > Author:     "Daniel Yeisley" <dan.yeisley@unisys.com>
> > > AuthorDate: Fri, 6 Mar 2009 10:11:49 -0500
> > > Commit:     Ingo Molnar <mingo@elte.hu>
> > > CommitDate: Fri, 6 Mar 2009 19:32:44 +0100
> > > 
> > > x86: MSI: check for BIOS assigned addresses
> > > 
> > > Impact: fix ACPI hotplug on certain systems
> > 
> > Note, this commit means that i moved the commit over from 
> > tip:x86/urgent into tip:x86/apic - i.e. with a 2.6.30 merge 
> > date. When exactly was this regression introduced? I think this 
> > code never really accepted the MSI settings from the firmware, 
> > so changing that behavior might affect a lot of systems.
> > 
> > So i'm uneasy about applying this fix this late in the .29 
> > cycle, but marked it for -stable backport so it could hit 
> > 2.6.29.1 if it stays problem-free.
> > 
> > 	Ingo
> 
> I initially noticed the problem on a 2.6.18 kernel.  ACPI 
> hotplugging didn't work unless I booted with pci=nomsi.  I 
> don't have a problem with waiting until 2.6.30 to make sure it 
> is thoroughly tested.

Okay - so it's an ancient bug. The current track should thus be 
fine.

Thanks,

	Ingo

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

* Re: [tip:x86/apic] x86: MSI: check for BIOS assigned addresses
  2009-03-06 19:13       ` Ingo Molnar
@ 2009-03-07  1:06         ` Yinghai Lu
  2009-03-08 10:54           ` Ingo Molnar
  0 siblings, 1 reply; 11+ messages in thread
From: Yinghai Lu @ 2009-03-07  1:06 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Daniel Yeisley, mingo, hpa, linux-kernel, stable, tglx, linux-pci

Ingo Molnar wrote:
> * Daniel Yeisley <dan.yeisley@unisys.com> wrote:
> 
>> On Fri, 2009-03-06 at 19:45 +0100, Ingo Molnar wrote:
>>> * Daniel Yeisley <dan.yeisley@unisys.com> wrote:
>>>
>>>> Commit-ID:  affd49d36e2e2df4cbb3e07a57db901c23823b6a
>>>> Gitweb:     http://git.kernel.org/tip/affd49d36e2e2df4cbb3e07a57db901c23823b6a
>>>> Author:     "Daniel Yeisley" <dan.yeisley@unisys.com>
>>>> AuthorDate: Fri, 6 Mar 2009 10:11:49 -0500
>>>> Commit:     Ingo Molnar <mingo@elte.hu>
>>>> CommitDate: Fri, 6 Mar 2009 19:32:44 +0100
>>>>
>>>> x86: MSI: check for BIOS assigned addresses
>>>>
>>>> Impact: fix ACPI hotplug on certain systems
>>> Note, this commit means that i moved the commit over from 
>>> tip:x86/urgent into tip:x86/apic - i.e. with a 2.6.30 merge 
>>> date. When exactly was this regression introduced? I think this 
>>> code never really accepted the MSI settings from the firmware, 
>>> so changing that behavior might affect a lot of systems.
>>>
>>> So i'm uneasy about applying this fix this late in the .29 
>>> cycle, but marked it for -stable backport so it could hit 
>>> 2.6.29.1 if it stays problem-free.
>>>
>>> 	Ingo
>> I initially noticed the problem on a 2.6.18 kernel.  ACPI 
>> hotplugging didn't work unless I booted with pci=nomsi.  I 
>> don't have a problem with waiting until 2.6.30 to make sure it 
>> is thoroughly tested.
> 
> Okay - so it's an ancient bug. The current track should thus be 
> fine.
> 

please drop this patch. it seems it break msi-x.
at least igbe with msi-x does not work anymore.

YH

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

* Re: [tip:x86/apic] x86: MSI: check for BIOS assigned addresses
  2009-03-07  1:06         ` Yinghai Lu
@ 2009-03-08 10:54           ` Ingo Molnar
  2009-03-09 14:32             ` Daniel Yeisley
  0 siblings, 1 reply; 11+ messages in thread
From: Ingo Molnar @ 2009-03-08 10:54 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Daniel Yeisley, mingo, hpa, linux-kernel, stable, tglx, linux-pci


* Yinghai Lu <yinghai@kernel.org> wrote:

> Ingo Molnar wrote:
> > * Daniel Yeisley <dan.yeisley@unisys.com> wrote:
> > 
> >> On Fri, 2009-03-06 at 19:45 +0100, Ingo Molnar wrote:
> >>> * Daniel Yeisley <dan.yeisley@unisys.com> wrote:
> >>>
> >>>> Commit-ID:  affd49d36e2e2df4cbb3e07a57db901c23823b6a
> >>>> Gitweb:     http://git.kernel.org/tip/affd49d36e2e2df4cbb3e07a57db901c23823b6a
> >>>> Author:     "Daniel Yeisley" <dan.yeisley@unisys.com>
> >>>> AuthorDate: Fri, 6 Mar 2009 10:11:49 -0500
> >>>> Commit:     Ingo Molnar <mingo@elte.hu>
> >>>> CommitDate: Fri, 6 Mar 2009 19:32:44 +0100
> >>>>
> >>>> x86: MSI: check for BIOS assigned addresses
> >>>>
> >>>> Impact: fix ACPI hotplug on certain systems
> >>> Note, this commit means that i moved the commit over from 
> >>> tip:x86/urgent into tip:x86/apic - i.e. with a 2.6.30 merge 
> >>> date. When exactly was this regression introduced? I think this 
> >>> code never really accepted the MSI settings from the firmware, 
> >>> so changing that behavior might affect a lot of systems.
> >>>
> >>> So i'm uneasy about applying this fix this late in the .29 
> >>> cycle, but marked it for -stable backport so it could hit 
> >>> 2.6.29.1 if it stays problem-free.
> >>>
> >>> 	Ingo
> >> I initially noticed the problem on a 2.6.18 kernel.  ACPI 
> >> hotplugging didn't work unless I booted with pci=nomsi.  I 
> >> don't have a problem with waiting until 2.6.30 to make sure it 
> >> is thoroughly tested.
> > 
> > Okay - so it's an ancient bug. The current track should thus be 
> > fine.
> > 
> 
> please drop this patch. it seems it break msi-x.
> at least igbe with msi-x does not work anymore.

Not just that, it also causes bootup crashes - attached below. 
I'v dropped the patch for now.

	Ingo

[    4.276015] calling  pcie_portdrv_init+0x0/0x4c @ 1
[    4.280343] bus: 'pci_express': registered
[    4.284017] bus: 'pci': add driver pcieport-driver
[    4.288046] bus: 'pci': driver_probe_device: matched device 0000:00:0b.0 with driver pcieport-driver
[    4.292014] bus: 'pci': really_probe: probing driver pcieport-driver with device 0000:00:0b.0
[    4.296148] pcieport-driver 0000:00:0b.0: setting latency timer to 64
[    4.304157]   alloc irq_desc for 16 on cpu 0 node 0
[    4.308008]   alloc kstat_irqs on cpu 0 node 0
[    4.308124] ------------[ cut here ]------------
[    4.312008] kernel BUG at arch/x86/kernel/apic/io_apic.c:1362!
[    4.312008] invalid opcode: 0000 [#1] SMP 
[    4.312008] last sysfs file: 
[    4.312008] CPU 0 
[    4.312008] Modules linked in:
[    4.312008] Pid: 7, comm: work_on_cpu/0 Not tainted 2.6.29-rc7-tip-02101-g8d7adfc-dirty #31737 SMP Sun Mar 8 11:50:18 CET 2009 System Product Name
[    4.312008] RIP: 0010:[<ffffffff80221e36>]  [<ffffffff80221e36>] __clear_irq_vector+0x1d/0xd8
[    4.312008] RSP: 0018:ffff88003f121bf0  EFLAGS: 00010046
[    4.312008] RAX: 0000000000000246 RBX: ffff88003ea0b1a0 RCX: 0000000000000002
[    4.312008] RDX: 8c6318c6318c6300 RSI: ffff88003ea0b1a0 RDI: 0000000000000010
[    4.312008] RBP: ffff88003f121c10 R08: ffff88003f118998 R09: 0000000000000070
[    4.312008] R10: 0000000000000000 R11: ffffffff80222c03 R12: 0000000000000010
[    4.312008] R13: 0000000000000010 R14: ffff88003f2a31b0 R15: 0000000000000001
[    4.312008] FS:  0000000000000000(0000) GS:ffff8800035b0000(0000) knlGS:0000000000000000
[    4.312008] CS:  0010 DS: 0018 ES: 0018 CR0: 000000008005003b
[    4.312008] CR2: 0000000000000000 CR3: 0000000000201000 CR4: 00000000000006e0
[    4.312008] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[    4.312008] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[    4.312008] Process work_on_cpu/0 (pid: 7, threadinfo ffff88003f120000, task ffff88003f118000)
[    4.312008] Stack:
[    4.312008]  0000000000000246 0000000000000010 ffff88003ea0b1a0 ffff88003f2a31b0
[    4.312008]  ffff88003f121c40 ffffffff80222c11 ffff88003f121c50 ffff88003ea060a8
[    4.312008]  ffff88003f2a38b0 ffff88003f2a38b0 ffff88003f121c50 ffffffff80222c36
[    4.312008] Call Trace:
[    4.312008]  [<ffffffff80222c11>] destroy_irq+0x4f/0x66
[    4.312008]  [<ffffffff80222c36>] arch_teardown_msi_irq+0xe/0x10
[    4.312008]  [<ffffffff80471099>] arch_teardown_msi_irqs+0x2c/0xa3
[    4.312008]  [<ffffffff80471188>] msi_free_irqs+0x5d/0xff
[    4.312008]  [<ffffffff80471a8f>] msi_capability_init+0x134/0x16d
[    4.312008]  [<ffffffff8047202e>] pci_enable_msi+0x7c/0x82
[    4.312008]  [<ffffffff804707ff>] assign_interrupt_mode+0xda/0x10b
[    4.312008]  [<ffffffff80470a0d>] pcie_port_device_register+0xb3/0x123
[    4.312008]  [<ffffffff80260f9c>] ? do_work_for_cpu+0x0/0x20
[    4.312008]  [<ffffffff807e10af>] pcie_portdrv_probe+0x8c/0xb4
[    4.312008]  [<ffffffff8046baee>] local_pci_probe+0x17/0x1b
[    4.312008]  [<ffffffff80260fb4>] do_work_for_cpu+0x18/0x20
[    4.312008]  [<ffffffff80261492>] run_workqueue+0x10c/0x221
[    4.312008]  [<ffffffff80261440>] ? run_workqueue+0xba/0x221
[    4.312008]  [<ffffffff802611b5>] ? test_ti_thread_flag+0x9/0x11
[    4.312008]  [<ffffffff80262282>] worker_thread+0xd6/0xe7
[    4.312008]  [<ffffffff802658b8>] ? autoremove_wake_function+0x0/0x3d
[    4.312008]  [<ffffffff802621ac>] ? worker_thread+0x0/0xe7
[    4.312008]  [<ffffffff802656d3>] kthread+0x4e/0x7b
[    4.312008]  [<ffffffff80c54140>] ? early_idt_handler+0x0/0x73
[    4.312008]  [<ffffffff8020d1ca>] child_rip+0xa/0x20
[    4.312008]  [<ffffffff8020cb90>] ? restore_args+0x0/0x30
[    4.312008]  [<ffffffff80265685>] ? kthread+0x0/0x7b
[    4.312008]  [<ffffffff8020d1c0>] ? child_rip+0x0/0x20
[    4.312008] Code: ff 8b 35 2e 78 a1 00 e8 cd ff ff ff c9 c3 55 48 89 e5 41 56 41 55 41 54 53 e8 97 9f fe ff 8a 56 1c 41 89 fd 48 89 f3 84 d2 75 04 <0f> 0b eb fe 0f b6 d2 4c 8b 35 d4 b1 5f 00 48 c7 c0 20 b0 00 00 
[    4.312008] RIP  [<ffffffff80221e36>] __clear_irq_vector+0x1d/0xd8
[    4.312008]  RSP <ffff88003f121bf0>

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

* Re: [tip:x86/apic] x86: MSI: check for BIOS assigned addresses
  2009-03-08 10:54           ` Ingo Molnar
@ 2009-03-09 14:32             ` Daniel Yeisley
  0 siblings, 0 replies; 11+ messages in thread
From: Daniel Yeisley @ 2009-03-09 14:32 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: Yinghai Lu, mingo, hpa, linux-kernel, stable, tglx, linux-pci

On Sun, 2009-03-08 at 11:54 +0100, Ingo Molnar wrote:
> * Yinghai Lu <yinghai@kernel.org> wrote:
> 
> > Ingo Molnar wrote:
> > > * Daniel Yeisley <dan.yeisley@unisys.com> wrote:
> > > 
> > >> On Fri, 2009-03-06 at 19:45 +0100, Ingo Molnar wrote:
> > >>> * Daniel Yeisley <dan.yeisley@unisys.com> wrote:
> > >>>
> > >>>> Commit-ID:  affd49d36e2e2df4cbb3e07a57db901c23823b6a
> > >>>> Gitweb:     http://git.kernel.org/tip/affd49d36e2e2df4cbb3e07a57db901c23823b6a
> > >>>> Author:     "Daniel Yeisley" <dan.yeisley@unisys.com>
> > >>>> AuthorDate: Fri, 6 Mar 2009 10:11:49 -0500
> > >>>> Commit:     Ingo Molnar <mingo@elte.hu>
> > >>>> CommitDate: Fri, 6 Mar 2009 19:32:44 +0100
> > >>>>
> > >>>> x86: MSI: check for BIOS assigned addresses
> > >>>>
> > >>>> Impact: fix ACPI hotplug on certain systems
> > >>> Note, this commit means that i moved the commit over from 
> > >>> tip:x86/urgent into tip:x86/apic - i.e. with a 2.6.30 merge 
> > >>> date. When exactly was this regression introduced? I think this 
> > >>> code never really accepted the MSI settings from the firmware, 
> > >>> so changing that behavior might affect a lot of systems.
> > >>>
> > >>> So i'm uneasy about applying this fix this late in the .29 
> > >>> cycle, but marked it for -stable backport so it could hit 
> > >>> 2.6.29.1 if it stays problem-free.
> > >>>
> > >>> 	Ingo
> > >> I initially noticed the problem on a 2.6.18 kernel.  ACPI 
> > >> hotplugging didn't work unless I booted with pci=nomsi.  I 
> > >> don't have a problem with waiting until 2.6.30 to make sure it 
> > >> is thoroughly tested.
> > > 
> > > Okay - so it's an ancient bug. The current track should thus be 
> > > fine.
> > > 
> > 
> > please drop this patch. it seems it break msi-x.
> > at least igbe with msi-x does not work anymore.
> 
> Not just that, it also causes bootup crashes - attached below. 
> I'v dropped the patch for now.
> 
> 	Ingo
> 
> [    4.276015] calling  pcie_portdrv_init+0x0/0x4c @ 1
> [    4.280343] bus: 'pci_express': registered
> [    4.284017] bus: 'pci': add driver pcieport-driver
> [    4.288046] bus: 'pci': driver_probe_device: matched device 0000:00:0b.0 with driver pcieport-driver
> [    4.292014] bus: 'pci': really_probe: probing driver pcieport-driver with device 0000:00:0b.0
> [    4.296148] pcieport-driver 0000:00:0b.0: setting latency timer to 64
> [    4.304157]   alloc irq_desc for 16 on cpu 0 node 0
> [    4.308008]   alloc kstat_irqs on cpu 0 node 0
> [    4.308124] ------------[ cut here ]------------
> [    4.312008] kernel BUG at arch/x86/kernel/apic/io_apic.c:1362!
> [    4.312008] invalid opcode: 0000 [#1] SMP 
> [    4.312008] last sysfs file: 
> [    4.312008] CPU 0 
> [    4.312008] Modules linked in:
> [    4.312008] Pid: 7, comm: work_on_cpu/0 Not tainted 2.6.29-rc7-tip-02101-g8d7adfc-dirty #31737 SMP Sun Mar 8 11:50:18 CET 2009 System Product Name
> [    4.312008] RIP: 0010:[<ffffffff80221e36>]  [<ffffffff80221e36>] __clear_irq_vector+0x1d/0xd8
> [    4.312008] RSP: 0018:ffff88003f121bf0  EFLAGS: 00010046
> [    4.312008] RAX: 0000000000000246 RBX: ffff88003ea0b1a0 RCX: 0000000000000002
> [    4.312008] RDX: 8c6318c6318c6300 RSI: ffff88003ea0b1a0 RDI: 0000000000000010
> [    4.312008] RBP: ffff88003f121c10 R08: ffff88003f118998 R09: 0000000000000070
> [    4.312008] R10: 0000000000000000 R11: ffffffff80222c03 R12: 0000000000000010
> [    4.312008] R13: 0000000000000010 R14: ffff88003f2a31b0 R15: 0000000000000001
> [    4.312008] FS:  0000000000000000(0000) GS:ffff8800035b0000(0000) knlGS:0000000000000000
> [    4.312008] CS:  0010 DS: 0018 ES: 0018 CR0: 000000008005003b
> [    4.312008] CR2: 0000000000000000 CR3: 0000000000201000 CR4: 00000000000006e0
> [    4.312008] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [    4.312008] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> [    4.312008] Process work_on_cpu/0 (pid: 7, threadinfo ffff88003f120000, task ffff88003f118000)
> [    4.312008] Stack:
> [    4.312008]  0000000000000246 0000000000000010 ffff88003ea0b1a0 ffff88003f2a31b0
> [    4.312008]  ffff88003f121c40 ffffffff80222c11 ffff88003f121c50 ffff88003ea060a8
> [    4.312008]  ffff88003f2a38b0 ffff88003f2a38b0 ffff88003f121c50 ffffffff80222c36
> [    4.312008] Call Trace:
> [    4.312008]  [<ffffffff80222c11>] destroy_irq+0x4f/0x66
> [    4.312008]  [<ffffffff80222c36>] arch_teardown_msi_irq+0xe/0x10
> [    4.312008]  [<ffffffff80471099>] arch_teardown_msi_irqs+0x2c/0xa3
> [    4.312008]  [<ffffffff80471188>] msi_free_irqs+0x5d/0xff
> [    4.312008]  [<ffffffff80471a8f>] msi_capability_init+0x134/0x16d
> [    4.312008]  [<ffffffff8047202e>] pci_enable_msi+0x7c/0x82
> [    4.312008]  [<ffffffff804707ff>] assign_interrupt_mode+0xda/0x10b
> [    4.312008]  [<ffffffff80470a0d>] pcie_port_device_register+0xb3/0x123
> [    4.312008]  [<ffffffff80260f9c>] ? do_work_for_cpu+0x0/0x20
> [    4.312008]  [<ffffffff807e10af>] pcie_portdrv_probe+0x8c/0xb4
> [    4.312008]  [<ffffffff8046baee>] local_pci_probe+0x17/0x1b
> [    4.312008]  [<ffffffff80260fb4>] do_work_for_cpu+0x18/0x20
> [    4.312008]  [<ffffffff80261492>] run_workqueue+0x10c/0x221
> [    4.312008]  [<ffffffff80261440>] ? run_workqueue+0xba/0x221
> [    4.312008]  [<ffffffff802611b5>] ? test_ti_thread_flag+0x9/0x11
> [    4.312008]  [<ffffffff80262282>] worker_thread+0xd6/0xe7
> [    4.312008]  [<ffffffff802658b8>] ? autoremove_wake_function+0x0/0x3d
> [    4.312008]  [<ffffffff802621ac>] ? worker_thread+0x0/0xe7
> [    4.312008]  [<ffffffff802656d3>] kthread+0x4e/0x7b
> [    4.312008]  [<ffffffff80c54140>] ? early_idt_handler+0x0/0x73
> [    4.312008]  [<ffffffff8020d1ca>] child_rip+0xa/0x20
> [    4.312008]  [<ffffffff8020cb90>] ? restore_args+0x0/0x30
> [    4.312008]  [<ffffffff80265685>] ? kthread+0x0/0x7b
> [    4.312008]  [<ffffffff8020d1c0>] ? child_rip+0x0/0x20
> [    4.312008] Code: ff 8b 35 2e 78 a1 00 e8 cd ff ff ff c9 c3 55 48 89 e5 41 56 41 55 41 54 53 e8 97 9f fe ff 8a 56 1c 41 89 fd 48 89 f3 84 d2 75 04 <0f> 0b eb fe 0f b6 d2 4c 8b 35 d4 b1 5f 00 48 c7 c0 20 b0 00 00 
> [    4.312008] RIP  [<ffffffff80221e36>] __clear_irq_vector+0x1d/0xd8
> [    4.312008]  RSP <ffff88003f121bf0>

Can someone try the patch below?  It only checks for MSI capability so
it shouldn't affect MSI-x.  


Signed-off-by: Dan Yeisley <dan.yeisley@unisys.com>

---
diff -Naur linux-2.6.29-rc5/arch/x86/kernel/io_apic.c linux-2.6.29-rc5-dpy/arch/x86/kernel/io_apic.c
--- linux-2.6.29-rc5/arch/x86/kernel/io_apic.c	2009-02-13 18:31:30.000000000 -0500
+++ linux-2.6.29-rc5-dpy/arch/x86/kernel/io_apic.c	2009-03-09 04:36:35.000000000 -0400
@@ -3258,6 +3258,7 @@
 	struct irq_cfg *cfg;
 	int err;
 	unsigned dest;
+	int pos;
 
 	cfg = irq_cfg(irq);
 	err = assign_irq_vector(irq, cfg, TARGET_CPUS);
@@ -3266,6 +3267,14 @@
 
 	dest = cpu_mask_to_apicid_and(cfg->domain, TARGET_CPUS);
 
+	pos = pci_find_capability(pdev, PCI_CAP_ID_MSI);
+	if (pos != 0) {
+		pci_read_config_dword(pdev, pos + PCI_MSI_ADDRESS_LO,
+        	                                &msg->address_lo);
+		pci_read_config_dword(pdev, pos + PCI_MSI_ADDRESS_HI,
+        	                                &msg->address_hi);
+	}
+
 #ifdef CONFIG_INTR_REMAP
 	if (irq_remapped(irq)) {
 		struct irte irte;
@@ -3295,17 +3304,18 @@
 	} else
 #endif
 	{
-		msg->address_hi = MSI_ADDR_BASE_HI;
-		msg->address_lo =
-			MSI_ADDR_BASE_LO |
-			((INT_DEST_MODE == 0) ?
-				MSI_ADDR_DEST_MODE_PHYSICAL:
-				MSI_ADDR_DEST_MODE_LOGICAL) |
-			((INT_DELIVERY_MODE != dest_LowestPrio) ?
-				MSI_ADDR_REDIRECTION_CPU:
-				MSI_ADDR_REDIRECTION_LOWPRI) |
-			MSI_ADDR_DEST_ID(dest);
-
+		if (!msg->address_hi && !msg->address_lo) {
+			msg->address_hi = MSI_ADDR_BASE_HI;
+			msg->address_lo = 
+				MSI_ADDR_BASE_LO |
+				((INT_DEST_MODE == 0) ?
+					MSI_ADDR_DEST_MODE_PHYSICAL:
+					MSI_ADDR_DEST_MODE_LOGICAL) |
+				((INT_DELIVERY_MODE != dest_LowestPrio) ?
+					MSI_ADDR_REDIRECTION_CPU:
+					MSI_ADDR_REDIRECTION_LOWPRI) |
+				MSI_ADDR_DEST_ID(dest); 
+		}
 		msg->data =
 			MSI_DATA_TRIGGER_EDGE |
 			MSI_DATA_LEVEL_ASSERT |



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

end of thread, other threads:[~2009-03-09 15:04 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-03-06 15:11 [PATCH] MSI: check for BIOS assigned addresses Daniel Yeisley
2009-03-06 16:24 ` Ingo Molnar
2009-03-06 15:56   ` Daniel Yeisley
2009-03-06 18:09 ` [tip:x86/urgent] x86: " Daniel Yeisley
2009-03-06 18:36 ` [tip:x86/apic] " Daniel Yeisley
2009-03-06 18:45   ` Ingo Molnar
2009-03-06 18:28     ` Daniel Yeisley
2009-03-06 19:13       ` Ingo Molnar
2009-03-07  1:06         ` Yinghai Lu
2009-03-08 10:54           ` Ingo Molnar
2009-03-09 14:32             ` Daniel Yeisley

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).