[RFC] Switch OFMEM to use phys_addr_t for physical addresses

[Mark Cave-Ayland]

Hi all,

Here is my first attempt at switching over OFMEM to use phys_addr_t for 
physical addresses. It was mainly a case of going through all of the 
OFMEM APIs by hand and then updating the definitions used to hold 
physical addresses, along with swapping over range_t to use phys_addr_t 
on the assumption that sizeof(phys_addr_t) >= sizeof(ucell).

Andreas - note I've not touched anything in ppc64 as I figure this is 
still experimental and I don't really have anything to test it with.

This patch seems to work on my SPARC64/PPC tests here, but since it 
touches a core part of OpenBIOS I'd like to get a little more feedback 
before committing it.

ATB,

Mark.

[Andreas Färber]

Hi,

Am 07.11.2010 um 12:16 schrieb Mark Cave-Ayland:
> Here is my first attempt at switching over OFMEM to use phys_addr_t  
> for physical addresses. It was mainly a case of going through all of  
> the OFMEM APIs by hand and then updating the definitions used to  
> hold physical addresses, along with swapping over range_t to use  
> phys_addr_t on the assumption that sizeof(phys_addr_t) >=  
> sizeof(ucell).

Looking good so far, except for one issue.
> Index: libopenbios/ofmem_common.c
>
> --- libopenbios/ofmem_common.c	(revision 946)
> +++ libopenbios/ofmem_common.c	(working copy)
> @@ -463,7 +463,7 @@
> }
>
> /* if align != 0, phys is ignored. Returns -1 on error */
> -ucell ofmem_claim_phys( ucell phys, ucell size, ucell align )
> +ucell ofmem_claim_phys( phys_addr_t phys, ucell size, ucell align )
> {
>     OFMEM_TRACE("ofmem_claim phys=" FMT_ucellx " size=" FMT_ucellx
>                 " align=" FMT_ucellx "\n",

This will break the trace code on sparc32 and ppc64. We'll need some  
target-specific define to replace FMT_ucellx in the trace code for the  
"phys" parameter since it may be larger.

Btw if we start deviating from ucell, we might as well use size_t size  
in a second step, but I don't see a real use case yet.
> Index: arch/sparc64/ofmem_sparc64.c
>
> --- arch/sparc64/ofmem_sparc64.c	(revision 946)
> +++ arch/sparc64/ofmem_sparc64.c	(working copy)
> @@ -514,7 +516,8 @@
> static void
> mem_claim( void )
> {
> ucell phys=-1UL, size, align;
> ucell size, align;
> phys_addr_t phys=-1UL;

I'd appreciate if you could enhance the code you refactored with  
spaces around the assignment, i.e. phys_addr_t phys = -1UL;, for  
readability.
> @@ -548,7 +552,8 @@
> static void
> mem_retain ( void )
> {
> ucell phys=-1UL, size, align;
> ucell size, align;
> phys_addr_t phys=-1UL;

Dito.
> @@ -393,7 +394,7 @@
>     }
> }
>
> -void ofmem_map_pages(ucell phys, ucell virt, ucell size, ucell mode)
> +void ofmem_map_pages(phys_addr_t phys, ucell virt, ucell size,  
> ucell mode)
> {
>    return map_pages(phys, virt, size, mode);
> }

Note that I find it slightly odd that there is a (trivial)  
ofmem_map_ranges() function in arch/sparc64/lib.c, but that's  
unrelated to phys_addr_t.
What's the difference to ofmem_map() in ofmem_common.c? We should  
either share it, rename it or drop it mid-term.

Regards,
Andreas

[RFC] Switch OFMEM to use phys_addr_t for physical addresses

[Mark Cave-Ayland]

Andreas Färber wrote:
> Looking good so far, except for one issue.
> > Index: libopenbios/ofmem_common.c
> >
> > --- libopenbios/ofmem_common.c    (revision 946)
> > +++ libopenbios/ofmem_common.c    (working copy)
> > @@ -463,7 +463,7 @@
> > }
> >
> > /* if align != 0, phys is ignored. Returns -1 on error */
> > -ucell ofmem_claim_phys( ucell phys, ucell size, ucell align )
> > +ucell ofmem_claim_phys( phys_addr_t phys, ucell size, ucell align )
> > {
> >     OFMEM_TRACE("ofmem_claim phys=" FMT_ucellx " size=" FMT_ucellx
> >                 " align=" FMT_ucellx "\n",
>
> This will break the trace code on sparc32 and ppc64. We'll need some 
> target-specific define to replace FMT_ucellx in the trace code for the 
> "phys" parameter since it may be larger.

Ah good spot - would FMT_phys_addr_t be a suitable name for this? I'll 
make the defaults for SPARC64 and PPC the same as those for ucell.
> > @@ -514,7 +516,8 @@
> > static void
> > mem_claim( void )
> > {
> > ucell phys=-1UL, size, align;
> > ucell size, align;
> > phys_addr_t phys=-1UL;
>
> I'd appreciate if you could enhance the code you refactored with spaces 
> around the assignment, i.e. phys_addr_t phys = -1UL;, for readability.
> > @@ -548,7 +552,8 @@
> > static void
> > mem_retain ( void )
> > {
> > ucell phys=-1UL, size, align;
> > ucell size, align;
> > phys_addr_t phys=-1UL;
>
> Dito.
> > @@ -393,7 +394,7 @@
> >     }
> > }

Yup no worries - I know some people don't like whitespace cleanups in 
the same patch, but it seems fine to me.
> > -void ofmem_map_pages(ucell phys, ucell virt, ucell size, ucell mode)
> > +void ofmem_map_pages(phys_addr_t phys, ucell virt, ucell size, ucell 
> > mode)
> > {
> >     return map_pages(phys, virt, size, mode);
> > }
>
> Note that I find it slightly odd that there is a (trivial) 
> ofmem_map_ranges() function in arch/sparc64/lib.c, but that's unrelated 
> to phys_addr_t.
> What's the difference to ofmem_map() in ofmem_common.c? We should either 
> share it, rename it or drop it mid-term.

It looks like it's called from ofmem_arch_early_map_pages() depending 
upon whether or not the TLB entry is marked as locked. I know that the 
OFMEM API does still need a bit a work looking at this comment in ofmem.h:

/* TODO: temporary migration interface */

I'm fairly sure we could simplify this if we had help from people 
experienced with other platforms, but I'm not sure it's something I yet 
feel confident enough to do myself. It's something we could definitely 
consider for the future though.

ATB,

Mark.

[Andreas Färber]

Am 07.11.2010 um 13:20 schrieb Mark Cave-Ayland:
> Andreas Färber wrote:
> > Looking good so far, except for one issue.
> > > Index: libopenbios/ofmem_common.c
> > >
> > > --- libopenbios/ofmem_common.c    (revision 946)
> > > +++ libopenbios/ofmem_common.c    (working copy)
> > > @@ -463,7 +463,7 @@
> > > }
> > >
> > > /* if align != 0, phys is ignored. Returns -1 on error */
> > > -ucell ofmem_claim_phys( ucell phys, ucell size, ucell align )
> > > +ucell ofmem_claim_phys( phys_addr_t phys, ucell size, ucell align )
> > > {
> > >    OFMEM_TRACE("ofmem_claim phys=" FMT_ucellx " size=" FMT_ucellx
> > >                " align=" FMT_ucellx "\n",
> >
> > This will break the trace code on sparc32 and ppc64. We'll need  
> > some target-specific define to replace FMT_ucellx in the trace code  
> > for the "phys" parameter since it may be larger.
>
> Ah good spot - would FMT_phys_addr_t be a suitable name for this?  
> I'll make the defaults for SPARC64 and PPC the same as those for  
> ucell.

Judging by include/arch/ppc/types.h I'd prefer FMT_physaddrt (without  
the trailing _t that makes it look like a type itself).
QEMU has TARGET_FMT_plx, so we might use FMT_plx (physical long  
hexadecimal?).

Would be nice to have their introduction in a separate, preceding patch.

Andreas

[RFC] Switch OFMEM to use phys_addr_t for physical addresses

[Mark Cave-Ayland]

Andreas Färber wrote:
> > Ah good spot - would FMT_phys_addr_t be a suitable name for this? I'll 
> > make the defaults for SPARC64 and PPC the same as those for ucell.
>
> Judging by include/arch/ppc/types.h I'd prefer FMT_physaddrt (without 
> the trailing _t that makes it look like a type itself).
> QEMU has TARGET_FMT_plx, so we might use FMT_plx (physical long 
> hexadecimal?).

(Looks like our previous emails have crossed)

I can see the argument for renaming the FMT argument to avoid confusion 
with proper types, but introducing TARGET_FMT_plx seems a little 
excessive. I'm merely sticking with the existing OpenBIOS convention 
rather than attempting to change it within the scope of this patch :)
> Would be nice to have their introduction in a separate, preceding patch.

I really can't get too excited about this as I don't see what benefit it 
would have over a single commit to implement the type change in this case.

ATB,

Mark.

[Andreas Färber]

Am 07.11.2010 um 14:30 schrieb Mark Cave-Ayland:
> Andreas Färber wrote:
> > > Ah good spot - would FMT_phys_addr_t be a suitable name for this?  
> > > I'll make the defaults for SPARC64 and PPC the same as those for  
> > > ucell.
> >
> > Judging by include/arch/ppc/types.h I'd prefer FMT_physaddrt  
> > (without the trailing _t that makes it look like a type itself).
> > QEMU has TARGET_FMT_plx, so we might use FMT_plx (physical long  
> > hexadecimal?).
>
> (Looks like our previous emails have crossed)

Yeah, again!
> I can see the argument for renaming the FMT argument to avoid  
> confusion with proper types, but
> introducing TARGET_FMT_plx seems a little excessive.

That's why I suggested FMT_plx without the TARGET_ prefix...
> I'm merely sticking with the existing OpenBIOS convention rather  
> than attempting to change it within the scope of this patch :)

...and it's not a change but a new definition. Blue was asking for  
name reuse from QEMU when I introduced the type, and Alex was recently  
asking for name reuse from Linux. Don't know about the latter.
> > Would be nice to have their introduction in a separate, preceding  
> > patch.
>
> I really can't get too excited about this as I don't see what  
> benefit it would have over a single commit to implement the type  
> change in this case.

Logically, it is simply an addition to my r932 and has nothing to do  
with ofmem.

Practically, if something breaks during/after the ofmem migration it  
is better for bisecting and possible reverting with tools like git- 
revert to leave separate things separate.

I'd be volunteering to supply a patch, after all I'm to blame for  
introducing phys_addr_t. ;)

Andreas

[Blue Swirl]

