RE: [PATCH] IB/qib: detour pcie_caps for certain chip sets

2012-01-12 Thread Mike Marciniszyn
> It doesn't seem to me like this has any place in the quirks thing
> either. Things seem to be working properly, the MaxPayLoad of 128 is
> clearly the highest the system will support correctly.
>
> Jason

Probably the best thing to do unwind the module parameter default in 8d4548f2b 
which would change the initial value back to 0.

That's the way the file has always been and that won't change the rc.

Mike

This message and any attached documents contain information from QLogic 
Corporation or its wholly-owned subsidiaries that may be confidential. If you 
are not the intended recipient, you may not read, copy, distribute, or use this 
information. If you have received this transmission in error, please notify the 
sender immediately by reply e-mail and then delete this message.

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


Re: [PATCH] IB/qib: detour pcie_caps for certain chip sets

2012-01-12 Thread Jason Gunthorpe
On Thu, Jan 12, 2012 at 02:14:12PM -0800, Mike Marciniszyn wrote:

> Actually, Jason is right.  The vendor check you reference here is in
> qib_tune_pcie_coalesce() and not the routine being patched.
> 
> A bit of background here is that the issue was noted with the
> indicated Harpertown root complex chip sets as follows:
> - The BIOS set the root complex MaxPayLoad to 128, but rc capabilities 
> indicate 256 is possible
> - To get the best performance we tried going to 256 on the rc and
> our card and noted the Poisoned TLP

I don't think it is appropriate for a driver to modify the pci
configuration of the root complex.. What if other drivers also try and
modify this configuration? Chaos.

It doesn't seem to me like this has any place in the quirks thing
either. Things seem to be working properly, the MaxPayLoad of 128 is
clearly the highest the system will support correctly.

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


RE: [PATCH] IB/qib: detour pcie_caps for certain chip sets

2012-01-12 Thread Mike Marciniszyn
>
>if (parent->vendor != 0x8086)
> return 1;
>
> so I guess you don't need another vendor check.

Actually, Jason is right.  The vendor check you reference here is in 
qib_tune_pcie_coalesce() and not the routine being patched.

A bit of background here is that the issue was noted with the indicated 
Harpertown root complex chip sets as follows:
- The BIOS set the root complex MaxPayLoad to 128, but rc capabilities indicate 
256 is possible
- To get the best performance we tried going to 256 on the rc and our card and 
noted the Poisoned TLP
- The patch is an effort to avoid having to use set pcie_caps at all as well as 
avoiding issues with the problematic chip sets
- The module parameter can still be used to experiment

We have never the issue with AMD or other Intel chipsets.  The problematic 
device ids are not in fixup.c in lib.

I can reissue a v2 with:
- the vendor check
- define use when available

We probably need to do something, since the current 3.2 rc has the above risk.

Mike


This message and any attached documents contain information from QLogic 
Corporation or its wholly-owned subsidiaries that may be confidential. If you 
are not the intended recipient, you may not read, copy, distribute, or use this 
information. If you have received this transmission in error, please notify the 
sender immediately by reply e-mail and then delete this message.

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


Re: [PATCH] IB/qib: detour pcie_caps for certain chip sets

2012-01-12 Thread Roland Dreier
On Thu, Jan 12, 2012 at 9:17 AM, Mike Marciniszyn
 wrote:
>> Does this work on systems where the broken chipset might
>> not be the immediate parent of the qib device (ie there are
>> some PCIe switches in between)?

> The code figures this out at the top of routine and returns, changing nothing.

OIC.

Also I see

   if (parent->vendor != 0x8086)
return 1;

so I guess you don't need another vendor check.

Although this might be better written as PCI_VENDOR_ID_INTEL instead of 0x8086.

I guess this is OK, although as Jason said it would be much better
if the PCI core knew about these chipset errata.

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


Re: [PATCH] IB/qib: detour pcie_caps for certain chip sets

2012-01-12 Thread Jason Gunthorpe
On Thu, Jan 12, 2012 at 08:02:52AM -0800, Mike Marciniszyn wrote:
> > Should whatever this issue is be a general PCI fixup? Like broken MSI,
> > etc.
> >
> Can you point me to some details on this?

