public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* RE: [PATCH] [broken?] Add MSI support to e1000
@ 2004-08-24 17:40 Nguyen, Tom L
  0 siblings, 0 replies; 16+ messages in thread
From: Nguyen, Tom L @ 2004-08-24 17:40 UTC (permalink / raw)
  To: Andi Kleen; +Cc: Roland Dreier, linux-kernel

On Monday, August 23, 2004  Andi Kleen wrote:
>I guess it's an x86-64 specific issue? Did you test that recently?

I just tested an x86-64 MSI support in 2.6.8.1 kernel. It works fine.

Thanks,
Long

^ permalink raw reply	[flat|nested] 16+ messages in thread
* RE: [PATCH] [broken?] Add MSI support to e1000
@ 2004-08-24 16:01 Nguyen, Tom L
  0 siblings, 0 replies; 16+ messages in thread
From: Nguyen, Tom L @ 2004-08-24 16:01 UTC (permalink / raw)
  To: Roland Dreier, Andi Kleen; +Cc: linux-kernel, Nguyen, Tom L

On Tuesday, August 24 Roland Dreier wrote: 
>    Andi> Yes, the flag word is 0x8b after the call. And
>    Andi> pci_enable_msi returns 0.
>
>Actually I bet the problem is that the driver is doing request_irq()
>on the wrong IRQ.  In s2io.c, s2io_init_nic() does
>
>	sp->irq = pdev->irq;
>
>and then sometime later s2io_open() does
>
>	err =
>	    request_irq((int) sp->irq, s2io_isr, SA_SHIRQ, sp->name,
dev);
>
>If you put the call to pci_enable_msi() after sp->irq is assigned, the
>driver will request the original irq (which will still be in INTx
>mode, of course), rather than the new vector assigned by the MSI core.

My guess is the same as Roland's bet. I've not tested MSI support in 
2.6.8.1 kernel. I'll provide an update later if MSI support
in x86-64 has any issue.

Thanks,
Long

^ permalink raw reply	[flat|nested] 16+ messages in thread
* RE: [PATCH] [broken?] Add MSI support to e1000
@ 2004-08-23 23:17 Nguyen, Tom L
  2004-08-24  2:15 ` Roland Dreier
  0 siblings, 1 reply; 16+ messages in thread
From: Nguyen, Tom L @ 2004-08-23 23:17 UTC (permalink / raw)
  To: Roland Dreier
  Cc: cramerj, Ronciak, John, Venkatesan, Ganesh, linux-net,
	linux-kernel, Nguyen, Tom L

On Monday, August 23, 2004 Roland Dreier wrote:
>It seems e1000 does not support the per-vector masking feature of MSI
>(see full PCI header dump below).

You're right that e1000 hardware does not support the per-vector masking
feature.

>However if I understand the x86 APIC properly, even without masking,
>edge-triggered MSI interrupts should work OK.  As I understand it,
>when the interrupt is dispatched, its bit is moved from the IRR to the
>ISR.  If the same interrupt is received while the interrupt handler is
>running, its bit will be set again in the IRR and it will be
>dispatched again as soon as the handler exits.

Agree. The question is that how many egde-triggered MSI with the same 
vector are generated by e1000 hardware when its service handler is 
still running.

>It seems this should work OK for e1000 -- the chip should not generate
>another MSI until the driver reads the ICR in the interrupt handler,
>although it might generate an interrupt immediately afterward (while
>the interrupt handler is still running).

I do not know much about e1000 hardware. I leave it to Ganesh for an
answer.

Thanks,
Long 

^ permalink raw reply	[flat|nested] 16+ messages in thread
* RE: [PATCH] [broken?] Add MSI support to e1000
@ 2004-08-23 19:41 Nguyen, Tom L
  2004-08-23 23:39 ` Andi Kleen
  0 siblings, 1 reply; 16+ messages in thread
From: Nguyen, Tom L @ 2004-08-23 19:41 UTC (permalink / raw)
  To: Andi Kleen, Roland Dreier; +Cc: linux-kernel, Nguyen, Tom L

