Re: Framebuffer mmap on PowerPC

Le 31/08/2023 à 16:41, Thomas Zimmermann a écrit :

quoted

Hi,

there's a per-architecture function called fb_pgprotect() that sets 
VMA's vm_page_prot for mmaped framebuffers. Most architectures use a 
simple implementation based on pgprot_writecomine() [1] or 
pgprot_noncached(). [2]

On PPC this function uses phys_mem_access_prot() and therefore 
requires the mmap call's file struct. [3] Removing the file argument 
would help with simplifying the caller of fb_pgprotect(). [4]

Why is the file even required on PPC?

Is it possible to replace phys_mem_access_prot() with something 
simpler that does not use the file struct?

Looks like phys_mem_access_prot() defaults to calling pgprot_noncached() 
see 
https://elixir.bootlin.com/linux/v6.5/source/arch/powerpc/mm/mem.c#L37 
but for a few platforms that's superseeded by 
pci_phys_mem_access_prot(), see 
https://elixir.bootlin.com/linux/v6.5/source/arch/powerpc/kernel/pci-common.c#L524

However, as far as I can see pci_phys_mem_access_prot() doesn't use file 
so you could likely drop that argument on phys_mem_access_prot() on 
powerpc. But when I for instance look at arm, I see that the file 
argument is used, see 
https://elixir.bootlin.com/linux/v6.5/source/arch/arm/mm/mmu.c#L713

So, the simplest is maybe the following, allthough that's probably worth 
a comment:

diff --git a/arch/powerpc/include/asm/fb.h b/arch/powerpc/include/asm/fb.h
index 5f1a2e5f7654..8b9b856f476e 100644
--- a/arch/powerpc/include/asm/fb.h
+++ b/arch/powerpc/include/asm/fb.h

@@ -6,10 +6,9 @@

  #include <asm/page.h>

-static inline void fb_pgprotect(struct file *file, struct 

vm_area_struct *vma,
-                unsigned long off)
+static inline void fb_pgprotect(struct vm_area_struct *vma, unsigned 
long off)
  {
-    vma->vm_page_prot = phys_mem_access_prot(file, off >> PAGE_SHIFT,
+    vma->vm_page_prot = phys_mem_access_prot(NULL, off >> PAGE_SHIFT,
                           vma->vm_end - vma->vm_start,
                           vma->vm_page_prot);
  }