On Sun, Nov 7, 2010 at 1:46 PM, Andreas Färber  wrote:
> Am 07.11.2010 um 14:30 schrieb Mark Cave-Ayland:
> > Andreas Färber wrote:
> > > > Ah good spot - would FMT_phys_addr_t be a suitable name for this? I'll
> > > > make the defaults for SPARC64 and PPC the same as those for ucell.
> > >
> > > Judging by include/arch/ppc/types.h I'd prefer FMT_physaddrt (without the
> > > trailing _t that makes it look like a type itself).
> > > QEMU has TARGET_FMT_plx, so we might use FMT_plx (physical long
> > > hexadecimal?).
> >
> > (Looks like our previous emails have crossed)
>
> Yeah, again!
> > I can see the argument for renaming the FMT argument to avoid confusion
> > with proper types, but
> > introducing TARGET_FMT_plx seems a little excessive.
>
> That's why I suggested FMT_plx without the TARGET_ prefix...

I'd agree. Otherwise the patch seems to be OK.
> > I'm merely sticking with the existing OpenBIOS convention rather than
> > attempting to change it within the scope of this patch :)
>
> ...and it's not a change but a new definition. Blue was asking for name
> reuse from QEMU when I introduced the type, and Alex was recently asking for
> name reuse from Linux. Don't know about the latter.
> > > Would be nice to have their introduction in a separate, preceding patch.
> >
> > I really can't get too excited about this as I don't see what benefit it
> > would have over a single commit to implement the type change in this case.
>
> Logically, it is simply an addition to my r932 and has nothing to do with
> ofmem.
>
> Practically, if something breaks during/after the ofmem migration it is
> better for bisecting and possible reverting with tools like git-revert to
> leave separate things separate.
>
> I'd be volunteering to supply a patch, after all I'm to blame for
> introducing phys_addr_t. ;)

Great!

[Andreas Färber]

Am 07.11.2010 um 16:07 schrieb Blue Swirl:
> On Sun, Nov 7, 2010 at 1:46 PM, Andreas Färber  
>  wrote:
> > Am 07.11.2010 um 14:30 schrieb Mark Cave-Ayland:
> > > Andreas Färber wrote:
> > > > > Ah good spot - would FMT_phys_addr_t be a suitable name for  
> > > > > this? I'll
> > > > > make the defaults for SPARC64 and PPC the same as those for ucell.
> > > >
> > > > Judging by include/arch/ppc/types.h I'd prefer FMT_physaddrt  
> > > > (without the
> > > > trailing _t that makes it look like a type itself).
> > > > QEMU has TARGET_FMT_plx, so we might use FMT_plx (physical long
> > > > hexadecimal?).
> > >
> > > I can see the argument for renaming the FMT argument to avoid  
> > > confusion
> > > with proper types, but
> > > introducing TARGET_FMT_plx seems a little excessive.
> >
> > That's why I suggested FMT_plx without the TARGET_ prefix...
>
> I'd agree.

With what exactly? FMT_plx? Or that being too excessive?

(Still investigating one ppc regression with v1 here, might be QEMU or  
my WIP.)

Andreas

[Blue Swirl]

On Sun, Nov 7, 2010 at 3:37 PM, Andreas Färber  wrote:
> Am 07.11.2010 um 16:07 schrieb Blue Swirl:
> > On Sun, Nov 7, 2010 at 1:46 PM, Andreas Färber 
> > wrote:
> > > Am 07.11.2010 um 14:30 schrieb Mark Cave-Ayland:
> > > > Andreas Färber wrote:
> > > > > > Ah good spot - would FMT_phys_addr_t be a suitable name for this? I'll
> > > > > > make the defaults for SPARC64 and PPC the same as those for ucell.
> > > > >
> > > > > Judging by include/arch/ppc/types.h I'd prefer FMT_physaddrt (without
> > > > > the
> > > > > trailing _t that makes it look like a type itself).
> > > > > QEMU has TARGET_FMT_plx, so we might use FMT_plx (physical long
> > > > > hexadecimal?).
> > > >
> > > > I can see the argument for renaming the FMT argument to avoid confusion
> > > > with proper types, but
> > > > introducing TARGET_FMT_plx seems a little excessive.
> > >
> > > That's why I suggested FMT_plx without the TARGET_ prefix...
> >
> > I'd agree.
>
> With what exactly? FMT_plx? Or that being too excessive?

To use FMT_plx. We don't need 'TARGET_' in TARGET_FMT_plx and
FMT_phys_addr_t (or FMT_phys) does not specify 'x' (compared to 'd')
format.

[PATCH] Introduce FMT_plx for phys_addr_t

[Andreas Färber]

Define a zero-padded format string to be used for phys_addr_t arguments.
Introduce PRIx{32,64} macros as needed.

Cc: Mark Cave-Ayland 
Cc: Blue Swirl 
Signed-off-by: Andreas Färber 
---
 To play safe for bootstrapping and the x86/amd64/ia64 architectures,
 I resorted to PRIx32 / PRIx64. Tested that with a custom printk()
 on ppc. In future patches I'd like to extend their use to the cell
 definitions to avoid lx vs. l uint32_t and llx vs. lx uint64_t issues.

Note that I intentionally used 9 for sparc32 to handle the additional
 nibble. On x86 since we're still using uint32_t I chose 8.

Andreas

include/arch/amd64/types.h   |    3 +++
 include/arch/ia64/types.h    |    3 +++
 include/arch/ppc/types.h     |    5 +++++
 include/arch/sparc32/types.h |    5 +++++
 include/arch/sparc64/types.h |    4 ++++
 include/arch/x86/types.h     |    3 +++
 6 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/include/arch/amd64/types.h b/include/arch/amd64/types.h
index 672f02c..44d2182 100644
--- a/include/arch/amd64/types.h
+++ b/include/arch/amd64/types.h
@@ -17,8 +17,11 @@
 #include "autoconf.h"

/* physical address */
+
 typedef uint64_t phys_addr_t;

+#define FMT_plx "%016" PRIx64
+
 /* cell based types */

typedef long long     cell;
diff --git a/include/arch/ia64/types.h b/include/arch/ia64/types.h
index d423461..3bd2edb 100644
--- a/include/arch/ia64/types.h
+++ b/include/arch/ia64/types.h
@@ -18,8 +18,11 @@
 #include <endian.h>

/* physical address */
+
 typedef uint64_t phys_addr_t;

+#define FMT_plx "%016" PRIx64
+
 /* cell based types */

typedef int64_t       cell;
diff --git a/include/arch/ppc/types.h b/include/arch/ppc/types.h
index aaa66fc..ed9100c 100644
--- a/include/arch/ppc/types.h
+++ b/include/arch/ppc/types.h
@@ -25,6 +25,9 @@ typedef short           int16_t;
 typedef int             int32_t;
 typedef long long       int64_t;
 typedef long            intptr_t;
+
+#define PRIx32 "x"
+#define PRIx64 "llx"
 #endif

/* endianess */
@@ -33,8 +36,10 @@ typedef long            intptr_t;
 /* physical address */
 #if defined(__powerpc64__)
 typedef uint64_t phys_addr_t;
+#define FMT_plx "%016" PRIx64
 #else
 typedef uint32_t phys_addr_t;
+#define FMT_plx "%08" PRIx32
 #endif

/* cell based types */
diff --git a/include/arch/sparc32/types.h b/include/arch/sparc32/types.h
index bf96f57..7840e5b 100644
--- a/include/arch/sparc32/types.h
+++ b/include/arch/sparc32/types.h
@@ -25,14 +25,19 @@ typedef short           int16_t;
 typedef int             int32_t;
 typedef long long       int64_t;
 typedef long            intptr_t;
+
+#define PRIx64 "llx"
 #endif

/* endianess */
 #include "autoconf.h"

/* physical address: 36 bits */
+
 typedef uint64_t phys_addr_t;

+#define FMT_plx "%09" PRIx64
+
 /* cell based types */

typedef int32_t		cell;
diff --git a/include/arch/sparc64/types.h b/include/arch/sparc64/types.h
index 8baa5ee..3a235d9 100644
--- a/include/arch/sparc64/types.h
+++ b/include/arch/sparc64/types.h
@@ -25,6 +25,8 @@ typedef short           int16_t;
 typedef int             int32_t;
 typedef long long       int64_t;
 typedef long            intptr_t;
+
+#define PRIx64 "llx"
 #endif

/* endianess */
@@ -33,6 +35,8 @@ typedef long            intptr_t;
 /* physical address */
 typedef uint64_t phys_addr_t;

+#define FMT_plx "%016" PRIx64
+
 /* cell based types */
 typedef long long          cell;
 typedef unsigned long long ucell;
diff --git a/include/arch/x86/types.h b/include/arch/x86/types.h
index 3ba4807..3b1b331 100644
--- a/include/arch/x86/types.h
+++ b/include/arch/x86/types.h
@@ -18,8 +18,11 @@
 #include "autoconf.h"

/* physical address: XXX theoretically 36 bits for PAE */
+
 typedef uint32_t phys_addr_t;

+#define FMT_plx "%08" PRIx32
+
 /* cell based types */

typedef int32_t		cell;

[PATCH] Introduce FMT_plx for phys_addr_t

[Andreas Färber]

Am 07.11.2010 um 18:39 schrieb Andreas Färber:
> Define a zero-padded format string to be used for phys_addr_t  
> arguments.
> Introduce PRIx{32,64} macros as needed.
>
> Cc: Mark Cave-Ayland 
> Cc: Blue Swirl 
> Signed-off-by: Andreas Färber

While at it, how would we format a uintptr_t? (thinking of escc)
Some places do an (unsigned long) cast and use %lx. Should we add some  
FMT_* instead?

Andreas

[PATCH] Introduce FMT_plx for phys_addr_t

[Mark Cave-Ayland]

Andreas Färber wrote:
> Define a zero-padded format string to be used for phys_addr_t arguments.
> Introduce PRIx{32,64} macros as needed.

Not that I know the lower-level multi-platform detail, but on the 
surface the patch looks reasonable - I'll defer to Blue on this one.

ATB,

Mark.

[PATCH] Introduce FMT_plx for phys_addr_t

[Blue Swirl]

On Sun, Nov 7, 2010 at 5:39 PM, Andreas Färber  wrote:
> Define a zero-padded format string to be used for phys_addr_t arguments.
> Introduce PRIx{32,64} macros as needed.
>
> Cc: Mark Cave-Ayland 
> Cc: Blue Swirl 
> Signed-off-by: Andreas Färber
>
> To play safe for bootstrapping and the x86/amd64/ia64 architectures,
>  I resorted to PRIx32 / PRIx64. Tested that with a custom printk()
>  on ppc. In future patches I'd like to extend their use to the cell
>  definitions to avoid lx vs. l uint32_t and llx vs. lx uint64_t issues.
>
> Note that I intentionally used 9 for sparc32 to handle the additional
>  nibble. On x86 since we're still using uint32_t I chose 8.
>
> Andreas