On Monday, August 23, 2004 Andi Kleen wrote:
>There seems to be something wrong with the MSI code in the kernel.
>I tried to add MSI support to the s2io driver on x86-64, but it just
didn't
>work (/proc/interrupts still displayed IO-APIC mode). I haven't 
>investigated in detail yet though.

Would you please tell me whether the MSI enable bit of the MSI
capability 
structure of the s2io device is set or not after successfully calling 
pci_enable_msi()?

Thanks,
Long

^ permalink raw reply	[flat|nested] 16+ messages in thread
* RE: [PATCH] [broken?] Add MSI support to e1000
@ 2004-08-23 19:09 Nguyen, Tom L
  2004-08-23 19:39 ` Roland Dreier
  0 siblings, 1 reply; 16+ messages in thread
From: Nguyen, Tom L @ 2004-08-23 19:09 UTC (permalink / raw)
  To: Roland Dreier
  Cc: cramerj, Ronciak, John, Venkatesan, Ganesh, linux-net,
	linux-kernel, Nguyen, Tom L

On Monday, August 23, 2004 Roland Dreier wrote: 
>    Tom> I do not see anything wrong with the patch and the kernel MSI
>    Tom> support because it works for a short time. Ganesh may provide
>    Tom> an answer on the MSI support in e1000 hardware.
>
>Based on the e1000 documentation I have, the only thing required for
>the e1000 to use MSI is to set the MSI enable bit in the PCI header.
>Of course there may be some e1000 erratum involving MSI but I have not
>been able to find any indication that this is the case.
>
>It seems possible that there could be some problem in the core Linux
>interrupt code even though some interrupts work -- for example there
>could be a race condition triggered when a second interrupt is
>delivered while handling the first interrupt.  However I couldn't find
>any such bug, although I am not at all an expert about low-level
>interrupt handling/APIC programming.

MSI is an edge trigger, which requires the synchronization handshake 
between the hardware device and its software device driver. For the 
MSI-X capability structure, the kernel handles the synchronization 
by masking and unmasking the MSI maskbits. For the MSI capability 
structure, the MSI maskbits is optional. If the e1000 hardware does not
support the MSI maskbits in its MSI capability structure, I guess it 
could be a race condition in e1000 hardware, which results an 
unpredictable behavior.

Thanks,
Long


^ permalink raw reply	[flat|nested] 16+ messages in thread
* RE: [PATCH] [broken?] Add MSI support to e1000
@ 2004-08-23 15:41 Nguyen, Tom L
  2004-08-23 17:25 ` Roland Dreier
  0 siblings, 1 reply; 16+ messages in thread
From: Nguyen, Tom L @ 2004-08-23 15:41 UTC (permalink / raw)
  To: Roland Dreier, cramerj, Ronciak, John, Venkatesan, Ganesh
  Cc: linux-net, linux-kernel, Nguyen, Tom L

On Friday, August 20 Roland Dreier wrote: 
>However, on my Dell box with 865 chipset (lspci below), loading e1000
>(from kernel 2.6.8.1 with this patch applied) with MSI=1 only works
>for a short time (maybe ~1000 e1000 interrupts) before network traffic
>stops.  
>
>Is there something wrong with the patch?  Something wrong with the
>kernel MSI support?  Something wrong with the hardware?

I do not see anything wrong with the patch and the kernel MSI support
because it works for a short time. Ganesh may provide an answer on the 
MSI support in e1000 hardware.

Thanks,
Long

^ permalink raw reply	[flat|nested] 16+ messages in thread
* [PATCH] [broken?] Add MSI support to e1000
@ 2004-08-20 21:37 Roland Dreier
  0 siblings, 0 replies; 16+ messages in thread
From: Roland Dreier @ 2004-08-20 21:37 UTC (permalink / raw)
  To: cramerj, john.ronciak, ganesh.venkatesan, tom.l.nguyen
  Cc: linux-net, linux-kernel

