Re: [PATCH 1/2] msi: set 'En' bit of MSI Mapping Capability on HT platform

2007-11-27 Thread Andrew Morton
On Sun, 25 Nov 2007 11:21:48 +0800 "peerchen" <[EMAIL PROTECTED]> wrote:

> According to the HyperTransport spec, 'En' indicate if the MSI Mapping is 
> active. So it should be set when enable the MSI.
> 
> The patch base on kernel 2.6.24-rc3
> 
> Signed-off-by: Andy Currid <[EMAIL PROTECTED]>
> Signed-off-by: Peer Chen <[EMAIL PROTECTED]>
> 
> ---
> --- linux-2.6.24-rc3/drivers/pci/msi.c.orig   2007-11-23 17:28:45.0 
> -0500
> +++ linux-2.6.24-rc3/drivers/pci/msi.c2007-11-23 17:50:59.0 
> -0500
> @@ -20,6 +20,8 @@
>  #include 
>  #include 
>  
> +#include 

This inclusion makes msi.c x86-specific whereas it previously was not.  I
assume that this change breaks arm, ia64, sparc64 and powerpc.

I'll queue the below ugliness to avoid this, but it would be nicer to move
these functions out to an arch-specific file and then require that all
MSI-using architectures implement them.

diff -puN 
drivers/pci/msi.c~msi-set-en-bit-of-msi-mapping-capability-on-ht-platform-fix 
drivers/pci/msi.c
--- 
a/drivers/pci/msi.c~msi-set-en-bit-of-msi-mapping-capability-on-ht-platform-fix
+++ a/drivers/pci/msi.c
@@ -20,7 +20,9 @@
 #include 
 #include 
 
+#ifdef CONFIG_X86
 #include 
+#endif
 
 #include "pci.h"
 #include "msi.h"
@@ -292,6 +294,7 @@ void pci_restore_msi_state(struct pci_de
 }
 #endif /* CONFIG_PM */
 
+#ifdef CONFIG_X86
 /*
  * pci_enable_msi_ht_cap - Set the HT MSI mapping capability En bit of
  * a device.
@@ -384,6 +387,12 @@ static int pci_check_msi_ht_cap(struct p
}
return 0;
 }
+#else
+static inline int pci_check_msi_ht_cap(struct pci_dev *dev)
+{
+   return 0;
+}
+#endif
 
 /**
  * msi_capability_init - configure device's MSI capability structure
_

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] msi: set 'En' bit of MSI Mapping Capability on HT platform

2007-11-27 Thread Andrew Morton
On Sun, 25 Nov 2007 11:21:45 +0100 Prakash Punnoor <[EMAIL PROTECTED]> wrote:

> Yay:

Sounds good.

> One thing I noticed in the patch:
> + if (((bridge_dev = pci_find_slot(0, 0)) != NULL) &&
> 
> You rather want to use pci_get_bus_and_slot instead, as pci_find_slot is 
> deprecated.

yup.  I'll queue this to get it a bit of testing but won't send it to Greg,
in the expectation that an updated version will be prepared.

Please do feed the diff through scripts/checkpatch.pl too - there are a
large number of trivial coding-style glitches in there.


Also, your [patch 1/2] will break the build unless your [patch 2/2] is
applied.  This will break git bisection so please avoid it.  It is
appropriate to fold both these patches into a single one.

And please avoid sending multiple patches under the same title, as these
two were.

Thanks.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] msi: set 'En' bit of MSI Mapping Capability on HT platform

2007-11-27 Thread Andrew Morton
On Sun, 25 Nov 2007 11:21:45 +0100 Prakash Punnoor [EMAIL PROTECTED] wrote:

 Yay:

Sounds good.

 One thing I noticed in the patch:
 + if (((bridge_dev = pci_find_slot(0, 0)) != NULL) 
 
 You rather want to use pci_get_bus_and_slot instead, as pci_find_slot is 
 deprecated.

yup.  I'll queue this to get it a bit of testing but won't send it to Greg,
in the expectation that an updated version will be prepared.

Please do feed the diff through scripts/checkpatch.pl too - there are a
large number of trivial coding-style glitches in there.


Also, your [patch 1/2] will break the build unless your [patch 2/2] is
applied.  This will break git bisection so please avoid it.  It is
appropriate to fold both these patches into a single one.

And please avoid sending multiple patches under the same title, as these
two were.

Thanks.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] msi: set 'En' bit of MSI Mapping Capability on HT platform

2007-11-27 Thread Andrew Morton
On Sun, 25 Nov 2007 11:21:48 +0800 peerchen [EMAIL PROTECTED] wrote:

 According to the HyperTransport spec, 'En' indicate if the MSI Mapping is 
 active. So it should be set when enable the MSI.
 
 The patch base on kernel 2.6.24-rc3
 
 Signed-off-by: Andy Currid [EMAIL PROTECTED]
 Signed-off-by: Peer Chen [EMAIL PROTECTED]
 
 ---
 --- linux-2.6.24-rc3/drivers/pci/msi.c.orig   2007-11-23 17:28:45.0 
 -0500
 +++ linux-2.6.24-rc3/drivers/pci/msi.c2007-11-23 17:50:59.0 
 -0500
 @@ -20,6 +20,8 @@
  #include asm/errno.h
  #include asm/io.h
  
 +#include asm/k8.h

This inclusion makes msi.c x86-specific whereas it previously was not.  I
assume that this change breaks arm, ia64, sparc64 and powerpc.

I'll queue the below ugliness to avoid this, but it would be nicer to move
these functions out to an arch-specific file and then require that all
MSI-using architectures implement them.

diff -puN 
drivers/pci/msi.c~msi-set-en-bit-of-msi-mapping-capability-on-ht-platform-fix 
drivers/pci/msi.c
--- 
a/drivers/pci/msi.c~msi-set-en-bit-of-msi-mapping-capability-on-ht-platform-fix
+++ a/drivers/pci/msi.c
@@ -20,7 +20,9 @@
 #include asm/errno.h
 #include asm/io.h
 
+#ifdef CONFIG_X86
 #include asm/k8.h
+#endif
 
 #include pci.h
 #include msi.h
@@ -292,6 +294,7 @@ void pci_restore_msi_state(struct pci_de
 }
 #endif /* CONFIG_PM */
 
