Re: [PATCH 4/6] MTHCA driver (infiniband) use new pci interfaces
> Actually even PCIe might not be that easy. For example with current > kernels on PowerPC 440SPe (SoC with PCIe), I just get: > > # lspci > 00:01.0 InfiniBand: Mellanox Technology: Unknown device 6274 (rev a0) > > ie no host bridge / root complex. Did somebody used the spec as toilet paper again ? Or is it just the kernel that isn't properly showing the root complex ? Ben. - 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 4/6] MTHCA driver (infiniband) use new pci interfaces
> I'm worried by this... At no point do you check the host bridge > capabilities, and thus will happily set the max read req size to some > value larger than the max the host bridge can cope... Well, it's disabled by default... the option is there as a quick way to fix "why is my bandwidth so low" when a broken BIOS sets these to minimum values. Maybe we should just strip out that code and point people who want to tweak this at setpci instead. > So for PCI-X, if we want tat, we need a pcibios hook for the platform > to validate the size requested. For PCI-E, we can use standard code to > look for the root complex (and bridges on the path to it) and get the > proper max value. Actually even PCIe might not be that easy. For example with current kernels on PowerPC 440SPe (SoC with PCIe), I just get: # lspci 00:01.0 InfiniBand: Mellanox Technology: Unknown device 6274 (rev a0) ie no host bridge / root complex. - R. - 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 4/6] MTHCA driver (infiniband) use new pci interfaces
I'm worried by this... At no point do you check the host bridge capabilities, and thus will happily set the max read req size to some value larger than the max the host bridge can cope... Well, it's disabled by default... the option is there as a quick way to fix why is my bandwidth so low when a broken BIOS sets these to minimum values. Maybe we should just strip out that code and point people who want to tweak this at setpci instead. So for PCI-X, if we want tat, we need a pcibios hook for the platform to validate the size requested. For PCI-E, we can use standard code to look for the root complex (and bridges on the path to it) and get the proper max value. Actually even PCIe might not be that easy. For example with current kernels on PowerPC 440SPe (SoC with PCIe), I just get: # lspci 00:01.0 InfiniBand: Mellanox Technology: Unknown device 6274 (rev a0) ie no host bridge / root complex. - R. - 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 4/6] MTHCA driver (infiniband) use new pci interfaces
Actually even PCIe might not be that easy. For example with current kernels on PowerPC 440SPe (SoC with PCIe), I just get: # lspci 00:01.0 InfiniBand: Mellanox Technology: Unknown device 6274 (rev a0) ie no host bridge / root complex. Did somebody used the spec as toilet paper again ? Or is it just the kernel that isn't properly showing the root complex ? Ben. - 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 4/6] MTHCA driver (infiniband) use new pci interfaces
On Mon, Dec 11, 2006 at 02:55:39PM +1100, Benjamin Herrenschmidt wrote: > On Fri, 2006-12-08 at 10:22 -0800, Stephen Hemminger wrote: > > plain text document attachment (mthca-rbc.patch) > > Use new pci interfaces to set read request tuning values > > Untested because of lack of hardware. Sorry...I missed that. I have mthca HW on publicly available IA64 machines. I'll contact Steve off list to check if he is interested/time. > I'm worried by this... At no point do you check the host bridge > capabilities, and thus will happily set the max read req size to some > value larger than the max the host bridge can cope... > > I've been having exactly that problem on a number of setups, for > example, the sky2 cards tend to start with a value of 512 while the G5's > host bridge can't cope with more than 256 (iirc). The firmware fixes > that up properly on the G5 at least (but not on all machines), but if > you allow drivers to go tweak the value without a way to go check what > are the host bridge capabilities, you are toast. > > Of course, on PCI-X, this is moot, there is no clear definition on how > to get to a host bridge config space (if any) ... > So for PCI-X, if we want tat, we need a pcibios hook for the platform > to validate the size requested. Yes, agreed. grant - 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 4/6] MTHCA driver (infiniband) use new pci interfaces
On Mon, Dec 11, 2006 at 02:55:39PM +1100, Benjamin Herrenschmidt wrote: On Fri, 2006-12-08 at 10:22 -0800, Stephen Hemminger wrote: plain text document attachment (mthca-rbc.patch) Use new pci interfaces to set read request tuning values Untested because of lack of hardware. Sorry...I missed that. I have mthca HW on publicly available IA64 machines. I'll contact Steve off list to check if he is interested/time. I'm worried by this... At no point do you check the host bridge capabilities, and thus will happily set the max read req size to some value larger than the max the host bridge can cope... I've been having exactly that problem on a number of setups, for example, the sky2 cards tend to start with a value of 512 while the G5's host bridge can't cope with more than 256 (iirc). The firmware fixes that up properly on the G5 at least (but not on all machines), but if you allow drivers to go tweak the value without a way to go check what are the host bridge capabilities, you are toast. Of course, on PCI-X, this is moot, there is no clear definition on how to get to a host bridge config space (if any) ... So for PCI-X, if we want tat, we need a pcibios hook for the platform to validate the size requested. Yes, agreed. grant - 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 4/6] MTHCA driver (infiniband) use new pci interfaces
Looks fine (assuming the PCI core interfaces it uses go in). Acked-by: Roland Dreier <[EMAIL PROTECTED]> - 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 4/6] MTHCA driver (infiniband) use new pci interfaces
Use new pci interfaces to set read request tuning values Untested because of lack of hardware. Signed-off-by: Stephen Hemminger <[EMAIL PROTECTED]> --- drivers/infiniband/hw/mthca/mthca_main.c | 38 +++ 1 file changed, 9 insertions(+), 29 deletions(-) --- pci-x.orig/drivers/infiniband/hw/mthca/mthca_main.c +++ pci-x/drivers/infiniband/hw/mthca/mthca_main.c @@ -100,45 +100,25 @@ static struct mthca_profile default_prof static int mthca_tune_pci(struct mthca_dev *mdev) { - int cap; - u16 val; - if (!tune_pci) return 0; /* First try to max out Read Byte Count */ - cap = pci_find_capability(mdev->pdev, PCI_CAP_ID_PCIX); - if (cap) { - if (pci_read_config_word(mdev->pdev, cap + PCI_X_CMD, )) { - mthca_err(mdev, "Couldn't read PCI-X command register, " + if (pci_find_capability(mdev->pdev, PCI_CAP_ID_PCIX)) { + if (pcix_set_mmrbc(mdev->pdev, 4096)) { + mthca_err(mdev, "Couldn't set PCI-X max read count, " "aborting.\n"); return -ENODEV; } - val = (val & ~PCI_X_CMD_MAX_READ) | (3 << 2); - if (pci_write_config_word(mdev->pdev, cap + PCI_X_CMD, val)) { - mthca_err(mdev, "Couldn't write PCI-X command register, " - "aborting.\n"); - return -ENODEV; - } - } else if (!(mdev->mthca_flags & MTHCA_FLAG_PCIE)) - mthca_info(mdev, "No PCI-X capability, not setting RBC.\n"); + } - cap = pci_find_capability(mdev->pdev, PCI_CAP_ID_EXP); - if (cap) { - if (pci_read_config_word(mdev->pdev, cap + PCI_EXP_DEVCTL, )) { - mthca_err(mdev, "Couldn't read PCI Express device control " - "register, aborting.\n"); - return -ENODEV; - } - val = (val & ~PCI_EXP_DEVCTL_READRQ) | (5 << 12); - if (pci_write_config_word(mdev->pdev, cap + PCI_EXP_DEVCTL, val)) { - mthca_err(mdev, "Couldn't write PCI Express device control " - "register, aborting.\n"); + if (pci_find_capability(mdev->pdev, PCI_CAP_ID_EXP)) { + if (pcie_set_readrq(mdev->pdev, 4096)) { + mthca_err(mdev, "Couldn't write PCI Express read request, " + "aborting.\n"); return -ENODEV; } - } else if (mdev->mthca_flags & MTHCA_FLAG_PCIE) - mthca_info(mdev, "No PCI Express capability, " - "not setting Max Read Request Size.\n"); + } return 0; } -- - 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 4/6] MTHCA driver (infiniband) use new pci interfaces
Use new pci interfaces to set read request tuning values Untested because of lack of hardware. Signed-off-by: Stephen Hemminger [EMAIL PROTECTED] --- drivers/infiniband/hw/mthca/mthca_main.c | 38 +++ 1 file changed, 9 insertions(+), 29 deletions(-) --- pci-x.orig/drivers/infiniband/hw/mthca/mthca_main.c +++ pci-x/drivers/infiniband/hw/mthca/mthca_main.c @@ -100,45 +100,25 @@ static struct mthca_profile default_prof static int mthca_tune_pci(struct mthca_dev *mdev) { - int cap; - u16 val; - if (!tune_pci) return 0; /* First try to max out Read Byte Count */ - cap = pci_find_capability(mdev-pdev, PCI_CAP_ID_PCIX); - if (cap) { - if (pci_read_config_word(mdev-pdev, cap + PCI_X_CMD, val)) { - mthca_err(mdev, Couldn't read PCI-X command register, + if (pci_find_capability(mdev-pdev, PCI_CAP_ID_PCIX)) { + if (pcix_set_mmrbc(mdev-pdev, 4096)) { + mthca_err(mdev, Couldn't set PCI-X max read count, aborting.\n); return -ENODEV; } - val = (val ~PCI_X_CMD_MAX_READ) | (3 2); - if (pci_write_config_word(mdev-pdev, cap + PCI_X_CMD, val)) { - mthca_err(mdev, Couldn't write PCI-X command register, - aborting.\n); - return -ENODEV; - } - } else if (!(mdev-mthca_flags MTHCA_FLAG_PCIE)) - mthca_info(mdev, No PCI-X capability, not setting RBC.\n); + } - cap = pci_find_capability(mdev-pdev, PCI_CAP_ID_EXP); - if (cap) { - if (pci_read_config_word(mdev-pdev, cap + PCI_EXP_DEVCTL, val)) { - mthca_err(mdev, Couldn't read PCI Express device control - register, aborting.\n); - return -ENODEV; - } - val = (val ~PCI_EXP_DEVCTL_READRQ) | (5 12); - if (pci_write_config_word(mdev-pdev, cap + PCI_EXP_DEVCTL, val)) { - mthca_err(mdev, Couldn't write PCI Express device control - register, aborting.\n); + if (pci_find_capability(mdev-pdev, PCI_CAP_ID_EXP)) { + if (pcie_set_readrq(mdev-pdev, 4096)) { + mthca_err(mdev, Couldn't write PCI Express read request, + aborting.\n); return -ENODEV; } - } else if (mdev-mthca_flags MTHCA_FLAG_PCIE) - mthca_info(mdev, No PCI Express capability, - not setting Max Read Request Size.\n); + } return 0; } -- - 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 4/6] MTHCA driver (infiniband) use new pci interfaces
Looks fine (assuming the PCI core interfaces it uses go in). Acked-by: Roland Dreier [EMAIL PROTECTED] - 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/