From: Luis R. Rodriguez <hidden> Date: 2015-06-19 22:10:56
From: "Luis R. Rodriguez" <redacted>
Arnd,
After a long-winded conversation with Bjorn over use of
EXPORT_SYMBOL_GPL() instead of EXPORT_SYMBOL() he's noted he'd
be OK if this goes through you as an alternative. This series
goes unmodified from the last posted v6 series, I am just
reposting and addressing this to you now in hopes you might
be comfortable with my use of EXPORT_SYMBOL_GPL() on new
PAT interfaces. I use EXPORT_SYMBOL_GPL() for new PAT symbols,
this parallels Ingo's stated preference as "we don't want
proprietary modules mucking around with new code PAT interfaces,
we only want modules we can analyze and fix in detail" [1].
Other than this Bjorn seemed comfortable with the series and
even had provided an Ack to the series, the EXPORT_SYMBOL_GPL()
thing was the ony thing he was not comfortable with, and also
that of the devres change going in with no users yet.
Tomi has already Ack'd the framebuffer driver specific changes
and is comfortable with the driver changes to go through the
tree that the depending symbols depend on.
I typically had submitted two series for these patches but since
the patches are all related and now already Acked I've bundled
them together and rebased them on top of today's linux-next tree.
This series is part of a larger effort to help avoid direct use
of MTRR code and instead replace it with more generic PAT
interfaces, I posted a full description of the entire series
recently [2].
Please let me know if there are any issues.
[0] http://lkml.kernel.org/r/CAErSpo7CNH1WpgqJCEU8EtxiFNp_PiQ3cBwnKiWQpUaD-fd4YA@mail.gmail.com
[1] http://article.gmane.org/gmane.linux.kernel.mm/129104
[2] http://lkml.kernel.org/r/CAB=NE6UgtdSoBsA=8+ueYRAZHDnWUSmQAoHhAaefqudBrSY7Zw@mail.gmail.com
Luis R. Rodriguez (9):
pci: add pci_ioremap_wc_bar()
video: fbdev: i740fb: use arch_phys_wc_add() and pci_ioremap_wc_bar()
video: fbdev: kyrofb: use arch_phys_wc_add() and pci_ioremap_wc_bar()
video: fbdev: gxt4500: use pci_ioremap_wc_bar() for framebuffer
PCI: Add pci_iomap_wc() variants
lib: devres: add pcim_iomap_wc() variants
video: fbdev: arkfb: use arch_phys_wc_add() and pci_iomap_wc()
video: fbdev: s3fb: use arch_phys_wc_add() and pci_iomap_wc()
video: fbdev: vt8623fb: use arch_phys_wc_add() and pci_iomap_wc()
drivers/pci/pci.c | 14 ++++++++
drivers/video/fbdev/arkfb.c | 36 +++----------------
drivers/video/fbdev/gxt4500.c | 2 +-
drivers/video/fbdev/i740fb.c | 35 ++++--------------
drivers/video/fbdev/kyro/fbdev.c | 33 ++++++-----------
drivers/video/fbdev/s3fb.c | 35 ++++--------------
drivers/video/fbdev/vt8623fb.c | 31 ++++------------
include/asm-generic/pci_iomap.h | 14 ++++++++
include/linux/pci.h | 3 ++
include/video/kyro.h | 4 +--
lib/devres.c | 78 ++++++++++++++++++++++++++++++++++++++++
lib/pci_iomap.c | 61 +++++++++++++++++++++++++++++++
12 files changed, 206 insertions(+), 140 deletions(-)
--
2.3.2.209.gd67f9d5.dirty
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
From: Luis R. Rodriguez <hidden> Date: 2015-06-19 22:13:10
From: "Luis R. Rodriguez" <redacted>
This lets drivers take advantage of PAT when available. This
should help with the transition of converting video drivers over
to ioremap_wc() to help with the goal of eventually using
_PAGE_CACHE_UC over _PAGE_CACHE_UC_MINUS on x86 on
ioremap_nocache() (de33c442e titled "x86 PAT: fix performance
drop for glx, use UC minus for ioremap(), ioremap_nocache() and
pci_mmap_page_range()")
Cc: Toshi Kani <redacted>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Suresh Siddha <redacted>
Cc: Ingo Molnar <redacted>
Cc: Thomas Gleixner <redacted>
Cc: Juergen Gross <jgross@suse.com>
Cc: Daniel Vetter <redacted>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <redacted>
Cc: Tomi Valkeinen <redacted>
Cc: Ville Syrjälä <syrjala@sci.fi>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Vlastimil Babka <redacted>
Cc: Borislav Petkov <redacted>
Cc: Davidlohr Bueso <redacted>
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Luis R. Rodriguez <redacted>
---
drivers/pci/pci.c | 14 ++++++++++++++
include/linux/pci.h | 1 +
2 files changed, 15 insertions(+)
From: Luis R. Rodriguez <hidden> Date: 2015-06-19 22:15:14
From: "Luis R. Rodriguez" <redacted>
Convert the driver from using the x86 specific MTRR code to
the architecture agnostic arch_phys_wc_add(). arch_phys_wc_add()
will avoid MTRR if write-combining is available, in order to
take advantage of that also ensure the ioremap'd area is requested
as write-combining.
There are a few motivations for this:
a) Take advantage of PAT when available
b) Help bury MTRR code away, MTRR is architecture specific and on
x86 its replaced by PAT
c) Help with the goal of eventually using _PAGE_CACHE_UC over
_PAGE_CACHE_UC_MINUS on x86 on ioremap_nocache() (see commit
de33c442e titled "x86 PAT: fix performance drop for glx,
use UC minus for ioremap(), ioremap_nocache() and
pci_mmap_page_range()")
The conversion done is expressed by the following Coccinelle
SmPL patch, it additionally required manual intervention to
address all the #ifdery and removal of redundant things which
arch_phys_wc_add() already addresses such as verbose message
about when MTRR fails and doing nothing when we didn't get
an MTRR.
@ mtrr_found @
expression index, base, size;
@@
-index = mtrr_add(base, size, MTRR_TYPE_WRCOMB, 1);
+index = arch_phys_wc_add(base, size);
@ mtrr_rm depends on mtrr_found @
expression mtrr_found.index, mtrr_found.base, mtrr_found.size;
@@
-mtrr_del(index, base, size);
+arch_phys_wc_del(index);
@ mtrr_rm_zero_arg depends on mtrr_found @
expression mtrr_found.index;
@@
-mtrr_del(index, 0, 0);
+arch_phys_wc_del(index);
@ mtrr_rm_fb_info depends on mtrr_found @
struct fb_info *info;
expression mtrr_found.index;
@@
-mtrr_del(index, info->fix.smem_start, info->fix.smem_len);
+arch_phys_wc_del(index);
@ ioremap_replace_nocache depends on mtrr_found @
struct fb_info *info;
expression base, size;
@@
-info->screen_base = ioremap_nocache(base, size);
+info->screen_base = ioremap_wc(base, size);
@ ioremap_replace_default depends on mtrr_found @
struct fb_info *info;
expression base, size;
@@
-info->screen_base = ioremap(base, size);
+info->screen_base = ioremap_wc(base, size);
Generated-by: Coccinelle SmPL
Cc: Jingoo Han <redacted>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Rob Clark <redacted>
Cc: Benoit Taine <redacted>
Cc: Suresh Siddha <redacted>
Cc: Ingo Molnar <redacted>
Cc: Thomas Gleixner <redacted>
Cc: Juergen Gross <jgross@suse.com>
Cc: Daniel Vetter <redacted>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <redacted>
Cc: Tomi Valkeinen <redacted>
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Acked-by: Tomi Valkeinen <redacted>
Signed-off-by: Luis R. Rodriguez <redacted>
---
drivers/video/fbdev/i740fb.c | 35 ++++++-----------------------------
1 file changed, 6 insertions(+), 29 deletions(-)
From: Luis R. Rodriguez <hidden> Date: 2015-06-19 22:17:29
From: "Luis R. Rodriguez" <redacted>
Convert the driver from using the x86 specific MTRR code to
the architecture agnostic arch_phys_wc_add(). arch_phys_wc_add()
will avoid MTRR if write-combining is available, in order to
take advantage of that also ensure the ioremap'd area is requested
as write-combining.
There are a few motivations for this:
a) Take advantage of PAT when available
b) Help bury MTRR code away, MTRR is architecture specific and on
x86 its replaced by PAT
c) Help with the goal of eventually using _PAGE_CACHE_UC over
_PAGE_CACHE_UC_MINUS on x86 on ioremap_nocache() (see commit
de33c442e titled "x86 PAT: fix performance drop for glx,
use UC minus for ioremap(), ioremap_nocache() and
pci_mmap_page_range()")
The conversion done is expressed by the following Coccinelle
SmPL patch, it additionally required manual intervention to
address all the #ifdery and removal of redundant things which
arch_phys_wc_add() already addresses such as verbose message
about when MTRR fails and doing nothing when we didn't get
an MTRR.
@ mtrr_found @
expression index, base, size;
@@
-index = mtrr_add(base, size, MTRR_TYPE_WRCOMB, 1);
+index = arch_phys_wc_add(base, size);
@ mtrr_rm depends on mtrr_found @
expression mtrr_found.index, mtrr_found.base, mtrr_found.size;
@@
-mtrr_del(index, base, size);
+arch_phys_wc_del(index);
@ mtrr_rm_zero_arg depends on mtrr_found @
expression mtrr_found.index;
@@
-mtrr_del(index, 0, 0);
+arch_phys_wc_del(index);
@ mtrr_rm_fb_info depends on mtrr_found @
struct fb_info *info;
expression mtrr_found.index;
@@
-mtrr_del(index, info->fix.smem_start, info->fix.smem_len);
+arch_phys_wc_del(index);
@ ioremap_replace_nocache depends on mtrr_found @
struct fb_info *info;
expression base, size;
@@
-info->screen_base = ioremap_nocache(base, size);
+info->screen_base = ioremap_wc(base, size);
@ ioremap_replace_default depends on mtrr_found @
struct fb_info *info;
expression base, size;
@@
-info->screen_base = ioremap(base, size);
+info->screen_base = ioremap_wc(base, size);
Generated-by: Coccinelle SmPL
Cc: Jingoo Han <redacted>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Suresh Siddha <redacted>
Cc: Ingo Molnar <redacted>
Cc: Thomas Gleixner <redacted>
Cc: Juergen Gross <jgross@suse.com>
Cc: Daniel Vetter <redacted>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <redacted>
Cc: Tomi Valkeinen <redacted>
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Acked-by: Tomi Valkeinen <redacted>
Signed-off-by: Luis R. Rodriguez <redacted>
---
drivers/video/fbdev/kyro/fbdev.c | 33 +++++++++++----------------------
include/video/kyro.h | 4 +---
2 files changed, 12 insertions(+), 25 deletions(-)
@@ -35,9 +35,7 @@ struct kyrofb_info {/* Useful to hold depth here for Linux */u8PIXDEPTH;-#ifdef CONFIG_MTRR-intmtrr_handle;-#endif+intwc_cookie;};externintkyro_dev_init(void);
--
2.3.2.209.gd67f9d5.dirty
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
From: Luis R. Rodriguez <hidden> Date: 2015-06-19 22:19:39
From: "Luis R. Rodriguez" <redacted>
The driver doesn't use mtrr_add() or arch_phys_wc_add() but
since we know the framebuffer is isolated already on an
ioremap() we can take advantage of write combining for
performance where possible.
In this case there are a few motivations for this:
a) Take advantage of PAT when available
b) Help with the goal of eventually using _PAGE_CACHE_UC over
_PAGE_CACHE_UC_MINUS on x86 on ioremap_nocache() (see commit
de33c442e titled "x86 PAT: fix performance drop for glx,
use UC minus for ioremap(), ioremap_nocache() and
pci_mmap_page_range()")
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Rob Clark <redacted>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Suresh Siddha <redacted>
Cc: Ingo Molnar <redacted>
Cc: Thomas Gleixner <redacted>
Cc: Juergen Gross <jgross@suse.com>
Cc: Daniel Vetter <redacted>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <redacted>
Cc: Tomi Valkeinen <redacted>
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Acked-by: Tomi Valkeinen <redacted>
Signed-off-by: Luis R. Rodriguez <redacted>
---
drivers/video/fbdev/gxt4500.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Luis R. Rodriguez <hidden> Date: 2015-06-19 22:21:50
From: "Luis R. Rodriguez" <redacted>
PCI BARs tell us whether prefetching is safe, but they don't say anything
about write combining (WC). WC changes ordering rules and allows writes to
be collapsed, so it's not safe in general to use it on a prefetchable
region.
Add pci_iomap_wc() and pci_iomap_wc_range() so drivers can take advantage
of write combining when they know it's safe.
On architectures that don't fully support WC, e.g., x86 without PAT,
drivers for legacy framebuffers may get some of the benefit by using
arch_phys_wc_add() in addition to pci_iomap_wc(). But arch_phys_wc_add()
is unreliable and should be avoided in general. On x86, it uses MTRRs,
which are limited in number and size, so the results will vary based on
driver loading order.
The goals of adding pci_iomap_wc() are to:
- Give drivers an architecture-independent way to use WC so they can stop
using interfaces like mtrr_add() (on x86, pci_iomap_wc() uses
PAT when available)
- Move toward using _PAGE_CACHE_MODE_UC, not _PAGE_CACHE_MODE_UC_MINUS,
on x86 on ioremap_nocache() (see de33c442ed2a ("x86 PAT: fix
performance drop for glx, use UC minus for ioremap(), ioremap_nocache()
and pci_mmap_page_range()")
Link: http://lkml.kernel.org/r/1426893517-2511-6-git-send-email-mcgrof@do-not-panic.com
Original-posting: http://lkml.kernel.org/r/1432163293-20965-1-git-send-email-mcgrof@do-not-panic.com
Cc: Toshi Kani <redacted>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Suresh Siddha <redacted>
Cc: Ingo Molnar <redacted>
Cc: Thomas Gleixner <redacted>
Cc: Juergen Gross <jgross@suse.com>
Cc: Daniel Vetter <redacted>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <redacted>
Cc: Tomi Valkeinen <redacted>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: venkatesh.pallipadi@intel.com
Cc: Stefan Bader <redacted>
Cc: Ville Syrjälä <syrjala@sci.fi>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Vlastimil Babka <redacted>
Cc: Borislav Petkov <redacted>
Cc: Davidlohr Bueso <redacted>
Cc: konrad.wilk@oracle.com
Cc: ville.syrjala@linux.intel.com
Cc: david.vrabel@citrix.com
Cc: jbeulich@suse.com
Cc: Roger Pau Monné <redacted>
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: xen-devel@lists.xensource.com
Signed-off-by: Luis R. Rodriguez <redacted>
---
include/asm-generic/pci_iomap.h | 14 ++++++++++
lib/pci_iomap.c | 61 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 75 insertions(+)
@@ -15,9 +15,13 @@ struct pci_dev;#ifdef CONFIG_PCI/* Create a virtual mapping cookie for a PCI BAR (memory or IO) */externvoid__iomem*pci_iomap(structpci_dev*dev,intbar,unsignedlongmax);+externvoid__iomem*pci_iomap_wc(structpci_dev*dev,intbar,unsignedlongmax);externvoid__iomem*pci_iomap_range(structpci_dev*dev,intbar,unsignedlongoffset,unsignedlongmaxlen);+externvoid__iomem*pci_iomap_wc_range(structpci_dev*dev,intbar,+unsignedlongoffset,+unsignedlongmaxlen);/* Create a virtual mapping cookie for a port on a given PCI device.*Donotcallthisdirectly,itexiststomakeiteasierforarchitectures*tooverride*/
@@ -34,12 +38,22 @@ static inline void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned lonreturnNULL;}+staticinlinevoid__iomem*pci_iomap_wc(structpci_dev*dev,intbar,unsignedlongmax)+{+returnNULL;+}staticinlinevoid__iomem*pci_iomap_range(structpci_dev*dev,intbar,unsignedlongoffset,unsignedlongmaxlen){returnNULL;}+staticinlinevoid__iomem*pci_iomap_wc_range(structpci_dev*dev,intbar,+unsignedlongoffset,+unsignedlongmaxlen)+{+returnNULL;+}#endif#endif /* __ASM_GENERIC_IO_H */
@@ -304,6 +304,30 @@ void __iomem *pcim_iomap(struct pci_dev *pdev, int bar, unsigned long maxlen)EXPORT_SYMBOL(pcim_iomap);/**+*pcim_iomap_wc-Managedpcim_iomap_wc()+*@pdev:PCIdevicetoiomapfor+*@bar:BARtoiomap+*@maxlen:Maximumlengthofiomap+*+*Managedpci_iomap_wc().Mapisautomaticallyunmappedondriver+*detach.+*/+void__iomem*pcim_iomap_wc(structpci_dev*pdev,intbar,unsignedlongmaxlen)+{+void__iomem**tbl;++BUG_ON(bar>=PCIM_IOMAP_MAX);++tbl=(void__iomem**)pcim_iomap_table(pdev);+if(!tbl||tbl[bar])/* duplicate mappings not allowed */+returnNULL;++tbl[bar]=pci_iomap_wc(pdev,bar,maxlen);+returntbl[bar];+}+EXPORT_SYMBOL_GPL(pcim_iomap_wc);++/***pcim_iounmap-Managedpci_iounmap()*@pdev:PCIdevicetoiounmapfor*@addr:Addresstounmap
@@ -383,6 +407,60 @@ int pcim_iomap_regions(struct pci_dev *pdev, int mask, const char *name)EXPORT_SYMBOL(pcim_iomap_regions);/**+*pcim_iomap_wc_regions-RequestandiomapPCIBARswithwrite-combining+*@pdev:PCIdevicetomapIOresourcesfor+*@mask:MaskofBARstorequestandiomap+*@name:Nameusedwhenrequestingregions+*+*Requestandiomapregionsspecifiedby@maskwithapreferencefor+*write-combining.+*/+intpcim_iomap_wc_regions(structpci_dev*pdev,intmask,constchar*name)+{+void__iomem*const*iomap;+inti,rc;++iomap=pcim_iomap_table(pdev);+if(!iomap)+return-ENOMEM;++for(i=0;i<DEVICE_COUNT_RESOURCE;i++){+unsignedlonglen;++if(!(mask&(1<<i)))+continue;++rc=-EINVAL;+len=pci_resource_len(pdev,i);+if(!len)+gotoerr_inval;++rc=pci_request_region(pdev,i,name);+if(rc)+gotoerr_inval;++rc=-ENOMEM;+if(!pcim_iomap_wc(pdev,i,0))+gotoerr_region;+}++return0;++err_region:+pci_release_region(pdev,i);+err_inval:+while(--i>=0){+if(!(mask&(1<<i)))+continue;+pcim_iounmap(pdev,iomap[i]);+pci_release_region(pdev,i);+}++returnrc;+}+EXPORT_SYMBOL_GPL(pcim_iomap_wc_regions);++/***pcim_iomap_regions_request_all-RequestallBARsandiomapspecifiedones*@pdev:PCIdevicetomapIOresourcesfor*@mask:MaskofBARstoiomap
--
2.3.2.209.gd67f9d5.dirty
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
From: Luis R. Rodriguez <hidden> Date: 2015-06-19 22:26:07
From: "Luis R. Rodriguez" <redacted>
Convert the driver from using the x86 specific MTRR code to
the architecture agnostic arch_phys_wc_add(). arch_phys_wc_add()
will avoid MTRR if write-combining is available, in order to
take advantage of that also ensure the ioremap'd area is requested
as write-combining.
There are a few motivations for this:
a) Take advantage of PAT when available
b) Help bury MTRR code away, MTRR is architecture specific and on
x86 its replaced by PAT
c) Help with the goal of eventually using _PAGE_CACHE_UC over
_PAGE_CACHE_UC_MINUS on x86 on ioremap_nocache() (see commit
de33c442e titled "x86 PAT: fix performance drop for glx,
use UC minus for ioremap(), ioremap_nocache() and
pci_mmap_page_range()")
The conversion done is expressed by the following Coccinelle
SmPL patch, it additionally required manual intervention to
address all the #ifdery and removal of redundant things which
arch_phys_wc_add() already addresses such as verbose message
about when MTRR fails and doing nothing when we didn't get
an MTRR.
@ mtrr_found @
expression index, base, size;
@@
-index = mtrr_add(base, size, MTRR_TYPE_WRCOMB, 1);
+index = arch_phys_wc_add(base, size);
@ mtrr_rm depends on mtrr_found @
expression mtrr_found.index, mtrr_found.base, mtrr_found.size;
@@
-mtrr_del(index, base, size);
+arch_phys_wc_del(index);
@ mtrr_rm_zero_arg depends on mtrr_found @
expression mtrr_found.index;
@@
-mtrr_del(index, 0, 0);
+arch_phys_wc_del(index);
@ mtrr_rm_fb_info depends on mtrr_found @
struct fb_info *info;
expression mtrr_found.index;
@@
-mtrr_del(index, info->fix.smem_start, info->fix.smem_len);
+arch_phys_wc_del(index);
@ ioremap_replace_nocache depends on mtrr_found @
struct fb_info *info;
expression base, size;
@@
-info->screen_base = ioremap_nocache(base, size);
+info->screen_base = ioremap_wc(base, size);
@ ioremap_replace_default depends on mtrr_found @
struct fb_info *info;
expression base, size;
@@
-info->screen_base = ioremap(base, size);
+info->screen_base = ioremap_wc(base, size);
Generated-by: Coccinelle SmPL
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
Cc: Suresh Siddha <redacted>
Cc: Ingo Molnar <redacted>
Cc: Thomas Gleixner <redacted>
Cc: Juergen Gross <jgross@suse.com>
Cc: Daniel Vetter <redacted>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <redacted>
Cc: Tomi Valkeinen <redacted>
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Acked-by: Tomi Valkeinen <redacted>
Signed-off-by: Luis R. Rodriguez <redacted>
---
drivers/video/fbdev/arkfb.c | 36 +++++-------------------------------
1 file changed, 5 insertions(+), 31 deletions(-)
@@ -26,13 +26,9 @@#include<linux/console.h> /* Why should fb driver call console functions? because console_lock() */#include<video/vga.h>-#ifdef CONFIG_MTRR-#include<asm/mtrr.h>-#endif-structarkfb_info{intmclk_freq;-intmtrr_reg;+intwc_cookie;structdac_info*dac;structvgastatestate;
@@ -102,10 +98,6 @@ static const struct svga_timing_regs ark_timing_regs = {staticchar*mode_option="640x480-8@60";-#ifdef CONFIG_MTRR-staticintmtrr=1;-#endif-MODULE_AUTHOR("(c) 2007 Ondrej Zajicek <santiago@crfreenet.org>");MODULE_LICENSE("GPL");MODULE_DESCRIPTION("fbdev driver for ARK 2000PV");
@@ -115,11 +107,6 @@ MODULE_PARM_DESC(mode_option, "Default video mode ('640x480-8@60', etc)");module_param_named(mode,mode_option,charp,0444);MODULE_PARM_DESC(mode,"Default video mode ('640x480-8@60', etc) (deprecated)");-#ifdef CONFIG_MTRR-module_param(mtrr,int,0444);-MODULE_PARM_DESC(mtrr,"Enable write-combining with MTRR (1=enable, 0=disable, default=1)");-#endif-staticintthreshold=4;module_param(threshold,int,0644);
@@ -1002,7 +989,7 @@ static int ark_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)info->fix.smem_len=pci_resource_len(dev,0);/* Map physical IO memory address into kernel space */-info->screen_base=pci_iomap(dev,0,0);+info->screen_base=pci_iomap_wc(dev,0,0);if(!info->screen_base){rc=-ENOMEM;dev_err(info->device,"iomap for framebuffer failed\n");
@@ -1057,14 +1044,8 @@ static int ark_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)/* Record a reference to the driver data */pci_set_drvdata(dev,info);--#ifdef CONFIG_MTRR-if(mtrr){-par->mtrr_reg=-1;-par->mtrr_reg=mtrr_add(info->fix.smem_start,info->fix.smem_len,MTRR_TYPE_WRCOMB,1);-}-#endif-+par->wc_cookie=arch_phys_wc_add(info->fix.smem_start,+info->fix.smem_len);return0;/* Error handling */
From: Luis R. Rodriguez <hidden> Date: 2015-06-19 22:28:24
From: "Luis R. Rodriguez" <redacted>
This driver uses the same area for MTRR as for the ioremap().
Convert the driver from using the x86 specific MTRR code to
the architecture agnostic arch_phys_wc_add(). arch_phys_wc_add()
will avoid MTRR if write-combining is available, in order to
take advantage of that also ensure the ioremap'd area is requested
as write-combining.
There are a few motivations for this:
a) Take advantage of PAT when available
b) Help bury MTRR code away, MTRR is architecture specific and on
x86 its replaced by PAT
c) Help with the goal of eventually using _PAGE_CACHE_UC over
_PAGE_CACHE_UC_MINUS on x86 on ioremap_nocache() (see commit
de33c442e titled "x86 PAT: fix performance drop for glx,
use UC minus for ioremap(), ioremap_nocache() and
pci_mmap_page_range()")
The conversion done is expressed by the following Coccinelle
SmPL patch, it additionally required manual intervention to
address all the #ifdery and removal of redundant things which
arch_phys_wc_add() already addresses such as verbose message
about when MTRR fails and doing nothing when we didn't get
an MTRR.
@ mtrr_found @
expression index, base, size;
@@
-index = mtrr_add(base, size, MTRR_TYPE_WRCOMB, 1);
+index = arch_phys_wc_add(base, size);
@ mtrr_rm depends on mtrr_found @
expression mtrr_found.index, mtrr_found.base, mtrr_found.size;
@@
-mtrr_del(index, base, size);
+arch_phys_wc_del(index);
@ mtrr_rm_zero_arg depends on mtrr_found @
expression mtrr_found.index;
@@
-mtrr_del(index, 0, 0);
+arch_phys_wc_del(index);
@ mtrr_rm_fb_info depends on mtrr_found @
struct fb_info *info;
expression mtrr_found.index;
@@
-mtrr_del(index, info->fix.smem_start, info->fix.smem_len);
+arch_phys_wc_del(index);
@ ioremap_replace_nocache depends on mtrr_found @
struct fb_info *info;
expression base, size;
@@
-info->screen_base = ioremap_nocache(base, size);
+info->screen_base = ioremap_wc(base, size);
@ ioremap_replace_default depends on mtrr_found @
struct fb_info *info;
expression base, size;
@@
-info->screen_base = ioremap(base, size);
+info->screen_base = ioremap_wc(base, size);
Generated-by: Coccinelle SmPL
Cc: Jean-Christophe Plagniol-Villard <redacted>
Cc: Tomi Valkeinen <redacted>
Cc: Jingoo Han <redacted>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Daniel Vetter <redacted>
Cc: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
Cc: Rickard Strandqvist <redacted>
Cc: Suresh Siddha <redacted>
Cc: Ingo Molnar <redacted>
Cc: Thomas Gleixner <redacted>
Cc: Juergen Gross <jgross@suse.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Acked-by: Tomi Valkeinen <redacted>
Signed-off-by: Luis R. Rodriguez <redacted>
---
drivers/video/fbdev/s3fb.c | 35 ++++++-----------------------------
1 file changed, 6 insertions(+), 29 deletions(-)
@@ -170,11 +162,8 @@ module_param(mode_option, charp, 0444);MODULE_PARM_DESC(mode_option,"Default video mode ('640x480-8@60', etc)");module_param_named(mode,mode_option,charp,0444);MODULE_PARM_DESC(mode,"Default video mode ('640x480-8@60', etc) (deprecated)");--#ifdef CONFIG_MTRRmodule_param(mtrr,int,0444);MODULE_PARM_DESC(mtrr,"Enable write-combining with MTRR (1=enable, 0=disable, default=1)");-#endifmodule_param(fasttext,int,0644);MODULE_PARM_DESC(fasttext,"Enable S3 fast text mode (1=enable, 0=disable, default=1)");
@@ -1168,7 +1157,7 @@ static int s3_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)info->fix.smem_len=pci_resource_len(dev,0);/* Map physical IO memory address into kernel space */-info->screen_base=pci_iomap(dev,0,0);+info->screen_base=pci_iomap_wc(dev,0,0);if(!info->screen_base){rc=-ENOMEM;dev_err(info->device,"iomap for framebuffer failed\n");
@@ -1365,12 +1354,9 @@ static int s3_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)/* Record a reference to the driver data */pci_set_drvdata(dev,info);-#ifdef CONFIG_MTRR-if(mtrr){-par->mtrr_reg=-1;-par->mtrr_reg=mtrr_add(info->fix.smem_start,info->fix.smem_len,MTRR_TYPE_WRCOMB,1);-}-#endif+if(mtrr)+par->wc_cookie=arch_phys_wc_add(info->fix.smem_start,+info->fix.smem_len);return0;
From: Luis R. Rodriguez <hidden> Date: 2015-06-19 22:30:34
From: "Luis R. Rodriguez" <redacted>
This driver uses the same area for MTRR as for the ioremap().
Convert the driver from using the x86 specific MTRR code to
the architecture agnostic arch_phys_wc_add(). arch_phys_wc_add()
will avoid MTRR if write-combining is available, in order to
take advantage of that also ensure the ioremap'd area is requested
as write-combining.
There are a few motivations for this:
a) Take advantage of PAT when available
b) Help bury MTRR code away, MTRR is architecture specific and on
x86 its replaced by PAT
c) Help with the goal of eventually using _PAGE_CACHE_UC over
_PAGE_CACHE_UC_MINUS on x86 on ioremap_nocache() (see commit
de33c442e titled "x86 PAT: fix performance drop for glx,
use UC minus for ioremap(), ioremap_nocache() and
pci_mmap_page_range()")
The conversion done is expressed by the following Coccinelle
SmPL patch, it additionally required manual intervention to
address all the #ifdery and removal of redundant things which
arch_phys_wc_add() already addresses such as verbose message
about when MTRR fails and doing nothing when we didn't get
an MTRR.
@ mtrr_found @
expression index, base, size;
@@
-index = mtrr_add(base, size, MTRR_TYPE_WRCOMB, 1);
+index = arch_phys_wc_add(base, size);
@ mtrr_rm depends on mtrr_found @
expression mtrr_found.index, mtrr_found.base, mtrr_found.size;
@@
-mtrr_del(index, base, size);
+arch_phys_wc_del(index);
@ mtrr_rm_zero_arg depends on mtrr_found @
expression mtrr_found.index;
@@
-mtrr_del(index, 0, 0);
+arch_phys_wc_del(index);
@ mtrr_rm_fb_info depends on mtrr_found @
struct fb_info *info;
expression mtrr_found.index;
@@
-mtrr_del(index, info->fix.smem_start, info->fix.smem_len);
+arch_phys_wc_del(index);
@ ioremap_replace_nocache depends on mtrr_found @
struct fb_info *info;
expression base, size;
@@
-info->screen_base = ioremap_nocache(base, size);
+info->screen_base = ioremap_wc(base, size);
@ ioremap_replace_default depends on mtrr_found @
struct fb_info *info;
expression base, size;
@@
-info->screen_base = ioremap(base, size);
+info->screen_base = ioremap_wc(base, size);
Generated-by: Coccinelle SmPL
Cc: Rob Clark <redacted>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Jingoo Han <redacted>
Cc: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
Cc: Suresh Siddha <redacted>
Cc: Ingo Molnar <redacted>
Cc: Thomas Gleixner <redacted>
Cc: Juergen Gross <jgross@suse.com>
Cc: Daniel Vetter <redacted>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <redacted>
Cc: Tomi Valkeinen <redacted>
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Acked-by: Tomi Valkeinen <redacted>
Signed-off-by: Luis R. Rodriguez <redacted>
---
drivers/video/fbdev/vt8623fb.c | 31 ++++++-------------------------
1 file changed, 6 insertions(+), 25 deletions(-)
@@ -112,11 +105,8 @@ module_param(mode_option, charp, 0644);MODULE_PARM_DESC(mode_option,"Default video mode ('640x480-8@60', etc)");module_param_named(mode,mode_option,charp,0);MODULE_PARM_DESC(mode,"Default video mode e.g. '648x480-8@60' (deprecated)");--#ifdef CONFIG_MTRRmodule_param(mtrr,int,0444);MODULE_PARM_DESC(mtrr,"Enable write-combining with MTRR (1=enable, 0=disable, default=1)");-#endif/* ------------------------------------------------------------------------- */
@@ -710,7 +700,7 @@ static int vt8623_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)info->fix.mmio_len=pci_resource_len(dev,1);/* Map physical IO memory address into kernel space */-info->screen_base=pci_iomap(dev,0,0);+info->screen_base=pci_iomap_wc(dev,0,0);if(!info->screen_base){rc=-ENOMEM;dev_err(info->device,"iomap for framebuffer failed\n");
@@ -781,12 +771,9 @@ static int vt8623_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)/* Record a reference to the driver data */pci_set_drvdata(dev,info);-#ifdef CONFIG_MTRR-if(mtrr){-par->mtrr_reg=-1;-par->mtrr_reg=mtrr_add(info->fix.smem_start,info->fix.smem_len,MTRR_TYPE_WRCOMB,1);-}-#endif+if(mtrr)+par->wc_cookie=arch_phys_wc_add(info->fix.smem_start,+info->fix.smem_len);return0;
On Friday 19 June 2015 15:08:23 Luis R. Rodriguez wrote:
From: "Luis R. Rodriguez" <redacted>
Arnd,
After a long-winded conversation with Bjorn over use of
EXPORT_SYMBOL_GPL() instead of EXPORT_SYMBOL() he's noted he'd
be OK if this goes through you as an alternative. This series
goes unmodified from the last posted v6 series, I am just
reposting and addressing this to you now in hopes you might
be comfortable with my use of EXPORT_SYMBOL_GPL() on new
PAT interfaces. I use EXPORT_SYMBOL_GPL() for new PAT symbols,
this parallels Ingo's stated preference as "we don't want
proprietary modules mucking around with new code PAT interfaces,
we only want modules we can analyze and fix in detail" [1].
Other than this Bjorn seemed comfortable with the series and
even had provided an Ack to the series, the EXPORT_SYMBOL_GPL()
thing was the ony thing he was not comfortable with, and also
that of the devres change going in with no users yet.
Tomi has already Ack'd the framebuffer driver specific changes
and is comfortable with the driver changes to go through the
tree that the depending symbols depend on.
I typically had submitted two series for these patches but since
the patches are all related and now already Acked I've bundled
them together and rebased them on top of today's linux-next tree.
This series is part of a larger effort to help avoid direct use
of MTRR code and instead replace it with more generic PAT
interfaces, I posted a full description of the entire series
recently [2].
Please let me know if there are any issues.
Hi Luis,
sorry for messing up your plans, but I'm currently on parental
leave and not taking any patches for arm-soc or asm-generic
until I'm back at work in around 3 months.
I've briefly looked at your patches and they all look reasonable
to me, but it would be best if you could find someone else to
take them. Please an my 'Acked-by' to patches 1, 5 and 6.
Arnd
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2015-06-23 22:45:08
On Fri, 2015-06-19 at 15:08 -0700, Luis R. Rodriguez wrote:
From: "Luis R. Rodriguez" <redacted>
PCI BARs tell us whether prefetching is safe, but they don't say anything
about write combining (WC). WC changes ordering rules and allows writes to
be collapsed, so it's not safe in general to use it on a prefetchable
region.
Well, the PCIe spec at least specifies that a prefetchable BAR also
tolerates write merging...
quoted hunk
Add pci_iomap_wc() and pci_iomap_wc_range() so drivers can take advantage
of write combining when they know it's safe.
On architectures that don't fully support WC, e.g., x86 without PAT,
drivers for legacy framebuffers may get some of the benefit by using
arch_phys_wc_add() in addition to pci_iomap_wc(). But arch_phys_wc_add()
is unreliable and should be avoided in general. On x86, it uses MTRRs,
which are limited in number and size, so the results will vary based on
driver loading order.
The goals of adding pci_iomap_wc() are to:
- Give drivers an architecture-independent way to use WC so they can stop
using interfaces like mtrr_add() (on x86, pci_iomap_wc() uses
PAT when available)
- Move toward using _PAGE_CACHE_MODE_UC, not _PAGE_CACHE_MODE_UC_MINUS,
on x86 on ioremap_nocache() (see de33c442ed2a ("x86 PAT: fix
performance drop for glx, use UC minus for ioremap(), ioremap_nocache()
and pci_mmap_page_range()")
Link: http://lkml.kernel.org/r/1426893517-2511-6-git-send-email-mcgrof@do-not-panic.com
Original-posting: http://lkml.kernel.org/r/1432163293-20965-1-git-send-email-mcgrof@do-not-panic.com
Cc: Toshi Kani <redacted>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Suresh Siddha <redacted>
Cc: Ingo Molnar <redacted>
Cc: Thomas Gleixner <redacted>
Cc: Juergen Gross <jgross@suse.com>
Cc: Daniel Vetter <redacted>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <redacted>
Cc: Tomi Valkeinen <redacted>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: venkatesh.pallipadi@intel.com
Cc: Stefan Bader <redacted>
Cc: Ville Syrjälä <syrjala@sci.fi>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Vlastimil Babka <redacted>
Cc: Borislav Petkov <redacted>
Cc: Davidlohr Bueso <redacted>
Cc: konrad.wilk@oracle.com
Cc: ville.syrjala@linux.intel.com
Cc: david.vrabel@citrix.com
Cc: jbeulich@suse.com
Cc: Roger Pau Monné <redacted>
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: xen-devel@lists.xensource.com
Signed-off-by: Luis R. Rodriguez <redacted>
---
include/asm-generic/pci_iomap.h | 14 ++++++++++
lib/pci_iomap.c | 61 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 75 insertions(+)
@@ -15,9 +15,13 @@ struct pci_dev;#ifdef CONFIG_PCI/* Create a virtual mapping cookie for a PCI BAR (memory or IO) */externvoid__iomem*pci_iomap(structpci_dev*dev,intbar,unsignedlongmax);+externvoid__iomem*pci_iomap_wc(structpci_dev*dev,intbar,unsignedlongmax);externvoid__iomem*pci_iomap_range(structpci_dev*dev,intbar,unsignedlongoffset,unsignedlongmaxlen);+externvoid__iomem*pci_iomap_wc_range(structpci_dev*dev,intbar,+unsignedlongoffset,+unsignedlongmaxlen);/* Create a virtual mapping cookie for a port on a given PCI device.*Donotcallthisdirectly,itexiststomakeiteasierforarchitectures*tooverride*/
@@ -34,12 +38,22 @@ static inline void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned lonreturnNULL;}+staticinlinevoid__iomem*pci_iomap_wc(structpci_dev*dev,intbar,unsignedlongmax)+{+returnNULL;+}staticinlinevoid__iomem*pci_iomap_range(structpci_dev*dev,intbar,unsignedlongoffset,unsignedlongmaxlen){returnNULL;}+staticinlinevoid__iomem*pci_iomap_wc_range(structpci_dev*dev,intbar,+unsignedlongoffset,+unsignedlongmaxlen)+{+returnNULL;+}#endif#endif /* __ASM_GENERIC_IO_H */
From: Luis R. Rodriguez <hidden> Date: 2015-06-24 16:38:37
On Wed, Jun 24, 2015 at 08:42:23AM +1000, Benjamin Herrenschmidt wrote:
On Fri, 2015-06-19 at 15:08 -0700, Luis R. Rodriguez wrote:
quoted
From: "Luis R. Rodriguez" <redacted>
PCI BARs tell us whether prefetching is safe, but they don't say anything
about write combining (WC). WC changes ordering rules and allows writes to
be collapsed, so it's not safe in general to use it on a prefetchable
region.
Well, the PCIe spec at least specifies that a prefetchable BAR also
tolerates write merging...
How can that be determined and can that be used as a full bullet proof hint
to enable wc ? And are you sure? :) Reason all this was stated was to be
apologetic over why we can't automate this behind the scenes. Otherwise
we could amend what you stated into the commit log to elaborate on our
technical apology. Let me know!
Luis
From: Luis R. Rodriguez <hidden> Date: 2015-06-24 22:29:49
On Wed, Jun 24, 2015 at 3:05 PM, Benjamin Herrenschmidt
[off-list ref] wrote:
On Wed, 2015-06-24 at 18:38 +0200, Luis R. Rodriguez wrote:
quoted
On Wed, Jun 24, 2015 at 08:42:23AM +1000, Benjamin Herrenschmidt wrote:
quoted
On Fri, 2015-06-19 at 15:08 -0700, Luis R. Rodriguez wrote:
quoted
From: "Luis R. Rodriguez" <redacted>
PCI BARs tell us whether prefetching is safe, but they don't say anything
about write combining (WC). WC changes ordering rules and allows writes to
be collapsed, so it's not safe in general to use it on a prefetchable
region.
Well, the PCIe spec at least specifies that a prefetchable BAR also
tolerates write merging...
How can that be determined and can that be used as a full bullet proof hint
to enable wc ? And are you sure? :)
Well, I"m sure the spec says that ;-) But it could be new to PCIe, I
haven't checked legacy PCI.
OK cool so to be clear from what I gather you are suggesting (or not
and letting me make it) is that we might be able to enforce
write-merging on prefetchable areas, and if we can *ensure* we do this
then automatically enable write-combining behind the scenes?
quoted
Reason all this was stated was to be
apologetic over why we can't automate this behind the scenes. Otherwise
we could amend what you stated into the commit log to elaborate on our
technical apology. Let me know!
At least on powerpc, for mmap of resource to userspace, we take off the
garded bit in the PTE for prefetchable BARs. This has the effect
architecturally of enabling both prefetch and write combine (ie. side
effect)
That's pretty darn sexy.
though afaik, the implementations probably don't actually
prefetch. We've done that for years.
Neat!
In fact we don't have a way to split the notions, it's either G or no G,
which carries both meanings.
Interesting.
Do you have example/case of a device having problems ?
Nope but at least what made me squint at this being a possible
"feature" was that in practice when reviewing all of the kernels
pending device drivers using MTRR (potential write-combine candidates)
I encountered a slew of them which had the architectural unfortunate
practice of combining PCI bars for MMIO and their respective
write-combined desirable area (framebuffer for video, PIO buffers for
infiniband, etc). Now, to me that read more as a practice for old
school devices when such things were likely still being evaluated,
more modern devices seem to adhere to sticking a full PCI bar with
write-combining or not. Did you not encounter such mismatch splits on
powerpc ? Was such possibility addressed?
If what you are implying here is applicable to the x86 world I'm all
for enabling this as we'd have less code to maintain but I'll note
that getting a clarification alone on that prefetchable !write-combining was in and of itself hard, I'd be surprised if we
could get full architectural buy-in to this as an immediate automatic
feature. Because of this and because PAT did have some errata as well,
I would not be surprised if some PCI bridges / devices would end up
finding corner cases, as such if we can really do what you're saying
and unless we can get some super sane certainty over it across the
board, I'd be inclined to leave such things as a part of a new API.
Maybe have some folks test using the new API for all calls and after
some sanity of testing / releases consider a full switch.
That is, unless of course you're sure all this is sane and would wager
all-in on it from the get-go.
Luis
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2015-06-24 23:40:03
On Wed, 2015-06-24 at 15:29 -0700, Luis R. Rodriguez wrote:
Nope but at least what made me squint at this being a possible
"feature" was that in practice when reviewing all of the kernels
pending device drivers using MTRR (potential write-combine candidates)
I encountered a slew of them which had the architectural unfortunate
practice of combining PCI bars for MMIO and their respective
write-combined desirable area (framebuffer for video, PIO buffers for
infiniband, etc). Now, to me that read more as a practice for old
school devices when such things were likely still being evaluated,
more modern devices seem to adhere to sticking a full PCI bar with
write-combining or not. Did you not encounter such mismatch splits on
powerpc ? Was such possibility addressed?
Yes, I remember we dealt with some networking (or maybe IB) stuff back
in the day. We dealt with it by using a WC mapping and explicit barriers
to prevent combine when not wanted.
It is to be noted that on powerpc at least, writel() and co will never
combine due to the memory barriers in them. Only "normal" stores (or
__raw_writel) will.
On Intel things I different I assume...
The problem I see is that architectures can provide widely different
mechanisms and semantics in those areas and it's hard to define a
generic driver interface.
If what you are implying here is applicable to the x86 world I'm all
for enabling this as we'd have less code to maintain but I'll note
that getting a clarification alone on that prefetchable !> write-combining was in and of itself hard, I'd be surprised if we
could get full architectural buy-in to this as an immediate automatic
feature.
I'm happy not to make it automatic for kernel space. As for user
mappings, maybe the right thing to do is to let us do what we do by
default with a quirk that can set a flag in pci_dev to disable that
behaviour (maybe on a per BAR basis ?). I think the common case is
that WC works.
Because of this and because PAT did have some errata as well,
I would not be surprised if some PCI bridges / devices would end up
finding corner cases, as such if we can really do what you're saying
and unless we can get some super sane certainty over it across the
board, I'd be inclined to leave such things as a part of a new API.
Maybe have some folks test using the new API for all calls and after
some sanity of testing / releases consider a full switch.
That is, unless of course you're sure all this is sane and would wager
all-in on it from the get-go.
From: Luis R. Rodriguez <hidden> Date: 2015-06-25 00:08:18
On Thu, Jun 25, 2015 at 09:38:01AM +1000, Benjamin Herrenschmidt wrote:
On Wed, 2015-06-24 at 15:29 -0700, Luis R. Rodriguez wrote:
quoted
Nope but at least what made me squint at this being a possible
"feature" was that in practice when reviewing all of the kernels
pending device drivers using MTRR (potential write-combine candidates)
I encountered a slew of them which had the architectural unfortunate
practice of combining PCI bars for MMIO and their respective
write-combined desirable area (framebuffer for video, PIO buffers for
infiniband, etc). Now, to me that read more as a practice for old
school devices when such things were likely still being evaluated,
more modern devices seem to adhere to sticking a full PCI bar with
write-combining or not. Did you not encounter such mismatch splits on
powerpc ? Was such possibility addressed?
Yes, I remember we dealt with some networking (or maybe IB) stuff back
in the day. We dealt with it by using a WC mapping and explicit barriers
to prevent combine when not wanted.
It is to be noted that on powerpc at least, writel() and co will never
combine due to the memory barriers in them. Only "normal" stores (or
__raw_writel) will.
On Intel things I different I assume...
And the people who really know seem to be eaten by volcanoes or not have time.
The problem I see is that architectures can provide widely different
mechanisms and semantics in those areas and it's hard to define a
generic driver interface.
Provided asm generic helpers are defined this should work though. The question
is just if there is enough motivation. Doesn't sound like it or as you note
maybe for userspace there might be. My position is that if it was too late for
PCIE or if this was too ambigious for PCIE perhaps the next generation bus
archicture or ammendments (I have no clue if this would would be possible) will
make this part of future device negotiation clear and fully expected, not a
wonderful side effect.
quoted
If what you are implying here is applicable to the x86 world I'm all
for enabling this as we'd have less code to maintain but I'll note
that getting a clarification alone on that prefetchable !> > write-combining was in and of itself hard, I'd be surprised if we
could get full architectural buy-in to this as an immediate automatic
feature.
I'm happy not to make it automatic for kernel space.
OK thanks I'll proceed with these patches then.
As for user mappings,
Which APIs were you considering in this regard BTW?
maybe the right thing to do is to let us do what we do by
default with a quirk that can set a flag in pci_dev to disable that
behaviour (maybe on a per BAR basis ?).
That might mean it could restrict userspace WC to require devices
to have WC parts on a full PCI BAR. Although this is restrictive
having reviewed most WC uses in the kernel I'd think this would be
a fair compromise to make, but again, if things are still murky
perhaps best we kiss this idea good bye for now and hope for it
to come in on future buses or ammendments (if that's even possible?).
I think the common case is that WC works.
If WC does not I will note one hack which migh be worth mentioning -- just for
the record, this was devised as a shortcoming of a device where they failed to
split things properly and that *without* WC performance suffered quite a bit so
they made one full PCI BAR WC and as a work around this:
http://lkml.kernel.org/r/20150416041837.GA5712@hykim-PC
That is for registers that needed it:
write; wmb;
Then if they wanted to wait till the NIC has seen the write, they did:
write; wmb; read;
Luis
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2015-06-25 00:53:43
On Thu, 2015-06-25 at 02:08 +0200, Luis R. Rodriguez wrote:
OK thanks I'll proceed with these patches then.
quoted
As for user mappings,
Which APIs were you considering in this regard BTW?
mmap of the generic /sys/bus/pci/.../resource*
quoted
maybe the right thing to do is to let us do what we do by
default with a quirk that can set a flag in pci_dev to disable that
behaviour (maybe on a per BAR basis ?).
That might mean it could restrict userspace WC to require devices
to have WC parts on a full PCI BAR. Although this is restrictive
having reviewed most WC uses in the kernel I'd think this would be
a fair compromise to make, but again, if things are still murky
perhaps best we kiss this idea good bye for now and hope for it
to come in on future buses or ammendments (if that's even possible?).
quoted
I think the common case is that WC works.
If WC does not I will note one hack which migh be worth mentioning --
just for
the record, this was devised as a shortcoming of a device where they
failed to
split things properly and that *without* WC performance suffered quite
a bit so
they made one full PCI BAR WC and as a work around this:
http://lkml.kernel.org/r/20150416041837.GA5712@hykim-PC
That is for registers that needed it:
write; wmb;
Then if they wanted to wait till the NIC has seen the write, they did:
write; wmb; read;
Right, and as I mentioned, on some archs like powerpc (and possibly
more), writel() and co contains an implicit mb()
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2015-06-25 01:01:57
On Wed, 2015-06-24 at 18:38 +0200, Luis R. Rodriguez wrote:
On Wed, Jun 24, 2015 at 08:42:23AM +1000, Benjamin Herrenschmidt wrote:
quoted
On Fri, 2015-06-19 at 15:08 -0700, Luis R. Rodriguez wrote:
quoted
From: "Luis R. Rodriguez" <redacted>
PCI BARs tell us whether prefetching is safe, but they don't say anything
about write combining (WC). WC changes ordering rules and allows writes to
be collapsed, so it's not safe in general to use it on a prefetchable
region.
Well, the PCIe spec at least specifies that a prefetchable BAR also
tolerates write merging...
How can that be determined and can that be used as a full bullet proof hint
to enable wc ? And are you sure? :)
Well, I"m sure the spec says that ;-) But it could be new to PCIe, I
haven't checked legacy PCI.
Reason all this was stated was to be
apologetic over why we can't automate this behind the scenes. Otherwise
we could amend what you stated into the commit log to elaborate on our
technical apology. Let me know!
At least on powerpc, for mmap of resource to userspace, we take off the
garded bit in the PTE for prefetchable BARs. This has the effect
architecturally of enabling both prefetch and write combine (ie. side
effect) though afaik, the implementations probably don't actually
prefetch. We've done that for years.
In fact we don't have a way to split the notions, it's either G or no G,
which carries both meanings.
Do you have example/case of a device having problems ?
Cheers,
Ben.
/ From: Benjamin Herrenschmidt [benh@kernel.crashing.org]
| Sent: Wednesday, June 24, 2015 4:38 PM
|
| It is to be noted that on powerpc at least, writel() and co will never
| combine due to the memory barriers in them. Only "normal" stores (or
\ __raw_writel) will.
/ From: Benjamin Herrenschmidt [benh@kernel.crashing.org]
| Sent: Wednesday, June 24, 2015 5:52 PM
|
| Right, and as I mentioned, on some archs like powerpc (and possibly
\ more), writel() and co contains an implicit mb()
Hhmmm, interesting. I definitely hadn't known that. In our network
drivers we explicitly manage all of our own memory barrier semantics.
(wmb() between writes to DMA Queues and writes to Device Registers
informing hardware of new data available to be DMA'ed, etc.) And we
do have code to take advantage of Write Combining Mappings using
writeq(). It looks like this is implemented eventually as out_be64()
in arch/powerpc/include/asm/io.h for PPC-BE:
#define DEF_MMIO_OUT_D(name, size, insn) \
static inline void name(volatile u##size __iomem *addr, u##size val) \
{ \
__asm__ __volatile__("sync;"#insn"%U0%X0 %1,%0" \
: "=m" (*addr) : "r" (val) : "memory"); \
IO_SET_SYNC_FLAG(); \
}
DEF_MMIO_OUT_D(out_be64, 64, std);
So, if understand your notes and the source correctly, all of
our code to do register reads/writes are getting SYNC instructions,
as are the 64-bit writeq()s we're attempting to use for our Write
Combining code on PowerPC. Hhmmm indeed ...
Is there a reference I can read on this so I can understand
when and where we can use the __raw_*() APIs? Can these
Raw Read/Write operations be reordered with respect to
each other or are the use of the various flavors of SYNC
instructions just to maintain order between Cached Memory
Accesses and I/O Instructions?
Casey
On Thursday 25 June 2015 15:01:56 Casey Leedom wrote:
Is there a reference I can read on this so I can understand
when and where we can use the __raw_*() APIs? Can these
Raw Read/Write operations be reordered with respect to
each other or are the use of the various flavors of SYNC
instructions just to maintain order between Cached Memory
Accesses and I/O Instructions?
The interpretation is not consistent across architectures.
My best description would be that the __raw_*() accessors should
only be used for accessing RAM areas that are known to have no
side-effects and can be read in any size (8-bit to 64-bit wide),
any alignment, and do not have a specific endianess.
If you are dealing with MMIO registers that have a fixed endianess
and size, the correct accessor would be readl_relaxed(), which
is like readl() but lacks the barriers on certain architectures.
Arnd
| From: Arnd Bergmann [arnd@arndb.de]
| Sent: Thursday, June 25, 2015 1:44 PM
|
| On Thursday 25 June 2015 15:01:56 Casey Leedom wrote:
| >
| > Is there a reference I can read on this so I can understand
| > when and where we can use the __raw_*() APIs? Can these
| > Raw Read/Write operations be reordered with respect to
| > each other or are the use of the various flavors of SYNC
| > instructions just to maintain order between Cached Memory
| > Accesses and I/O Instructions?
|
| The interpretation is not consistent across architectures.
|
| My best description would be that the __raw_*() accessors should
| only be used for accessing RAM areas that are known to have no
| side-effects and can be read in any size (8-bit to 64-bit wide),
| any alignment, and do not have a specific endianess.
|
| If you are dealing with MMIO registers that have a fixed endianess
| and size, the correct accessor would be readl_relaxed(), which
| is like readl() but lacks the barriers on certain architectures.
Ah, thanks. I see now that the __raw_*() APIs don't do any of the
Endian Swizzling. Unfortunately the *_relaxed() APIs on PowerPC
are just defined as the normal *() routines. From
arch/powerpc/include/asm/io.h:
/*
* We don't do relaxed operations yet, at least not with this semantic
*/
#define readb_relaxed(addr) readb(addr)
#define readw_relaxed(addr) readw(addr)
#define readl_relaxed(addr) readl(addr)
#define readq_relaxed(addr) readq(addr)
#define writeb_relaxed(v, addr) writeb(v, addr)
#define writew_relaxed(v, addr) writew(v, addr)
#define writel_relaxed(v, addr) writel(v, addr)
#define writeq_relaxed(v, addr) writeq(v, addr)
(And in fact, this is true for most of the architectures.)
So we could go ahead and use these but for now there wouldn't be
any effect.
Hhmmm, so what do PowerPC Drivers do when they want to take
advantage of Write Combining? Do their own Endian Swizzling
with the __raw_*() APIs?
Casey
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2015-06-26 02:46:34
On Thu, 2015-06-25 at 21:40 +0000, Casey Leedom wrote:
Ah, thanks. I see now that the __raw_*() APIs don't do any of the
Endian Swizzling. Unfortunately the *_relaxed() APIs on PowerPC
are just defined as the normal *() routines. From
arch/powerpc/include/asm/io.h:
/*
* We don't do relaxed operations yet, at least not with this
semantic
*/
Yes so I was looking at this but there are some difficulties.
Architecturally, even with I=1 G=1 mappings (normal ioremap), we have no
guarantee of ordering of load vs. store unless I misunderstood
something. I think all current implementations provide some of that but
without barriers in the accessors, we aren't architecturally correct.
However, having those barriers will cause issues with G=0 (write
combine). It's unclear whether eieio() will provide the required
ordering for I=1 G=0 mappings and it will probably break write combine.
I'm looking into it with our HW guys and will try to come up with a
solution for power, but it doesn't help that our memory model conflates
write combining with other relaxations and that all our barriers also
prevent write combine.
Maybe we can bias the relaxed accessors toward write, by having no
barriers in it, and putting extra ones in reads.
Cheers,
Ben.
Thanks for looking into this Ben. As it stands now, it seems as
if Write Combined mappings simply aren't supported and/or all
driver writers trying to utilize Write Combined mappings have to
"hand roll" their own solutions which really isn't supportable.
One thing that might be considered is simply to treat the desire
to utilize the Write Combining hardware as a separate issue and
develop writel_wc(), writeq_wc(), etc. Those could be defined
as legal only for Write Combined Mappings and would "do the
right thing" for each architecture. The initial implementations
could simply be writel(), writeq(), etc. till each architecture'si
ssues were investigated and understood.
Additionally, it would be very useful to have some sort of
predicate which could be used to determine if an architecture
supported Write Combining. Our drivers would use this to
eliminate a wasteful attempt to use write combining on those
architectures where it didn't work.
Casey
________________________________________
From: Benjamin Herrenschmidt [benh@kernel.crashing.org]
Sent: Thursday, June 25, 2015 7:02 PM
To: Casey Leedom
Cc: Arnd Bergmann; Luis R. Rodriguez; Michael S. Tsirkin; Bjorn Helgaas; Toshi Kani; Andy Lutomirski; Juergen Gross; Tomi Valkeinen; linux-pci@vger.kernel.org; linux-kernel@vger.kernel.org; xen-devel@lists.xensource.com; linux-fbdev; Suresh Siddha; Ingo Molnar; Thomas Gleixner; Daniel Vetter; Dave Airlie; Antonino Daplas; Jean-Christophe Plagniol-Villard; Dave Hansen; venkatesh.pallipadi@intel.com; Stefan Bader; ville.syrjala@linux.intel.com; David Vrabel; Jan Beulich; Roger Pau Monné
Subject: Re: [PATCH v7 5/9] PCI: Add pci_iomap_wc() variants
On Thu, 2015-06-25 at 21:40 +0000, Casey Leedom wrote:
Ah, thanks. I see now that the __raw_*() APIs don't do any of the
Endian Swizzling. Unfortunately the *_relaxed() APIs on PowerPC
are just defined as the normal *() routines. From
arch/powerpc/include/asm/io.h:
/*
* We don't do relaxed operations yet, at least not with this
semantic
*/
Yes so I was looking at this but there are some difficulties.
Architecturally, even with I=1 G=1 mappings (normal ioremap), we have no
guarantee of ordering of load vs. store unless I misunderstood
something. I think all current implementations provide some of that but
without barriers in the accessors, we aren't architecturally correct.
However, having those barriers will cause issues with G=0 (write
combine). It's unclear whether eieio() will provide the required
ordering for I=1 G=0 mappings and it will probably break write combine.
I'm looking into it with our HW guys and will try to come up with a
solution for power, but it doesn't help that our memory model conflates
write combining with other relaxations and that all our barriers also
prevent write combine.
Maybe we can bias the relaxed accessors toward write, by having no
barriers in it, and putting extra ones in reads.
Cheers,
Ben.
From: Luis R. Rodriguez <hidden> Date: 2015-06-26 19:32:43
On Fri, Jun 26, 2015 at 08:51:58AM +1000, Benjamin Herrenschmidt wrote:
On Thu, 2015-06-25 at 21:40 +0000, Casey Leedom wrote:
quoted
Hhmmm, so what do PowerPC Drivers do when they want to take
advantage of Write Combining? Do their own Endian Swizzling
with the __raw_*() APIs?
Yeah either, we need to fix our relaxed implementation (patch
welcome :-)
Yikes, so although powerpc has useful heuristics to automatically
enable WC the default write ops have been nullifying its effects?
Shouldn't this have been blatantly obvious in performance benchmarks
and expectations? If not a change should give considerable performance
gains... If the WC was nullified on powerpc then the experience and
value of the heuristics have less value and even if they are going
to be only considered for userspace mmap() it should be taken with
a bit grain of salt it seems.
Luis
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2015-06-26 22:02:25
On Fri, 2015-06-26 at 16:24 +0000, Casey Leedom wrote:
Thanks for looking into this Ben. As it stands now, it seems as
if Write Combined mappings simply aren't supported and/or all
driver writers trying to utilize Write Combined mappings have to
"hand roll" their own solutions which really isn't supportable.
One thing that might be considered is simply to treat the desire
to utilize the Write Combining hardware as a separate issue and
develop writel_wc(), writeq_wc(), etc. Those could be defined
as legal only for Write Combined Mappings and would "do the
right thing" for each architecture.
The question then is what is "the right thing". In the powerpc case,
we'll have a non-garded mapping, which means we also get no ordering
between load and stores.
Ah, thanks. I see now that the __raw_*() APIs don't do any of the
Endian Swizzling. Unfortunately the *_relaxed() APIs on PowerPC
are just defined as the normal *() routines. From
arch/powerpc/include/asm/io.h:
/*
* We don't do relaxed operations yet, at least not with this
semantic
*/
Yes so I was looking at this but there are some difficulties.
Architecturally, even with I=1 G=1 mappings (normal ioremap), we have no
guarantee of ordering of load vs. store unless I misunderstood
something. I think all current implementations provide some of that but
without barriers in the accessors, we aren't architecturally correct.
However, having those barriers will cause issues with G=0 (write
combine). It's unclear whether eieio() will provide the required
ordering for I=1 G=0 mappings and it will probably break write combine.
I'm looking into it with our HW guys and will try to come up with a
solution for power, but it doesn't help that our memory model conflates
write combining with other relaxations and that all our barriers also
prevent write combine.
Maybe we can bias the relaxed accessors toward write, by having no
barriers in it, and putting extra ones in reads.
Cheers,
Ben.
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2015-06-26 22:05:43
On Fri, 2015-06-26 at 21:31 +0200, Luis R. Rodriguez wrote:
quoted
Yeah either, we need to fix our relaxed implementation (patch
welcome :-)
Yikes, so although powerpc has useful heuristics to automatically
enable WC the default write ops have been nullifying its effects?
The heuristic is for userspace mapping which don't use the kernel
accessors. To cut a long story short, this was all done back in the day
to speed up X.org which doesn't use fancy accessors with barriers to
access the framebuffer (neither does the kernel fbdev layer).
Shouldn't this have been blatantly obvious in performance benchmarks
and expectations?
If not a change should give considerable performance
gains... If the WC was nullified on powerpc then the experience and
value of the heuristics have less value and even if they are going
to be only considered for userspace mmap() it should be taken with
a bit grain of salt it seems.
It wasn't nullified for the main user at the time, the fb. And I
mentioned an IB adapter or two for which the code had been hand tuned.
Cheers,
Ben.
From: Luis R. Rodriguez <hidden> Date: 2015-07-02 18:49:17
On Sat, Jun 27, 2015 at 08:00:48AM +1000, Benjamin Herrenschmidt wrote:
On Fri, 2015-06-26 at 16:24 +0000, Casey Leedom wrote:
quoted
Thanks for looking into this Ben. As it stands now, it seems as
if Write Combined mappings simply aren't supported and/or all
driver writers trying to utilize Write Combined mappings have to
"hand roll" their own solutions which really isn't supportable.
One thing that might be considered is simply to treat the desire
to utilize the Write Combining hardware as a separate issue and
develop writel_wc(), writeq_wc(), etc.
That seems rather sloppy and cumbersome, its best to just provide the
infrastructure for initial mapping for an area and let the hardware do it for
you. With the current design drivers would just use regular read/write on all
areas and the only thing that will set them apart will be the mapping. With
what you propose we'd end up having to shift a whole bunch of reads/writes for
just the purpose of adding WC to an area that didn't have wc before. That's
a waste of code and time.
quoted
Those could be defined
as legal only for Write Combined Mappings and would "do the
right thing" for each architecture.
Yuck.
The question then is what is "the right thing". In the powerpc case,
we'll have a non-garded mapping, which means we also get no ordering
between load and stores.
I don't follow, you *ordering* between load and stores for WC? We should
not need that for WC, its why WC is used for only very specific things
such as framebuffer and PIO (which BTw I still don't quite get all this
use case for infiniband to be honest, and I will note I do see some
proprietary hardware extensions like bursts but nothing covering all
this in a general doc, I think I think it all just has to do that this
is a hardware hack in reality, which we sell as a feature).
Luis
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2015-07-02 22:28:14
On Thu, 2015-07-02 at 20:49 +0200, Luis R. Rodriguez wrote:
quoted
The question then is what is "the right thing". In the powerpc case,
we'll have a non-garded mapping, which means we also get no ordering
between load and stores.
I don't follow, you *ordering* between load and stores for WC? We should
not need that for WC, its why WC is used for only very specific things
such as framebuffer and PIO (which BTw I still don't quite get all this
use case for infiniband to be honest, and I will note I do see some
proprietary hardware extensions like bursts but nothing covering all
this in a general doc, I think I think it all just has to do that this
is a hardware hack in reality, which we sell as a feature).
Well, that's the problem, the semantics that we provide to drivers
aren't well defined.
The words "write combine" themselves only specify that writes to subsequent
addresses can be combined into larger transactions. That's in itself is already
quite vague (are their boundaries, limits ? some can depend on bus type, etc...)
though in practice is probably sufficient.
However, overloading a _wc mapping with additional memory model differences
such as loss of ordering between load and stores, etc... is not an obvious
thing to do. I agree it would make *my* life easier if we did it since this
is precisely the semantics provided by a "G=0" mapping on ppc, but we need
to agree and *document* it, otherwise bad things will happen eventually.
We also need to document in that case which barriers can be used to explicitly
enforce the ordering on such a mapping and which barriers can be used to break
write combine (they don't necessarily have to be the same).
We also need to document which accessors will actually provide the write combine
"feature" of a _wc mapping. For example while writel() will do it on Intel, it
will not on ppc and I wouldn't be surprised if a bunch of other archs fall in
the same bucket as ppc (basically anything that has barriers in their writel
implementation to order vs. DMA etc...).
So we might need to explicitly document that writel_relaxed() needs to be used.
Finally what are the precise guaranteed semantics of writeX/readX,
writeX_relaxed/readX_relaxed and __raw_ (everything else) on a _wc mapping, what
do we mandate and document, what do we leave to be implementation dependent ?
Cheers,
Ben.
On Jul 2, 2015, at 11:49 AM, Luis R. Rodriguez [off-list ref] wrote:
On Sat, Jun 27, 2015 at 08:00:48AM +1000, Benjamin Herrenschmidt wrote:
quoted
On Fri, 2015-06-26 at 16:24 +0000, Casey Leedom wrote:
quoted
Thanks for looking into this Ben. As it stands now, it seems as
if Write Combined mappings simply aren't supported and/or all
driver writers trying to utilize Write Combined mappings have to
"hand roll" their own solutions which really isn't supportable.
One thing that might be considered is simply to treat the desire
to utilize the Write Combining hardware as a separate issue and
develop writel_wc(), writeq_wc(), etc.
That seems rather sloppy and cumbersome, its best to just provide the
infrastructure for initial mapping for an area and let the hardware do it for
you. With the current design drivers would just use regular read/write on all
areas and the only thing that will set them apart will be the mapping. With
what you propose we'd end up having to shift a whole bunch of reads/writes for
just the purpose of adding WC to an area that didn't have wc before. That's
a waste of code and time.
quoted
quoted
Those could be defined
as legal only for Write Combined Mappings and would "do the
right thing" for each architecture.
Yuck.
Yeah, probably. But it sounds like from Ben’s response, we should really use write_relaxed() as an already existing API for this. But most of the architectures just define this as writel() so some work would need to be done to get it to work.
quoted
The question then is what is "the right thing". In the powerpc case,
we'll have a non-garded mapping, which means we also get no ordering
between load and stores.
I don't follow, you *ordering* between load and stores for WC? We should
not need that for WC, its why WC is used for only very specific things
such as framebuffer and PIO (which BTw I still don't quite get all this
use case for infiniband to be honest, and I will note I do see some
proprietary hardware extensions like bursts but nothing covering all
this in a general doc, I think I think it all just has to do that this
is a hardware hack in reality, which we sell as a feature).
I was talking about the work that our drivers (cxgb4, cxgb4vf, etc.) do to ensure correct ordering between writes to memory and I/O space. For instance, issuing a wmb() between writes to a DMA buffer and the write to a register telling the hardware that the data is available. This isn’t necessary on the Strongly Ordered Intel architectures but is necessary on other architectures.
Casey