+#ifdef CONFIG_X86
 /*
  * pci_enable_msi_ht_cap - Set the HT MSI mapping capability En bit of
  * a device.
@@ -384,6 +387,12 @@ static int pci_check_msi_ht_cap(struct p
}
return 0;
 }
+#else
+static inline int pci_check_msi_ht_cap(struct pci_dev *dev)
+{
+   return 0;
+}
+#endif
 
 /**
  * msi_capability_init - configure device's MSI capability structure
_

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] msi: set 'En' bit of MSI Mapping Capability on HT platform

2007-11-26 Thread Sébastien Dugué
On Sun, 25 Nov 2007 11:21:48 +0800 "peerchen" <[EMAIL PROTECTED]> wrote:

> According to the HyperTransport spec, 'En' indicate if the MSI Mapping is
> active. So it should be set when enable the MSI.
> 

  Cool, I had a patch that added a quirk to enable MSI Mapping on Broadcom's
HT1000 so that the builtin BCM5706S Gigabit Ethernet would use MSI. This one
looks even cleaner.

  As a side note, the bnx2 driver tries to use MSI by default (in the kernel I'm
using - 2.6.21.5-rt10) and falls back to INTx if no interrupt is received within
a reasonable amount of time. If the HT MSI Mapping is not enabled beforehand,
the first MSI message (before reverting to INTx) is not trapped by the bridge
and lands in memory thus corrupting what is there at that time. In my case
it's the mapcount of a struct page in the freelist which is overwritten with
the MSI message.

  Sébastien.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] msi: set 'En' bit of MSI Mapping Capability on HT platform

2007-11-26 Thread Sébastien Dugué
On Sun, 25 Nov 2007 11:21:48 +0800 peerchen [EMAIL PROTECTED] wrote:

 According to the HyperTransport spec, 'En' indicate if the MSI Mapping is
 active. So it should be set when enable the MSI.
 

  Cool, I had a patch that added a quirk to enable MSI Mapping on Broadcom's
HT1000 so that the builtin BCM5706S Gigabit Ethernet would use MSI. This one
looks even cleaner.

  As a side note, the bnx2 driver tries to use MSI by default (in the kernel I'm
using - 2.6.21.5-rt10) and falls back to INTx if no interrupt is received within
a reasonable amount of time. If the HT MSI Mapping is not enabled beforehand,
the first MSI message (before reverting to INTx) is not trapped by the bridge
and lands in memory thus corrupting what is there at that time. In my case
it's the mapcount of a struct page in the freelist which is overwritten with
the MSI message.

  Sébastien.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH 1/2] msi: set 'En' bit of MSI Mapping Capability on HT platform

2007-11-25 Thread Andy Currid

> Isn't there a way we can make this work for any upstream HT
> bridge, rather than only for specific NVIDIA chipsets?


The lines Peer indicates below will work for any vendor's bridge device
that implements an HT MSI mapping and is an upstream bridge of the
endpoint requesting MSI.

On some NVIDIA chipsets, the host bridge that implements HT MSI mapping
is not hierarchically upstream from the MSI endpoint; it may be a peer
on the same bus as the endpoint or the PCIe root complex that's above
the endpoint. The NVIDIA-specific code in the patch is to detect those
specific chipsets where this can occur. We have tested the patch with
both internal and PCI Express MSI endpoints on each of these NVIDIA
chipsets.

It may be that other vendors have Hypertransport chipsets with similar
requirements for HT MSI mapping, but we don't have that information or
the ability to test code on those vendors' chipsets.

Regards,

Andy
--
Andy Currid, NVIDIA Corporation
[EMAIL PROTECTED]  408 566 6743


-Original Message-
From: Peer Chen 
Sent: Sunday, November 25, 2007 20:02
To: Robert Hancock; peerchen
Cc: linux-kernel; akpm; Andy Currid
Subject: RE: [PATCH 1/2] msi: set 'En' bit of MSI Mapping Capability on
HT platform

I think the following lines are suitable for other bridges besides
nvidia's, :) :
===
+   if (pci_enable_msi_ht_cap(dev) != 0) {
+   return 0;
+   } else {
+   /* Get upstream bridge device handle */
+
+   bridge_dev = dev->bus->self;
+   while(bridge_dev != 0) {
+   if (pci_enable_msi_ht_cap(bridge_dev) !=
0) {
+   return 0;
+   } else
+   bridge_dev =
bridge_dev->bus->self;
+   }
+
+   return 1;
+   } 