Looks OK, except I'd always define PRIx64 etc. for consistency.
> include/arch/amd64/types.h   |    3 +++
>  include/arch/ia64/types.h    |    3 +++
>  include/arch/ppc/types.h     |    5 +++++
>  include/arch/sparc32/types.h |    5 +++++
>  include/arch/sparc64/types.h |    4 ++++
>  include/arch/x86/types.h     |    3 +++
>  6 files changed, 23 insertions(+), 0 deletions(-)
>
> diff --git a/include/arch/amd64/types.h b/include/arch/amd64/types.h
> index 672f02c..44d2182 100644
> --- a/include/arch/amd64/types.h
> +++ b/include/arch/amd64/types.h
> @@ -17,8 +17,11 @@
>  #include "autoconf.h"
>
> /* physical address */
>
> typedef uint64_t phys_addr_t;
>
> +#define FMT_plx "%016" PRIx64
>
> /* cell based types */
>
> typedef long long     cell;
> diff --git a/include/arch/ia64/types.h b/include/arch/ia64/types.h
> index d423461..3bd2edb 100644
> --- a/include/arch/ia64/types.h
> +++ b/include/arch/ia64/types.h
> @@ -18,8 +18,11 @@
>  #include <endian.h>
>
> /* physical address */
>
> typedef uint64_t phys_addr_t;
>
> +#define FMT_plx "%016" PRIx64
>
> /* cell based types */
>
> typedef int64_t       cell;
> diff --git a/include/arch/ppc/types.h b/include/arch/ppc/types.h
> index aaa66fc..ed9100c 100644
> --- a/include/arch/ppc/types.h
> +++ b/include/arch/ppc/types.h
> @@ -25,6 +25,9 @@ typedef short           int16_t;
>  typedef int             int32_t;
>  typedef long long       int64_t;
>  typedef long            intptr_t;
>
> +#define PRIx32 "x"
> +#define PRIx64 "llx"
>  #endif
>
> /* endianess */
> @@ -33,8 +36,10 @@ typedef long            intptr_t;
>  /* physical address */
>  #if defined(__powerpc64__)
>  typedef uint64_t phys_addr_t;
> +#define FMT_plx "%016" PRIx64
>  #else
>  typedef uint32_t phys_addr_t;
> +#define FMT_plx "%08" PRIx32
>  #endif
>
> /* cell based types */
> diff --git a/include/arch/sparc32/types.h b/include/arch/sparc32/types.h
> index bf96f57..7840e5b 100644
> --- a/include/arch/sparc32/types.h
> +++ b/include/arch/sparc32/types.h
> @@ -25,14 +25,19 @@ typedef short           int16_t;
>  typedef int             int32_t;
>  typedef long long       int64_t;
>  typedef long            intptr_t;
>
> +#define PRIx64 "llx"
>  #endif
>
> /* endianess */
>  #include "autoconf.h"
>
> /* physical address: 36 bits */
>
> typedef uint64_t phys_addr_t;
>
> +#define FMT_plx "%09" PRIx64
>
> /* cell based types */
>
> typedef int32_t                cell;
> diff --git a/include/arch/sparc64/types.h b/include/arch/sparc64/types.h
> index 8baa5ee..3a235d9 100644
> --- a/include/arch/sparc64/types.h
> +++ b/include/arch/sparc64/types.h
> @@ -25,6 +25,8 @@ typedef short           int16_t;
>  typedef int             int32_t;
>  typedef long long       int64_t;
>  typedef long            intptr_t;
>
> +#define PRIx64 "llx"

glibc uses just "lx" on 64 bit hosts.
> #endif
>
> /* endianess */
> @@ -33,6 +35,8 @@ typedef long            intptr_t;
>  /* physical address */
>  typedef uint64_t phys_addr_t;
>
> +#define FMT_plx "%016" PRIx64
>
> /* cell based types */
>  typedef long long          cell;
>  typedef unsigned long long ucell;
> diff --git a/include/arch/x86/types.h b/include/arch/x86/types.h
> index 3ba4807..3b1b331 100644
> --- a/include/arch/x86/types.h
> +++ b/include/arch/x86/types.h
> @@ -18,8 +18,11 @@
>  #include "autoconf.h"
>
> /* physical address: XXX theoretically 36 bits for PAE */
>
> typedef uint32_t phys_addr_t;
>
> +#define FMT_plx "%08" PRIx32
>
> /* cell based types */
> typedef int32_t                cell;
>
> 1.7.3

[PATCH] Introduce FMT_plx for phys_addr_t

[Andreas Färber]

Am 08.11.2010 um 19:20 schrieb Blue Swirl:
> On Sun, Nov 7, 2010 at 5:39 PM, Andreas Färber  
>  wrote:
> > Define a zero-padded format string to be used for phys_addr_t  
> > arguments.
> > Introduce PRIx{32,64} macros as needed.
> >
> > Cc: Mark Cave-Ayland 
> > Cc: Blue Swirl 
> > Signed-off-by: Andreas Färber
>
> Looks OK, except I'd always define PRIx64 etc. for consistency.

Will do.
> > diff --git a/include/arch/sparc64/types.h b/include/arch/sparc64/ 
> > types.h
> > index 8baa5ee..3a235d9 100644
> > --- a/include/arch/sparc64/types.h
> > +++ b/include/arch/sparc64/types.h
> > @@ -25,6 +25,8 @@ typedef short           int16_t;
> >  typedef int             int32_t;
> >  typedef long long       int64_t;
> >  typedef long            intptr_t;
> >
> > +#define PRIx64 "llx"
>
> glibc uses just "lx" on 64 bit hosts.

Possibly, I was surprised too. But the format should match our type  
definition above.

When introducing phys_addr_t I ran into some long long assumptions so  
that I turned to uint64_t rather than unsigned long.
For intptr_t I believe the use of long was a convenient shortcut to  
avoid #ifdef'ed definitions, which happened to work since there was no  
prior art.
I therefore consider a long long -> long conversion material for a  
follow-on patch, if desired.

Andreas

[PATCH] Introduce FMT_plx for phys_addr_t

[Andreas Färber]

Am 08.11.2010 um 20:27 schrieb Andreas Färber:
> Am 08.11.2010 um 19:20 schrieb Blue Swirl:
> > On Sun, Nov 7, 2010 at 5:39 PM, Andreas Färber <[e-mail]
> > > wrote:
> > > Define a zero-padded format string to be used for phys_addr_t  
> > > arguments.
> > > Introduce PRIx{32,64} macros as needed.
> > >
> > > Cc: Mark Cave-Ayland 
> > > Cc: Blue Swirl 
> > > Signed-off-by: Andreas Färber
> >
> > Looks OK, except I'd always define PRIx64 etc. for consistency.

Done, thanks, applied in r949.

Andreas

[RFC] Switch OFMEM to use phys_addr_t for physical addresses

[Mark Cave-Ayland]

Mark Cave-Ayland wrote:
> Andreas Färber wrote:
> > Looking good so far, except for one issue.

(lots cut)

Okay - here's a v2 re-spin of the patchset based upon your other 
comments. Do you feel this is now ready for commit?

ATB,

Mark.

[Andreas Färber]

Am 07.11.2010 um 14:05 schrieb Mark Cave-Ayland:
> here's a v2 re-spin of the patchset based upon your other comments.  
> Do you feel this is now ready for commit?

No, please see my other message.

Also, we need the FMT_* definition for all archs, not just sparc64 and  
ppc.
I'd prefer to place it alongside the phys_addr_t typedef, following  
the *cell example, to avoid new #ifdef'ery. The ppc one should be  
%016llx for ppc64.

Still on my way to testing v1...
Since the phys_addr_t format stuff is something that will affect more  
parts of the code, I'd like consensus on the name first and I would  
like to hear further review from Blue (cc'ed).
If you're eager to continue with the sparc32 ofmem migration I'd  
recommend a tool like Git (or maybe Quilt) to create a refineable  
patch series.

Andreas

[Andreas Färber]

Am 07.11.2010 um 13:02 schrieb Andreas Färber:
> Am 07.11.2010 um 12:16 schrieb Mark Cave-Ayland:
> > Here is my first attempt at switching over OFMEM to use phys_addr_t  
> > for physical addresses. It was mainly a case of going through all  
> > of the OFMEM APIs by hand and then updating the definitions used to  
> > hold physical addresses, along with swapping over range_t to use  
> > phys_addr_t on the assumption that sizeof(phys_addr_t) >=  
> > sizeof(ucell).
>
> Looking good so far, except for one issue.

With trace disabled, works as before for:

Debian4/ppc
Debian4/ppc64-via-ppc
Haiku/ppc
FreeBSD8/ppc
AIX6/ppc
AIX6/ppc64-via-ppc
Milax/sparc64
Solaris10/sparc64

Andreas

[RFC] Switch OFMEM to use phys_addr_t for physical addresses

[Mark Cave-Ayland]

Andreas Färber wrote:
> With trace disabled, works as before for:
>
> Debian4/ppc
> Debian4/ppc64-via-ppc
> Haiku/ppc
> FreeBSD8/ppc
> AIX6/ppc
> AIX6/ppc64-via-ppc
> Milax/sparc64
> Solaris10/sparc64
>
> Andreas

Okay, thanks for testing :)

I'll wait on your FMT_plx patch to finalise before committing something.

ATB,

Mark.

[Andreas Färber]

[snip]

Now that ppc64 is basically working I continued playing with Mark's  
v1. I noticed a few glitches, some of which I've started fixing. ppc64  
would no longer reach the prompt with v1 applied; I noticed that,  
e.g., the claim_phys functions did not yet return a phys_addr_t  
despite using it internally. With some fixes like these it is now  
working.

ppc64 required some more work for the MMU translations - I needed to  
tweak some sizes and squeeze in some additional cells for the  
properties.
Still need to add some more .properties pretty-printing to confirm  
whether the values are actually right now.

http://repo.or.cz/w/openbios/afaerber.git/shortlog/refs/heads/ppc64-ofmem

What I didn't yet look into is that the add_range() etc. functions  
still operate on ucell despite the range_t using a phys_addr_t now.
ppc's ea_to_phys() also still has some assumptions about memory layout  
that need fixing.

Andreas

[Andreas Färber]

Am 24.11.2010 um 23:18 schrieb Andreas Färber:
> Now that ppc64 is basically working I continued playing with Mark's  
> v1. I noticed a few glitches, some of which I've started fixing.  
> ppc64 would no longer reach the prompt with v1 applied; I noticed  
> that, e.g., the claim_phys functions did not yet return a  
> phys_addr_t despite using it internally. With some fixes like these  
> it is now working.
>
> ppc64 required some more work for the MMU translations - I needed to  
> tweak some sizes and squeeze in some additional cells for the  
> properties.
> Still need to add some more .properties pretty-printing to confirm  
> whether the values are actually right now.
>
> http://repo.or.cz/w/openbios/afaerber.git/shortlog/refs/heads/ppc64-ofmem

FYI a rebased version that applies on top of the ppc cleanup is now  
pushed. My add-ons are still WIP.

[PATCH v3 1/2] ofmem: Use phys_addr_t for physical addresses

[Andreas Färber]

Based on Mark's previous work, let ofmem handle addresses wider than one cell.
This is based on the assumption that sizeof(phys_addr_t) >= sizeof(ucell).
Affected are in particular sparc32 (36 bits) and ppc64 (64 bits).

