mmap and ppc460gt

mmap and ppc460gt

[vb]

I am trying to port an application from ppc854x to ppc440. The
application, among other things, maps register file of a PCI device
into its memory space to gain direct access to the device. mmap() is
used to accomplish the mapping.

I don't seem to be able to port this to ppc460gt: on this architecture
the PCI register space can be mapped only to 36 bit address ranges
with nonzero top nibbles. But mmap()  (at least the one I have in
2.6.25)  accepts only 32 bit values for addresses, hence there is no
way to map PCI space into user space.

Am I missing something and there is a way to map 36 bit address in 32
bit mode? Or maybe the kernel/library were not compiled properly and
mmap() is supposed to accept 64 bit addresses?

Any hints/suggestions will be highly appreciated,
TIA,
Vadim

Re: mmap and ppc460gt

[Roland Dreier]

 > I don't seem to be able to port this to ppc460gt: on this architecture
 > the PCI register space can be mapped only to 36 bit address ranges
 > with nonzero top nibbles. But mmap()  (at least the one I have in
 > 2.6.25)  accepts only 32 bit values for addresses, hence there is no
 > way to map PCI space into user space.

In the past I've been able to use mmap64() on ppc440 to get at 36-bit
addresses.  As far as I know this should still work.

Also you should be able to get at the BAR of a PCI device by using
mmap() on a resource file under /sys/devices, eg something like
/sys/devices/pci0000:00/0000:00:04.0/0000:0b:00.0/resource0
(where the bus numbers obviously depend on your system and the resource
number depends on what BAR your device has registers in)

 - R.

Re: mmap and ppc460gt

[Arnd Bergmann]

On Wednesday 27 August 2008, Roland Dreier wrote:
> =A0> I don't seem to be able to port this to ppc460gt: on this architectu=
re
> =A0> the PCI register space can be mapped only to 36 bit address ranges
> =A0> with nonzero top nibbles. But mmap() =A0(at least the one I have in
> =A0> 2.6.25) =A0accepts only 32 bit values for addresses, hence there is =
no
> =A0> way to map PCI space into user space.
>=20
> In the past I've been able to use mmap64() on ppc440 to get at 36-bit
> addresses. =A0As far as I know this should still work.

Right, this is supposed to work. If it doesn't, report it as a bug.

> Also you should be able to get at the BAR of a PCI device by using
> mmap() on a resource file under /sys/devices, eg something like
> /sys/devices/pci0000:00/0000:00:04.0/0000:0b:00.0/resource0
> (where the bus numbers obviously depend on your system and the resource
> number depends on what BAR your device has registers in)

This should work as well.

The best way to do it however would be to implement a UIO device driver
for your device, see e.g. http://lwn.net/Articles/232575/.

That gives you proper control over the permissions and lets you handle
interrupts and other things as well.

	Arnd <><

Re: mmap and ppc460gt

[vb]

On Wed, Aug 27, 2008 at 1:13 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday 27 August 2008, Roland Dreier wrote:
>>  > I don't seem to be able to port this to ppc460gt: on this architecture
>>  > the PCI register space can be mapped only to 36 bit address ranges
>>  > with nonzero top nibbles. But mmap()  (at least the one I have in
>>  > 2.6.25)  accepts only 32 bit values for addresses, hence there is no
>>  > way to map PCI space into user space.
>>
>> In the past I've been able to use mmap64() on ppc440 to get at 36-bit
>> addresses.  As far as I know this should still work.
>
> Right, this is supposed to work. If it doesn't, report it as a bug.
>
>> Also you should be able to get at the BAR of a PCI device by using
>> mmap() on a resource file under /sys/devices, eg something like
>> /sys/devices/pci0000:00/0000:00:04.0/0000:0b:00.0/resource0
>> (where the bus numbers obviously depend on your system and the resource
>> number depends on what BAR your device has registers in)
>
> This should work as well.
>
> The best way to do it however would be to implement a UIO device driver
> for your device, see e.g. http://lwn.net/Articles/232575/.
>
> That gives you proper control over the permissions and lets you handle
> interrupts and other things as well.
>
>        Arnd <><
>

Arnd, Roland, thank you for your replies, I did manage to get it
mapped using mmap64(), but as it often happens clearing one hurdle
just brings us to the next one :-)

The problem now is that that PCI register space gets mapped with
caching enabled (by looking at the TLB contents), so I still can't
control the device. I did some search and indeed UIO device driver
came up, I'll read the article. I was wondering  though if there is a
simpler way to modify cache attributes of a region. mmap() doesn't
seem to provide an interface for that, is there some other function to
call to configure 'cache inhibit' attribute for a region?

TIA,
Vadim

Re: mmap and ppc460gt