BRs
Peer Chen

-Original Message-
From: Robert Hancock [mailto:[EMAIL PROTECTED] 
Sent: Monday, November 26, 2007 2:34 AM
To: peerchen
Cc: linux-kernel; akpm; Peer Chen; Andy Currid
Subject: Re: [PATCH 1/2] msi: set 'En' bit of MSI Mapping Capability on
HT platform

peerchen wrote:
> According to the HyperTransport spec, 'En' indicate if the MSI Mapping
is active. So it should be set when enable the MSI.
> 
> The patch base on kernel 2.6.24-rc3
> 
> Signed-off-by: Andy Currid <[EMAIL PROTECTED]>
> Signed-off-by: Peer Chen <[EMAIL PROTECTED]>

Isn't there a way we can make this work for any upstream HT bridge,
rather than only for specific NVIDIA chipsets?

-- 
Robert Hancock  Saskatoon, SK, Canada
To email, remove "nospam" from [EMAIL PROTECTED] Home Page:
http://www.roberthancock.com/

---
This email message is for the sole use of the intended recipient(s) and may 
contain
confidential information.  Any unauthorized review, use, disclosure or 
distribution
is prohibited.  If you are not the intended recipient, please contact the 
sender by
reply email and destroy all copies of the original message.
---
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH 1/2] msi: set 'En' bit of MSI Mapping Capability on HT platform

2007-11-25 Thread Peer Chen
I think the following lines are suitable for other bridges besides
nvidia's, :) :
===
+   if (pci_enable_msi_ht_cap(dev) != 0) {
+   return 0;
+   } else {
+   /* Get upstream bridge device handle */
+
+   bridge_dev = dev->bus->self;
+   while(bridge_dev != 0) {
+   if (pci_enable_msi_ht_cap(bridge_dev) !=
0) {
+   return 0;
+   } else
+   bridge_dev =
bridge_dev->bus->self;
+   }
+
+   return 1;
+   } 


BRs
Peer Chen

-Original Message-
From: Robert Hancock [mailto:[EMAIL PROTECTED] 
Sent: Monday, November 26, 2007 2:34 AM
To: peerchen
Cc: linux-kernel; akpm; Peer Chen; Andy Currid
Subject: Re: [PATCH 1/2] msi: set 'En' bit of MSI Mapping Capability on
HT platform

peerchen wrote:
> According to the HyperTransport spec, 'En' indicate if the MSI Mapping
is active. So it should be set when enable the MSI.
> 
> The patch base on kernel 2.6.24-rc3
> 
> Signed-off-by: Andy Currid <[EMAIL PROTECTED]>
> Signed-off-by: Peer Chen <[EMAIL PROTECTED]>

Isn't there a way we can make this work for any upstream HT bridge,
rather than only for specific NVIDIA chipsets?

-- 
Robert Hancock  Saskatoon, SK, Canada
To email, remove "nospam" from [EMAIL PROTECTED] Home Page:
http://www.roberthancock.com/

---
This email message is for the sole use of the intended recipient(s) and may 
contain
confidential information.  Any unauthorized review, use, disclosure or 
distribution
is prohibited.  If you are not the intended recipient, please contact the 
sender by
reply email and destroy all copies of the original message.
---
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] msi: set 'En' bit of MSI Mapping Capability on HT platform

