Re: [TG3]: Workaround MSI bug on 5714/5780.

Re: [TG3]: Workaround MSI bug on 5714/5780.

[Andy Gospodarek]

On 9/5/07, Michael Chan <mchan@broadcom.com> wrote:
> [TG3]: Workaround MSI bug on 5714/5780.
>
> A hardware bug was revealed after a recent PCI MSI patch was made to
> always disable legacy INTX when enabling MSI.  The 5714/5780 chips
> will not generate MSI when INTX is disabled, causing MSI failure
> messages to be reported, and another patch was made to workaround the
> problem by disabling MSI on ServerWorks HT1000 bridge chips commonly
> found with the 5714.
>
> We workaround this chip bug by enabling INTX after we enable MSI and
> after we resume from suspend.
>
> Update version to 3.81.
>
> This problem was discovered by David Miller.
>
> Signed-off-by: Michael Chan <mchan@broadcom.com>
>

Seems reasonable.

Acked-by: Andy Gospodarek <andy@greyhouse.net>

[TG3]: Workaround MSI bug on 5714/5780.

[Michael Chan]

[TG3]: Workaround MSI bug on 5714/5780.

A hardware bug was revealed after a recent PCI MSI patch was made to
always disable legacy INTX when enabling MSI.  The 5714/5780 chips
will not generate MSI when INTX is disabled, causing MSI failure
messages to be reported, and another patch was made to workaround the
problem by disabling MSI on ServerWorks HT1000 bridge chips commonly
found with the 5714.

We workaround this chip bug by enabling INTX after we enable MSI and
after we resume from suspend.

Update version to 3.81.

This problem was discovered by David Miller.

Signed-off-by: Michael Chan <mchan@broadcom.com>

diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 5874042..9034a05 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -64,8 +64,8 @@
 
 #define DRV_MODULE_NAME		"tg3"
 #define PFX DRV_MODULE_NAME	": "
-#define DRV_MODULE_VERSION	"3.80"
-#define DRV_MODULE_RELDATE	"August 2, 2007"
+#define DRV_MODULE_VERSION	"3.81"
+#define DRV_MODULE_RELDATE	"September 5, 2007"
 
 #define TG3_DEF_MAC_MODE	0
 #define TG3_DEF_RX_MODE		0
