| Message ID | 1429730795-6721-1-git-send-email-mcgrof@do-not-panic.com (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
On Wed, Apr 22, 2015 at 12:26 PM, Luis R. Rodriguez <mcgrof@do-not-panic.com> wrote: > From: "Luis R. Rodriguez" <mcgrof@suse.com> > > We are burrying direct access to MTRR code support on > x86 in order to take advantage of PAT. In the future we > also want to make the default behaviour of ioremap_nocache() > to use strong UC, use of mtrr_add() on those systems > would make write-combining void. > > In order to help both enable us to later make strong > UC default and in order to phase out direct MTRR access > code port the driver over to arch_phys_wc_add() and > annotate that the device driver requires systems to > boot with PAT disabled, with the nopat kernel parameter. > > This is a worthy compromise given that the ipath device > driver powers the old HTX bus cards that only work in > AMD systems, while the newer IB/qib device driver > powers all PCI-e cards. The ipath device driver is > obsolete, hardware hard to find and because of this > this its a reasonable compromise to make to require > users of ipath to boot with nopat. Hey folks, I realize its being discussed whether or not to remove the driver entirely from the kernel but in the meantime, is this a reasonable compromise ? Luis -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2015-04-27 at 09:46 -0700, Luis R. Rodriguez wrote: > On Wed, Apr 22, 2015 at 12:26 PM, Luis R. Rodriguez > <mcgrof@do-not-panic.com> wrote: > > From: "Luis R. Rodriguez" <mcgrof@suse.com> > > > > We are burrying direct access to MTRR code support on > > x86 in order to take advantage of PAT. In the future we > > also want to make the default behaviour of ioremap_nocache() > > to use strong UC, use of mtrr_add() on those systems > > would make write-combining void. > > > > In order to help both enable us to later make strong > > UC default and in order to phase out direct MTRR access > > code port the driver over to arch_phys_wc_add() and > > annotate that the device driver requires systems to > > boot with PAT disabled, with the nopat kernel parameter. > > > > This is a worthy compromise given that the ipath device > > driver powers the old HTX bus cards that only work in > > AMD systems, while the newer IB/qib device driver > > powers all PCI-e cards. The ipath device driver is > > obsolete, hardware hard to find and because of this > > this its a reasonable compromise to make to require > > users of ipath to boot with nopat. > > Hey folks, I realize its being discussed whether or not to remove the > driver entirely from the kernel but in the meantime, is this a > reasonable compromise ? [ trimmed Cc: list to probably the only people that care ] I would think so. I think we might as well mark this driver as deprecated and put a tentative date on removal while we are at it. Mike, any specific input here? I would suggest mark it deprecated with a planned removal sometime in late 2015/early 2016.
On Mon, Apr 27, 2015 at 11:38 AM, Doug Ledford <dledford@redhat.com> wrote: > On Mon, 2015-04-27 at 09:46 -0700, Luis R. Rodriguez wrote: >> On Wed, Apr 22, 2015 at 12:26 PM, Luis R. Rodriguez >> <mcgrof@do-not-panic.com> wrote: >> > From: "Luis R. Rodriguez" <mcgrof@suse.com> >> > >> > We are burrying direct access to MTRR code support on >> > x86 in order to take advantage of PAT. In the future we >> > also want to make the default behaviour of ioremap_nocache() >> > to use strong UC, use of mtrr_add() on those systems >> > would make write-combining void. >> > >> > In order to help both enable us to later make strong >> > UC default and in order to phase out direct MTRR access >> > code port the driver over to arch_phys_wc_add() and >> > annotate that the device driver requires systems to >> > boot with PAT disabled, with the nopat kernel parameter. >> > >> > This is a worthy compromise given that the ipath device >> > driver powers the old HTX bus cards that only work in >> > AMD systems, while the newer IB/qib device driver >> > powers all PCI-e cards. The ipath device driver is >> > obsolete, hardware hard to find and because of this >> > this its a reasonable compromise to make to require >> > users of ipath to boot with nopat. >> >> Hey folks, I realize its being discussed whether or not to remove the >> driver entirely from the kernel but in the meantime, is this a >> reasonable compromise ? > > [ trimmed Cc: list to probably the only people that care ] > > I would think so. OK great, can this be merged then? I am waiting for this patch to help move forward with deprecating MTRR upstream. > I think we might as well mark this driver as > deprecated and put a tentative date on removal while we are at it. OK great, that can be done as a separate patch. > Mike, any specific input here? I would suggest mark it deprecated with > a planned removal sometime in late 2015/early 2016. I will note that feature-removal file was removed from Linux so if we want to deprecate something now we just do it. Luis -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2015-04-29 at 09:01 -0700, Luis R. Rodriguez wrote: > On Mon, Apr 27, 2015 at 11:38 AM, Doug Ledford <dledford@redhat.com> wrote: > > On Mon, 2015-04-27 at 09:46 -0700, Luis R. Rodriguez wrote: > >> On Wed, Apr 22, 2015 at 12:26 PM, Luis R. Rodriguez > >> <mcgrof@do-not-panic.com> wrote: > >> > From: "Luis R. Rodriguez" <mcgrof@suse.com> > >> > > >> > We are burrying direct access to MTRR code support on > >> > x86 in order to take advantage of PAT. In the future we > >> > also want to make the default behaviour of ioremap_nocache() > >> > to use strong UC, use of mtrr_add() on those systems > >> > would make write-combining void. > >> > > >> > In order to help both enable us to later make strong > >> > UC default and in order to phase out direct MTRR access > >> > code port the driver over to arch_phys_wc_add() and > >> > annotate that the device driver requires systems to > >> > boot with PAT disabled, with the nopat kernel parameter. > >> > > >> > This is a worthy compromise given that the ipath device > >> > driver powers the old HTX bus cards that only work in > >> > AMD systems, while the newer IB/qib device driver > >> > powers all PCI-e cards. The ipath device driver is > >> > obsolete, hardware hard to find and because of this > >> > this its a reasonable compromise to make to require > >> > users of ipath to boot with nopat. > >> > >> Hey folks, I realize its being discussed whether or not to remove the > >> driver entirely from the kernel but in the meantime, is this a > >> reasonable compromise ? > > > > [ trimmed Cc: list to probably the only people that care ] > > > > I would think so. > > OK great, can this be merged then? I am waiting for this patch to help > move forward with deprecating MTRR upstream. I'm generally OK with picking this up and merging it in. However, it relies on pat_enabled being an exported symbol, and that isn't the case. Are you submitting an additional patch via a different tree to make that symbol exported to modules? > > I think we might as well mark this driver as > > deprecated and put a tentative date on removal while we are at it. > > OK great, that can be done as a separate patch. > > > Mike, any specific input here? I would suggest mark it deprecated with > > a planned removal sometime in late 2015/early 2016. > > I will note that feature-removal file was removed from Linux so if we > want to deprecate something now we just do it. > > Luis > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Apr 29, 2015 at 9:57 AM, Doug Ledford <dledford@redhat.com> wrote: > On Wed, 2015-04-29 at 09:01 -0700, Luis R. Rodriguez wrote: >> On Mon, Apr 27, 2015 at 11:38 AM, Doug Ledford <dledford@redhat.com> wrote: >> > On Mon, 2015-04-27 at 09:46 -0700, Luis R. Rodriguez wrote: >> >> On Wed, Apr 22, 2015 at 12:26 PM, Luis R. Rodriguez >> >> <mcgrof@do-not-panic.com> wrote: >> >> > From: "Luis R. Rodriguez" <mcgrof@suse.com> >> >> > >> >> > We are burrying direct access to MTRR code support on >> >> > x86 in order to take advantage of PAT. In the future we >> >> > also want to make the default behaviour of ioremap_nocache() >> >> > to use strong UC, use of mtrr_add() on those systems >> >> > would make write-combining void. >> >> > >> >> > In order to help both enable us to later make strong >> >> > UC default and in order to phase out direct MTRR access >> >> > code port the driver over to arch_phys_wc_add() and >> >> > annotate that the device driver requires systems to >> >> > boot with PAT disabled, with the nopat kernel parameter. >> >> > >> >> > This is a worthy compromise given that the ipath device >> >> > driver powers the old HTX bus cards that only work in >> >> > AMD systems, while the newer IB/qib device driver >> >> > powers all PCI-e cards. The ipath device driver is >> >> > obsolete, hardware hard to find and because of this >> >> > this its a reasonable compromise to make to require >> >> > users of ipath to boot with nopat. >> >> >> >> Hey folks, I realize its being discussed whether or not to remove the >> >> driver entirely from the kernel but in the meantime, is this a >> >> reasonable compromise ? >> > >> > [ trimmed Cc: list to probably the only people that care ] >> > >> > I would think so. >> >> OK great, can this be merged then? I am waiting for this patch to help >> move forward with deprecating MTRR upstream. > > I'm generally OK with picking this up and merging it in. However, it > relies on pat_enabled being an exported symbol, and that isn't the case. > Are you submitting an additional patch via a different tree to make that > symbol exported to modules? Thanks for spotting that I had not but I will do so as it is also needed for ivtv. I'll submit that. This might need to go through another tree then, can I add your Acked-by ? Luis -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Yes, you can add my ack Sent from my iPhone > On Apr 29, 2015, at 1:25 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote: > >> On Wed, Apr 29, 2015 at 9:57 AM, Doug Ledford <dledford@redhat.com> wrote: >>> On Wed, 2015-04-29 at 09:01 -0700, Luis R. Rodriguez wrote: >>>> On Mon, Apr 27, 2015 at 11:38 AM, Doug Ledford <dledford@redhat.com> wrote: >>>>> On Mon, 2015-04-27 at 09:46 -0700, Luis R. Rodriguez wrote: >>>>> On Wed, Apr 22, 2015 at 12:26 PM, Luis R. Rodriguez >>>>> <mcgrof@do-not-panic.com> wrote: >>>>>> From: "Luis R. Rodriguez" <mcgrof@suse.com> >>>>>> >>>>>> We are burrying direct access to MTRR code support on >>>>>> x86 in order to take advantage of PAT. In the future we >>>>>> also want to make the default behaviour of ioremap_nocache() >>>>>> to use strong UC, use of mtrr_add() on those systems >>>>>> would make write-combining void. >>>>>> >>>>>> In order to help both enable us to later make strong >>>>>> UC default and in order to phase out direct MTRR access >>>>>> code port the driver over to arch_phys_wc_add() and >>>>>> annotate that the device driver requires systems to >>>>>> boot with PAT disabled, with the nopat kernel parameter. >>>>>> >>>>>> This is a worthy compromise given that the ipath device >>>>>> driver powers the old HTX bus cards that only work in >>>>>> AMD systems, while the newer IB/qib device driver >>>>>> powers all PCI-e cards. The ipath device driver is >>>>>> obsolete, hardware hard to find and because of this >>>>>> this its a reasonable compromise to make to require >>>>>> users of ipath to boot with nopat. >>>>> >>>>> Hey folks, I realize its being discussed whether or not to remove the >>>>> driver entirely from the kernel but in the meantime, is this a >>>>> reasonable compromise ? >>>> >>>> [ trimmed Cc: list to probably the only people that care ] >>>> >>>> I would think so. >>> >>> OK great, can this be merged then? I am waiting for this patch to help >>> move forward with deprecating MTRR upstream. >> >> I'm generally OK with picking this up and merging it in. However, it >> relies on pat_enabled being an exported symbol, and that isn't the case. >> Are you submitting an additional patch via a different tree to make that >> symbol exported to modules? > > Thanks for spotting that I had not but I will do so as it is also > needed for ivtv. I'll submit that. This might need to go through > another tree then, can I add your Acked-by ? > > Luis -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/infiniband/hw/ipath/Kconfig b/drivers/infiniband/hw/ipath/Kconfig index 1d9bb11..8fe54ff 100644 --- a/drivers/infiniband/hw/ipath/Kconfig +++ b/drivers/infiniband/hw/ipath/Kconfig @@ -9,3 +9,6 @@ config INFINIBAND_IPATH as IP-over-InfiniBand as well as with userspace applications (in conjunction with InfiniBand userspace access). For QLogic PCIe QLE based cards, use the QIB driver instead. + + If you have this hardware you will need to boot with PAT disabled + on your x86-64 systems, use the nopat kernel parameter. diff --git a/drivers/infiniband/hw/ipath/ipath_driver.c b/drivers/infiniband/hw/ipath/ipath_driver.c index bd0caed..3ef592c 100644 --- a/drivers/infiniband/hw/ipath/ipath_driver.c +++ b/drivers/infiniband/hw/ipath/ipath_driver.c @@ -42,6 +42,9 @@ #include <linux/bitmap.h> #include <linux/slab.h> #include <linux/module.h> +#ifdef CONFIG_X86_64 +#include <asm/pat.h> +#endif #include "ipath_kernel.h" #include "ipath_verbs.h" @@ -395,6 +398,14 @@ static int ipath_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) unsigned long long addr; u32 bar0 = 0, bar1 = 0; +#ifdef CONFIG_X86_64 + if (WARN(pat_enabled, + "ipath needs PAT disabled, boot with nopat kernel parameter\n")) { + ret = EINVAL; + goto bail; + } +#endif + dd = ipath_alloc_devdata(pdev); if (IS_ERR(dd)) { ret = PTR_ERR(dd); @@ -542,6 +553,7 @@ static int ipath_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) dd->ipath_kregbase = __ioremap(addr, len, (_PAGE_NO_CACHE|_PAGE_WRITETHRU)); #else + /* XXX: split this properly to enable on PAT */ dd->ipath_kregbase = ioremap_nocache(addr, len); #endif @@ -587,12 +599,8 @@ static int ipath_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) ret = ipath_enable_wc(dd); - if (ret) { - ipath_dev_err(dd, "Write combining not enabled " - "(err %d): performance may be poor\n", - -ret); + if (ret) ret = 0; - } ipath_verify_pioperf(dd); diff --git a/drivers/infiniband/hw/ipath/ipath_kernel.h b/drivers/infiniband/hw/ipath/ipath_kernel.h index e08db70..f0f9471 100644 --- a/drivers/infiniband/hw/ipath/ipath_kernel.h +++ b/drivers/infiniband/hw/ipath/ipath_kernel.h @@ -463,9 +463,7 @@ struct ipath_devdata { /* offset in HT config space of slave/primary interface block */ u8 ipath_ht_slave_off; /* for write combining settings */ - unsigned long ipath_wc_cookie; - unsigned long ipath_wc_base; - unsigned long ipath_wc_len; + int wc_cookie; /* ref count for each pkey */ atomic_t ipath_pkeyrefs[4]; /* shadow copy of struct page *'s for exp tid pages */ diff --git a/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c b/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c index 70c1f3a..7b6e4c8 100644 --- a/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c +++ b/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c @@ -37,7 +37,6 @@ */ #include <linux/pci.h> -#include <asm/mtrr.h> #include <asm/processor.h> #include "ipath_kernel.h" @@ -122,27 +121,14 @@ int ipath_enable_wc(struct ipath_devdata *dd) } if (!ret) { - int cookie; - ipath_cdbg(VERBOSE, "Setting mtrr for chip to WC " - "(addr %llx, len=0x%llx)\n", - (unsigned long long) pioaddr, - (unsigned long long) piolen); - cookie = mtrr_add(pioaddr, piolen, MTRR_TYPE_WRCOMB, 1); - if (cookie < 0) { - { - dev_info(&dd->pcidev->dev, - "mtrr_add() WC for PIO bufs " - "failed (%d)\n", - cookie); - ret = -EINVAL; - } - } else { - ipath_cdbg(VERBOSE, "Set mtrr for chip to WC, " - "cookie is %d\n", cookie); - dd->ipath_wc_cookie = cookie; - dd->ipath_wc_base = (unsigned long) pioaddr; - dd->ipath_wc_len = (unsigned long) piolen; - } + dd->wc_cookie = arch_phys_wc_add(pioaddr, piolen); + if (dd->wc_cookie < 0) { + ipath_dev_err(dd, "Seting mtrr failed on PIO buffers\n"); + ret = -ENODEV; + } else if (dd->wc_cookie == 0) + ipath_cdbg(VERBOSE, "Set mtrr for chip to WC not needed\n"); + else + ipath_cdbg(VERBOSE, "Set mtrr for chip to WC\n"); } return ret; @@ -154,16 +140,5 @@ int ipath_enable_wc(struct ipath_devdata *dd) */ void ipath_disable_wc(struct ipath_devdata *dd) { - if (dd->ipath_wc_cookie) { - int r; - ipath_cdbg(VERBOSE, "undoing WCCOMB on pio buffers\n"); - r = mtrr_del(dd->ipath_wc_cookie, dd->ipath_wc_base, - dd->ipath_wc_len); - if (r < 0) - dev_info(&dd->pcidev->dev, - "mtrr_del(%lx, %lx, %lx) failed: %d\n", - dd->ipath_wc_cookie, dd->ipath_wc_base, - dd->ipath_wc_len, r); - dd->ipath_wc_cookie = 0; /* even on failure */ - } + arch_phys_wc_del(dd->wc_cookie); }