2007-11-25 Thread Robert Hancock

peerchen wrote:

According to the HyperTransport spec, 'En' indicate if the MSI Mapping is 
active. So it should be set when enable the MSI.

The patch base on kernel 2.6.24-rc3

Signed-off-by: Andy Currid <[EMAIL PROTECTED]>
Signed-off-by: Peer Chen <[EMAIL PROTECTED]>


Isn't there a way we can make this work for any upstream HT bridge, 
rather than only for specific NVIDIA chipsets?


--
Robert Hancock  Saskatoon, SK, Canada
To email, remove "nospam" from [EMAIL PROTECTED]
Home Page: http://www.roberthancock.com/

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] msi: set 'En' bit of MSI Mapping Capability on HT platform

2007-11-25 Thread Prakash Punnoor
Hi,

> According to the HyperTransport spec, 'En' indicate if the MSI Mapping is
> active. So it should be set when enable the MSI.

Great! This is what I needed to get MSI going on my MCP51 board. I added some 
ids, but I think there were not necessary, as I only got

PCI: :00:00.0: enabled HT MSI mapping
PCI: :00:10.1: enabled HT MSI mapping

were those devices are:
00:00.0 RAM memory: nVidia Corporation C51 Host Bridge (rev a2)
Subsystem: ASUSTeK Computer Inc. Unknown device 81c0
Flags: bus master, 66MHz, fast devsel, latency 0
Capabilities: [44] HyperTransport: Slave or Primary Interface
Capabilities: [e0] HyperTransport: MSI Mapping Enable+ Fixed-
00: de 10 f0 02 06 00 b0 00 a2 00 00 05 00 00 80 00
10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 43 10 c0 81
30: 00 00 00 00 44 00 00 00 00 00 00 00 ff 00 00 00

According to your patch, you named this device
+#define PCI_DEVICE_ID_NVIDIA_NFORCE_C51_MEMC0   0x02F0
So is the lspci database not correct or your naming? lspci lists some 
memcontrollers for me, but they don't seem to have MSI specific items.


00:10.1 Audio device: nVidia Corporation MCP51 High Definition Audio (rev a2)
Subsystem: ASUSTeK Computer Inc. Unknown device 81cb
Flags: bus master, 66MHz, fast devsel, latency 0, IRQ 319
Memory at fe024000 (32-bit, non-prefetchable) [size=16K]
Capabilities: [44] Power Management version 2
Capabilities: [50] Message Signalled Interrupts: Mask+ 64bit+ 
Queue=0/0 Enable+
Capabilities: [6c] HyperTransport: MSI Mapping Enable+ Fixed+
00: de 10 6c 02 06 04 b0 00 a2 00 03 04 00 00 80 00
10: 00 40 02 fe 00 00 00 00 00 00 00 00 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 43 10 cb 81
30: 00 00 00 00 44 00 00 00 00 00 00 00 05 02 02 05

Yay:
cat /proc/interrupts
319: 26   9357   PCI-MSI-edge  HDA Intel


So, now sata_nv and nvidia binary graphics driver could activate MSI, as 
well. :-)

One thing I noticed in the patch:
+   if (((bridge_dev = pci_find_slot(0, 0)) != NULL) &&

You rather want to use pci_get_bus_and_slot instead, as pci_find_slot is 
deprecated.

Thanks!
-- 
(°= =°)
//\ Prakash Punnoor /\\
V_/ \_V


signature.asc
Description: This is a digitally signed message part.


Re: [PATCH 1/2] msi: set 'En' bit of MSI Mapping Capability on HT platform

2007-11-25 Thread Prakash Punnoor
Hi,

 According to the HyperTransport spec, 'En' indicate if the MSI Mapping is
 active. So it should be set when enable the MSI.

Great! This is what I needed to get MSI going on my MCP51 board. I added some 
ids, but I think there were not necessary, as I only got

PCI: :00:00.0: enabled HT MSI mapping
PCI: :00:10.1: enabled HT MSI mapping

were those devices are:
00:00.0 RAM memory: nVidia Corporation C51 Host Bridge (rev a2)
Subsystem: ASUSTeK Computer Inc. Unknown device 81c0
Flags: bus master, 66MHz, fast devsel, latency 0
Capabilities: [44] HyperTransport: Slave or Primary Interface
Capabilities: [e0] HyperTransport: MSI Mapping Enable+ Fixed-
00: de 10 f0 02 06 00 b0 00 a2 00 00 05 00 00 80 00
10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 43 10 c0 81
30: 00 00 00 00 44 00 00 00 00 00 00 00 ff 00 00 00

According to your patch, you named this device
+#define PCI_DEVICE_ID_NVIDIA_NFORCE_C51_MEMC0   0x02F0
So is the lspci database not correct or your naming? lspci lists some 
memcontrollers for me, but they don't seem to have MSI specific items.


00:10.1 Audio device: nVidia Corporation MCP51 High Definition Audio (rev a2)
Subsystem: ASUSTeK Computer Inc. Unknown device 81cb
Flags: bus master, 66MHz, fast devsel, latency 0, IRQ 319
Memory at fe024000 (32-bit, non-prefetchable) [size=16K]
Capabilities: [44] Power Management version 2
Capabilities: [50] Message Signalled Interrupts: Mask+ 64bit+ 
Queue=0/0 Enable+
Capabilities: [6c] HyperTransport: MSI Mapping Enable+ Fixed+
00: de 10 6c 02 06 04 b0 00 a2 00 03 04 00 00 80 00
10: 00 40 02 fe 00 00 00 00 00 00 00 00 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 43 10 cb 81
30: 00 00 00 00 44 00 00 00 00 00 00 00 05 02 02 05

Yay:
cat /proc/interrupts
319: 26   9357   PCI-MSI-edge  HDA Intel


So, now sata_nv and nvidia binary graphics driver could activate MSI, as 
well. :-)