@@ -7127,6 +7127,10 @@ static int tg3_open(struct net_device *dev)
 		} else if (pci_enable_msi(tp->pdev) == 0) {
 			u32 msi_mode;
 
+			/* Hardware bug - MSI won't work if INTX disabled. */
+			if (tp->tg3_flags2 & TG3_FLG2_5780_CLASS)
+				pci_intx(tp->pdev, 1);
+
 			msi_mode = tr32(MSGINT_MODE);
 			tw32(MSGINT_MODE, msi_mode | MSGINT_MODE_ENABLE);
 			tp->tg3_flags2 |= TG3_FLG2_USING_MSI;
@@ -12172,6 +12176,11 @@ static int tg3_resume(struct pci_dev *pdev)
 	if (err)
 		return err;
 
+	/* Hardware bug - MSI won't work if INTX disabled. */
+	if ((tp->tg3_flags2 & TG3_FLG2_5780_CLASS) &&
+	    (tp->tg3_flags2 & TG3_FLG2_USING_MSI))
+		pci_intx(tp->pdev, 1);
+
 	netif_device_attach(dev);
 
 	tg3_full_lock(tp, 0);

Re: [TG3]: Workaround MSI bug on 5714/5780.

[David Miller]

From: "Michael Chan" <mchan@broadcom.com>
Date: Wed, 05 Sep 2007 13:07:08 -0700

> [TG3]: Workaround MSI bug on 5714/5780.
> 
> A hardware bug was revealed after a recent PCI MSI patch was made to
> always disable legacy INTX when enabling MSI.  The 5714/5780 chips
> will not generate MSI when INTX is disabled, causing MSI failure
> messages to be reported, and another patch was made to workaround the
> problem by disabling MSI on ServerWorks HT1000 bridge chips commonly
> found with the 5714.
> 
> We workaround this chip bug by enabling INTX after we enable MSI and
> after we resume from suspend.
> 
> Update version to 3.81.
> 
> This problem was discovered by David Miller.
> 
> Signed-off-by: Michael Chan <mchan@broadcom.com>

Thanks a lot Michael, I'll apply this and work on reverting that
incorrect MSI chipset quirk too.

Re: [TG3]: Workaround MSI bug on 5714/5780.

[Andy Gospodarek]

On Thu, Sep 06, 2007 at 04:02:35AM -0700, David Miller wrote:
> From: "Michael Chan" <mchan@broadcom.com>
> Date: Wed, 05 Sep 2007 13:07:08 -0700
> 
> > [TG3]: Workaround MSI bug on 5714/5780.
> > 
> > A hardware bug was revealed after a recent PCI MSI patch was made to
> > always disable legacy INTX when enabling MSI.  The 5714/5780 chips
> > will not generate MSI when INTX is disabled, causing MSI failure
> > messages to be reported, and another patch was made to workaround the
> > problem by disabling MSI on ServerWorks HT1000 bridge chips commonly
> > found with the 5714.
> > 
> > We workaround this chip bug by enabling INTX after we enable MSI and
> > after we resume from suspend.
> > 
> > Update version to 3.81.
> > 
> > This problem was discovered by David Miller.
> > 
> > Signed-off-by: Michael Chan <mchan@broadcom.com>
> 
> Thanks a lot Michael, I'll apply this and work on reverting that
> incorrect MSI chipset quirk too.

Michael,

Is is really necessary to get rid of the HT1000 patch?

commit e3008dedff4bdc96a5f67224cd3d8d12237082a0
Author: Andy Gospodarek <andy@greyhouse.net>
Date:   Thu May 10 22:58:57 2007 -0700

    PCI: disable MSI by default on systems with Serverworks HT1000 chips

This patch was designed to address tg3 and bnx2 messages printed on
systems with HT1000 chips IIRC.  Are we going back to those messages on
bnx2 or is there a workaround in that driver needed too?

-andy

Re: [TG3]: Workaround MSI bug on 5714/5780.

[David Miller]

From: Andy Gospodarek <andy@greyhouse.net>
Date: Thu, 6 Sep 2007 09:45:16 -0400

> Is is really necessary to get rid of the HT1000 patch?

Yes, absolutely and without a single doubt.

> commit e3008dedff4bdc96a5f67224cd3d8d12237082a0
> Author: Andy Gospodarek <andy@greyhouse.net>
> Date:   Thu May 10 22:58:57 2007 -0700
> 
>     PCI: disable MSI by default on systems with Serverworks HT1000 chips
> 
> This patch was designed to address tg3 and bnx2 messages printed on
> systems with HT1000 chips IIRC.  Are we going back to those messages on
> bnx2 or is there a workaround in that driver needed too?

Every report of that problem is with tg3 chips afflicted by this
INTX bug which is now worked around properly.

Even my Niagara T1000 box was hit by this absolutely stupid and
wrong MSI quirk, which is how I found this problem to begin with.

I'm reverting it.

Re: [TG3]: Workaround MSI bug on 5714/5780.

[Andy Gospodarek]

On 9/6/07, David Miller <davem@davemloft.net> wrote:
> From: Andy Gospodarek <andy@greyhouse.net>
> Date: Thu, 6 Sep 2007 09:45:16 -0400
>
> > Is is really necessary to get rid of the HT1000 patch?
>
> Yes, absolutely and without a single doubt.
>
> > commit e3008dedff4bdc96a5f67224cd3d8d12237082a0
> > Author: Andy Gospodarek <andy@greyhouse.net>
> > Date:   Thu May 10 22:58:57 2007 -0700
> >
> >     PCI: disable MSI by default on systems with Serverworks HT1000 chips
> >
> > This patch was designed to address tg3 and bnx2 messages printed on
> > systems with HT1000 chips IIRC.  Are we going back to those messages on
> > bnx2 or is there a workaround in that driver needed too?
>
> Every report of that problem is with tg3 chips afflicted by this
> INTX bug which is now worked around properly.
>
> Even my Niagara T1000 box was hit by this absolutely stupid and
> wrong MSI quirk, which is how I found this problem to begin with.
>
> I'm reverting it.
>

Sounds great.  I never liked it that much to begin with but no one
NACKED it, so it slipped in through Greg KH's tree.

Re: [TG3]: Workaround MSI bug on 5714/5780.

[Michael Chan]

Andy Gospodarek wrote:

> This patch was designed to address tg3 and bnx2 messages printed on
> systems with HT1000 chips IIRC.  Are we going back to those 
> messages on
> bnx2 or is there a workaround in that driver needed too?
> 

Andy, I've checked all the Broadcom chips including bnx2 chips, only
the 5714 and 5780 have this problem.

Re: [TG3]: Workaround MSI bug on 5714/5780.

[Andy Gospodarek]

On Thu, Sep 06, 2007 at 07:34:23AM -0700, David Miller wrote:
> From: Andy Gospodarek <andy@greyhouse.net>
> Date: Thu, 6 Sep 2007 09:45:16 -0400
> 
> > Is is really necessary to get rid of the HT1000 patch?
> 
> Yes, absolutely and without a single doubt.
> 
> > commit e3008dedff4bdc96a5f67224cd3d8d12237082a0
> > Author: Andy Gospodarek <andy@greyhouse.net>
> > Date:   Thu May 10 22:58:57 2007 -0700
> > 
> >     PCI: disable MSI by default on systems with Serverworks HT1000 chips
> > 
> > This patch was designed to address tg3 and bnx2 messages printed on
> > systems with HT1000 chips IIRC.  Are we going back to those messages on
> > bnx2 or is there a workaround in that driver needed too?
> 
> Every report of that problem is with tg3 chips afflicted by this
> INTX bug which is now worked around properly.
> 

Not according to this:

https://bugzilla.redhat.com/show_bug.cgi?id=227657

Which was precisely why I chose to look at it as a chipset issue.

> Even my Niagara T1000 box was hit by this absolutely stupid and
> wrong MSI quirk, which is how I found this problem to begin with.
> 
> I'm reverting it.
> 

If that's the case, do we need to consider that some of the bnx2
hardware (like in this case the 5706) needs a similar workaround?

> -
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [TG3]: Workaround MSI bug on 5714/5780.

[Andy Gospodarek]

On Thu, Sep 06, 2007 at 12:05:30PM -0700, Michael Chan wrote:
> On Thu, 2007-09-06 at 11:41 -0400, Andy Gospodarek wrote:
> > On Thu, Sep 06, 2007 at 07:34:23AM -0700, David Miller wrote:
> > > From: Andy Gospodarek <andy@greyhouse.net>
> > > Date: Thu, 6 Sep 2007 09:45:16 -0400
> > > 
> > > > Is is really necessary to get rid of the HT1000 patch?
> > > 
> > > Yes, absolutely and without a single doubt.
> > > 
> > > > commit e3008dedff4bdc96a5f67224cd3d8d12237082a0
> > > > Author: Andy Gospodarek <andy@greyhouse.net>
> > > > Date:   Thu May 10 22:58:57 2007 -0700
> > > > 
> > > >     PCI: disable MSI by default on systems with Serverworks HT1000 chips
> > > > 
> > > > This patch was designed to address tg3 and bnx2 messages printed on
> > > > systems with HT1000 chips IIRC.  Are we going back to those messages on
> > > > bnx2 or is there a workaround in that driver needed too?
> > > 
> > > Every report of that problem is with tg3 chips afflicted by this
> > > INTX bug which is now worked around properly.
> > > 
> > 
> > Not according to this:
> > 
> > https://bugzilla.redhat.com/show_bug.cgi?id=227657
> > 
> > Which was precisely why I chose to look at it as a chipset issue.
> 
> The HT1000 bridge may very well have an MSI issue.  I'm checking with
> ServerWorks and I will do some testing to confirm.  If confirmed, we can
> disable MSI behind the HT1000 bridge instead of globally.  The 5714
> issue is not caused by the HT1000 as it is not behind the HT1000.
> 

Thanks for checking that out.  You can probably get the information more
easily than I can. ;-)