[Roland Dreier]

 > The problem now is that that PCI register space gets mapped with
 > caching enabled (by looking at the TLB contents), so I still can't
 > control the device. I did some search and indeed UIO device driver
 > came up, I'll read the article. I was wondering  though if there is a
 > simpler way to modify cache attributes of a region. mmap() doesn't
 > seem to provide an interface for that, is there some other function to
 > call to configure 'cache inhibit' attribute for a region?

The issue would be related to phys_mem_access_prot() not doing the right
thing in this case.  In fact it looks like the arch/powerpc
page_is_ram() implementation has the same bug that I fixed a long time
ago for arch/ppc in 8b150478 ("[PATCH] ppc: make phys_mem_access_prot()
work with pfns instead of addresses").  I'll send a patch a little later
that you can try out.

 - R.

Re: mmap and ppc460gt

[vb]

On Wed, Aug 27, 2008 at 5:11 PM, Roland Dreier <rdreier@cisco.com> wrote:
>  > The problem now is that that PCI register space gets mapped with
>  > caching enabled (by looking at the TLB contents), so I still can't
>  > control the device. I did some search and indeed UIO device driver
>  > came up, I'll read the article. I was wondering  though if there is a
>  > simpler way to modify cache attributes of a region. mmap() doesn't
>  > seem to provide an interface for that, is there some other function to
>  > call to configure 'cache inhibit' attribute for a region?
>
> The issue would be related to phys_mem_access_prot() not doing the right
> thing in this case.  In fact it looks like the arch/powerpc
> page_is_ram() implementation has the same bug that I fixed a long time
> ago for arch/ppc in 8b150478 ("[PATCH] ppc: make phys_mem_access_prot()
> work with pfns instead of addresses").  I'll send a patch a little later
> that you can try out.
>

Ronald, I will appreciate the patch that very much!

TIA,
/vb


>  - R.
>

Re: mmap and ppc460gt

[Roland Dreier]

OK, please try this and let me know if it helps:

diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index 1c93c25..98d7bf9 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -75,11 +75,10 @@ static inline pte_t *virt_to_kpte(unsigned long vaddr)
 
 int page_is_ram(unsigned long pfn)
 {
-	unsigned long paddr = (pfn << PAGE_SHIFT);
-
 #ifndef CONFIG_PPC64	/* XXX for now */
-	return paddr < __pa(high_memory);
+	return pfn < max_pfn;
 #else
+	unsigned long paddr = (pfn << PAGE_SHIFT);
 	int i;
 	for (i=0; i < lmb.memory.cnt; i++) {
 		unsigned long base;

Re: mmap and ppc460gt

[vb]

On Wed, Aug 27, 2008 at 8:12 PM, Roland Dreier <rdreier@cisco.com> wrote:
> OK, please try this and let me know if it helps:
>
> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
> index 1c93c25..98d7bf9 100644
> --- a/arch/powerpc/mm/mem.c
> +++ b/arch/powerpc/mm/mem.c
> @@ -75,11 +75,10 @@ static inline pte_t *virt_to_kpte(unsigned long vaddr)
>
>  int page_is_ram(unsigned long pfn)
>  {
> -       unsigned long paddr = (pfn << PAGE_SHIFT);
> -
>  #ifndef CONFIG_PPC64   /* XXX for now */
> -       return paddr < __pa(high_memory);
> +       return pfn < max_pfn;
>  #else
> +       unsigned long paddr = (pfn << PAGE_SHIFT);
>        int i;
>        for (i=0; i < lmb.memory.cnt; i++) {
>                unsigned long base;
>

Ronald, thank you for the hint,  I actually tried something slightly different:

return pfn < (__pa(high_memory) >> PAGE_SHIFT);

and it worked. I guess your fix is faster, I'll try it tomorrow.

I also checked that the problem is there in the top of the tree in
Linus' git - isn't it amazing - I guess very few people use mmap()
nowadays, everybody must be using UIO device :-)

cheers,
vadim

Re: mmap and ppc460gt

[Josh Boyer]

On Wed, 27 Aug 2008 20:47:24 -0700
vb <vb@vsbe.com> wrote:

> On Wed, Aug 27, 2008 at 8:12 PM, Roland Dreier <rdreier@cisco.com> wrote:
> > OK, please try this and let me know if it helps:
> >
> > diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
> > index 1c93c25..98d7bf9 100644
> > --- a/arch/powerpc/mm/mem.c
> > +++ b/arch/powerpc/mm/mem.c
> > @@ -75,11 +75,10 @@ static inline pte_t *virt_to_kpte(unsigned long vaddr)
> >
> >  int page_is_ram(unsigned long pfn)
> >  {
> > -       unsigned long paddr = (pfn << PAGE_SHIFT);
> > -
> >  #ifndef CONFIG_PPC64   /* XXX for now */
> > -       return paddr < __pa(high_memory);
> > +       return pfn < max_pfn;
> >  #else
> > +       unsigned long paddr = (pfn << PAGE_SHIFT);
> >        int i;
> >        for (i=0; i < lmb.memory.cnt; i++) {
> >                unsigned long base;
> >
> 
> Ronald, thank you for the hint,  I actually tried something slightly different:
> 
> return pfn < (__pa(high_memory) >> PAGE_SHIFT);
> 
> and it worked. I guess your fix is faster, I'll try it tomorrow.
> 
> I also checked that the problem is there in the top of the tree in
> Linus' git - isn't it amazing - I guess very few people use mmap()
> nowadays, everybody must be using UIO device :-)

No, most people use device drivers in the kernel.

josh

[PATCH] powerpc: Avoid integer overflow in page_is_ram()

[Roland Dreier]

Commit 8b150478 ("ppc: make phys_mem_access_prot() work with pfns
instead of addresses") fixed page_is_ram() in arch/ppc to avoid overflow
for addresses above 4G on 32-bit kernels.  However arch/powerpc's
page_is_ram() is missing the same fix -- it computes a physical address
by doing pfn << PAGE_SHIFT, which overflows if pfn corresponds to a page
above 4G.

In particular this causes pages above 4G to be mapped with the wrong
caching attribute; for example many ppc440-based SoCs have PCI space
above 4G, and mmap()ing MMIO space may end up with a mapping that has
caching enabled.

Fix this by working with the pfn and avoiding the conversion to
physical address that causes the overflow.  This patch compares the
pfn to max_pfn, which is a semantic change from the old code -- that
code compared the physical address to high_memory, which corresponds
to max_low_pfn.  However, I think that was is another bug, since
highmem pages are still RAM.

Reported-by: vb <vb@vsbe.com>
Signed-off-by: Roland Dreier <rolandd@cisco.com>
---
This is a fix but the bug is pretty long-standing -- I think this is
2.6.28 material.

 arch/powerpc/mm/mem.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index 1c93c25..98d7bf9 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -75,11 +75,10 @@ static inline pte_t *virt_to_kpte(unsigned long vaddr)
 
 int page_is_ram(unsigned long pfn)
 {
-	unsigned long paddr = (pfn << PAGE_SHIFT);
-
 #ifndef CONFIG_PPC64	/* XXX for now */
-	return paddr < __pa(high_memory);
+	return pfn < max_pfn;
 #else
+	unsigned long paddr = (pfn << PAGE_SHIFT);
 	int i;
 	for (i=0; i < lmb.memory.cnt; i++) {
 		unsigned long base;

Re: [PATCH] powerpc: Avoid integer overflow in page_is_ram()

[Benjamin Herrenschmidt]

On Fri, 2008-08-29 at 20:39 -0700, Roland Dreier wrote:
> Commit 8b150478 ("ppc: make phys_mem_access_prot() work with pfns
> instead of addresses") fixed page_is_ram() in arch/ppc to avoid overflow
> for addresses above 4G on 32-bit kernels.  However arch/powerpc's
> page_is_ram() is missing the same fix -- it computes a physical address
> by doing pfn << PAGE_SHIFT, which overflows if pfn corresponds to a page
> above 4G.
> 
> In particular this causes pages above 4G to be mapped with the wrong
> caching attribute; for example many ppc440-based SoCs have PCI space
> above 4G, and mmap()ing MMIO space may end up with a mapping that has
> caching enabled.
> 
> Fix this by working with the pfn and avoiding the conversion to
> physical address that causes the overflow.  This patch compares the
> pfn to max_pfn, which is a semantic change from the old code -- that
> code compared the physical address to high_memory, which corresponds
> to max_low_pfn.  However, I think that was is another bug, since
> highmem pages are still RAM.
> 
> Reported-by: vb <vb@vsbe.com>
> Signed-off-by: Roland Dreier <rolandd@cisco.com>

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

> ---
> This is a fix but the bug is pretty long-standing -- I think this is
> 2.6.28 material.
> 
>  arch/powerpc/mm/mem.c |    5 ++---
>  1 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
> index 1c93c25..98d7bf9 100644
> --- a/arch/powerpc/mm/mem.c
> +++ b/arch/powerpc/mm/mem.c
> @@ -75,11 +75,10 @@ static inline pte_t *virt_to_kpte(unsigned long vaddr)
>  
>  int page_is_ram(unsigned long pfn)
>  {
> -	unsigned long paddr = (pfn << PAGE_SHIFT);
> -
>  #ifndef CONFIG_PPC64	/* XXX for now */
> -	return paddr < __pa(high_memory);
> +	return pfn < max_pfn;
>  #else
> +	unsigned long paddr = (pfn << PAGE_SHIFT);
>  	int i;
>  	for (i=0; i < lmb.memory.cnt; i++) {
>  		unsigned long base;