One thing I noticed in the patch:
+   if (((bridge_dev = pci_find_slot(0, 0)) != NULL) 

You rather want to use pci_get_bus_and_slot instead, as pci_find_slot is 
deprecated.

Thanks!
-- 
(°= =°)
//\ Prakash Punnoor /\\
V_/ \_V


signature.asc
Description: This is a digitally signed message part.


Re: [PATCH 1/2] msi: set 'En' bit of MSI Mapping Capability on HT platform

2007-11-25 Thread Robert Hancock

peerchen wrote:

According to the HyperTransport spec, 'En' indicate if the MSI Mapping is 
active. So it should be set when enable the MSI.

The patch base on kernel 2.6.24-rc3

Signed-off-by: Andy Currid [EMAIL PROTECTED]
Signed-off-by: Peer Chen [EMAIL PROTECTED]


Isn't there a way we can make this work for any upstream HT bridge, 
rather than only for specific NVIDIA chipsets?


--
Robert Hancock  Saskatoon, SK, Canada
To email, remove nospam from [EMAIL PROTECTED]
Home Page: http://www.roberthancock.com/

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH 1/2] msi: set 'En' bit of MSI Mapping Capability on HT platform

2007-11-25 Thread Peer Chen
I think the following lines are suitable for other bridges besides
nvidia's, :) :
===
+   if (pci_enable_msi_ht_cap(dev) != 0) {
+   return 0;
+   } else {
+   /* Get upstream bridge device handle */
+
+   bridge_dev = dev-bus-self;
+   while(bridge_dev != 0) {
+   if (pci_enable_msi_ht_cap(bridge_dev) !=
0) {
+   return 0;
+   } else
+   bridge_dev =
bridge_dev-bus-self;
+   }
+
+   return 1;
+   } 


BRs
Peer Chen

-Original Message-
From: Robert Hancock [mailto:[EMAIL PROTECTED] 
Sent: Monday, November 26, 2007 2:34 AM
To: peerchen
Cc: linux-kernel; akpm; Peer Chen; Andy Currid
Subject: Re: [PATCH 1/2] msi: set 'En' bit of MSI Mapping Capability on
HT platform

peerchen wrote:
 According to the HyperTransport spec, 'En' indicate if the MSI Mapping
is active. So it should be set when enable the MSI.
 
 The patch base on kernel 2.6.24-rc3
 
 Signed-off-by: Andy Currid [EMAIL PROTECTED]
 Signed-off-by: Peer Chen [EMAIL PROTECTED]

Isn't there a way we can make this work for any upstream HT bridge,
rather than only for specific NVIDIA chipsets?

-- 
Robert Hancock  Saskatoon, SK, Canada
To email, remove nospam from [EMAIL PROTECTED] Home Page:
http://www.roberthancock.com/

---
This email message is for the sole use of the intended recipient(s) and may 
contain
confidential information.  Any unauthorized review, use, disclosure or 
distribution
is prohibited.  If you are not the intended recipient, please contact the 
sender by
reply email and destroy all copies of the original message.
---
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH 1/2] msi: set 'En' bit of MSI Mapping Capability on HT platform

2007-11-25 Thread Andy Currid

 Isn't there a way we can make this work for any upstream HT
 bridge, rather than only for specific NVIDIA chipsets?


The lines Peer indicates below will work for any vendor's bridge device
that implements an HT MSI mapping and is an upstream bridge of the
endpoint requesting MSI.