As a consequence, some range_t related code shared with virtual/effective
addresses needed to be migrated, too. Since both address types are unsigned
this should be okay.

v3:
* Rebased: Prefer FMT_plx for trace output.
* Convert some more places to phys_addr_t to avoid breakage on ppc64.
* Add helpers ofmem_arch_{get_physaddr_cellsize,encode_physaddr}(),
  requested by Mark.

v2:
* Use FMT_physaddr_t in ofmem trace code. (by Mark)

Cc: Mark Cave-Ayland 
Signed-off-by: Andreas Färber 
---
 Mark, please insert an SoB for your original patch. Thanks!

arch/ppc/qemu/ofmem.c        |   51 ++++++++++++++++++++++---------
 arch/sparc64/lib.c           |   21 ++++++++-----
 arch/sparc64/ofmem_sparc64.c |   15 ++++++++-
 include/libopenbios/ofmem.h  |   22 +++++++------
 libopenbios/ofmem_common.c   |   68 +++++++++++++++++++++++------------------
 5 files changed, 112 insertions(+), 65 deletions(-)

diff --git a/arch/ppc/qemu/ofmem.c b/arch/ppc/qemu/ofmem.c
index c62e42c..2892d54 100644
--- a/arch/ppc/qemu/ofmem.c
+++ b/arch/ppc/qemu/ofmem.c
@@ -120,7 +120,7 @@ void ofmem_arch_unmap_pages(ucell virt, ucell size)
     /* kill page mappings in provided range */
 }

-void ofmem_arch_early_map_pages(ucell phys, ucell virt, ucell size, ucell mode)
+void ofmem_arch_early_map_pages(phys_addr_t phys, ucell virt, ucell size, ucell mode)
 {
     /* none yet */
 }
@@ -131,10 +131,29 @@ retain_t *ofmem_arch_get_retained(void)
     return NULL;
 }

+int ofmem_arch_get_physaddr_cellsize(void)
+{
+#ifdef CONFIG_PPC64
+    return 2;
+#else
+    return 1;
+#endif
+}
+
+int ofmem_arch_encode_physaddr(ucell *p, phys_addr_t value)
+{
+    int n = 0;
+#ifdef CONFIG_PPC64
+    p[n++] = value >> 32;
+#endif
+    p[n++] = value;
+    return n;
+}
+
 /* Return size of a single MMU package translation property entry in cells */
 int ofmem_arch_get_translation_entry_size(void)
 {
-    return 4;
+    return 3 + ofmem_arch_get_physaddr_cellsize();
 }

/* Generate translation property entry for PPC.
@@ -149,10 +168,12 @@ int ofmem_arch_get_translation_entry_size(void)
  */
 void ofmem_arch_create_translation_entry(ucell *transentry, translation_t *t)
 {
-    transentry[0] = t->virt;
-    transentry[1] = t->size;
-    transentry[2] = t->phys;
-    transentry[3] = t->mode;
+    int i = 0;
+
+    transentry[i++] = t->virt;
+    transentry[i++] = t->size;
+    i += ofmem_arch_encode_physaddr(&transentry[i], t->phys);
+    transentry[i++] = t->mode;
 }

/************************************************************************/
@@ -182,7 +203,7 @@ realloc(void *ptr, size_t size)
 /*	misc								*/
 /************************************************************************/