Re: [TG3]: Workaround MSI bug on 5714/5780.

[Michael Chan]

On Thu, 2007-09-06 at 11:41 -0400, Andy Gospodarek wrote:
> On Thu, Sep 06, 2007 at 07:34:23AM -0700, David Miller wrote:
> > From: Andy Gospodarek <andy@greyhouse.net>
> > Date: Thu, 6 Sep 2007 09:45:16 -0400
> > 
> > > Is is really necessary to get rid of the HT1000 patch?
> > 
> > Yes, absolutely and without a single doubt.
> > 
> > > commit e3008dedff4bdc96a5f67224cd3d8d12237082a0
> > > Author: Andy Gospodarek <andy@greyhouse.net>
> > > Date:   Thu May 10 22:58:57 2007 -0700
> > > 
> > >     PCI: disable MSI by default on systems with Serverworks HT1000 chips
> > > 
> > > This patch was designed to address tg3 and bnx2 messages printed on
> > > systems with HT1000 chips IIRC.  Are we going back to those messages on
> > > bnx2 or is there a workaround in that driver needed too?
> > 
> > Every report of that problem is with tg3 chips afflicted by this
> > INTX bug which is now worked around properly.
> > 
> 
> Not according to this:
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=227657
> 
> Which was precisely why I chose to look at it as a chipset issue.

