ppc32/64: Map prefetchable PCI without guarded bit
Subsystems (3)
PATCH 1/1 ppc32/64: Map prefetchable PCI without guarded bit
Hi !
While experimenting with framebuffer access performances, we noticed a very significant improvement in write access to it when not setting the "guarded" bit on the MMU mappings. This bit basically says that reads and writes won't have side effects (it allows speculation). It appears that it also disables write combining.
This patch implements a new phys_mem_access_prot() arch callback for use by /dev/mem and fbdev when available, implements it for ppc32 and ppc64, and modifies /dev/mem and fbdev to use it, respectively when available or on ppc. I didn't change fbdev to use it on all archs when available because there is already a whole lot of arch specific mess in there (more than in /dev/mem !) that I didn't feel like messing with, but archs maintainers are welcome to give it a go).
The old mecanism in /dev/mem is still there, but arch maintainers should probably switch to this once which is more consistent imho.
Finally, the ppc32 and ppc64 implementation of this and of the PCI mmap calls (used by /proc and /sys) are modified to check if the mapping happens in a prefetchable PCI resource, in which case, the guarded bit is not set for the pgprot. In fact, ppc32 implementation of this code is updated to be identical to ppc64.
This improves framebuffer write performance on a simple test paul wrote from about 50Mb/sec to 200Mb/sec on my M9 based laptop and on a G5. The new hook will automatically catch up Xfree mmap's from /dev/mem, so it will work out of the box with existing X servers. Kernel fbdev accesses aren't improved yet as ioremap doesn't use that mecanism.
drivers/char/mem.c +7 -9
@@ -76,14 +76,6 @@* On ia64, we ignore O_SYNC because we cannot tolerate memory attribute aliases. */ return !(efi_mem_attributes (addr) &EFI_MEMORY_WB ); -#elif defined(CONFIG_PPC64 ) -/* On PPC64, we always do non-cacheable access to the IO hole and -* cacheable elsewhere. Cache paradox can checkstop the CPU and -* the high_memory heuristic below is wrong on machines with memory -* above the IO hole... Ah, and of course, XFree86 doesn't pass -* O_SYNC when mapping us to tap IO space. Surprised ? -*/ -return !page_is_ram (addr >>PAGE_SHIFT );#else /* * Accessing memory above the top the kernel knows about or through a file pointer
@@ -238,7 +230,13 @@static int mmap_mem (struct file * file,struct vm_area_struct * vma) { -#ifdef pgprot_noncached +#if defined(__HAVE_PHYS_MEM_ACCESS_PROT) +unsigned long offset = vma->vm_pgoff <<PAGE_SHIFT ; + + vma->vm_page_prot =phys_mem_access_prot (file, offset, + vma->vm_end - vma->vm_start, + vma->vm_page_prot); +#elif defined(pgprot_noncached)unsigned long offset = vma->vm_pgoff <<PAGE_SHIFT ;int uncached;
include/asm-ppc64/machdep.h +7 -0
@@ -21,6 +21,7 @@struct device_node ;struct iommu_table ;struct rtc_time ; +struct file ;#ifdef CONFIG_SMP struct smp_ops_t {
@@ -131,6 +132,12 @@/* Get legacy PCI/IDE interrupt mapping */ int (*pci_get_legacy_ide_irq)(struct pci_dev *dev,int channel); +/* Get access protection for /dev/mem */ +pgprot_t (*phys_mem_access_prot)(struct file *file, +unsigned long offset, +unsigned long size, +pgprot_t vma_prot); + };extern struct machdep_calls ppc_md;
drivers/video/fbmem.c +3 -1
@@ -957,7 +957,9 @@ }#endif #elif defined(__powerpc__) -pgprot_val (vma->vm_page_prot) |= _PAGE_NO_CACHE|_PAGE_GUARDED; + vma->vm_page_prot =phys_mem_access_prot (file, off, + vma->vm_end - vma->vm_start, + vma->vm_page_prot);#elif defined(__alpha__)/* Caching is off in the I/O space quadrant by design. */ #elif defined(__i386__) || defined(__x86_64__)
include/asm-ppc64/pgtable.h +5 -0
@@ -472,6 +472,11 @@*/ #define pgprot_noncached (prot) (__pgprot(pgprot_val(prot) | _PAGE_NO_CACHE | _PAGE_GUARDED)) +struct file ; +extern pgprot_t phys_mem_access_prot (struct file *file,unsigned long addr, +unsigned long size,pgprot_t vma_prot); +#define __HAVE_PHYS_MEM_ACCESS_PROT +#define __HAVE_ARCH_PTE_SAME#define pte_same (A ,B ) (((pte_val(A) ^ pte_val(B)) & ~_PAGE_HPTEFLAGS) == 0)
arch/ppc64/kernel/pci.c +102 -37
197 lines not on this page · raw message
arch/ppc64/mm/init.c +13 -0
@@ -912,3 +912,16 @@if (!zero_cache)panic ("pgtable_cache_init(): could not create zero_cache!\n" ); } + +pgprot_t phys_mem_access_prot (struct file *file,unsigned long addr, +unsigned long size,pgprot_t vma_prot) +{ +if (ppc_md.phys_mem_access_prot) +return ppc_md.phys_mem_access_prot (file, addr, size, vma_prot); + +if (!page_is_ram (addr >>PAGE_SHIFT )) + vma_prot =__pgprot (pgprot_val (vma_prot) + | _PAGE_GUARDED | _PAGE_NO_CACHE); +return vma_prot; +} +EXPORT_SYMBOL (phys_mem_access_prot);
include/asm-ppc64/pci.h +7 -0
@@ -130,6 +130,13 @@extern void pcibios_add_platform_entries (struct pci_dev *dev); +struct file ; +extern pgprot_t pci_phys_mem_access_prot (struct file *file, +unsigned long offset, +unsigned long size, +pgprot_t prot); + +#endif /* __KERNEL__ */ #endif /* __PPC64_PCI_H */
include/asm-ppc/pci.h +6 -0
@@ -97,6 +97,12 @@extern void pcibios_add_platform_entries (struct pci_dev *dev); +struct file ; +extern pgprot_t pci_phys_mem_access_prot (struct file *file, +unsigned long offset, +unsigned long size, +pgprot_t prot); +#endif /* __KERNEL__ */ #endif /* __PPC_PCI_H */
arch/ppc/kernel/pci.c +127 -74
238 lines not on this page · raw message
include/asm-ppc/pgtable.h +5 -0
@@ -623,6 +623,11 @@*/ #define pgprot_noncached (prot) (__pgprot(pgprot_val(prot) | _PAGE_NO_CACHE | _PAGE_GUARDED)) +struct file ; +extern pgprot_t phys_mem_access_prot (struct file *file,unsigned long addr, +unsigned long size,pgprot_t vma_prot); +#define __HAVE_PHYS_MEM_ACCESS_PROT +#define __HAVE_ARCH_PTE_SAME#define pte_same (A ,B ) (((pte_val(A) ^ pte_val(B)) & ~_PAGE_HASHPTE) == 0)
arch/ppc/platforms/chrp_setup.c +2 -0
@@ -527,6 +527,8 @@ ppc_md.init = chrp_init2; + ppc_md.phys_mem_access_prot = pci_phys_mem_access_prot; + ppc_md.restart = chrp_restart; ppc_md.power_off = chrp_power_off; ppc_md.halt = chrp_halt;
include/asm-ppc/page.h +2 -0
@@ -137,6 +137,8 @@#define ___va (paddr) ((paddr)+PPC_MEMOFFSET)#endif +extern int page_is_ram (unsigned long pfn); +#define __pa (x) ___pa((unsigned long)(x))#define __va (x) ((void *)(___va((unsigned long)(x))))
include/asm-ppc/machdep.h +8 -0
@@ -6,6 +6,7 @@#include <linux/init.h> #include <asm/setup.h> +#include <asm/page.h> #ifdef CONFIG_APUS #include <asm-m68k/machdep.h>
@@ -15,6 +16,7 @@struct pci_bus ;struct pci_dev ;struct seq_file ; +struct file ;/* We export this macro for external modules like Alsa to know if * ppc_md.feature_call is implemented or not
@@ -93,6 +95,12 @@/* Called at then very end of pcibios_init() */ void (*pcibios_after_init)(void ); +/* Get access protection for /dev/mem */ +pgprot_t (*phys_mem_access_prot)(struct file *file, +unsigned long offset, +unsigned long size, +pgprot_t vma_prot); +/* this is for modules, since _machine can be a define -- Cort */ int ppc_machine;
arch/ppc/platforms/prep_setup.c +2 -0
@@ -1144,6 +1144,8 @@/* this gets changed later on if we have an OpenPIC -- Cort */ ppc_md.get_irq = i8259_irq; + ppc_md.phys_mem_access_prot = pci_phys_mem_access_prot; + ppc_md.restart = prep_restart; ppc_md.power_off =NULL ;/* set in prep_setup_arch() */ ppc_md.halt = prep_halt;
arch/ppc/platforms/pmac_setup.c +1 -0
@@ -669,6 +669,7 @@ ppc_md.pcibios_fixup = pmac_pcibios_fixup; ppc_md.pcibios_enable_device_hook = pmac_pci_enable_device_hook; ppc_md.pcibios_after_init = pmac_pcibios_after_init; + ppc_md.phys_mem_access_prot = pci_phys_mem_access_prot; ppc_md.restart = pmac_restart; ppc_md.power_off = pmac_power_off;
arch/ppc/mm/init.c +24 -0
@@ -641,3 +641,27 @@ }#endif } + +/* +* This is called by /dev/mem to know if a given address has to +* be mapped non-cacheable or not +*/ +int page_is_ram (unsigned long pfn) +{ +unsigned long paddr = (pfn <<PAGE_SHIFT ); + +return paddr <__pa (high_memory); +} + +pgprot_t phys_mem_access_prot (struct file *file,unsigned long addr, +unsigned long size,pgprot_t vma_prot) +{ +if (ppc_md.phys_mem_access_prot) +return ppc_md.phys_mem_access_prot (file, addr, size, vma_prot); + +if (!page_is_ram (addr >>PAGE_SHIFT )) + vma_prot =__pgprot (pgprot_val (vma_prot) + | _PAGE_GUARDED | _PAGE_NO_CACHE); +return vma_prot; +} +EXPORT_SYMBOL (phys_mem_access_prot);
Quoted 5 lines · Benjamin Herrenschmidt
While experimenting with framebuffer access performances, we noticed a very significant improvement in write access to it when not setting the "guarded" bit on the MMU mappings. This bit basically says that reads and writes won't have side effects (it allows speculation). It appears that it also disables write combining.
Doesn't pgprot_writecombine imply non-guarded, so can't you use it instead? Either way, you'll probably want to fix fbmem.c as well and turn off _PAGE_GUARDED?
Maybe it's time for a more generic call to support this stuff, both for in-kernel mappings and ones that we export to userspace.
Jesse
Quoted 10 lines · Jesse Barnes
Quoted 5 lines · Benjamin Herrenschmidt
While experimenting with framebuffer access performances, we noticed a very significant improvement in write access to it when not setting the "guarded" bit on the MMU mappings. This bit basically says that reads and writes won't have side effects (it allows speculation). It appears that it also disables write combining.
Doesn't
pgprot_writecombineimply non-guarded, so can't you use it instead? Either way, you'll probably want to fixfbmem.cas well and turn off_PAGE_GUARDED?
Nevermind about this bit, I just scrolled a little further into your patch :)
Jesse
Quoted 10 lines · Jesse Barnes
Quoted 5 lines · Benjamin Herrenschmidt
While experimenting with framebuffer access performances, we noticed a very significant improvement in write access to it when not setting the "guarded" bit on the MMU mappings. This bit basically says that reads and writes won't have side effects (it allows speculation). It appears that it also disables write combining.
Doesn't
pgprot_writecombineimply non-guarded, so can't you use it instead? Either way, you'll probably want to fixfbmem.cas well and turn off_PAGE_GUARDED?
I'm not sure we implement pgprot_writecombine. But anyway, that wouldn't help as X uses /dev/mem which doesn't use that, and sysfs new mmap interface doesn't have anything for setting writecombine.
The interface I propose could be made generic, that is the whole point. It's basically a mean for the arch to say "heh, somebody wants to map this bit of physical address space, what pgprot should I use" :) The decision of wether to do uncacheable or writecombine, all the page_in_ram() trickery in /dev/mem or fbmem can be moved to arch specific routines and cleanup the generic stuff.
Quoted 2 lines
Maybe it's time for a more generic call to support this stuff, both for in-kernel mappings and ones that we export to userspace.
Yah, in-kernel mappings aren't covered yet, as my interface supposes a "struct file" but then, I don't use the struct file argument in my ppc implementation (I exposed it for the sake of archs that may want it). We could just define that in-kernel mappings can call this with NULL for struct file.
Ben.
Quoted 4 lines
While experimenting with framebuffer access performances, we noticed a very significant improvement in write access to it when not setting the "guarded" bit on the MMU mappings. This bit basically says that reads and writes won't have side effects (it allows speculation).
Unless the data is already in cache.
Quoted 1 line
It appears that it also disables write combining.
When the page is also cache-inhibited, it indeed does.
Btw, did you ever get to fix the problem with mapping the last page of physical address space via /dev/mem ?
Segher
Quoted 7 lines · Segher Boessenkool
Quoted 4 lines
While experimenting with framebuffer access performances, we noticed a very significant improvement in write access to it when not setting the "guarded" bit on the MMU mappings. This bit basically says that reads and writes won't have side effects (it allows speculation).
Unless the data is already in cache.
Quoted 7 lines
Quoted 1 line
It appears that it also disables write combining.
When the page is also cache-inhibited, it indeed does.
Btw, did you ever get to fix the problem with mapping the last page of physical address space via
/dev/mem?
I don't think so, but I'll have to double check.
Ben.