* Framebuffer mmap on PowerPC
@ 2023-08-31 14:41 ` Thomas Zimmermann
0 siblings, 0 replies; 10+ messages in thread
From: Thomas Zimmermann @ 2023-08-31 14:41 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Christophe Leroy,
Arnd Bergmann, Helge Deller
Cc: linuxppc-dev, Linux-Arch, Linux Fbdev development list, dri-devel
[-- Attachment #1.1: Type: text/plain, Size: 1185 bytes --]
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?
Best 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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Framebuffer mmap on PowerPC
@ 2023-08-31 14:41 ` Thomas Zimmermann
0 siblings, 0 replies; 10+ messages in thread
From: Thomas Zimmermann @ 2023-08-31 14:41 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Christophe Leroy,
Arnd Bergmann, Helge Deller
Cc: Linux-Arch, Linux Fbdev development list, linuxppc-dev, dri-devel
[-- Attachment #1.1: Type: text/plain, Size: 1185 bytes --]
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?
Best 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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Framebuffer mmap on PowerPC
2023-08-31 14:41 ` Thomas Zimmermann
@ 2023-08-31 17:38 ` Christophe Leroy
-1 siblings, 0 replies; 10+ messages in thread
From: Christophe Leroy @ 2023-08-31 17:38 UTC (permalink / raw)
To: Thomas Zimmermann, Michael Ellerman, Nicholas Piggin,
Arnd Bergmann, Helge Deller
Cc: linuxppc-dev, Linux-Arch, Linux Fbdev development list, dri-devel
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
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);
}
Christophe
>
> Best 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
>
>
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: Framebuffer mmap on PowerPC
@ 2023-08-31 17:38 ` Christophe Leroy
0 siblings, 0 replies; 10+ messages in thread
From: Christophe Leroy @ 2023-08-31 17:38 UTC (permalink / raw)
To: Thomas Zimmermann, Michael Ellerman, Nicholas Piggin,
Arnd Bergmann, Helge Deller
Cc: Linux-Arch, Linux Fbdev development list, linuxppc-dev, dri-devel
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
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);
}
Christophe
>
> Best 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
>
>
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: Framebuffer mmap on PowerPC
2023-08-31 17:38 ` Christophe Leroy
@ 2023-08-31 17:41 ` Christophe Leroy
-1 siblings, 0 replies; 10+ messages in thread
From: Christophe Leroy @ 2023-08-31 17:41 UTC (permalink / raw)
To: Thomas Zimmermann, Michael Ellerman, Nicholas Piggin,
Arnd Bergmann, Helge Deller
Cc: linuxppc-dev, Linux-Arch, Linux Fbdev development list, dri-devel
Le 31/08/2023 à 19:38, Christophe Leroy a écrit :
>
>
> 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
>
> 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);
> }
And while at it, maybe also replace off >> PAGE_SHIFT by PHYS_PFN(off)
Christophe
>
>
> Christophe
>
>
>>
>> Best 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
>>
>>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Framebuffer mmap on PowerPC
@ 2023-08-31 17:41 ` Christophe Leroy
0 siblings, 0 replies; 10+ messages in thread
From: Christophe Leroy @ 2023-08-31 17:41 UTC (permalink / raw)
To: Thomas Zimmermann, Michael Ellerman, Nicholas Piggin,
Arnd Bergmann, Helge Deller
Cc: Linux-Arch, Linux Fbdev development list, linuxppc-dev, dri-devel
Le 31/08/2023 à 19:38, Christophe Leroy a écrit :
>
>
> 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
>
> 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);
> }
And while at it, maybe also replace off >> PAGE_SHIFT by PHYS_PFN(off)
Christophe
>
>
> Christophe
>
>
>>
>> Best 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
>>
>>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Framebuffer mmap on PowerPC
2023-08-31 14:41 ` Thomas Zimmermann
@ 2023-09-01 1:44 ` Arnd Bergmann
-1 siblings, 0 replies; 10+ messages in thread
From: Arnd Bergmann @ 2023-09-01 1:44 UTC (permalink / raw)
To: Thomas Zimmermann, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Helge Deller
Cc: linuxppc-dev, Linux-Arch, Linux Fbdev development list, dri-devel
On Thu, Aug 31, 2023, at 10:41, Thomas Zimmermann wrote:
> 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?
What what I can tell, the structure of the code is a result of
these constraints:
- some powerpc platforms use different page table flags for
prefetchable vs nonprefetchable BARs on PCI memory.
- page table flags must match between all mappings, in particular
here between /dev/fb0 and /dev/mem, as mismatched attributes
cause a checkstop. On other architectures this may cause
undefined behavior instead of a checkstop
It's unfortunate that we have multiple incompatible ways
to determine the page flags based on firmware (ia64),
pci (powerpc) or file->f_flags (arm, csky), when they all
try to solve the same problem here.
Christophe's suggested approach to simplify it is probably
fine, another way would be to pass the f_flags value instead
of the file pointer.
Arnd
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Framebuffer mmap on PowerPC
@ 2023-09-01 1:44 ` Arnd Bergmann
0 siblings, 0 replies; 10+ messages in thread
From: Arnd Bergmann @ 2023-09-01 1:44 UTC (permalink / raw)
To: Thomas Zimmermann, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Helge Deller
Cc: Linux-Arch, Linux Fbdev development list, linuxppc-dev, dri-devel
On Thu, Aug 31, 2023, at 10:41, Thomas Zimmermann wrote:
> 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?
What what I can tell, the structure of the code is a result of
these constraints:
- some powerpc platforms use different page table flags for
prefetchable vs nonprefetchable BARs on PCI memory.
- page table flags must match between all mappings, in particular
here between /dev/fb0 and /dev/mem, as mismatched attributes
cause a checkstop. On other architectures this may cause
undefined behavior instead of a checkstop
It's unfortunate that we have multiple incompatible ways
to determine the page flags based on firmware (ia64),
pci (powerpc) or file->f_flags (arm, csky), when they all
try to solve the same problem here.
Christophe's suggested approach to simplify it is probably
fine, another way would be to pass the f_flags value instead
of the file pointer.
Arnd
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Framebuffer mmap on PowerPC
2023-08-31 17:38 ` Christophe Leroy
@ 2023-09-01 6:45 ` Thomas Zimmermann
-1 siblings, 0 replies; 10+ messages in thread
From: Thomas Zimmermann @ 2023-09-01 6:45 UTC (permalink / raw)
To: Christophe Leroy, Michael Ellerman, Nicholas Piggin,
Arnd Bergmann, Helge Deller
Cc: linuxppc-dev, Linux-Arch, Linux Fbdev development list, dri-devel
[-- Attachment #1.1: Type: text/plain, Size: 3246 bytes --]
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);
> }
>
>
> Christophe
>
>
>>
>> Best 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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Framebuffer mmap on PowerPC
@ 2023-09-01 6:45 ` Thomas Zimmermann
0 siblings, 0 replies; 10+ messages in thread
From: Thomas Zimmermann @ 2023-09-01 6:45 UTC (permalink / raw)
To: Christophe Leroy, Michael Ellerman, Nicholas Piggin,
Arnd Bergmann, Helge Deller
Cc: Linux-Arch, Linux Fbdev development list, linuxppc-dev, dri-devel
[-- Attachment #1.1: Type: text/plain, Size: 3246 bytes --]
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);
> }
>
>
> Christophe
>
>
>>
>> Best 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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-09-01 6:46 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-31 14:41 Framebuffer mmap on PowerPC Thomas Zimmermann
2023-08-31 14:41 ` Thomas Zimmermann
2023-08-31 17:38 ` Christophe Leroy
2023-08-31 17:38 ` Christophe Leroy
2023-08-31 17:41 ` Christophe Leroy
2023-08-31 17:41 ` Christophe Leroy
2023-09-01 6:45 ` Thomas Zimmermann
2023-09-01 6:45 ` Thomas Zimmermann
2023-09-01 1:44 ` Arnd Bergmann
2023-09-01 1:44 ` Arnd Bergmann
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.