[RFC] Switch OFMEM to use phys_addr_t for physical addresses
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. -- Mark Cave-Ayland - Senior Technical Architect PostgreSQL - PostGIS Sirius Corporation plc - control through freedom http://www.siriusit.co.uk t: +44 870 608 0063 Sirius Labs: http://www.siriusit.co.uk/labs
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
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. -- Mark Cave-Ayland - Senior Technical Architect PostgreSQL - PostGIS Sirius Corporation plc - control through freedom http://www.siriusit.co.uk t: +44 870 608 0063 Sirius Labs: http://www.siriusit.co.uk/labs
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
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. -- Mark Cave-Ayland - Senior Technical Architect PostgreSQL - PostGIS Sirius Corporation plc - control through freedom http://www.siriusit.co.uk t: +44 870 608 0063 Sirius Labs: http://www.siriusit.co.uk/labs
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
On Sun, Nov 7, 2010 at 1:46 PM, Andreas Färber <andreas.faerber@web.de> 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!
Am 07.11.2010 um 16:07 schrieb Blue Swirl:
On Sun, Nov 7, 2010 at 1:46 PM, Andreas Färber <andreas.faerber@web.de> 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
On Sun, Nov 7, 2010 at 3:37 PM, Andreas Färber <andreas.faerber@web.de> wrote:
Am 07.11.2010 um 16:07 schrieb Blue Swirl:
On Sun, Nov 7, 2010 at 1:46 PM, Andreas Färber <andreas.faerber@web.de> 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.
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 <mark.cave-ayland@siriusit.co.uk> Cc: Blue Swirl <blauwirbel@gmail.com> Signed-off-by: Andreas Färber <andreas.faerber@web.de> --- 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; -- 1.7.3
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 <mark.cave-ayland@siriusit.co.uk> Cc: Blue Swirl <blauwirbel@gmail.com> Signed-off-by: Andreas Färber <andreas.faerber@web.de> ---
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
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. -- Mark Cave-Ayland - Senior Technical Architect PostgreSQL - PostGIS Sirius Corporation plc - control through freedom http://www.siriusit.co.uk t: +44 870 608 0063 Sirius Labs: http://www.siriusit.co.uk/labs
On Sun, Nov 7, 2010 at 5:39 PM, Andreas Färber <andreas.faerber@web.de> 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 <mark.cave-ayland@siriusit.co.uk> Cc: Blue Swirl <blauwirbel@gmail.com> Signed-off-by: Andreas Färber <andreas.faerber@web.de> --- 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
Am 08.11.2010 um 19:20 schrieb Blue Swirl:
On Sun, Nov 7, 2010 at 5:39 PM, Andreas Färber <andreas.faerber@web.de> 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 <mark.cave-ayland@siriusit.co.uk> Cc: Blue Swirl <blauwirbel@gmail.com> Signed-off-by: Andreas Färber <andreas.faerber@web.de> ---
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
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 <andreas.faerber@web.de
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 <mark.cave-ayland@siriusit.co.uk> Cc: Blue Swirl <blauwirbel@gmail.com> Signed-off-by: Andreas Färber <andreas.faerber@web.de> ---
Looks OK, except I'd always define PRIx64 etc. for consistency.
Done, thanks, applied in r949. Andreas
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. -- Mark Cave-Ayland - Senior Technical Architect PostgreSQL - PostGIS Sirius Corporation plc - control through freedom http://www.siriusit.co.uk t: +44 870 608 0063 Sirius Labs: http://www.siriusit.co.uk/labs
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
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
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. -- Mark Cave-Ayland - Senior Technical Architect PostgreSQL - PostGIS Sirius Corporation plc - control through freedom http://www.siriusit.co.uk t: +44 870 608 0063 Sirius Labs: http://www.siriusit.co.uk/labs
[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
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.
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 <mark.cave-ayland@siriusit.co.uk> Signed-off-by: Andreas Färber <andreas.faerber@web.de> --- 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; } -- 1.7.3
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 <andreas.faerber@web.de> --- 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; -- 1.7.3
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 <andreas.faerber@web.de>
Applied as r982.
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. -- Mark Cave-Ayland - Senior Technical Architect PostgreSQL - PostGIS Sirius Corporation plc - control through freedom http://www.siriusit.co.uk t: +44 870 608 0063 Sirius Labs: http://www.siriusit.co.uk/labs
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 <mark.cave-ayland@siriusit.co.uk> Signed-off-by: Andreas Färber <andreas.faerber@web.de> --- 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; } -- 1.7.3
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 <mark.cave-ayland@siriusit.co.uk> ATB, Mark. -- Mark Cave-Ayland - Senior Technical Architect PostgreSQL - PostGIS Sirius Corporation plc - control through freedom http://www.siriusit.co.uk t: +44 870 608 0063 Sirius Labs: http://www.siriusit.co.uk/labs
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 <mark.cave-ayland@siriusit.co.uk>
Thanks, applied as r981. Andreas
participants (3)
-
Andreas Färber -
Blue Swirl -
Mark Cave-Ayland