-ucell ofmem_arch_default_translation_mode(ucell phys)
+ucell ofmem_arch_default_translation_mode(phys_addr_t phys)
 {
     /* XXX: Guard bit not set as it should! */
     if (phys < IO_BASE)
@@ -195,10 +216,10 @@ ucell ofmem_arch_default_translation_mode(ucell phys)
 /*	page fault handler						*/
 /************************************************************************/

-static ucell
+static phys_addr_t
 ea_to_phys(ucell ea, ucell *mode)
 {
-    ucell phys;
+    phys_addr_t phys;

if (ea >= OF_CODE_START) {
         /* ROM into RAM */
@@ -221,7 +242,7 @@ ea_to_phys(ucell ea, ucell *mode)
 }

static void
-hash_page_64(ucell ea, ucell phys, ucell mode)
+hash_page_64(ucell ea, phys_addr_t phys, ucell mode)
 {
     static int next_grab_slot = 0;
     uint64_t vsid_mask, page_mask, pgidx, hash;
@@ -270,7 +291,7 @@ hash_page_64(ucell ea, ucell phys, ucell mode)
         .h = 0,
         .v = 1,

-        .rpn = (phys & ~0xfff) >> 12,
+        .rpn = (phys & ~0xfffUL) >> 12,
         .r = mode & (1 << 8) ? 1 : 0,
         .c = mode & (1 << 7) ? 1 : 0,
         .w = mode & (1 << 6) ? 1 : 0,
@@ -287,7 +308,7 @@ hash_page_64(ucell ea, ucell phys, ucell mode)
 }

static void
-hash_page_32(ucell ea, ucell phys, ucell mode)
+hash_page_32(ucell ea, phys_addr_t phys, ucell mode)
 {
 #ifndef __powerpc64__
     static int next_grab_slot = 0;
@@ -341,7 +362,7 @@ static int is_ppc64(void)
 }

/* XXX Remove these ugly constructs when legacy 64-bit support is dropped. */
-static void hash_page(unsigned long ea, unsigned long phys, ucell mode)
+static void hash_page(unsigned long ea, phys_addr_t phys, ucell mode)
 {
     if (is_ppc64())
         hash_page_64(ea, phys, mode);
@@ -354,7 +375,7 @@ dsi_exception(void)
 {
     unsigned long dar, dsisr;
     ucell mode;
-    ucell phys;
+    phys_addr_t phys;

asm volatile("mfdar %0" : "=r" (dar) : );
     asm volatile("mfdsisr %0" : "=r" (dsisr) : );
@@ -368,7 +389,7 @@ isi_exception(void)
 {
     unsigned long nip, srr1;
     ucell mode;
-    ucell phys;
+    phys_addr_t phys;

asm volatile("mfsrr0 %0" : "=r" (nip) : );
     asm volatile("mfsrr1 %0" : "=r" (srr1) : );
diff --git a/arch/sparc64/lib.c b/arch/sparc64/lib.c
index 03dc754..1a48649 100644
--- a/arch/sparc64/lib.c
+++ b/arch/sparc64/lib.c
@@ -120,7 +120,8 @@ void ofmem_walk_boot_map(translation_entry_cb cb)
 static void
 mmu_translate(void)
 {
-    ucell virt, phys, mode;
+    ucell virt, mode;
+    phys_addr_t phys;

virt = POP();

@@ -352,7 +353,7 @@ itlb_miss_handler(void)
 }

static void
-map_pages(unsigned long phys, unsigned long virt,
+map_pages(phys_addr_t phys, unsigned long virt,
    	  unsigned long size, unsigned long mode)
 {
    unsigned long tte_data, currsize;
@@ -393,7 +394,7 @@ map_pages(unsigned long phys, unsigned long virt,
     }
 }

-void ofmem_map_pages(ucell phys, ucell virt, ucell size, ucell mode)
+void ofmem_map_pages(phys_addr_t phys, ucell virt, ucell size, ucell mode)
 {
    return map_pages(phys, virt, size, mode);
 }
@@ -405,7 +406,8 @@ void ofmem_map_pages(ucell phys, ucell virt, ucell size, ucell mode)
 static void
 mmu_map(void)
 {
-    ucell virt, size, mode, phys;
+    ucell virt, size, mode;
+    phys_addr_t phys;

mode = POP();
     size = POP();
@@ -453,7 +455,7 @@ void ofmem_arch_unmap_pages(ucell virt, ucell size)
    unmap_pages(virt, size);
 }

-void ofmem_arch_early_map_pages(ucell phys, ucell virt, ucell size, ucell mode)
+void ofmem_arch_early_map_pages(phys_addr_t phys, ucell virt, ucell size, ucell mode)
 {
    if (mode & SPITFIRE_TTE_LOCKED) {
    	// install locked tlb entries now
@@ -514,7 +516,8 @@ mmu_release(void)
 static void
 mem_claim( void )
 {
-    ucell phys=-1UL, size, align;
+    ucell size, align;
+    phys_addr_t phys=-1UL;

align = POP();
     size = POP();
@@ -534,7 +537,8 @@ mem_claim( void )
 static void
 mem_release( void )
 {
-    ucell phys, size;
+    phys_addr_t phys;
+    ucell size;

size = POP();
     phys = POP();
@@ -548,7 +552,8 @@ mem_release( void )
 static void
 mem_retain ( void )
 {
-    ucell phys=-1UL, size, align;
+    ucell size, align;
+    phys_addr_t phys=-1UL;

align = POP();
     size = POP();
diff --git a/arch/sparc64/ofmem_sparc64.c b/arch/sparc64/ofmem_sparc64.c
index 7c05c40..4d13038 100644
--- a/arch/sparc64/ofmem_sparc64.c
+++ b/arch/sparc64/ofmem_sparc64.c
@@ -95,7 +95,18 @@ void ofmem_arch_create_translation_entry(ucell *transentry, translation_t *t)
 /* misc                                                                 */
 /************************************************************************/

-ucell ofmem_arch_default_translation_mode( ucell phys )
+int ofmem_arch_get_physaddr_cellsize(void)
+{
+    return 1;
+}
+
+int ofmem_arch_encode_physaddr(ucell *p, phys_addr_t value)
+{
+    p[0] = value;
+    return 1;
+}
+
+ucell ofmem_arch_default_translation_mode( phys_addr_t phys )
 {
    /* Writable, cacheable */
    /* not privileged and not locked */
@@ -109,7 +120,7 @@ ucell ofmem_arch_default_translation_mode( ucell phys )
 /* init / cleanup                                                       */
 /************************************************************************/

-static int remap_page_range( ucell phys, ucell virt, ucell size, ucell mode )
+static int remap_page_range( phys_addr_t phys, ucell virt, ucell size, ucell mode )
 {
    ofmem_claim_phys(phys, size, 0);
    ofmem_claim_virt(virt, size, 0);
diff --git a/include/libopenbios/ofmem.h b/include/libopenbios/ofmem.h
index 74f1efd..6472008 100644
--- a/include/libopenbios/ofmem.h
+++ b/include/libopenbios/ofmem.h
@@ -26,7 +26,7 @@ typedef struct alloc_desc {

typedef struct mem_range {
    struct mem_range	*next;
-	ucell			start;
+	phys_addr_t		start;			/* sizeof(phys) >= sizeof(virt), e.g SPARC32 */
    ucell			size;
 } range_t;

@@ -34,7 +34,7 @@ typedef struct trans {
    struct trans		*next;
    ucell			virt;			/* chain is sorted by virt */
    ucell			size;
-	ucell			phys;
+	phys_addr_t		phys;
    ucell			mode;
 } translation_t;

@@ -63,14 +63,16 @@ extern void*    	ofmem_arch_get_malloc_base(void);
 extern ucell    	ofmem_arch_get_heap_top(void);
 extern ucell    	ofmem_arch_get_virt_top(void);
 extern retain_t*	ofmem_arch_get_retained(void);
+extern int			ofmem_arch_get_physaddr_cellsize(void);
+extern int			ofmem_arch_encode_physaddr(ucell *p, phys_addr_t value);
 extern int 		ofmem_arch_get_translation_entry_size(void);
 extern void 		ofmem_arch_create_translation_entry(ucell *transentry, translation_t *t);
-extern ucell    	ofmem_arch_default_translation_mode( ucell phys );
-extern void     	ofmem_arch_early_map_pages(ucell phys, ucell virt, ucell size,
+extern ucell    	ofmem_arch_default_translation_mode( phys_addr_t phys );
+extern void     	ofmem_arch_early_map_pages(phys_addr_t phys, ucell virt, ucell size,
                                            ucell mode);
 extern void     	ofmem_arch_unmap_pages(ucell virt, ucell size);
 /* sparc64 uses this method */
-extern int      	ofmem_map_page_range( ucell phys, ucell virt, ucell size,
+extern int      	ofmem_map_page_range( phys_addr_t phys, ucell virt, ucell size,
                                       ucell mode );

/* malloc interface */
@@ -93,18 +95,18 @@ extern void	ofmem_init( void );
 extern void ofmem_register( phandle_t ph_memory, phandle_t ph_mmu );

extern ucell ofmem_claim( ucell addr, ucell size, ucell align );
-extern ucell ofmem_claim_phys( ucell mphys, ucell size, ucell align );
+extern phys_addr_t ofmem_claim_phys( phys_addr_t mphys, ucell size, ucell align );
 extern ucell ofmem_claim_virt( ucell mvirt, ucell size, ucell align );

-extern ucell ofmem_retain( ucell phys, ucell size, ucell align );
+extern phys_addr_t ofmem_retain( phys_addr_t phys, ucell size, ucell align );

-extern int   ofmem_map( ucell phys, ucell virt, ucell size, ucell mode );
+extern int   ofmem_map( phys_addr_t phys, ucell virt, ucell size, ucell mode );
 extern int   ofmem_unmap( ucell virt, ucell size );

extern void  ofmem_release( ucell virt, ucell size );
-extern void  ofmem_release_phys( ucell phys, ucell size );
+extern void  ofmem_release_phys( phys_addr_t phys, ucell size );
 extern void  ofmem_release_virt( ucell virt, ucell size );
-extern ucell ofmem_translate( ucell virt, ucell *ret_mode );
+extern phys_addr_t ofmem_translate( ucell virt, ucell *ret_mode );

#ifdef CONFIG_PPC
 #define PAGE_SHIFT   12
diff --git a/libopenbios/ofmem_common.c b/libopenbios/ofmem_common.c
index 64695b6..368bba5 100644
--- a/libopenbios/ofmem_common.c
+++ b/libopenbios/ofmem_common.c
@@ -49,7 +49,7 @@ print_range( range_t *r, char *str )
 {
    printk("--- Range %s ---\n", str );
    for( ; r; r=r->next )
-		printk("%08lx - %08lx\n", r->start, r->start + r->size -1 );
+		printk(FMT_plx " - " FMT_plx "\n", r->start, r->start + r->size - 1);
    printk("\n");
 }

@@ -72,7 +72,7 @@ print_trans( void )

printk("--- Translations ---\n");
    for( ; t; t=t->next )
-		printk("%08lx -> %08lx [size %lx]\n", t->virt, t->phys, t->size );
+		printk("%08lx -> " FMT_plx " [size %lx]\n", t->virt, t->phys, t->size);
    printk("\n");
 }
 #endif
@@ -246,8 +246,8 @@ static void ofmem_update_memory_available( phandle_t ph, range_t *range,
 {
    range_t *r;
    int ncells, prop_used, prop_size;
-
-	ucell start, size, *prop;
+	phys_addr_t start;
+	ucell size, *prop;

if (s_phandle_memory == 0)
    	return;
@@ -257,7 +257,8 @@ static void ofmem_update_memory_available( phandle_t ph, range_t *range,
    }

/* inverse of phys_range list could take 2 more cells for the tail */
-	prop_used = (ncells+1) * sizeof(ucell) * 2;
+	prop_used = (ncells + 1) * sizeof(ucell) *
+		(((ph == s_phandle_memory) ? ofmem_arch_get_physaddr_cellsize() : 1) + 1);

if (prop_used > *mem_prop_size) {

@@ -291,7 +292,10 @@ static void ofmem_update_memory_available( phandle_t ph, range_t *range,

size = r->start - start;
    	if (size) {
-			prop[ncells++] = start;
+			if (ph == s_phandle_memory)
+				ncells += ofmem_arch_encode_physaddr(&prop[ncells], start);
+			else
+				prop[ncells++] = start;
    		prop[ncells++] = size;
    	}
    	start = r->start + r->size;
@@ -299,7 +303,10 @@ static void ofmem_update_memory_available( phandle_t ph, range_t *range,

/* tail */
    if (start < top_address) {
-		prop[ncells++] = start;
+		if (ph == s_phandle_memory)
+			ncells += ofmem_arch_encode_physaddr(&prop[ncells], start);
+		else
+			prop[ncells++] = start;
    	prop[ncells++] = top_address - start;
    }

@@ -323,7 +330,7 @@ static void ofmem_update_translations( void )
 /* client interface                                                     */
 /************************************************************************/

-static int is_free( ucell ea, ucell size, range_t *r )
+static int is_free( phys_addr_t ea, ucell size, range_t *r )
 {
    if( size == 0 )
    	return 1;
@@ -336,7 +343,7 @@ static int is_free( ucell ea, ucell size, range_t *r )
    return 1;
 }

-static void add_entry_( ucell ea, ucell size, range_t **r )
+static void add_entry_( phys_addr_t ea, ucell size, range_t **r )
 {
    range_t *nr;

@@ -350,7 +357,7 @@ static void add_entry_( ucell ea, ucell size, range_t **r )
    *r = nr;
 }

-static int add_entry( ucell ea, ucell size, range_t **r )
+static int add_entry( phys_addr_t ea, ucell size, range_t **r )
 {
    if( !is_free( ea, size, *r ) ) {
    	OFMEM_TRACE("add_entry: range not free!\n");
@@ -380,7 +387,7 @@ static void join_ranges( range_t **rr )
    }
 }

-static void fill_range( ucell ea, ucell size, range_t **rr )
+static void fill_range( phys_addr_t ea, ucell size, range_t **rr )
 {
    add_entry_( ea, size, rr );
    join_ranges( rr );
@@ -388,9 +395,9 @@ static void fill_range( ucell ea, ucell size, range_t **rr )
 #endif

static ucell find_area( ucell align, ucell size, range_t *r,
-		ucell min, ucell max, int reverse )
+		phys_addr_t min, phys_addr_t max, int reverse )
 {
-	ucell base = min;
+	phys_addr_t base = min;
    range_t *r2;

if( (align & (align-1)) ) {
@@ -438,7 +445,7 @@ static ucell find_area( ucell align, ucell size, range_t *r,
    return -1;
 }

-static ucell ofmem_claim_phys_( ucell phys, ucell size, ucell align,
+static phys_addr_t ofmem_claim_phys_( phys_addr_t phys, ucell size, ucell align,
    	ucell min, ucell max, int reverse )
 {
    ofmem_t *ofmem = ofmem_arch_get_private();
@@ -463,9 +470,9 @@ static ucell ofmem_claim_phys_( ucell phys, ucell size, ucell align,
 }

/* if align != 0, phys is ignored. Returns -1 on error */
-ucell ofmem_claim_phys( ucell phys, ucell size, ucell align )
+phys_addr_t ofmem_claim_phys( phys_addr_t phys, ucell size, ucell align )
 {
-    OFMEM_TRACE("ofmem_claim phys=" FMT_ucellx " size=" FMT_ucellx
+    OFMEM_TRACE("ofmem_claim phys=" FMT_plx " size=" FMT_ucellx
                 " align=" FMT_ucellx "\n",
                 phys, size, align);

@@ -506,12 +513,12 @@ ucell ofmem_claim_virt( ucell virt, ucell size, ucell align )
 }

/* if align != 0, phys is ignored. Returns -1 on error */
-ucell ofmem_retain( ucell phys, ucell size, ucell align )
+phys_addr_t ofmem_retain( phys_addr_t phys, ucell size, ucell align )
 {
     retain_t *retained = ofmem_arch_get_retained();
-    ucell retain_phys;
+    phys_addr_t retain_phys;

-    OFMEM_TRACE("ofmem_retain phys=" FMT_ucellx " size=" FMT_ucellx
+    OFMEM_TRACE("ofmem_retain phys=" FMT_plx " size=" FMT_ucellx
                 " align=" FMT_ucellx "\n",
                 phys, size, align);

@@ -530,7 +537,8 @@ ucell ofmem_retain( ucell phys, ucell size, ucell align )
 ucell ofmem_claim( ucell addr, ucell size, ucell align )
 {
    ofmem_t *ofmem = ofmem_arch_get_private();
-	ucell virt, phys;
+	ucell virt;
+	phys_addr_t phys;
    ucell offs = addr & 0xfff;

OFMEM_TRACE("ofmem_claim " FMT_ucellx " " FMT_ucellx " " FMT_ucellx "\n", addr, size, align );
@@ -595,13 +603,13 @@ static void split_trans( ucell virt )
    }
 }

-int ofmem_map_page_range( ucell phys, ucell virt, ucell size, ucell mode )
+int ofmem_map_page_range( phys_addr_t phys, ucell virt, ucell size, ucell mode )
 {
    ofmem_t *ofmem = ofmem_arch_get_private();
    translation_t *t, **tt;

OFMEM_TRACE("ofmem_map_page_range " FMT_ucellx
-			" -> " FMT_ucellx " " FMT_ucellx " mode " FMT_ucellx "\n",
+			" -> " FMT_plx " " FMT_ucellx " mode " FMT_ucellx "\n",
    		virt, phys, size, mode );

split_trans( virt );
@@ -677,7 +685,7 @@ static int unmap_page_range( ucell virt, ucell size )
    		*plinkentry = t->next;

OFMEM_TRACE("unmap_page_range found "
-					FMT_ucellx " -> " FMT_ucellx " " FMT_ucellx
+					FMT_ucellx " -> " FMT_plx " " FMT_ucellx
    				" mode " FMT_ucellx "\n",
    				t->virt, t->phys, t->size, t->mode );

@@ -693,7 +701,7 @@ static int unmap_page_range( ucell virt, ucell size )
    return 0;
 }

-int ofmem_map( ucell phys, ucell virt, ucell size, ucell mode )
+int ofmem_map( phys_addr_t phys, ucell virt, ucell size, ucell mode )
 {
    /* printk("+ofmem_map: %08lX --> %08lX (size %08lX, mode 0x%02X)\n",
       virt, phys, size, mode ); */
@@ -701,7 +709,7 @@ int ofmem_map( ucell phys, ucell virt, ucell size, ucell mode )
    if( (phys & 0xfff) || (virt & 0xfff) || (size & 0xfff) ) {

OFMEM_TRACE("ofmem_map: Bad parameters ("
-				FMT_ucellX " " FMT_ucellX " " FMT_ucellX ")\n",
+				FMT_plx " " FMT_ucellX " " FMT_ucellX ")\n",
    			phys, virt, size );

phys &= ~0xfff;
@@ -751,7 +759,7 @@ int ofmem_unmap( ucell virt, ucell size )
 }

/* virtual -> physical. */
-ucell ofmem_translate( ucell virt, ucell *mode )
+phys_addr_t ofmem_translate( ucell virt, ucell *mode )
 {
    ofmem_t *ofmem = ofmem_arch_get_private();
    translation_t *t;
@@ -776,9 +784,9 @@ static void remove_range( ucell ea, ucell size, range_t **r )
 }

/* release memory allocated by ofmem_claim_phys */
-void ofmem_release_phys( ucell phys, ucell size )
+void ofmem_release_phys( phys_addr_t phys, ucell size )
 {
-    OFMEM_TRACE("ofmem_release_phys addr=" FMT_ucellx " size=" FMT_ucellx "\n",
+    OFMEM_TRACE("ofmem_release_phys addr=" FMT_plx " size=" FMT_ucellx "\n",
                 phys, size);

ofmem_t *ofmem = ofmem_arch_get_private();
@@ -802,8 +810,8 @@ void ofmem_release( ucell virt, ucell size )
                 __func__, virt, size);

ucell mode;
-    ucell phys = ofmem_translate(virt, &mode);
-    if (phys == (ucell)-1) {
+    phys_addr_t phys = ofmem_translate(virt, &mode);
+    if (phys == (phys_addr_t)-1) {
         OFMEM_TRACE("%s: no mapping\n", __func__);
         return;
     }

[PATCH 2/2] ppc: Use unsigned long for effective addresses

[Andreas Färber]

Adjust the special handling of ROM-to-RAM for ppc64.

In particular this allows ISI and DSI exceptions to set up TLB entries for
high addresses on ppc64.

Signed-off-by: Andreas Färber 
---
 arch/ppc/qemu/ofmem.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/ppc/qemu/ofmem.c b/arch/ppc/qemu/ofmem.c
index 2892d54..c21a112 100644
--- a/arch/ppc/qemu/ofmem.c
+++ b/arch/ppc/qemu/ofmem.c
@@ -47,7 +47,7 @@ extern void setup_mmu(unsigned long code_base);
 #define FREE_BASE		0x00004000UL
 #define OF_CODE_START	0xfff00000UL
 #define OF_CODE_SIZE    0x00100000
-#define IO_BASE			0x80000000
+#define IO_BASE			0x80000000UL

#ifdef __powerpc64__
 #define HASH_BITS		18
@@ -85,7 +85,7 @@ get_ram_bottom(void)
     return FREE_BASE;
 }

-static ucell get_heap_top(void)
+static unsigned long get_heap_top(void)
 {
     return get_hash_base() - (32 + 64 + 64) * 1024;
 }
@@ -217,11 +217,11 @@ ucell ofmem_arch_default_translation_mode(phys_addr_t phys)
 /************************************************************************/

static phys_addr_t
-ea_to_phys(ucell ea, ucell *mode)
+ea_to_phys(unsigned long ea, ucell *mode)
 {
     phys_addr_t phys;

-    if (ea >= OF_CODE_START) {
+    if (ea >= OF_CODE_START && ea <= 0xffffffffUL) {
         /* ROM into RAM */
         ea -= OF_CODE_START;
         phys = get_rom_base() + ea;
@@ -242,7 +242,7 @@ ea_to_phys(ucell ea, ucell *mode)
 }

static void
-hash_page_64(ucell ea, phys_addr_t phys, ucell mode)
+hash_page_64(unsigned long ea, phys_addr_t phys, ucell mode)
 {
     static int next_grab_slot = 0;
     uint64_t vsid_mask, page_mask, pgidx, hash;
@@ -308,7 +308,7 @@ hash_page_64(ucell ea, phys_addr_t phys, ucell mode)
 }

static void
-hash_page_32(ucell ea, phys_addr_t phys, ucell mode)
+hash_page_32(unsigned long ea, phys_addr_t phys, ucell mode)
 {
 #ifndef __powerpc64__
     static int next_grab_slot = 0;

[PATCH 2/2] ppc: Use unsigned long for effective addresses

[Andreas Färber]

Am 05.12.2010 um 19:08 schrieb Andreas Färber:
> Adjust the special handling of ROM-to-RAM for ppc64.
>
> In particular this allows ISI and DSI exceptions to set up TLB  
> entries for
> high addresses on ppc64.
>
> Signed-off-by: Andreas Färber

Applied as r982.

[PATCH v3 1/2] ofmem: Use phys_addr_t for physical addresses

[Mark Cave-Ayland]

Hi Andreas,

Just some general comments:

Andreas Färber wrote:
> @@ -257,7 +257,8 @@ static void ofmem_update_memory_available( phandle_t ph, range_t *range,
>    }
>
> /* inverse of phys_range list could take 2 more cells for the tail */
> prop_used = (ncells+1) * sizeof(ucell) * 2;
> prop_used = (ncells + 1) * sizeof(ucell) *
> (((ph == s_phandle_memory) ? ofmem_arch_get_physaddr_cellsize() : 1) + 1);

I think a comment here explaining that physical address size can be >= 
virtual address size would help readability here.
> @@ -291,7 +292,10 @@ static void ofmem_update_memory_available( phandle_t ph, range_t *range,
> size = r->start - start;
> if (size) {
> 	prop[ncells++] = start;
> 	if (ph == s_phandle_memory)

Add some braces to the if() to emulate QEMU style (even if it's a single 
line), plus add a comment e.g. /* For physical addresses */
> 		ncells += ofmem_arch_encode_physaddr(&prop[ncells], start);
> 	else

Same here, except comment should read e.g. /* For virtual addresses */
> 		prop[ncells++] = start;
> prop[ncells++] = size;
> }
> start = r->start + r->size;
>
> @@ -299,7 +303,10 @@ static void ofmem_update_memory_available( phandle_t ph, range_t *range,
>
> /* tail */
>    if (start < top_address) {
> prop[ncells++] = start;
> if (ph == s_phandle_memory)

Same here.
> 	ncells += ofmem_arch_encode_physaddr(&prop[ncells], start);
> else

And here.
> 	prop[ncells++] = start;
> prop[ncells++] = top_address - start;
> }

Other than that, it looks good to me. As long as there are no 
regressions, I'm happy for commit.

ATB,

Mark.

[PATCH v4 1/2] ofmem: Use phys_addr_t for physical addresses

[Andreas Färber]

Based on Mark's previous work, let ofmem handle addresses wider than one cell.
This is based on the assumption that sizeof(phys_addr_t) >= sizeof(ucell).
Affected are in particular sparc32 (36 bits) and ppc64 (64 bits).

As a consequence, some range_t related code shared with virtual/effective
addresses needed to be migrated, too. Since both address types are unsigned
this should be okay.

v4:
* Coding style fixes and comments, as suggested by Mark.

v3:
* Rebased: Prefer FMT_plx for trace output.
* Convert some more places to phys_addr_t to avoid breakage on ppc64.
* Add helpers ofmem_arch_{get_physaddr_cellsize,encode_physaddr}(),
  requested by Mark.

v2:
* Use FMT_physaddr_t in ofmem trace code. (by Mark)

Cc: Mark Cave-Ayland 
Signed-off-by: Andreas Färber 
---
 arch/ppc/qemu/ofmem.c        |   51 +++++++++++++++++++--------
 arch/sparc64/lib.c           |   21 +++++++----
 arch/sparc64/ofmem_sparc64.c |   15 +++++++-
 include/libopenbios/ofmem.h  |   22 ++++++-----
 libopenbios/ofmem_common.c   |   77 +++++++++++++++++++++++++-----------------
 5 files changed, 120 insertions(+), 66 deletions(-)

diff --git a/arch/ppc/qemu/ofmem.c b/arch/ppc/qemu/ofmem.c
index c62e42c..2892d54 100644
--- a/arch/ppc/qemu/ofmem.c
+++ b/arch/ppc/qemu/ofmem.c
@@ -120,7 +120,7 @@ void ofmem_arch_unmap_pages(ucell virt, ucell size)
     /* kill page mappings in provided range */
 }

-void ofmem_arch_early_map_pages(ucell phys, ucell virt, ucell size, ucell mode)
+void ofmem_arch_early_map_pages(phys_addr_t phys, ucell virt, ucell size, ucell mode)
 {
     /* none yet */
 }
@@ -131,10 +131,29 @@ retain_t *ofmem_arch_get_retained(void)
     return NULL;
 }

+int ofmem_arch_get_physaddr_cellsize(void)
+{
+#ifdef CONFIG_PPC64
+    return 2;
+#else
+    return 1;
+#endif
+}
+
+int ofmem_arch_encode_physaddr(ucell *p, phys_addr_t value)
+{
+    int n = 0;
+#ifdef CONFIG_PPC64
+    p[n++] = value >> 32;
+#endif
+    p[n++] = value;
+    return n;
+}
+
 /* Return size of a single MMU package translation property entry in cells */
 int ofmem_arch_get_translation_entry_size(void)
 {
-    return 4;
+    return 3 + ofmem_arch_get_physaddr_cellsize();
 }

/* Generate translation property entry for PPC.
@@ -149,10 +168,12 @@ int ofmem_arch_get_translation_entry_size(void)
  */
 void ofmem_arch_create_translation_entry(ucell *transentry, translation_t *t)
 {
-    transentry[0] = t->virt;
-    transentry[1] = t->size;
-    transentry[2] = t->phys;
-    transentry[3] = t->mode;
+    int i = 0;
+
+    transentry[i++] = t->virt;
+    transentry[i++] = t->size;
+    i += ofmem_arch_encode_physaddr(&transentry[i], t->phys);
+    transentry[i++] = t->mode;
 }

/************************************************************************/
@@ -182,7 +203,7 @@ realloc(void *ptr, size_t size)
 /*	misc								*/
 /************************************************************************/

-ucell ofmem_arch_default_translation_mode(ucell phys)
+ucell ofmem_arch_default_translation_mode(phys_addr_t phys)
 {
     /* XXX: Guard bit not set as it should! */
     if (phys < IO_BASE)
@@ -195,10 +216,10 @@ ucell ofmem_arch_default_translation_mode(ucell phys)
 /*	page fault handler						*/
 /************************************************************************/

-static ucell
+static phys_addr_t
 ea_to_phys(ucell ea, ucell *mode)
 {
-    ucell phys;
+    phys_addr_t phys;

if (ea >= OF_CODE_START) {
         /* ROM into RAM */
@@ -221,7 +242,7 @@ ea_to_phys(ucell ea, ucell *mode)
 }

static void
-hash_page_64(ucell ea, ucell phys, ucell mode)
+hash_page_64(ucell ea, phys_addr_t phys, ucell mode)
 {
     static int next_grab_slot = 0;
     uint64_t vsid_mask, page_mask, pgidx, hash;
@@ -270,7 +291,7 @@ hash_page_64(ucell ea, ucell phys, ucell mode)
         .h = 0,
         .v = 1,

-        .rpn = (phys & ~0xfff) >> 12,
+        .rpn = (phys & ~0xfffUL) >> 12,
         .r = mode & (1 << 8) ? 1 : 0,
         .c = mode & (1 << 7) ? 1 : 0,
         .w = mode & (1 << 6) ? 1 : 0,
@@ -287,7 +308,7 @@ hash_page_64(ucell ea, ucell phys, ucell mode)
 }

static void
-hash_page_32(ucell ea, ucell phys, ucell mode)
+hash_page_32(ucell ea, phys_addr_t phys, ucell mode)
 {
 #ifndef __powerpc64__
     static int next_grab_slot = 0;
@@ -341,7 +362,7 @@ static int is_ppc64(void)
 }

/* XXX Remove these ugly constructs when legacy 64-bit support is dropped. */
-static void hash_page(unsigned long ea, unsigned long phys, ucell mode)
+static void hash_page(unsigned long ea, phys_addr_t phys, ucell mode)
 {
     if (is_ppc64())
         hash_page_64(ea, phys, mode);
@@ -354,7 +375,7 @@ dsi_exception(void)
 {
     unsigned long dar, dsisr;
     ucell mode;
-    ucell phys;
+    phys_addr_t phys;

asm volatile("mfdar %0" : "=r" (dar) : );
     asm volatile("mfdsisr %0" : "=r" (dsisr) : );
@@ -368,7 +389,7 @@ isi_exception(void)
 {
     unsigned long nip, srr1;
     ucell mode;
-    ucell phys;
+    phys_addr_t phys;

asm volatile("mfsrr0 %0" : "=r" (nip) : );
     asm volatile("mfsrr1 %0" : "=r" (srr1) : );
diff --git a/arch/sparc64/lib.c b/arch/sparc64/lib.c
index 03dc754..1a48649 100644
--- a/arch/sparc64/lib.c
+++ b/arch/sparc64/lib.c
@@ -120,7 +120,8 @@ void ofmem_walk_boot_map(translation_entry_cb cb)
 static void
 mmu_translate(void)
 {
-    ucell virt, phys, mode;
+    ucell virt, mode;
+    phys_addr_t phys;

virt = POP();

@@ -352,7 +353,7 @@ itlb_miss_handler(void)
 }

static void
-map_pages(unsigned long phys, unsigned long virt,
+map_pages(phys_addr_t phys, unsigned long virt,
    	  unsigned long size, unsigned long mode)
 {
    unsigned long tte_data, currsize;
@@ -393,7 +394,7 @@ map_pages(unsigned long phys, unsigned long virt,
     }
 }

-void ofmem_map_pages(ucell phys, ucell virt, ucell size, ucell mode)
+void ofmem_map_pages(phys_addr_t phys, ucell virt, ucell size, ucell mode)
 {
    return map_pages(phys, virt, size, mode);
 }
@@ -405,7 +406,8 @@ void ofmem_map_pages(ucell phys, ucell virt, ucell size, ucell mode)
 static void
 mmu_map(void)
 {
-    ucell virt, size, mode, phys;
+    ucell virt, size, mode;
+    phys_addr_t phys;

mode = POP();
     size = POP();
@@ -453,7 +455,7 @@ void ofmem_arch_unmap_pages(ucell virt, ucell size)
    unmap_pages(virt, size);
 }

-void ofmem_arch_early_map_pages(ucell phys, ucell virt, ucell size, ucell mode)
+void ofmem_arch_early_map_pages(phys_addr_t phys, ucell virt, ucell size, ucell mode)
 {
    if (mode & SPITFIRE_TTE_LOCKED) {
    	// install locked tlb entries now
@@ -514,7 +516,8 @@ mmu_release(void)
 static void
 mem_claim( void )
 {
-    ucell phys=-1UL, size, align;
+    ucell size, align;
+    phys_addr_t phys=-1UL;

align = POP();
     size = POP();
@@ -534,7 +537,8 @@ mem_claim( void )
 static void
 mem_release( void )
 {
-    ucell phys, size;
+    phys_addr_t phys;
+    ucell size;

size = POP();
     phys = POP();
@@ -548,7 +552,8 @@ mem_release( void )
 static void
 mem_retain ( void )
 {
-    ucell phys=-1UL, size, align;
+    ucell size, align;
+    phys_addr_t phys=-1UL;

align = POP();
     size = POP();
diff --git a/arch/sparc64/ofmem_sparc64.c b/arch/sparc64/ofmem_sparc64.c
index 7c05c40..4d13038 100644
--- a/arch/sparc64/ofmem_sparc64.c
+++ b/arch/sparc64/ofmem_sparc64.c
@@ -95,7 +95,18 @@ void ofmem_arch_create_translation_entry(ucell *transentry, translation_t *t)
 /* misc                                                                 */
 /************************************************************************/

-ucell ofmem_arch_default_translation_mode( ucell phys )
+int ofmem_arch_get_physaddr_cellsize(void)
+{
+    return 1;
+}
+
+int ofmem_arch_encode_physaddr(ucell *p, phys_addr_t value)
+{
+    p[0] = value;
+    return 1;
+}
+
+ucell ofmem_arch_default_translation_mode( phys_addr_t phys )
 {
    /* Writable, cacheable */
    /* not privileged and not locked */
@@ -109,7 +120,7 @@ ucell ofmem_arch_default_translation_mode( ucell phys )
 /* init / cleanup                                                       */
 /************************************************************************/

-static int remap_page_range( ucell phys, ucell virt, ucell size, ucell mode )
+static int remap_page_range( phys_addr_t phys, ucell virt, ucell size, ucell mode )
 {
    ofmem_claim_phys(phys, size, 0);
    ofmem_claim_virt(virt, size, 0);
diff --git a/include/libopenbios/ofmem.h b/include/libopenbios/ofmem.h
index 74f1efd..6472008 100644
--- a/include/libopenbios/ofmem.h
+++ b/include/libopenbios/ofmem.h
@@ -26,7 +26,7 @@ typedef struct alloc_desc {

typedef struct mem_range {
    struct mem_range	*next;
-	ucell			start;
+	phys_addr_t		start;			/* sizeof(phys) >= sizeof(virt), e.g SPARC32 */
    ucell			size;
 } range_t;

@@ -34,7 +34,7 @@ typedef struct trans {
    struct trans		*next;
    ucell			virt;			/* chain is sorted by virt */
    ucell			size;
-	ucell			phys;
+	phys_addr_t		phys;
    ucell			mode;
 } translation_t;

@@ -63,14 +63,16 @@ extern void*    	ofmem_arch_get_malloc_base(void);
 extern ucell    	ofmem_arch_get_heap_top(void);
 extern ucell    	ofmem_arch_get_virt_top(void);
 extern retain_t*	ofmem_arch_get_retained(void);
+extern int			ofmem_arch_get_physaddr_cellsize(void);
+extern int			ofmem_arch_encode_physaddr(ucell *p, phys_addr_t value);
 extern int 		ofmem_arch_get_translation_entry_size(void);
 extern void 		ofmem_arch_create_translation_entry(ucell *transentry, translation_t *t);
-extern ucell    	ofmem_arch_default_translation_mode( ucell phys );
-extern void     	ofmem_arch_early_map_pages(ucell phys, ucell virt, ucell size,
+extern ucell    	ofmem_arch_default_translation_mode( phys_addr_t phys );
+extern void     	ofmem_arch_early_map_pages(phys_addr_t phys, ucell virt, ucell size,
                                            ucell mode);
 extern void     	ofmem_arch_unmap_pages(ucell virt, ucell size);
 /* sparc64 uses this method */
-extern int      	ofmem_map_page_range( ucell phys, ucell virt, ucell size,
+extern int      	ofmem_map_page_range( phys_addr_t phys, ucell virt, ucell size,
                                       ucell mode );

/* malloc interface */
@@ -93,18 +95,18 @@ extern void	ofmem_init( void );
 extern void ofmem_register( phandle_t ph_memory, phandle_t ph_mmu );

extern ucell ofmem_claim( ucell addr, ucell size, ucell align );
-extern ucell ofmem_claim_phys( ucell mphys, ucell size, ucell align );
+extern phys_addr_t ofmem_claim_phys( phys_addr_t mphys, ucell size, ucell align );
 extern ucell ofmem_claim_virt( ucell mvirt, ucell size, ucell align );

-extern ucell ofmem_retain( ucell phys, ucell size, ucell align );
+extern phys_addr_t ofmem_retain( phys_addr_t phys, ucell size, ucell align );

-extern int   ofmem_map( ucell phys, ucell virt, ucell size, ucell mode );
+extern int   ofmem_map( phys_addr_t phys, ucell virt, ucell size, ucell mode );
 extern int   ofmem_unmap( ucell virt, ucell size );

extern void  ofmem_release( ucell virt, ucell size );
-extern void  ofmem_release_phys( ucell phys, ucell size );
+extern void  ofmem_release_phys( phys_addr_t phys, ucell size );
 extern void  ofmem_release_virt( ucell virt, ucell size );
-extern ucell ofmem_translate( ucell virt, ucell *ret_mode );
+extern phys_addr_t ofmem_translate( ucell virt, ucell *ret_mode );

#ifdef CONFIG_PPC
 #define PAGE_SHIFT   12
diff --git a/libopenbios/ofmem_common.c b/libopenbios/ofmem_common.c
index 64695b6..20a4cba 100644
--- a/libopenbios/ofmem_common.c
+++ b/libopenbios/ofmem_common.c
@@ -49,7 +49,7 @@ print_range( range_t *r, char *str )
 {
    printk("--- Range %s ---\n", str );
    for( ; r; r=r->next )
-		printk("%08lx - %08lx\n", r->start, r->start + r->size -1 );
+		printk(FMT_plx " - " FMT_plx "\n", r->start, r->start + r->size - 1);
    printk("\n");
 }

@@ -72,7 +72,7 @@ print_trans( void )

printk("--- Translations ---\n");
    for( ; t; t=t->next )
-		printk("%08lx -> %08lx [size %lx]\n", t->virt, t->phys, t->size );
+		printk("%08lx -> " FMT_plx " [size %lx]\n", t->virt, t->phys, t->size);
    printk("\n");
 }
 #endif
@@ -246,8 +246,8 @@ static void ofmem_update_memory_available( phandle_t ph, range_t *range,
 {
    range_t *r;
    int ncells, prop_used, prop_size;
-
-	ucell start, size, *prop;
+	phys_addr_t start;
+	ucell size, *prop;

if (s_phandle_memory == 0)
    	return;
@@ -256,8 +256,10 @@ static void ofmem_update_memory_available( phandle_t ph, range_t *range,
    for( r = range, ncells = 0; r ; r=r->next, ncells++ ) {
    }

-	/* inverse of phys_range list could take 2 more cells for the tail */
-	prop_used = (ncells+1) * sizeof(ucell) * 2;
+	/* inverse of phys_range list could take 2 or more additional cells for the tail
+	   For /memory, physical addresses may be wider than one ucell. */
+	prop_used = (ncells + 1) * sizeof(ucell) *
+		(((ph == s_phandle_memory) ? ofmem_arch_get_physaddr_cellsize() : 1) + 1);

if (prop_used > *mem_prop_size) {

@@ -291,7 +293,13 @@ static void ofmem_update_memory_available( phandle_t ph, range_t *range,

size = r->start - start;
    	if (size) {
-			prop[ncells++] = start;
+			if (ph == s_phandle_memory) {
+				/* physical address for /memory */
+				ncells += ofmem_arch_encode_physaddr(&prop[ncells], start);
+			} else {
+				/* virtual address for MMU */
+				prop[ncells++] = start;
+			}
    		prop[ncells++] = size;
    	}
    	start = r->start + r->size;
@@ -299,7 +307,13 @@ static void ofmem_update_memory_available( phandle_t ph, range_t *range,

/* tail */
    if (start < top_address) {
-		prop[ncells++] = start;
+		if (ph == s_phandle_memory) {
+			/* physical address for /memory */
+			ncells += ofmem_arch_encode_physaddr(&prop[ncells], start);
+		} else {
+			/* virtual address for MMU */
+			prop[ncells++] = start;
+		}
    	prop[ncells++] = top_address - start;
    }

@@ -323,7 +337,7 @@ static void ofmem_update_translations( void )
 /* client interface                                                     */
 /************************************************************************/

-static int is_free( ucell ea, ucell size, range_t *r )
+static int is_free( phys_addr_t ea, ucell size, range_t *r )
 {
    if( size == 0 )
    	return 1;
@@ -336,7 +350,7 @@ static int is_free( ucell ea, ucell size, range_t *r )
    return 1;
 }

-static void add_entry_( ucell ea, ucell size, range_t **r )
+static void add_entry_( phys_addr_t ea, ucell size, range_t **r )
 {
    range_t *nr;

@@ -350,7 +364,7 @@ static void add_entry_( ucell ea, ucell size, range_t **r )
    *r = nr;
 }

-static int add_entry( ucell ea, ucell size, range_t **r )
+static int add_entry( phys_addr_t ea, ucell size, range_t **r )
 {
    if( !is_free( ea, size, *r ) ) {
    	OFMEM_TRACE("add_entry: range not free!\n");
@@ -380,7 +394,7 @@ static void join_ranges( range_t **rr )
    }
 }

-static void fill_range( ucell ea, ucell size, range_t **rr )
+static void fill_range( phys_addr_t ea, ucell size, range_t **rr )
 {
    add_entry_( ea, size, rr );
    join_ranges( rr );
@@ -388,9 +402,9 @@ static void fill_range( ucell ea, ucell size, range_t **rr )
 #endif

static ucell find_area( ucell align, ucell size, range_t *r,
-		ucell min, ucell max, int reverse )
+		phys_addr_t min, phys_addr_t max, int reverse )
 {
-	ucell base = min;
+	phys_addr_t base = min;
    range_t *r2;

if( (align & (align-1)) ) {
@@ -438,7 +452,7 @@ static ucell find_area( ucell align, ucell size, range_t *r,
    return -1;
 }

-static ucell ofmem_claim_phys_( ucell phys, ucell size, ucell align,
+static phys_addr_t ofmem_claim_phys_( phys_addr_t phys, ucell size, ucell align,
    	ucell min, ucell max, int reverse )
 {
    ofmem_t *ofmem = ofmem_arch_get_private();
@@ -463,9 +477,9 @@ static ucell ofmem_claim_phys_( ucell phys, ucell size, ucell align,
 }

/* if align != 0, phys is ignored. Returns -1 on error */
-ucell ofmem_claim_phys( ucell phys, ucell size, ucell align )
+phys_addr_t ofmem_claim_phys( phys_addr_t phys, ucell size, ucell align )
 {
-    OFMEM_TRACE("ofmem_claim phys=" FMT_ucellx " size=" FMT_ucellx
+    OFMEM_TRACE("ofmem_claim phys=" FMT_plx " size=" FMT_ucellx
                 " align=" FMT_ucellx "\n",
                 phys, size, align);

@@ -506,12 +520,12 @@ ucell ofmem_claim_virt( ucell virt, ucell size, ucell align )
 }

/* if align != 0, phys is ignored. Returns -1 on error */
-ucell ofmem_retain( ucell phys, ucell size, ucell align )
+phys_addr_t ofmem_retain( phys_addr_t phys, ucell size, ucell align )
 {
     retain_t *retained = ofmem_arch_get_retained();
-    ucell retain_phys;
+    phys_addr_t retain_phys;

-    OFMEM_TRACE("ofmem_retain phys=" FMT_ucellx " size=" FMT_ucellx
+    OFMEM_TRACE("ofmem_retain phys=" FMT_plx " size=" FMT_ucellx
                 " align=" FMT_ucellx "\n",
                 phys, size, align);

@@ -530,7 +544,8 @@ ucell ofmem_retain( ucell phys, ucell size, ucell align )
 ucell ofmem_claim( ucell addr, ucell size, ucell align )
 {
    ofmem_t *ofmem = ofmem_arch_get_private();
-	ucell virt, phys;
+	ucell virt;
+	phys_addr_t phys;
    ucell offs = addr & 0xfff;

OFMEM_TRACE("ofmem_claim " FMT_ucellx " " FMT_ucellx " " FMT_ucellx "\n", addr, size, align );
@@ -595,13 +610,13 @@ static void split_trans( ucell virt )
    }
 }

-int ofmem_map_page_range( ucell phys, ucell virt, ucell size, ucell mode )
+int ofmem_map_page_range( phys_addr_t phys, ucell virt, ucell size, ucell mode )
 {
    ofmem_t *ofmem = ofmem_arch_get_private();
    translation_t *t, **tt;

OFMEM_TRACE("ofmem_map_page_range " FMT_ucellx
-			" -> " FMT_ucellx " " FMT_ucellx " mode " FMT_ucellx "\n",
+			" -> " FMT_plx " " FMT_ucellx " mode " FMT_ucellx "\n",
    		virt, phys, size, mode );

split_trans( virt );
@@ -677,7 +692,7 @@ static int unmap_page_range( ucell virt, ucell size )
    		*plinkentry = t->next;

OFMEM_TRACE("unmap_page_range found "
-					FMT_ucellx " -> " FMT_ucellx " " FMT_ucellx
+					FMT_ucellx " -> " FMT_plx " " FMT_ucellx
    				" mode " FMT_ucellx "\n",
    				t->virt, t->phys, t->size, t->mode );

@@ -693,7 +708,7 @@ static int unmap_page_range( ucell virt, ucell size )
    return 0;
 }

-int ofmem_map( ucell phys, ucell virt, ucell size, ucell mode )
+int ofmem_map( phys_addr_t phys, ucell virt, ucell size, ucell mode )
 {
    /* printk("+ofmem_map: %08lX --> %08lX (size %08lX, mode 0x%02X)\n",
       virt, phys, size, mode ); */
@@ -701,7 +716,7 @@ int ofmem_map( ucell phys, ucell virt, ucell size, ucell mode )
    if( (phys & 0xfff) || (virt & 0xfff) || (size & 0xfff) ) {

OFMEM_TRACE("ofmem_map: Bad parameters ("
-				FMT_ucellX " " FMT_ucellX " " FMT_ucellX ")\n",
+				FMT_plx " " FMT_ucellX " " FMT_ucellX ")\n",
    			phys, virt, size );

phys &= ~0xfff;
@@ -751,7 +766,7 @@ int ofmem_unmap( ucell virt, ucell size )
 }

/* virtual -> physical. */
-ucell ofmem_translate( ucell virt, ucell *mode )
+phys_addr_t ofmem_translate( ucell virt, ucell *mode )
 {
    ofmem_t *ofmem = ofmem_arch_get_private();
    translation_t *t;
@@ -776,9 +791,9 @@ static void remove_range( ucell ea, ucell size, range_t **r )
 }

/* release memory allocated by ofmem_claim_phys */
-void ofmem_release_phys( ucell phys, ucell size )
+void ofmem_release_phys( phys_addr_t phys, ucell size )
 {
-    OFMEM_TRACE("ofmem_release_phys addr=" FMT_ucellx " size=" FMT_ucellx "\n",
+    OFMEM_TRACE("ofmem_release_phys addr=" FMT_plx " size=" FMT_ucellx "\n",
                 phys, size);

ofmem_t *ofmem = ofmem_arch_get_private();
@@ -802,8 +817,8 @@ void ofmem_release( ucell virt, ucell size )
                 __func__, virt, size);

ucell mode;
-    ucell phys = ofmem_translate(virt, &mode);
-    if (phys == (ucell)-1) {
+    phys_addr_t phys = ofmem_translate(virt, &mode);
+    if (phys == (phys_addr_t)-1) {
         OFMEM_TRACE("%s: no mapping\n", __func__);
         return;
     }

[PATCH v4 1/2] ofmem: Use phys_addr_t for physical addresses

[Mark Cave-Ayland]

Andreas Färber wrote:
> Based on Mark's previous work, let ofmem handle addresses wider than one cell.
> This is based on the assumption that sizeof(phys_addr_t) >= sizeof(ucell).
> Affected are in particular sparc32 (36 bits) and ppc64 (64 bits).
>
> As a consequence, some range_t related code shared with virtual/effective
> addresses needed to be migrated, too. Since both address types are unsigned
> this should be okay.
>
> v4:
> Coding style fixes and comments, as suggested by Mark.

Looks good to me.

Signed-off-by: Mark Cave-Ayland

ATB,

Mark.

[PATCH v4 1/2] ofmem: Use phys_addr_t for physical addresses

[Andreas Färber]

Am 09.12.2010 um 11:10 schrieb Mark Cave-Ayland:
> Andreas Färber wrote:
> > Based on Mark's previous work, let ofmem handle addresses wider  
> > than one cell.
> > This is based on the assumption that sizeof(phys_addr_t) >=  
> > sizeof(ucell).
> > Affected are in particular sparc32 (36 bits) and ppc64 (64 bits).
> > As a consequence, some range_t related code shared with virtual/ 
> > effective
> > addresses needed to be migrated, too. Since both address types are  
> > unsigned
> > this should be okay.
> > v4:
> > Coding style fixes and comments, as suggested by Mark.
>
> Looks good to me.
>
> Signed-off-by: Mark Cave-Ayland

Thanks, applied as r981.

Andreas