I can explain the broken MSI stuff, as an example. As I noted I'm not
sure what you are working around here, but if there are limits imposed
on otherwise correct values in the PCI capabilities block then I think
it is broadly applicable to handle this in core code...

There are flags in pci.h like:

PCI_BUS_FLAGS_NO_MSI   = (__force pci_bus_flags_t) 1,

Which are quirk things.. Look in drivers/pci/quirks.c to see how it is
set.

So broadly you'd make a new appropriate bus flag to control
whatever you are working around and then test and set it in quirks,
and provide core code to traverse the bus path from a device to ensure
nothing in the path sets that quirk.

Really depends what the problem actually is.

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


RE: [PATCH] IB/qib: detour pcie_caps for certain chip sets

2012-01-12 Thread Mike Marciniszyn

> Does this work on systems where the broken chipset might
> not be the immediate parent of the qib device (ie there are
> some PCIe switches in between)?
>

The code figures this out at the top of routine and returns, changing nothing.



This message and any attached documents contain information from QLogic 
Corporation or its wholly-owned subsidiaries that may be confidential. If you 
are not the intended recipient, you may not read, copy, distribute, or use this 
information. If you have received this transmission in error, please notify the 
sender immediately by reply e-mail and then delete this message.

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


RE: [PATCH] IB/qib: detour pcie_caps for certain chip sets

2012-01-12 Thread Mike Marciniszyn
> Should whatever this issue is be a general PCI fixup? Like broken MSI,
> etc.
>
Can you point me to some details on this?

> Might be nice to include what 0x51 tunes in the commit to aide other
> peoole with the broken chipset :)
>
> Isn't it necesary to check the PCI vendor as well as the devid?
>
Will do both of these in a V2.

Mike

This message and any attached documents contain information from QLogic 
Corporation or its wholly-owned subsidiaries that may be confidential. If you 
are not the intended recipient, you may not read, copy, distribute, or use this 
information. If you have received this transmission in error, please notify the 
sender immediately by reply e-mail and then delete this message.

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


Re: [PATCH] IB/qib: detour pcie_caps for certain chip sets

2012-01-11 Thread Roland Dreier
On Wed, Jan 11, 2012 at 9:48 PM, Jason Gunthorpe
 wrote:
> Isn't it necesary to check the PCI vendor as well as the devid?

That definitely seems like a good idea.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] IB/qib: detour pcie_caps for certain chip sets

2012-01-11 Thread Roland Dreier
On Wed, Jan 11, 2012 at 7:00 PM, Mike Marciniszyn
 wrote:
> +       if (!qib_pcie_caps) {
> +               u16 devid = parent->device;
> +               if ((devid >= 0x25e2 && devid <= 0x25fa) ||
> +                   (devid >= 0x65e2 && devid <= 0x65fa) ||
> +                   (devid >= 0x4021 && devid <= 0x402e))
> +                       caps = 0;
> +       } else

Does this work on systems where the broken chipset might
not be the immediate parent of the qib device (ie there are
some PCIe switches in between)?

Pushing knowledge of completely unrelated devices out into
every device driver looks it will be impossible to maintain.
If you need to know about the whole PCIe fabric to do this
type of tuning, the only way I can see to make this work
is to add some helper functions to set these values for
a given device, and then add all the workaround handling
to the core PCI driver.

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


Re: [PATCH] IB/qib: detour pcie_caps for certain chip sets

2012-01-11 Thread Jason Gunthorpe
On Wed, Jan 11, 2012 at 10:00:49PM -0500, Mike Marciniszyn wrote:
> Commit 8d4548f2b ("IB/qib: Default some module parameters optimally")
> introduced an issue with older root complexes.  They cannot handle
> the pcie_caps of 0x51.

Should whatever this issue is be a general PCI fixup? Like broken MSI,
etc.

Might be nice to include what 0x51 tunes in the commit to aide other
peoole with the broken chipset :)

Isn't it necesary to check the PCI vendor as well as the devid?

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