On some NVIDIA chipsets, the host bridge that implements HT MSI mapping
is not hierarchically upstream from the MSI endpoint; it may be a peer
on the same bus as the endpoint or the PCIe root complex that's above
the endpoint. The NVIDIA-specific code in the patch is to detect those
specific chipsets where this can occur. We have tested the patch with
both internal and PCI Express MSI endpoints on each of these NVIDIA
chipsets.

It may be that other vendors have Hypertransport chipsets with similar
requirements for HT MSI mapping, but we don't have that information or
the ability to test code on those vendors' chipsets.

Regards,

Andy
--
Andy Currid, NVIDIA Corporation
[EMAIL PROTECTED]  408 566 6743


-Original Message-
From: Peer Chen 
Sent: Sunday, November 25, 2007 20:02
To: Robert Hancock; peerchen
Cc: linux-kernel; akpm; Andy Currid
Subject: RE: [PATCH 1/2] msi: set 'En' bit of MSI Mapping Capability on
HT platform

I think the following lines are suitable for other bridges besides
nvidia's, :) :
===
+   if (pci_enable_msi_ht_cap(dev) != 0) {
+   return 0;
+   } else {
+   /* Get upstream bridge device handle */
+
+   bridge_dev = dev-bus-self;
+   while(bridge_dev != 0) {
+   if (pci_enable_msi_ht_cap(bridge_dev) !=
0) {
+   return 0;
+   } else
+   bridge_dev =
bridge_dev-bus-self;
+   }
+
+   return 1;
+   } 


BRs
Peer Chen

-Original Message-
From: Robert Hancock [mailto:[EMAIL PROTECTED] 
Sent: Monday, November 26, 2007 2:34 AM
To: peerchen
Cc: linux-kernel; akpm; Peer Chen; Andy Currid
Subject: Re: [PATCH 1/2] msi: set 'En' bit of MSI Mapping Capability on
HT platform

peerchen wrote:
 According to the HyperTransport spec, 'En' indicate if the MSI Mapping
is active. So it should be set when enable the MSI.
 
 The patch base on kernel 2.6.24-rc3
 
 Signed-off-by: Andy Currid [EMAIL PROTECTED]
 Signed-off-by: Peer Chen [EMAIL PROTECTED]

Isn't there a way we can make this work for any upstream HT bridge,
rather than only for specific NVIDIA chipsets?

-- 
Robert Hancock  Saskatoon, SK, Canada
To email, remove nospam from [EMAIL PROTECTED] Home Page:
http://www.roberthancock.com/

---
This email message is for the sole use of the intended recipient(s) and may 
contain
confidential information.  Any unauthorized review, use, disclosure or 
distribution
is prohibited.  If you are not the intended recipient, please contact the 
sender by
reply email and destroy all copies of the original message.
---
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 1/2] msi: set 'En' bit of MSI Mapping Capability on HT platform

2007-11-24 Thread peerchen
According to the HyperTransport spec, 'En' indicate if the MSI Mapping is 
active. So it should be set when enable the MSI.

The patch base on kernel 2.6.24-rc3

Signed-off-by: Andy Currid <[EMAIL PROTECTED]>
Signed-off-by: Peer Chen <[EMAIL PROTECTED]>

---
--- linux-2.6.24-rc3/drivers/pci/msi.c.orig 2007-11-23 17:28:45.0 
-0500
+++ linux-2.6.24-rc3/drivers/pci/msi.c  2007-11-23 17:50:59.0 -0500
@@ -20,6 +20,8 @@
 #include 
 #include 
 
+#include 
+
 #include "pci.h"
 #include "msi.h"
 
@@ -290,6 +292,99 @@ void pci_restore_msi_state(struct pci_de
 }
 #endif /* CONFIG_PM */
 