I recently tried to add MSI support to the e1000 driver, since on my
box, several interrupts seem to be wired together with the e1000:

     18:    1028608   IO-APIC-level  uhci_hcd, libata, eth0

I came up with the patch at the end of this email.

However, on my Dell box with 865 chipset (lspci below), loading e1000
(from kernel 2.6.8.1 with this patch applied) with MSI=1 only works
for a short time (maybe ~1000 e1000 interrupts) before network traffic
stops.  I often get the following as well:

    CPU 0: Machine Check Exception: 0000000000000000
    CPU 0: EIP: 00000000 EFLAGS: 00000000
            eax: 00000000 ebx: 00000000 ecx: 00000000 edx: 00000000
            esi: 00000000 edi: 00000000 ebp: 00000000 esp: 00000000

I also tried the same patch on a dual 3.4 GHz Xeon system with
Lindenhurst chipset with the same results.

Is there something wrong with the patch?  Something wrong with the
kernel MSI support?  Something wrong with the hardware?

I'm glad to provide any further information required to debug this.

Thanks,
  Roland

Here's the lspci output:

    0000:00:00.0 Host bridge: Intel Corp. 82865G/PE/P DRAM Controller/Host-Hub Interface (rev 02)
    0000:00:01.0 PCI bridge: Intel Corp. 82865G/PE/P PCI to AGP Controller (rev 02)
    0000:00:1d.0 USB Controller: Intel Corp. 82801EB/ER (ICH5/ICH5R) USB UHCI #1 (rev 02)
    0000:00:1d.1 USB Controller: Intel Corp. 82801EB/ER (ICH5/ICH5R) USB UHCI #2 (rev 02)
    0000:00:1d.2 USB Controller: Intel Corp. 82801EB/ER (ICH5/ICH5R) USB UHCI #3 (rev 02)
    0000:00:1d.3 USB Controller: Intel Corp. 82801EB/ER (ICH5/ICH5R) USB UHCI #4 (rev 02)
    0000:00:1d.7 USB Controller: Intel Corp. 82801EB/ER (ICH5/ICH5R) USB2 EHCI Controller (rev 02)
    0000:00:1e.0 PCI bridge: Intel Corp. 82801 PCI Bridge (rev c2)
    0000:00:1f.0 ISA bridge: Intel Corp. 82801EB/ER (ICH5/ICH5R) LPC Bridge (rev 02)
    0000:00:1f.1 IDE interface: Intel Corp. 82801EB/ER (ICH5/ICH5R) Ultra ATA 100 Storage Controller (rev 02)
    0000:00:1f.2 IDE interface: Intel Corp. 82801EB (ICH5) Serial ATA 150 Storage Controller (rev 02)
    0000:00:1f.3 SMBus: Intel Corp. 82801EB/ER (ICH5/ICH5R) SMBus Controller (rev 02)
    0000:00:1f.5 Multimedia audio controller: Intel Corp. 82801EB/ER (ICH5/ICH5R) AC'97 Audio Controller (rev 02)
    0000:01:00.0 VGA compatible controller: nVidia Corporation NV34 [GeForce FX 5200] (rev a1)
    0000:02:0c.0 Ethernet controller: Intel Corp. 82540EM Gigabit Ethernet Controller (rev 02)

Here's the patch I tried:

Index: linux-2.6.8.1/drivers/net/e1000/e1000.h
===================================================================
--- linux-2.6.8.1.orig/drivers/net/e1000/e1000.h	2004-08-14 03:54:47.000000000 -0700
+++ linux-2.6.8.1/drivers/net/e1000/e1000.h	2004-08-20 13:10:55.000000000 -0700
@@ -251,8 +251,10 @@
 	struct e1000_desc_ring test_tx_ring;
 	struct e1000_desc_ring test_rx_ring;
 
-
 	uint32_t pci_state[16];
 	int msg_enable;