The HT1000 bridge may very well have an MSI issue.  I'm checking with
ServerWorks and I will do some testing to confirm.  If confirmed, we can
disable MSI behind the HT1000 bridge instead of globally.  The 5714
issue is not caused by the HT1000 as it is not behind the HT1000.

> 
> > Even my Niagara T1000 box was hit by this absolutely stupid and
> > wrong MSI quirk, which is how I found this problem to begin with.
> > 
> > I'm reverting it.
> > 
> 
> If that's the case, do we need to consider that some of the bnx2
> hardware (like in this case the 5706) needs a similar workaround?

5706 doesn't have this same MSI problem as the 5714.

Re: [TG3]: Workaround MSI bug on 5714/5780.

[David Miller]

From: "Michael Chan" <mchan@broadcom.com>
Date: Thu, 06 Sep 2007 12:05:30 -0700

> The HT1000 bridge may very well have an MSI issue.  I'm checking with
> ServerWorks and I will do some testing to confirm.  If confirmed, we can
> disable MSI behind the HT1000 bridge instead of globally.  The 5714
> issue is not caused by the HT1000 as it is not behind the HT1000.

What I'm going to do at this point is just merge the tg3
fix into the current 2.6.23 tree right now.

Meanwhile I'll have the HT1000 MSI quirk revert ready and,
unless we find a reason not to, I'll ask Greg KH to merge
that patch into 2.6.24

It seems the safest and most reasonable way to handle this
right now.

Re: [TG3]: Workaround MSI bug on 5714/5780.

[Andy Gospodarek]

On Thu, Sep 06, 2007 at 12:50:19PM -0700, David Miller wrote:
> From: "Michael Chan" <mchan@broadcom.com>
> Date: Thu, 06 Sep 2007 12:05:30 -0700
> 
> > The HT1000 bridge may very well have an MSI issue.  I'm checking with
> > ServerWorks and I will do some testing to confirm.  If confirmed, we can
> > disable MSI behind the HT1000 bridge instead of globally.  The 5714
> > issue is not caused by the HT1000 as it is not behind the HT1000.
> 
> What I'm going to do at this point is just merge the tg3
> fix into the current 2.6.23 tree right now.
> 
> Meanwhile I'll have the HT1000 MSI quirk revert ready and,
> unless we find a reason not to, I'll ask Greg KH to merge
> that patch into 2.6.24
> 
> It seems the safest and most reasonable way to handle this
> right now.

Sounds great.  Hopefully Michael will hear something back from his folks
before too long and we can use whatever they suggest instead of just the
revert.

Re: [TG3]: Workaround MSI bug on 5714/5780.

[Michael Chan]

On Thu, 2007-09-06 at 12:50 -0700, David Miller wrote:
> From: "Michael Chan" <mchan@broadcom.com>
> Date: Thu, 06 Sep 2007 12:05:30 -0700
> 
> > The HT1000 bridge may very well have an MSI issue.  I'm checking with
> > ServerWorks and I will do some testing to confirm.  If confirmed, we can
> > disable MSI behind the HT1000 bridge instead of globally.  The 5714
> > issue is not caused by the HT1000 as it is not behind the HT1000.
> 
> What I'm going to do at this point is just merge the tg3
> fix into the current 2.6.23 tree right now.
> 
> Meanwhile I'll have the HT1000 MSI quirk revert ready and,
> unless we find a reason not to, I'll ask Greg KH to merge
> that patch into 2.6.24
> 
David, I see that you have already done the revert in your 2.6.23 tree.
So the following patch assumes the revert is already done.  I think it
is quite safe for this to go into 2.6.23.