+/*
+ * pci_enable_msi_ht_cap - Set the HT MSI mapping capability En bit of
+ * a device.
+ *
+ * @dev: pointer to the pci_dev data structure of MSI device function
+ */
+
+static int pci_enable_msi_ht_cap(struct pci_dev *dev)
+{
+   int pos;
+   u8 flags;
+
+   if ((pos = pci_find_ht_capability(dev, HT_CAPTYPE_MSI_MAPPING)) != 0)
+   {   
+   pci_read_config_byte(dev, pos + HT_MSI_FLAGS, );
+   pci_write_config_byte(dev, pos + HT_MSI_FLAGS,
+ flags | HT_MSI_FLAGS_ENABLE);
+
+   printk(KERN_INFO "PCI: %s: enabled HT MSI mapping\n", 
pci_name(dev));
+   }
+
+   return pos;
+}
+
+/**
+ * pci_check_msi_ht_cap - check for and enable the MSI mapping capability En 
bit
+ * of devices or upstream bridge on HT-base system.
+ * @dev: pointer to the pci_dev data structure of MSI device function
+ *
+ * Search if device support ht MSI mapping capability on HT-base 
+ * platform, if yes, enable the En bit. If device can't support MSI mapping,
+ * search the the upstream bridge for that capability, enable En bit find it, 
+ * otherwise disable the MSI function if device and upstream bridge can't 
+ * support MSI mapping capability.
+ **/
+
+static int pci_check_msi_ht_cap(struct pci_dev *dev)
+{
+   struct pci_dev *bridge_dev;
+   
+   if (num_k8_northbridges != 0) { /* If the system is the HT-base */
+
+   /* Check for upstream NVIDIA host bridges */
+
+   if (((bridge_dev = pci_find_slot(0, 0)) != NULL) &&
+(bridge_dev->vendor == PCI_VENDOR_ID_NVIDIA)) {
+   switch (bridge_dev->device) {
+   case PCI_DEVICE_ID_NVIDIA_NFORCE_C51_MEMC0:
+   case PCI_DEVICE_ID_NVIDIA_NFORCE_C51_MEMC1:
+   case PCI_DEVICE_ID_NVIDIA_NFORCE_C51_MEMC2:
+   case PCI_DEVICE_ID_NVIDIA_NFORCE_C51_MEMC3:
+   case PCI_DEVICE_ID_NVIDIA_NFORCE_C51_MEMC4:
+   case PCI_DEVICE_ID_NVIDIA_NFORCE_C51_MEMC5:
+   case PCI_DEVICE_ID_NVIDIA_NFORCE_C51_MEMC6:
+   case PCI_DEVICE_ID_NVIDIA_NFORCE_C51_MEMC7:
+   case PCI_DEVICE_ID_NVIDIA_NFORCE_MCP55_MEMC:
+
+   pci_enable_msi_ht_cap(bridge_dev);
+
+   bridge_dev = NULL;
+   while ((bridge_dev = 
pci_get_device(PCI_VENDOR_ID_NVIDIA,
+   PCI_DEVICE_ID_NVIDIA_NFORCE_MCP55_MEMC, 
bridge_dev))
+   != NULL) {
+   pci_enable_msi_ht_cap(bridge_dev);
+   }
+
+   break;
+
+   default:
+   break;
+   }
+   }
+
+
+   if (pci_enable_msi_ht_cap(dev) != 0) {
+   return 0;
+   } else {
+   /* Get upstream bridge device handle */
+
+   bridge_dev = dev->bus->self;
+   while(bridge_dev != 0) {
+   if (pci_enable_msi_ht_cap(bridge_dev) != 0) {
+   return 0;
+   } else
+   bridge_dev = bridge_dev->bus->self;
+   }
+
+   return 1;
+   }
+   }
+   return 0;
+}
+
 /**
  * msi_capability_init - configure device's MSI capability structure
  * @dev: pointer to the pci_dev data structure of MSI device function
@@ -510,6 +605,10 @@ int pci_enable_msi(struct pci_dev* dev)
status = pci_msi_check_device(dev, 1, PCI_CAP_ID_MSI);
if (status)
return status;
+   
+   status = pci_check_msi_ht_cap(dev);
+   if(status)
+   return status;
 
WARN_ON(!!dev->msi_enabled);
 
@@ -606,6 +705,10 @@ int pci_enable_msix(struct pci_dev* dev,
if (status)
return status;
 
+   status = pci_check_msi_ht_cap(dev);
+   if(status)
+   return status;
+
pos = pci_find_capability(dev, PCI_CAP_ID_MSIX);
pci_read_config_word(dev, msi_control_reg(pos), );
nr_entries 

[PATCH 1/2] msi: set 'En' bit of MSI Mapping Capability on HT platform

2007-11-24 Thread peerchen
According to the HyperTransport spec, 'En' indicate if the MSI Mapping is 
active. So it should be set when enable the MSI.

The patch base on kernel 2.6.24-rc3

Signed-off-by: Andy Currid [EMAIL PROTECTED]
Signed-off-by: Peer Chen [EMAIL PROTECTED]

---
--- linux-2.6.24-rc3/drivers/pci/msi.c.orig 2007-11-23 17:28:45.0 
-0500
+++ linux-2.6.24-rc3/drivers/pci/msi.c  2007-11-23 17:50:59.0 -0500
@@ -20,6 +20,8 @@
 #include asm/errno.h
 #include asm/io.h
 
+#include asm/k8.h
+
 #include pci.h
 #include msi.h
 
@@ -290,6 +292,99 @@ void pci_restore_msi_state(struct pci_de
 }
 #endif /* CONFIG_PM */
 
