Created attachment 299312 [details]
rhel5-test0.patch

This patch would probably resolve the issue.  

I just need to check to make sure our rhel5 tg3 driver is carrying the fixup
added upstream to address the original issue.

This patch:

commit 2fbe43f6f631dd7ce19fb1499d6164a5bdb34568
Author: Michael Chan <mchan@broadcom.com>
Date:	Thu Sep 6 12:04:29 2007 +0100

    [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.

and this one:

commit ba698ad4b7e466cbb4a8bde6b9da8080ab06808d
Author: David Miller <davem@davemloft.net>
Date:	Thu Oct 25 01:16:30 2007 -0700

    PCI: Add quirk for devices which disable MSI when INTX_DISABLE is set.

    A reasonably common problem with some devices is that they will
    disable MSI generation when the INTX_DISABLE bit is set in the
    PCI_COMMAND register.

    Quirk this explicitly, guarding the pci_intx() calls in msi.c with
    this quirk indication.

    The first entries for this quirk are for 5714 and 5780 Tigon3 chips,
    and thus we can remove the workaround code from the tg3.c driver.

We have the first one patch listed in comment #2, so we should be fine.  The
second fix in comment #2 would be nice, but not necessary since it's a general
fix and I'm not sure it's kABI-safe since it changes the size of struct pci_dev.

Anantha,

We can try and get this added to RHEL5.2, but we will need some help.  If I can
get test kernels to you by tomorrow morning (at the latest) can you verify them
for me?  I don't seem to have any HT1000 systems around anymore.

Thanks!

Yes we can verify them, thanks.

My test kernels have been updated to include a patch for this bugzilla.

http://people.redhat.com/agospoda/#rhel5

Please test them and report back your results.

Joe T., please test this kernel on 5714 using MSI. You don't need to use an 
HT1000 system as long as the driver can successfully run in MSI mode.

Thanks.

It would be great if someone (Joe T?) could at least boot an HT1000 system as
well just to be sure that MSI status will depend on BIOS/system configuration
rather than just blindly disabling MSI on the entire system.

I will be specifically testing on a HT1000 system.

Tested on a HT1000 system. Good news is that MSI is usable on the system.

Bad news is that MSI is being force enabled on HT1000 bridge and there is no way
for BIOS to control this. MSI capability block in the PXB is hardwired and it is
not possible for the BIOS to turn it off. 

Is it possible to either use the later version of HT1000 patch which enables /
disables MSI based on the BIOS settings, or if that is dependent on too many
other things just revert the HT1000 blacklist patch (back to RHEL5 state)

Ok, I will look more closely at the upstream changes and backport as any of them as I can.

I cannot justify a performance issue at this time for R5.2.  Moving out to R5.3.

I don't believe there is a performance issue. Note that this is not an issue of
using MSI behind HT1000 PXB bridge, but just that system which have HT1000 chip
are being blacklisted for MSI. So MSI is basically unusable on these systems. 

Also note that this blacklisting was not there in RHEL5 and we have not had any
complaints on performance from any of our customers.

thx

Anantha, I did not have a chance to get a patch to you today, but I will shoot
for a new patch and test kernel tomorrow with hopefully all the upstream fixes
that are needed.

Anantha,

I was looking at comment #11 again, but I really don't see any difference
between what is being included in 2.6.25 and what was in the patch I gave you. 
It looks like something was changed during the 2.6.24 development cycle though.
 Check this out:

commit 6bae1d96c6d7dde078994f6cb98235fd46f8736b
Author: Sebastien Dugue <sebastien.dugue@bull.net>
Date:   Thu Dec 13 16:09:25 2007 -0800

    PCI: quirk: enable MSI Mapping on HT1000

    Add a quirk to enable the MSI mapping capability on HyperTransport bridges.

    Wire Broadcom's HT1000 to use the quirk.

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 2fbf60e..63357f0 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -1711,9 +1711,33 @@ 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);
+
+
+/*
+ *  Force enable MSI mapping capability on HT bridges
+ */
+static void __devinit quirk_msi_ht_cap_enable(struct pci_dev *dev)
+{
+       int pos, ttl = 48;
+
+       pos = pci_find_ht_capability(dev, HT_CAPTYPE_MSI_MAPPING);
+       while (pos && ttl--) {
+               u8 flags;
+
+               if (pci_read_config_byte(dev, pos + HT_MSI_FLAGS, &flags) == 0) {
+                       printk(KERN_INFO "PCI: Enabling HT MSI Mapping on %s\n",
+                              pci_name(dev));
+

This patch seems to drop the bits that you feel would be good for RHEL.  Michael
Chan apparently ACK'd it, so maybe we should ask him why it might be better to
do this than checking with the bios.

Sorry to pin this on you, Michael, but I'm curious if you can provide some
insight as to why you ACK'd 6bae1d96c6d7dde078994f6cb98235fd46f8736b.  The
upstream discussion seemed pretty sparse.  Thanks!

I was initially CC'ed because I did an earlier patch to check HT1000 to see if 
we could use MSI or not.  Some older BIOS did not properly enable the HT-MSI 
mapping bit, causing BNX2 driver to complain about MSI not working.  This was 
the earlier patch which essentially would silence the BNX2 warning:

1d84b5424efbcce69a1c955ba181147d23d43a14

The patch that you mentioned above superseded this patch that I did.  To me, 
it seemed to be an improvement over my patch.  The new patch would allow users 
with older BIOS to use MSI and so I ACK'ed it.

It's possible that there may be yet another better way of doing this.

Thanks for the update, Michael.  The bug reported (Anantha) was hoping we would
include the commit you mentioned in comment #20, but I questioned if that was
better or worse than the latest upstream.

I presumed the later patch was better since it allowed systems that did not
explicitly enable MSI in BIOS to still use MSI with the HT1000, which you have
confirmed.

Anantha, if you find out anything else from your investigation let me know.

Anantha, I'm going to re-work my patch to the latest in 2.6.25 (the one that
does not check the possible BIOS status) and simply enables MSI on HT1000
devices.  I will not be much different from the one I gave you the other day.

We can work on a patch that will normally enable MSI, but on some specific
systems where you can disable MSI in the BIOS we can allow checking of HT
capabilities and disable if the BIOS has set it that way, but this needs to be
tested first and pushed upstream before we ship it in RHEL.

Off hand, Do you know of any specific systems that support disabling MSI in the
BIOS?  I might like to try and get my hands on one so I could test out some
patches in the future.

OK, so the patch I would propose it no different from the one included in my
gtest.45 kernel.

Andy, the current patch is ok by us. If required, we can revisit the patch that
provides BIOS control later, thanks.