[PCI]: Add MSI quirk for ServerWorks HT1000 PCIX bridge.

This is the fix for the following problem:

https://bugzilla.redhat.com/show_bug.cgi?id=227657

The bnx2 device 5706 complains about MSI not working behind a
ServerWorks HT1000 PCIX bridge. An earlier commit to fix the problem:

e3008dedff4bdc96a5f67224cd3d8d12237082a0:

"PCI: disable MSI by default on systems with Serverworks HT1000 chips"

was not entirely correct, and has been reverted.

MSI does not work on the PCIX bus because the BIOS did not set the
HT_MSI_FLAGS_ENABLE bit in the HyperTransport MSI capability on the
bridge.  We use the existing quirk_msi_ht_cap() to detect the problem
and disable MSI in all buses behind it.

Signed-off-by: Michael Chan <mchan@broadcom.com>
Cc: Anantha Subramanyam <ananth@broadcom.com>
Cc: Naren Sankar <nsankar@broadcom.com>

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 6da5a5d..c58429b 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -1703,6 +1703,9 @@ static void __devinit quirk_msi_ht_cap(struct pci_dev *dev)
 }
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_SERVERWORKS, PCI_DEVICE_ID_SERVERWORKS_HT2000_PCIE,
 			quirk_msi_ht_cap);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_SERVERWORKS,
+			PCI_DEVICE_ID_SERVERWORKS_HT1000_PXB,
+			quirk_msi_ht_cap);
 
 /* The nVidia CK804 chipset may have 2 HT MSI mappings.
  * MSI are supported if the MSI capability set in any of these mappings.
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 3e34dc0..1bdf8be 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -1428,6 +1428,7 @@
 #define PCI_DEVICE_ID_SERVERWORKS_HE	  0x0008
 #define PCI_DEVICE_ID_SERVERWORKS_LE	  0x0009
 #define PCI_DEVICE_ID_SERVERWORKS_GCNB_LE 0x0017
+#define PCI_DEVICE_ID_SERVERWORKS_HT1000_PXB	0x0036
 #define PCI_DEVICE_ID_SERVERWORKS_EPB	  0x0103
 #define PCI_DEVICE_ID_SERVERWORKS_HT2000_PCIE	0x0132
 #define PCI_DEVICE_ID_SERVERWORKS_OSB4	  0x0200

Re: [TG3]: Workaround MSI bug on 5714/5780.

[David Miller]

From: "Michael Chan" <mchan@broadcom.com>
Date: Fri, 07 Sep 2007 19:26:21 -0700

> David, I see that you have already done the revert in your 2.6.23 tree.

I intend to revert that and only push the tg3 fix into
2.6.23

Earlier I had the idea to undo the quirk too, but that's
something I decided was not a good idea.

Re: [TG3]: Workaround MSI bug on 5714/5780.

[David Miller]

From: "Michael Chan" <mchan@broadcom.com>
Date: Fri, 07 Sep 2007 19:26:21 -0700

> [PCI]: Add MSI quirk for ServerWorks HT1000 PCIX bridge.
> 
> This is the fix for the following problem:
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=227657
> 
> The bnx2 device 5706 complains about MSI not working behind a
> ServerWorks HT1000 PCIX bridge. An earlier commit to fix the problem:
> 
> e3008dedff4bdc96a5f67224cd3d8d12237082a0:
> 
> "PCI: disable MSI by default on systems with Serverworks HT1000 chips"
> 
> was not entirely correct, and has been reverted.
> 
> MSI does not work on the PCIX bus because the BIOS did not set the
> HT_MSI_FLAGS_ENABLE bit in the HyperTransport MSI capability on the
> bridge.  We use the existing quirk_msi_ht_cap() to detect the problem
> and disable MSI in all buses behind it.
> 
> Signed-off-by: Michael Chan <mchan@broadcom.com>
> Cc: Anantha Subramanyam <ananth@broadcom.com>
> Cc: Naren Sankar <nsankar@broadcom.com>

Michael, I've queued this and my revert patch into a tree which
I will push to the PCI maintainer for 2.6.24

Thanks.