+/*
+ * pci_enable_msi_ht_cap - Set the HT MSI mapping capability En bit of
+ * a device.
+ *
+ * @dev: pointer to the pci_dev data structure of MSI device function
+ */
+
+static int pci_enable_msi_ht_cap(struct pci_dev *dev)
+{
+   int pos;
+   u8 flags;
+
+   if ((pos = pci_find_ht_capability(dev, HT_CAPTYPE_MSI_MAPPING)) != 0)
+   {   
+   pci_read_config_byte(dev, pos + HT_MSI_FLAGS, flags);
+   pci_write_config_byte(dev, pos + HT_MSI_FLAGS,
+ flags | HT_MSI_FLAGS_ENABLE);
+
+   printk(KERN_INFO PCI: %s: enabled HT MSI mapping\n, 
pci_name(dev));
+   }
+
+   return pos;
+}
+
+/**
+ * pci_check_msi_ht_cap - check for and enable the MSI mapping capability En 
bit
+ * of devices or upstream bridge on HT-base system.
+ * @dev: pointer to the pci_dev data structure of MSI device function
+ *
+ * Search if device support ht MSI mapping capability on HT-base 
+ * platform, if yes, enable the En bit. If device can't support MSI mapping,
+ * search the the upstream bridge for that capability, enable En bit find it, 
+ * otherwise disable the MSI function if device and upstream bridge can't 
+ * support MSI mapping capability.
+ **/
+
+static int pci_check_msi_ht_cap(struct pci_dev *dev)
+{
+   struct pci_dev *bridge_dev;
+   
+   if (num_k8_northbridges != 0) { /* If the system is the HT-base */
+
+   /* Check for upstream NVIDIA host bridges */
+
+   if (((bridge_dev = pci_find_slot(0, 0)) != NULL) 
+(bridge_dev-vendor == PCI_VENDOR_ID_NVIDIA)) {
+   switch (bridge_dev-device) {
+   case PCI_DEVICE_ID_NVIDIA_NFORCE_C51_MEMC0:
+   case PCI_DEVICE_ID_NVIDIA_NFORCE_C51_MEMC1:
+   case PCI_DEVICE_ID_NVIDIA_NFORCE_C51_MEMC2:
+   case PCI_DEVICE_ID_NVIDIA_NFORCE_C51_MEMC3:
+   case PCI_DEVICE_ID_NVIDIA_NFORCE_C51_MEMC4:
+   case PCI_DEVICE_ID_NVIDIA_NFORCE_C51_MEMC5:
+   case PCI_DEVICE_ID_NVIDIA_NFORCE_C51_MEMC6:
+   case PCI_DEVICE_ID_NVIDIA_NFORCE_C51_MEMC7:
+   case PCI_DEVICE_ID_NVIDIA_NFORCE_MCP55_MEMC:
+
+   pci_enable_msi_ht_cap(bridge_dev);
+
+   bridge_dev = NULL;
+   while ((bridge_dev = 
pci_get_device(PCI_VENDOR_ID_NVIDIA,
+   PCI_DEVICE_ID_NVIDIA_NFORCE_MCP55_MEMC, 
bridge_dev))
+   != NULL) {
+   pci_enable_msi_ht_cap(bridge_dev);
+   }
+
+   break;
+
+   default:
+   break;
+   }
+   }
+
+
+   if (pci_enable_msi_ht_cap(dev) != 0) {
+   return 0;
+   } else {
+   /* Get upstream bridge device handle */
+
+   bridge_dev = dev-bus-self;
+   while(bridge_dev != 0) {
+   if (pci_enable_msi_ht_cap(bridge_dev) != 0) {
+   return 0;
+   } else
+   bridge_dev = bridge_dev-bus-self;
+   }
+
+   return 1;
+   }
+   }
+   return 0;
+}
+
 /**
  * msi_capability_init - configure device's MSI capability structure
  * @dev: pointer to the pci_dev data structure of MSI device function
@@ -510,6 +605,10 @@ int pci_enable_msi(struct pci_dev* dev)
status = pci_msi_check_device(dev, 1, PCI_CAP_ID_MSI);
if (status)
return status;
+   
+   status = pci_check_msi_ht_cap(dev);
+   if(status)
+   return status;
 
WARN_ON(!!dev-msi_enabled);
 
@@ -606,6 +705,10 @@ int pci_enable_msix(struct pci_dev* dev,
if (status)
return status;
 
+   status = pci_check_msi_ht_cap(dev);
+   if(status)
+   return status;
+
pos = pci_find_capability(dev, PCI_CAP_ID_MSIX);
pci_read_config_word(dev, msi_control_reg(pos), control);