+
+	int use_msi;
+	int msi_enabled;
 };
 #endif /* _E1000_H_ */
Index: linux-2.6.8.1/drivers/net/e1000/e1000_main.c
===================================================================
--- linux-2.6.8.1.orig/drivers/net/e1000/e1000_main.c	2004-08-14 03:55:10.000000000 -0700
+++ linux-2.6.8.1/drivers/net/e1000/e1000_main.c	2004-08-20 13:22:28.000000000 -0700
@@ -541,6 +541,10 @@
 	DPRINTK(PROBE, INFO, "Intel(R) PRO/1000 Network Connection\n");
 	e1000_check_options(adapter);
 
+	/* Message Signaled Interrupts */
+	if (adapter->use_msi && !pci_enable_msi(pdev))
+		adapter->msi_enabled = 1;
+
 	/* Initial Wake on LAN setting
 	 * If APM wake is enabled in the EEPROM,
 	 * enable the ACPI Magic Packet filter
@@ -624,6 +628,8 @@
 	e1000_phy_hw_reset(&adapter->hw);
 
 	iounmap(adapter->hw.hw_addr);
+	if (adapter->msi_enabled)
+		pci_disable_msi(pdev);
 	pci_release_regions(pdev);
 
 	free_netdev(netdev);
Index: linux-2.6.8.1/drivers/net/e1000/e1000_param.c
===================================================================
--- linux-2.6.8.1.orig/drivers/net/e1000/e1000_param.c	2004-08-20 13:06:34.000000000 -0700
+++ linux-2.6.8.1/drivers/net/e1000/e1000_param.c	2004-08-20 13:32:22.000000000 -0700
@@ -194,6 +194,15 @@
 
 E1000_PARAM(InterruptThrottleRate, "Interrupt Throttling Rate");
 
+/* Message Signaled Interrupts
+ *
+ * Valid Range: 0-1 (0=off, 1=on)
+ *
+ * Default Value: 0
+ */
+
+E1000_PARAM(MSI, "Message Signaled Interrupts");
+
 #define AUTONEG_ADV_DEFAULT  0x2F
 #define AUTONEG_ADV_MASK     0x2F
 #define FLOW_CONTROL_DEFAULT FLOW_CONTROL_FULL
@@ -459,6 +468,18 @@
 			break;
 		}
 	}
+	{ /* Message Signaled Interrupts */
+		struct e1000_option opt = {
+			.type = enable_option,
+			.name = "Message Signaled Interrupts",
+			.err  = "defaulting to Disabled",
+			.def  = OPTION_DISABLED
+		};
+
+		int use_msi = MSI[bd];
+		e1000_validate_option(&use_msi, &opt, adapter);
+		adapter->use_msi = use_msi;
+	}
 
 	switch(adapter->hw.media_type) {
 	case e1000_media_type_fiber:

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

end of thread, other threads:[~2004-08-24 23:38 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <2wpoS-1ai-1@gated-at.bofh.it>
     [not found] ` <2wqXF-2jm-29@gated-at.bofh.it>
2004-08-23 18:17   ` [PATCH] [broken?] Add MSI support to e1000 Andi Kleen
2004-08-23 18:26     ` Roland Dreier
2004-08-24 17:40 Nguyen, Tom L
  -- strict thread matches above, loose matches on Subject: below --
2004-08-24 16:01 Nguyen, Tom L
2004-08-23 23:17 Nguyen, Tom L
2004-08-24  2:15 ` Roland Dreier
2004-08-23 19:41 Nguyen, Tom L
2004-08-23 23:39 ` Andi Kleen
2004-08-24 14:19   ` Roland Dreier
2004-08-23 19:09 Nguyen, Tom L
2004-08-23 19:39 ` Roland Dreier
2004-08-23 15:41 Nguyen, Tom L
2004-08-23 17:25 ` Roland Dreier
2004-08-24 21:49   ` Chris Leech
2004-08-24 23:36     ` Roland Dreier
2004-08-20 21:37 Roland Dreier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox