Hi Am 31.08.23 um 19:38 schrieb Christophe Leroy:
Le 31/08/2023 à 16:41, Thomas Zimmermann a écrit :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
Right, I've seen these various implementations. Luckily, the ARM framebuffers use a plain pgprot_writecombine() without any references on to file.
So, the simplest is maybe the following, allthough that's probably worth a comment:
Could we drop the file argument from PPC's internal functions and provide this interface to fb_pgprotect()? phys_mem_access_prot() would be a trivial wrapper around that internal API. I'd provide a patch to do that.
Best regards Thomas
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); } ChristopheBest regards Thomas [1] https://elixir.bootlin.com/linux/v6.5/source/include/asm-generic/fb.h#L19 [2] https://elixir.bootlin.com/linux/v6.5/source/arch/mips/include/asm/fb.h#L11 [3] https://elixir.bootlin.com/linux/v6.5/source/arch/powerpc/include/asm/fb.h#L12 [4] https://elixir.bootlin.com/linux/v6.5/source/drivers/video/fbdev/core/fbmem.c#L1299
-- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg)
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
