11:38 a.m.
A casual glance at the source code suggests that pointers returned by ofmem_malloc() are
aligned, but in fact they are not
because sizeof(alloc_desc_t) is added to the final pointer return value. Create some
additional padding after the previous value
of ofmem->next_malloc so we can store the alloc_desc_t underneath the final return
pointer in memory, thus ensuring that the
final pointer is aligned correctly.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland(a)siriusit.co.uk>
---
openbios-devel/libopenbios/ofmem_common.c | 21 +++++++++++++--------
1 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/openbios-devel/libopenbios/ofmem_common.c
b/openbios-devel/libopenbios/ofmem_common.c
index 20a4cba..a4e199c 100644
--- a/openbios-devel/libopenbios/ofmem_common.c
+++ b/openbios-devel/libopenbios/ofmem_common.c
@@ -33,6 +33,11 @@ static inline size_t ALIGN_SIZE(size_t x, size_t a)
return (x + a - 1) & ~(a-1);
}
+static inline void * ALIGN_PTR(uintptr_t x, size_t a)
+{
+ return (void *)((x + a - 1) & ~(a-1));
+}
+
static ucell get_ram_size( void )
{
ofmem_t *ofmem = ofmem_arch_get_private();
@@ -102,29 +107,29 @@ void* ofmem_malloc( size_t size )
/* waste at most 4K by taking an entry from the freelist */
if( *pp && (**pp).size < size + 0x1000 ) {
- ret = (char*)*pp + sizeof(alloc_desc_t);
- memset( ret, 0, (**pp).size - sizeof(alloc_desc_t) );
+ ret = (char *)((uintptr_t)*pp + sizeof(alloc_desc_t));
+ memset( (char *)ret, 0, (**pp).size - sizeof(alloc_desc_t) );
*pp = (**pp).next;
- return ret;
+ return (void *)ret;
}
top = ofmem_arch_get_heap_top();
- if( pointer2cell(ofmem->next_malloc) + size > top ) {
+ ret = (char *)ALIGN_PTR((uintptr_t)ofmem->next_malloc + sizeof(alloc_desc_t),
CONFIG_OFMEM_MALLOC_ALIGN);
+ if( pointer2cell((void *)ret) + size > top ) {
printk("out of malloc memory (%x)!\n", size );
return NULL;
}
- d = (alloc_desc_t*) ofmem->next_malloc;
+ d = (alloc_desc_t*)((uintptr_t)ret - sizeof(alloc_desc_t));
ofmem->next_malloc += size;
d->next = NULL;
d->size = size;
- ret = (char*)d + sizeof(alloc_desc_t);
- memset( ret, 0, size - sizeof(alloc_desc_t) );
+ memset( (char *)ret, 0, size - sizeof(alloc_desc_t) );
- return ret;
+ return (void *)ret;
}
void ofmem_free( void *ptr )
--
1.7.1
11:38 a.m.
New subject: [PATCH 2/5] Introduce ofmem_malloc_align() function for arbitrary alignment.
Rework ofmem_malloc() so that it takes a second parameter specifying the alignment for the
allocation and rename it to
ofmem_malloc_align(). Then create ofmem_malloc() as a simple wrapper function onto
ofmem_malloc_align() using a default
alignment of CONFIG_OFMEM_MALLOC_ALIGN.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland(a)siriusit.co.uk>
---
openbios-devel/include/libopenbios/ofmem.h | 1 +
openbios-devel/libopenbios/ofmem_common.c | 11 ++++++++---
2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/openbios-devel/include/libopenbios/ofmem.h
b/openbios-devel/include/libopenbios/ofmem.h
index 6472008..72247d9 100644
--- a/openbios-devel/include/libopenbios/ofmem.h
+++ b/openbios-devel/include/libopenbios/ofmem.h
@@ -76,6 +76,7 @@ extern int ofmem_map_page_range( phys_addr_t phys, ucell virt,
ucell size,
ucell mode );
/* malloc interface */
+extern void* ofmem_malloc_align( size_t size, int alignment );
extern void* ofmem_malloc( size_t size );
extern void ofmem_free( void *ptr );
extern void* ofmem_realloc( void *ptr, size_t size );
diff --git a/openbios-devel/libopenbios/ofmem_common.c
b/openbios-devel/libopenbios/ofmem_common.c
index a4e199c..e892260 100644
--- a/openbios-devel/libopenbios/ofmem_common.c
+++ b/openbios-devel/libopenbios/ofmem_common.c
@@ -86,7 +86,7 @@ print_trans( void )
/* OF private allocations */
/************************************************************************/
-void* ofmem_malloc( size_t size )
+void* ofmem_malloc_align( size_t size, int alignment )
{
ofmem_t *ofmem = ofmem_arch_get_private();
alloc_desc_t *d, **pp;
@@ -99,7 +99,7 @@ void* ofmem_malloc( size_t size )
if( !ofmem->next_malloc )
ofmem->next_malloc = (char*)ofmem_arch_get_malloc_base();
- size = ALIGN_SIZE(size + sizeof(alloc_desc_t), CONFIG_OFMEM_MALLOC_ALIGN);
+ size = ALIGN_SIZE(size + sizeof(alloc_desc_t), alignment);
/* look in the freelist */
for( pp=&ofmem->mfree; *pp && (**pp).size < size; pp =
&(**pp).next ) {
@@ -115,7 +115,7 @@ void* ofmem_malloc( size_t size )
top = ofmem_arch_get_heap_top();
- ret = (char *)ALIGN_PTR((uintptr_t)ofmem->next_malloc + sizeof(alloc_desc_t),
CONFIG_OFMEM_MALLOC_ALIGN);
+ ret = (char *)ALIGN_PTR((uintptr_t)ofmem->next_malloc + sizeof(alloc_desc_t),
alignment);
if( pointer2cell((void *)ret) + size > top ) {
printk("out of malloc memory (%x)!\n", size );
return NULL;
@@ -132,6 +132,11 @@ void* ofmem_malloc( size_t size )
return (void *)ret;
}
+void* ofmem_malloc( size_t size )
+{
+ return ofmem_malloc_align( size, CONFIG_OFMEM_MALLOC_ALIGN );
+}
+
void ofmem_free( void *ptr )
{
ofmem_t *ofmem = ofmem_arch_get_private();
--
1.7.1
10:02 p.m.
New subject: [PATCH 2/5] Introduce ofmem_malloc_align() function for arbitrary alignment.
On Tue, Dec 21, 2010 at 10:38 AM, Mark Cave-Ayland
<mark.cave-ayland(a)siriusit.co.uk> wrote:
Rework ofmem_malloc() so that it takes a second
parameter specifying the alignment for the allocation and rename it to
ofmem_malloc_align(). Then create ofmem_malloc() as a simple wrapper function onto
ofmem_malloc_align() using a default
alignment of CONFIG_OFMEM_MALLOC_ALIGN.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland(a)siriusit.co.uk>
---
openbios-devel/include/libopenbios/ofmem.h | 1 +
openbios-devel/libopenbios/ofmem_common.c | 11 ++++++++---
2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/openbios-devel/include/libopenbios/ofmem.h
b/openbios-devel/include/libopenbios/ofmem.h
index 6472008..72247d9 100644
--- a/openbios-devel/include/libopenbios/ofmem.h
+++ b/openbios-devel/include/libopenbios/ofmem.h
@@ -76,6 +76,7 @@ extern int ofmem_map_page_range( phys_addr_t phys, ucell
virt, ucell size,
ucell mode );
/* malloc interface */
+extern void* ofmem_malloc_align( size_t size, int alignment );
The standard function with this signature would be posix_memalign(),
so how about ofmem_memalign() or even ofmem_posix_memalign()?
'extern' isn't useful for function prototypes.
extern void* ofmem_malloc( size_t size );
extern void ofmem_free( void *ptr );
extern void* ofmem_realloc( void *ptr, size_t size );
diff --git a/openbios-devel/libopenbios/ofmem_common.c
b/openbios-devel/libopenbios/ofmem_common.c
index a4e199c..e892260 100644
--- a/openbios-devel/libopenbios/ofmem_common.c
+++ b/openbios-devel/libopenbios/ofmem_common.c
@@ -86,7 +86,7 @@ print_trans( void )
/* OF private allocations */
/************************************************************************/
-void* ofmem_malloc( size_t size )
+void* ofmem_malloc_align( size_t size, int alignment )
{
ofmem_t *ofmem = ofmem_arch_get_private();
alloc_desc_t *d, **pp;
@@ -99,7 +99,7 @@ void* ofmem_malloc( size_t size )
if( !ofmem->next_malloc )
ofmem->next_malloc = (char*)ofmem_arch_get_malloc_base();
- size = ALIGN_SIZE(size + sizeof(alloc_desc_t), CONFIG_OFMEM_MALLOC_ALIGN);
+ size = ALIGN_SIZE(size + sizeof(alloc_desc_t), alignment);
/* look in the freelist */
for( pp=&ofmem->mfree; *pp && (**pp).size < size; pp =
&(**pp).next ) {
@@ -115,7 +115,7 @@ void* ofmem_malloc( size_t size )
top = ofmem_arch_get_heap_top();
- ret = (char *)ALIGN_PTR((uintptr_t)ofmem->next_malloc + sizeof(alloc_desc_t),
CONFIG_OFMEM_MALLOC_ALIGN);
+ ret = (char *)ALIGN_PTR((uintptr_t)ofmem->next_malloc + sizeof(alloc_desc_t),
alignment);
if( pointer2cell((void *)ret) + size > top ) {
printk("out of malloc memory (%x)!\n", size );
return NULL;
@@ -132,6 +132,11 @@ void* ofmem_malloc( size_t size )
return (void *)ret;
}
+void* ofmem_malloc( size_t size )
+{
+ return ofmem_malloc_align( size, CONFIG_OFMEM_MALLOC_ALIGN );
+}
+
void ofmem_free( void *ptr )
{
ofmem_t *ofmem = ofmem_arch_get_private();
--
1.7.1
--
OpenBIOS http://openbios.org/
Mailinglist: http://lists.openbios.org/mailman/listinfo
Free your System - May the Forth be with you
11:28 a.m.
New subject: [PATCH 2/5] Introduce ofmem_malloc_align() function for arbitrary alignment.
Blue Swirl wrote:
ofmem_posix_memalign() seems to fit the convention better, so I'll go
with that unless anyone has any objections?
/* malloc
interface */
+extern void* ofmem_malloc_align( size_t size, int alignment );
The standard function with this signature would be posix_memalign(),
so how about ofmem_memalign() or even ofmem_posix_memalign()? 'extern' isn't useful for function
prototypes.
Yeah... again just copying the existing style of the file.
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
11:38 a.m.
New subject: [PATCH 3/5] Alter ofmem_malloc_align() so that the resulting pointer is aligned for both physical and virtual addresses.
Some uses of ofmem_malloc_align() need to ensure that the alignment requirement is for
physical as well as virtual addresses,
e.g. for MMU page tables.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland(a)siriusit.co.uk>
---
openbios-devel/include/arch/ppc/io.h | 20 ++++++++++++++++++++
openbios-devel/libopenbios/ofmem_common.c | 11 ++++++++---
2 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/openbios-devel/include/arch/ppc/io.h b/openbios-devel/include/arch/ppc/io.h
index 3449c5b..20586e4 100644
--- a/openbios-devel/include/arch/ppc/io.h
+++ b/openbios-devel/include/arch/ppc/io.h
@@ -9,6 +9,26 @@
extern char _start, _end;
extern unsigned long virt_offset;
+static inline unsigned long
+va2pa(unsigned long va)
+{
+ if ((va >= (unsigned long)&_start) &&
+ (va < (unsigned long)&_end))
+ return va - virt_offset;
+ else
+ return va;
+}
+
+static inline unsigned long
+pa2va(unsigned long pa)
+{
+ if ((pa + virt_offset >= (unsigned long)&_start) &&
+ (pa + virt_offset < (unsigned long)&_end))
+ return pa + virt_offset;
+ else
+ return pa;
+}
+
#define phys_to_virt(phys) ((void *) ((unsigned long) (phys) - virt_offset))
#define virt_to_phys(virt) ((unsigned long) (virt) + virt_offset)
diff --git a/openbios-devel/libopenbios/ofmem_common.c
b/openbios-devel/libopenbios/ofmem_common.c
index e892260..50cd7d8 100644
--- a/openbios-devel/libopenbios/ofmem_common.c
+++ b/openbios-devel/libopenbios/ofmem_common.c
@@ -33,9 +33,9 @@ static inline size_t ALIGN_SIZE(size_t x, size_t a)
return (x + a - 1) & ~(a-1);
}
-static inline void * ALIGN_PTR(uintptr_t x, size_t a)
+static inline phys_addr_t ALIGN_PTR(phys_addr_t x, size_t a)
{
- return (void *)((x + a - 1) & ~(a-1));
+ return (x + a - 1) & ~(a-1);
}
static ucell get_ram_size( void )
@@ -92,6 +92,7 @@ void* ofmem_malloc_align( size_t size, int alignment )
alloc_desc_t *d, **pp;
char *ret;
ucell top;
+ phys_addr_t pa;
if( !size )
return NULL;
@@ -115,7 +116,11 @@ void* ofmem_malloc_align( size_t size, int alignment )
top = ofmem_arch_get_heap_top();
- ret = (char *)ALIGN_PTR((uintptr_t)ofmem->next_malloc + sizeof(alloc_desc_t),
alignment);
+ /* Alignment should be on physical not virtual address */
+ pa = va2pa((uintptr_t)ofmem->next_malloc + sizeof(alloc_desc_t));
+ pa = ALIGN_PTR(pa, alignment);
+ ret = (char *)pa2va(pa);
+
if( pointer2cell((void *)ret) + size > top ) {
printk("out of malloc memory (%x)!\n", size );
return NULL;
--
1.7.1
10:06 p.m.
New subject: [PATCH 3/5] Alter ofmem_malloc_align() so that the resulting pointer is aligned for both physical and virtual addresses.
On Tue, Dec 21, 2010 at 10:38 AM, Mark Cave-Ayland
<mark.cave-ayland(a)siriusit.co.uk> wrote:
Some uses of ofmem_malloc_align() need to ensure that
the alignment requirement is for physical as well as virtual addresses,
e.g. for MMU page tables.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland(a)siriusit.co.uk>
---
openbios-devel/include/arch/ppc/io.h | 20 ++++++++++++++++++++
openbios-devel/libopenbios/ofmem_common.c | 11 ++++++++---
2 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/openbios-devel/include/arch/ppc/io.h b/openbios-devel/include/arch/ppc/io.h
index 3449c5b..20586e4 100644
--- a/openbios-devel/include/arch/ppc/io.h
+++ b/openbios-devel/include/arch/ppc/io.h
@@ -9,6 +9,26 @@
extern char _start, _end;
extern unsigned long virt_offset;
+static inline unsigned long
phys_addr_t?
+va2pa(unsigned long va)
+{
+ if ((va >= (unsigned long)&_start) &&
+ (va < (unsigned long)&_end))
+ return va - virt_offset;
+ else
+ return va;
+}
+
+static inline unsigned long
+pa2va(unsigned long pa)
phys_addr_t pa?
+{
+ if ((pa + virt_offset >= (unsigned long)&_start) &&
+ (pa + virt_offset < (unsigned long)&_end))
+ return pa + virt_offset;
+ else
+ return pa;
+}
+
#define phys_to_virt(phys) ((void *) ((unsigned long) (phys) - virt_offset))
#define virt_to_phys(virt) ((unsigned long) (virt) + virt_offset)
diff --git a/openbios-devel/libopenbios/ofmem_common.c
b/openbios-devel/libopenbios/ofmem_common.c
index e892260..50cd7d8 100644
--- a/openbios-devel/libopenbios/ofmem_common.c
+++ b/openbios-devel/libopenbios/ofmem_common.c
@@ -33,9 +33,9 @@ static inline size_t ALIGN_SIZE(size_t x, size_t a)
return (x + a - 1) & ~(a-1);
}
-static inline void * ALIGN_PTR(uintptr_t x, size_t a)
+static inline phys_addr_t ALIGN_PTR(phys_addr_t x, size_t a)
{
- return (void *)((x + a - 1) & ~(a-1));
+ return (x + a - 1) & ~(a-1);
}
static ucell get_ram_size( void )
@@ -92,6 +92,7 @@ void* ofmem_malloc_align( size_t size, int alignment )
alloc_desc_t *d, **pp;
char *ret;
ucell top;
+ phys_addr_t pa;
if( !size )
return NULL;
@@ -115,7 +116,11 @@ void* ofmem_malloc_align( size_t size, int alignment )
top = ofmem_arch_get_heap_top();
- ret = (char *)ALIGN_PTR((uintptr_t)ofmem->next_malloc + sizeof(alloc_desc_t),
alignment);
+ /* Alignment should be on physical not virtual address */
+ pa = va2pa((uintptr_t)ofmem->next_malloc + sizeof(alloc_desc_t));
+ pa = ALIGN_PTR(pa, alignment);
+ ret = (char *)pa2va(pa);
+
if( pointer2cell((void *)ret) + size > top ) {
printk("out of malloc memory (%x)!\n", size );
return NULL;
--
1.7.1
--
OpenBIOS http://openbios.org/
Mailinglist: http://lists.openbios.org/mailman/listinfo
Free your System - May the Forth be with you
11:30 a.m.
New subject: [PATCH 3/5] Alter ofmem_malloc_align() so that the resulting pointer is aligned for both physical and virtual addresses.
Blue Swirl wrote:
Currently none of the SPARC pa <-> va conversion functions use
phys_addr_t so I just copied what was already there. Perhaps it would
make sense to change these to phys_addr_t before this 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
diff --git
a/openbios-devel/include/arch/ppc/io.h b/openbios-devel/include/arch/ppc/io.h
index 3449c5b..20586e4 100644
--- a/openbios-devel/include/arch/ppc/io.h
+++ b/openbios-devel/include/arch/ppc/io.h
@@ -9,6 +9,26 @@
extern char _start, _end;
extern unsigned long virt_offset;
+static inline unsigned long
phys_addr_t?
+va2pa(unsigned long va)
+{
+ if ((va >= (unsigned long)&_start) &&
+ (va < (unsigned long)&_end))
+ return va - virt_offset;
+ else
+ return va;
+}
+
+static inline unsigned long
+pa2va(unsigned long pa)
phys_addr_t pa?
> +{
> + if ((pa + virt_offset >= (unsigned long)&_start) &&
> + (pa + virt_offset < (unsigned long)&_end))
> + return pa + virt_offset;
> + else
> + return pa;
> +} 
10:30 p.m.
New subject: [PATCH 3/5] Alter ofmem_malloc_align() so that the resulting pointer is aligned for both physical and virtual addresses.
Am 21.12.2010 um 11:38 schrieb Mark Cave-Ayland:
Some uses of ofmem_malloc_align() need to ensure that
the alignment
requirement is for physical as well as virtual addresses,
e.g. for MMU page tables.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland(a)siriusit.co.uk>
---
openbios-devel/include/arch/ppc/io.h | 20 ++++++++++++++++++++
openbios-devel/libopenbios/ofmem_common.c | 11 ++++++++---
2 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/openbios-devel/include/arch/ppc/io.h b/openbios-devel/
include/arch/ppc/io.h
index 3449c5b..20586e4 100644
--- a/openbios-devel/include/arch/ppc/io.h
+++ b/openbios-devel/include/arch/ppc/io.h
@@ -9,6 +9,26 @@
extern char _start, _end;
extern unsigned long virt_offset;
+static inline unsigned long
+va2pa(unsigned long va)
+{
+ if ((va >= (unsigned long)&_start) &&
+ (va < (unsigned long)&_end))
+ return va - virt_offset;
+ else
+ return va;
+}
+
+static inline unsigned long
+pa2va(unsigned long pa)
+{
+ if ((pa + virt_offset >= (unsigned long)&_start) &&
+ (pa + virt_offset < (unsigned long)&_end))
+ return pa + virt_offset;
+ else
+ return pa;
+}
--verbose please! What is this supposed to do? Seems like a duplicate
of ofmem.c:ea_to_phys() but different?
Andreas
+
#define phys_to_virt(phys) ((void *) ((unsigned long) (phys) -
virt_offset))
#define virt_to_phys(virt) ((unsigned long) (virt) + virt_offset)
11:38 a.m.
New subject: [PATCH 3/5] Alter ofmem_malloc_align() so that the resulting pointer is aligned for both physical and virtual addresses.
Andreas Färber wrote:
--verbose please! What is this supposed to do? Seems
like a duplicate of
ofmem.c:ea_to_phys() but different?
Andreas
It's used to convert between physical/virtual addresses. For alignment
purposes, we need to ensure that memory allocated to a specific
alignment is aligned both physically and virtually and since I couldn't
see these functions in the PPC io.h file I just added them.
From what you're saying it sounds like the proper fix would be to
rename the PPC functions ea_to_phys() (and also it's reverse mapping
function) to match the SPARC definitions so they can be called from
OFMEM. Does that sound reasonable?
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
1:53 p.m.
New subject: [PATCH 3/5] Alter ofmem_malloc_align() so that the resulting pointer is aligned for both physical and virtual addresses.
Am 22.12.2010 um 11:38 schrieb Mark Cave-Ayland:
Andreas Färber wrote:
No. The virtual address on ppc64 is something like 80 bits, whereas
the effective address (64 bits) really is what we want, similar
differences for 32-bit, so it shouldn't be renamed. It may need to be
wrapped.
I don't think it belongs in io.h either. I wouldn't know where
virt_offset is initialized, so I assume that during runtime (as
opposed to forthstrap) it will be zero and all you're doing there is
identity-mapping things, which buys us nothing.
We do have ofmem_arch_translate() or similar for the virtual-to-
physical direction. Maybe we need another ofmem_* one for the opposite
direction? sparc implementations could then call their *va* macros
from there if it suits them. The way we map OpenBIOS into RAM in
ea2phys() should guarantee physical alignment of aligned effective
addresses, so that there's no need to consider it in other functions
(this is an ugly hack, in theory OpenBIOS would need to be fully
relocatable and just copied there).
Andreas
--verbose please! What is this supposed to do?
Seems like a
duplicate of ofmem.c:ea_to_phys() but different?
Andreas
It's used to convert between physical/virtual addresses. For
alignment purposes, we need to ensure that memory allocated to a
specific alignment is aligned both physically and virtually and
since I couldn't see these functions in the PPC io.h file I just
added them.
From what you're saying it sounds like the proper fix would be to
rename the PPC functions ea_to_phys() (and also it's reverse mapping
function) to match the SPARC definitions so they can be called from
OFMEM. Does that sound reasonable?
2:07 p.m.
New subject: [PATCH 3/5] Alter ofmem_malloc_align() so that the resulting pointer is aligned for both physical and virtual addresses.
Andreas Färber wrote:
For SPARC, the equivalent is va_shift which is calculated at the very
beginning of arch/<foo>/entry.S (i.e. just after the MMU is enabled).
Hmmm so it looks like virt_offset isn't set for PPC at all! This is
definitely where my lack of understanding the PPC code shows up.
From what
you're saying it sounds like the proper fix would be to
rename the PPC functions ea_to_phys() (and also it's reverse mapping
function) to match the SPARC definitions so they can be called from
OFMEM. Does that sound reasonable?
No. The virtual address on ppc64 is something like 80 bits, whereas the
effective address (64 bits) really is what we want, similar differences
for 32-bit, so it shouldn't be renamed. It may need to be wrapped.
I don't think it belongs in io.h either. I wouldn't know where
virt_offset is initialized, so I assume that during runtime (as opposed
to forthstrap) it will be zero and all you're doing there is
identity-mapping things, which buys us nothing. We do have ofmem_arch_translate() or similar for the
virtual-to-physical
direction. Maybe we need another ofmem_* one for the opposite direction?
sparc implementations could then call their *va* macros from there if it
suits them. The way we map OpenBIOS into RAM in ea2phys() should
guarantee physical alignment of aligned effective addresses, so that
there's no need to consider it in other functions (this is an ugly hack,
in theory OpenBIOS would need to be fully relocatable and just copied
there).
As long as we have two functions that convert from phys to virt and virt
to phys for each arch (preferably called pa2va and va2pa) then I'm
happy. Note that this is not for the alignment of OpenBIOS itself, but
for allow callers of ofmem_malloc() to request arbitrary alignment on
requested regions. This is mainly because the various IOMMU/MMU page
tables require this restriction on physical addresses at the same time
for obvious reasons.
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
12:47 p.m.
New subject: [PATCH 3/5] Alter ofmem_malloc_align() so that the resulting pointer is aligned for both physical and virtual addresses.
On 22/12/10 12:53, Andreas Färber wrote:
No. The virtual address on ppc64 is something like 80
bits, whereas the
effective address (64 bits) really is what we want, similar differences
for 32-bit, so it shouldn't be renamed. It may need to be wrapped.
I don't think it belongs in io.h either. I wouldn't know where
virt_offset is initialized, so I assume that during runtime (as opposed
to forthstrap) it will be zero and all you're doing there is
identity-mapping things, which buys us nothing.
We do have ofmem_arch_translate() or similar for the virtual-to-physical
direction. Maybe we need another ofmem_* one for the opposite direction?
sparc implementations could then call their *va* macros from there if it
suits them. The way we map OpenBIOS into RAM in ea2phys() should
guarantee physical alignment of aligned effective addresses, so that
there's no need to consider it in other functions (this is an ugly hack,
in theory OpenBIOS would need to be fully relocatable and just copied
there).
Hi Andreas,
I've had a play at using the following code in arch/ppc/qemu/ofmem.c
(compared to the original patch which placed it in io.h) and it seems to
work for me in tests here. It's based upon the code in ea2phys().
/* Private functions for mapping between physical/virtual addresses */
inline phys_addr_t
va2pa(unsigned long va)
{
if (va >= OF_CODE_START && va < OF_CODE_START + OF_CODE_SIZE) {
return (phys_addr_t)get_rom_base() - OF_CODE_START + va;
} else {
return (phys_addr_t)va;
}
}
inline unsigned long
pa2va(phys_addr_t pa)
{
if ((pa - get_rom_base() + OF_CODE_START >= OF_CODE_START) &&
(pa - get_rom_base() + OF_CODE_START < OF_CODE_START +
OF_CODE_SIZE))
return (unsigned long)pa - get_rom_base() + OF_CODE_START;
else
return (unsigned long)pa;
}
The main reason I'm trying to get a resolution for this is because I've
made some improvements based upon previous comments, and this is the
only part remaining before I feel this patchset is ready for commit.
Once this is then complete, it unblocks the SPARC32 OFMEM patchset which
would be really good to get in before the end of the holidays :)
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
11:38 a.m.
New subject: [PATCH 4/5] Introduce ofmem_arch_get_phys_top() for specifying the top of physical memory in OFMEM.
At the same time, teach OFMEM to make use of it when calculating the /memory available
property.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland(a)siriusit.co.uk>
---
openbios-devel/arch/ppc/qemu/ofmem.c | 7 +++++++
openbios-devel/arch/sparc64/ofmem_sparc64.c | 7 +++++++
openbios-devel/include/libopenbios/ofmem.h | 1 +
openbios-devel/libopenbios/ofmem_common.c | 2 +-
4 files changed, 16 insertions(+), 1 deletions(-)
diff --git a/openbios-devel/arch/ppc/qemu/ofmem.c b/openbios-devel/arch/ppc/qemu/ofmem.c
index c21a112..ef8cfc3 100644
--- a/openbios-devel/arch/ppc/qemu/ofmem.c
+++ b/openbios-devel/arch/ppc/qemu/ofmem.c
@@ -115,6 +115,13 @@ ucell ofmem_arch_get_virt_top(void)
return IO_BASE;
}
+phys_addr_t ofmem_arch_get_phys_top(void)
+{
+ ofmem_t *ofmem = ofmem_arch_get_private();
+
+ return ofmem->ramsize;
+}
+
void ofmem_arch_unmap_pages(ucell virt, ucell size)
{
/* kill page mappings in provided range */
diff --git a/openbios-devel/arch/sparc64/ofmem_sparc64.c
b/openbios-devel/arch/sparc64/ofmem_sparc64.c
index 4d13038..0ec8632 100644
--- a/openbios-devel/arch/sparc64/ofmem_sparc64.c
+++ b/openbios-devel/arch/sparc64/ofmem_sparc64.c
@@ -63,6 +63,13 @@ ucell ofmem_arch_get_virt_top(void)
return (ucell)TOP_OF_RAM;
}
+phys_addr_t ofmem_arch_get_phys_top(void)
+{
+ ofmem_t *ofmem = ofmem_arch_get_private();
+
+ return ofmem->ramsize;
+}
+
retain_t *ofmem_arch_get_retained(void)
{
/* Retained area is at the top of physical RAM */
diff --git a/openbios-devel/include/libopenbios/ofmem.h
b/openbios-devel/include/libopenbios/ofmem.h
index 72247d9..b7469c2 100644
--- a/openbios-devel/include/libopenbios/ofmem.h
+++ b/openbios-devel/include/libopenbios/ofmem.h
@@ -62,6 +62,7 @@ extern ofmem_t* ofmem_arch_get_private(void);
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 phys_addr_t ofmem_arch_get_phys_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);
diff --git a/openbios-devel/libopenbios/ofmem_common.c
b/openbios-devel/libopenbios/ofmem_common.c
index 50cd7d8..f235e98 100644
--- a/openbios-devel/libopenbios/ofmem_common.c
+++ b/openbios-devel/libopenbios/ofmem_common.c
@@ -341,7 +341,7 @@ static void ofmem_update_translations( void )
ofmem_t *ofmem = ofmem_arch_get_private();
ofmem_update_memory_available(s_phandle_memory, ofmem->phys_range,
- &phys_range_prop, &phys_range_prop_size, &phys_range_prop_used,
get_ram_size());
+ &phys_range_prop, &phys_range_prop_size, &phys_range_prop_used,
ofmem_arch_get_phys_top());
ofmem_update_memory_available(s_phandle_mmu, ofmem->virt_range,
&virt_range_prop, &virt_range_prop_size, &virt_range_prop_used, -1ULL);
ofmem_update_mmu_translations();
--
1.7.1
10:27 p.m.
New subject: [PATCH 4/5] Introduce ofmem_arch_get_phys_top() for specifying the top of physical memory in OFMEM.
Am 21.12.2010 um 11:38 schrieb Mark Cave-Ayland:
At the same time, teach OFMEM to make use of it when
calculating
the /memory available property.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland(a)siriusit.co.uk>
---
openbios-devel/arch/ppc/qemu/ofmem.c | 7 +++++++
openbios-devel/arch/sparc64/ofmem_sparc64.c | 7 +++++++
openbios-devel/include/libopenbios/ofmem.h | 1 +
openbios-devel/libopenbios/ofmem_common.c | 2 +-
4 files changed, 16 insertions(+), 1 deletions(-)
diff --git a/openbios-devel/libopenbios/ofmem_common.c
b/openbios-
devel/libopenbios/ofmem_common.c
index 50cd7d8..f235e98 100644
--- a/openbios-devel/libopenbios/ofmem_common.c
+++ b/openbios-devel/libopenbios/ofmem_common.c
@@ -341,7 +341,7 @@ static void ofmem_update_translations( void )
ofmem_t *ofmem = ofmem_arch_get_private();
ofmem_update_memory_available(s_phandle_memory, ofmem->phys_range,
- &phys_range_prop, &phys_range_prop_size, &phys_range_prop_used,
get_ram_size());
+ &phys_range_prop, &phys_range_prop_size, &phys_range_prop_used,
ofmem_arch_get_phys_top());
I like this change. Is get_ram_size() still needed elsewhere?
Andreas
11:34 a.m.
New subject: [PATCH 4/5] Introduce ofmem_arch_get_phys_top() for specifying the top of physical memory in OFMEM.
Andreas Färber wrote:
I like this change. Is get_ram_size() still needed
elsewhere?
It's used in a couple of other places within OFMEM - it's more not
knowing what should actually be in the properties rather than being a
coding issue. Whichever way we find to be correct will be a really easy
change :)
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
11:38 a.m.
New subject: [PATCH 5/5] Change OFMEM to allocate memory from the top of RAM downwards.
At the moment OFMEM allocates memory from the bottom of RAM upwards, however many
loaders/kernels initialise themselves in lower
memory areas which can cause them to be overwritten by subsequent memory allocations.
Mimic the existing SPARC32 behaviour by
allocating physical and virtual memory from the top of RAM downwards.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland(a)siriusit.co.uk>
---
openbios-devel/libopenbios/ofmem_common.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/openbios-devel/libopenbios/ofmem_common.c
b/openbios-devel/libopenbios/ofmem_common.c
index f235e98..97493e6 100644
--- a/openbios-devel/libopenbios/ofmem_common.c
+++ b/openbios-devel/libopenbios/ofmem_common.c
@@ -498,7 +498,7 @@ phys_addr_t ofmem_claim_phys( phys_addr_t phys, ucell size, ucell
align )
" align=" FMT_ucellx "\n",
phys, size, align);
- return ofmem_claim_phys_( phys, size, align, 0, get_ram_size(), 0 );
+ return ofmem_claim_phys_( phys, size, align, 0, ofmem_arch_get_phys_top(), 1 );
}
static ucell ofmem_claim_virt_( ucell virt, ucell size, ucell align,
@@ -531,7 +531,7 @@ ucell ofmem_claim_virt( ucell virt, ucell size, ucell align )
/* printk("+ ofmem_claim virt %08lx %lx %ld\n", virt, size, align ); */
return ofmem_claim_virt_( virt, size, align,
- get_ram_size(), ofmem_arch_get_virt_top(), 0 );
+ get_ram_size(), ofmem_arch_get_virt_top(), 1 );
}
/* if align != 0, phys is ignored. Returns -1 on error */
--
1.7.1
10:37 p.m.
New subject: [PATCH 5/5] Change OFMEM to allocate memory from the top of RAM downwards.
Am 21.12.2010 um 11:38 schrieb Mark Cave-Ayland:
At the moment OFMEM allocates memory from the bottom
of RAM upwards,
however many loaders/kernels initialise themselves in lower
memory areas which can cause them to be overwritten by subsequent
memory allocations. Mimic the existing SPARC32 behaviour by
allocating physical and virtual memory from the top of RAM downwards.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland(a)siriusit.co.uk>
While I'm not against reversing the direction, your above statement
worries me. Our claim code should only claim free memory. So either
the respective boot loader doesn't actually claim the memory it's
using (the recent ppc bug that broke Haiku), or there's a bug in ofmem
that you're hiding by this change. Both directions should in theory
work, I believe.
Regards,
Andreas
11:49 a.m.
New subject: [PATCH 5/5] Change OFMEM to allocate memory from the top of RAM downwards.
Andreas Färber wrote:
While I'm not against reversing the direction,
your above statement
worries me. Our claim code should only claim free memory. So either the
respective boot loader doesn't actually claim the memory it's using (the
recent ppc bug that broke Haiku), or there's a bug in ofmem that you're
hiding by this change. Both directions should in theory work, I believe.
Okay - having played with this a bit further, it seems that it's the
physical memory allocations that need to be reversed in order to prevent
my Solaris 8 kernel from crashing mid-boot. Below is the log from my
OFMEM-enabled SPARC32 OpenBIOS booting Solaris 8 with physical memory
allocated from the bottom upwards:
OFMEM: ofmem_map_page_range ffe27000 -> d00000000 00001000 mode 0000003c
Configuration device id QEMU version 1 machine id 32
OFMEM: ofmem_map_page_range ffe26000 -> 010000000 00001000 mode 0000003c
OFMEM: ofmem_map_page_range ffe25000 -> 071100000 00001000 mode 0000003c
OFMEM: ofmem_map_page_range ffe24000 -> 071000000 00001000 mode 0000003c
OFMEM: ofmem_map_page_range ffe22000 -> 071200000 00002000 mode 0000003c
OFMEM: ofmem_map_page_range ffe21000 -> 071400000 00001000 mode 0000003c
OFMEM: ofmem_map_page_range ffe20000 -> 071910000 00001000 mode 0000003c
OFMEM: ofmem_map_page_range ffe1f000 -> 071f00000 00001000 mode 0000003c
OFMEM: ofmem_map_page_range ffe0e000 -> 071d00000 00011000 mode 0000003c
OFMEM: ofmem_map_page_range ffdfd000 -> 071e00000 00011000 mode 0000003c
OFMEM: ofmem_map_page_range ffdfc000 -> 007fff000 00001000 mode 0000003c
OFMEM: ofmem_map_page_range ffdfb000 -> 010001000 00001000 mode 0000003c
OFMEM: ofmem_map_page_range ffdfa000 -> 078400000 00001000 mode 0000003c
OFMEM: ofmem_map_page_range ffdf9000 -> 078800000 00001000 mode 0000003c
CPUs: 1 x FMI,MB86904
UUID: 00000000-0000-0000-0000-000000000000
Welcome to OpenBIOS v1.0 built on Dec 22 2010 01:15
Type 'help' for detailed information
Trying cdrom:d...
Not a bootable ELF image
Loading a.out image...
Loaded 7680 bytes
entry point is 0x4000
bootpath: /iommu/sbus/espdma/esp/sd@2,0:d
Jumping to entry point 00004000 for type 00000005...
switching to new context:
OFMEM: ofmem_claim phys=ffffffffffffffff size=00040000 align=00000008
OFMEM: ofmem_claim phys returned 000000000
OFMEM: ofmem_claim_virt virt=f0040000 size=00040000 align=00000000
OFMEM: ofmem_map_page_range f0040000 -> 000000000 00040000 mode 000000bc
OFMEM: ofmem_claim phys=ffffffffffffffff size=00019000 align=00000008
OFMEM: ofmem_claim phys returned 000040000
OFMEM: ofmem_claim_virt virt=f0240000 size=00019000 align=00000000
OFMEM: ofmem_map_page_range f0240000 -> 000040000 00019000 mode 000000bc
OFMEM: ofmem_claim phys=ffffffffffffffff size=00007000 align=00000008
OFMEM: ofmem_claim phys returned 000059000
OFMEM: ofmem_claim_virt virt=f0080000 size=00007000 align=00000000
OFMEM: ofmem_map_page_range f0080000 -> 000059000 00007000 mode 000000bc
OFMEM: ofmem_claim phys=ffffffffffffffff size=00001000 align=00000008
OFMEM: ofmem_claim phys returned 000060000
OFMEM: ofmem_claim_virt virt=f0087000 size=00001000 align=00000000
OFMEM: ofmem_map_page_range f0087000 -> 000060000 00001000 mode 000000bc
OFMEM: ofmem_claim phys=ffffffffffffffff size=00001000 align=00000008
OFMEM: ofmem_claim phys returned 000061000
OFMEM: ofmem_claim_virt virt=f0088000 size=00001000 align=00000000
OFMEM: ofmem_map_page_range f0088000 -> 000061000 00001000 mode 000000bc
OFMEM: ofmem_claim phys=ffffffffffffffff size=00003000 align=00000008
OFMEM: ofmem_claim phys returned 000062000
OFMEM: ofmem_claim_virt virt=f0089000 size=00003000 align=00000000
OFMEM: ofmem_map_page_range f0089000 -> 000062000 00003000 mode 000000bc
OFMEM: ofmem_claim phys=ffffffffffffffff size=00003000 align=00000008
OFMEM: ofmem_claim phys returned 000065000
OFMEM: ofmem_claim_virt virt=f008c000 size=00003000 align=00000000
OFMEM: ofmem_map_page_range f008c000 -> 000065000 00003000 mode 000000bc
OFMEM: ofmem_claim phys=ffffffffffffffff size=00001000 align=00000008
OFMEM: ofmem_claim phys returned 000068000
OFMEM: ofmem_claim_virt virt=f0259000 size=00001000 align=00000000
OFMEM: ofmem_map_page_range f0259000 -> 000068000 00001000 mode 000000bc
OFMEM: ofmem_claim phys=ffffffffffffffff size=00001000 align=00000008
OFMEM: ofmem_claim phys returned 000069000
OFMEM: ofmem_claim_virt virt=f025a000 size=00001000 align=00000000
OFMEM: ofmem_map_page_range f025a000 -> 000069000 00001000 mode 000000bc
OFMEM: ofmem_claim phys=ffffffffffffffff size=00001000 align=00000008
OFMEM: ofmem_claim phys returned 00006a000
OFMEM: ofmem_claim_virt virt=f025b000 size=00001000 align=00000000
OFMEM: ofmem_map_page_range f025b000 -> 00006a000 00001000 mode 000000bc
OFMEM: ofmem_claim phys=ffffffffffffffff size=00001000 align=00000008
OFMEM: ofmem_claim phys returned 00006b000
OFMEM: ofmem_claim_virt virt=f025c000 size=00001000 align=00000000
OFMEM: ofmem_map_page_range f025c000 -> 00006b000 00001000 mode 000000bc
OFMEM: ofmem_claim phys=ffffffffffffffff size=000c9000 align=00000008
OFMEM: ofmem_claim phys returned 00006c000
OFMEM: ofmem_claim_virt virt=f008f000 size=000c9000 align=00000000
OFMEM: ofmem_map_page_range f008f000 -> 00006c000 000c9000 mode 000000bc
OFMEM: ofmem_claim phys=ffffffffffffffff size=00018000 align=00000008
OFMEM: ofmem_claim phys returned 000135000
OFMEM: ofmem_claim_virt virt=f025d000 size=00018000 align=00000000
OFMEM: ofmem_map_page_range f025d000 -> 000135000 00018000 mode 000000bc
(hangs at this point)
The Solaris boot loader appears to be claiming specific virtual
addresses, but claiming the physical addresses from the pool. The last
physical address allocated from the pool before the crash is 0x135000
which I think may be overwriting the kernel itself as I have something
in the back of my mind that says the kernel is loaded at 0x100000 - Blue
any ideas?
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
1:26 p.m.
New subject: [PATCH 5/5] Change OFMEM to allocate memory from the top of RAM downwards.
Am 22.12.2010 um 11:49 schrieb Mark Cave-Ayland:
Andreas Färber wrote:
[...]
While I'm not against reversing the
direction, your above statement
worries me. Our claim code should only claim free memory. So either
the respective boot loader doesn't actually claim the memory it's
using (the recent ppc bug that broke Haiku), or there's a bug in
ofmem that you're hiding by this change. Both directions should in
theory work, I believe.
Okay - having played with this a bit further, it seems that it's the
physical memory allocations that need to be reversed in order to
prevent my Solaris 8 kernel from crashing mid-boot. Below is the log
from my OFMEM-enabled SPARC32 OpenBIOS booting Solaris 8 with
physical memory allocated from the bottom upwards: OFMEM: ofmem_claim phys=ffffffffffffffff size=00018000
align=00000008
OFMEM: ofmem_claim phys returned 000135000
OFMEM: ofmem_claim_virt virt=f025d000 size=00018000 align=00000000
OFMEM: ofmem_map_page_range f025d000 -> 000135000 00018000 mode
000000bc
(hangs at this point)
The Solaris boot loader appears to be claiming specific virtual
addresses, but claiming the physical addresses from the pool. The
last physical address allocated from the pool before the crash is
0x135000 which I think may be overwriting the kernel itself as I
have something in the back of my mind that says the kernel is loaded
at 0x100000 - Blue any ideas?
As a general piece of advice I would suggest to turn on debugging for
the CIF. I would assume that the client simply calls "claim", then
something might be wrong with the mapping to ofmem. Or Solaris makes
specific assumptions, which Tarl might be able to answer. Maybe you
need to claim specific parts of memory during initial setup so that
they are exempt.
Andreas
1:50 p.m.
New subject: [PATCH 5/5] Change OFMEM to allocate memory from the top of RAM downwards.
Andreas Färber wrote:
As a general piece of advice I would suggest to turn
on debugging for
the CIF. I would assume that the client simply calls "claim", then
something might be wrong with the mapping to ofmem. Or Solaris makes
specific assumptions, which Tarl might be able to answer. Maybe you need
to claim specific parts of memory during initial setup so that they are
exempt.
Okay. SPARC32 doesn't have CIF, rather it has a block of function
pointers (called romvec) which used instead. Attached is the same output
with romvec debugging enabled.
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
11:36 p.m.
New subject: [PATCH 5/5] Change OFMEM to allocate memory from the top of RAM downwards.
On 2010-12-22 7:26 AM, Andreas Färber wrote:
As a general piece of advice I would suggest to turn on debugging for the
CIF. I would assume that the client simply calls "claim", then something
might be wrong with the mapping to ofmem. Or Solaris makes specific
assumptions, which Tarl might be able to answer. Maybe you need to claim
specific parts of memory during initial setup so that they are exempt.
I'm sure Solaris makes assumptions about memory regions, but they aren't
documented anywhere I know of. OBP allocates physical memory from top
downwards, and Solaris is supposed to avoid any OBP memory allocations by
respecting the /memory and /virtual-memory nodes.
If we're running into a problem with virtual allocations, Solaris does
"know"
that OBP lives in F000.0000 and above. It expects to allocate specific virtual
addresses for itself (and 0x10.0000 does sound right for the initial kernel
address), but these should also be kept from conflicting by the
/virtual-memory properties. What happens if a specific virtual address that it
expects to use is already occupied is an interesting question.
Looking at the OFMEM trace, the claim seems to go just fine (last claim phys
was given 0x13.5000, last claim virt took 0xf025.d000), it was after the
map_page_range mapping that physical memory to virtual address 0xf025.d000
that we got stuck.
OFMEM: ofmem_claim phys=ffffffffffffffff size=00018000
align=00000008
OFMEM: ofmem_claim phys returned 000135000
OFMEM: ofmem_claim_virt virt=f025d000 size=00018000 align=00000000
OFMEM: ofmem_map_page_range f025d000 -> 000135000 00018000 mode 000000bc
That should be mapping f025.d000 through f027.5000 as virtual addresses of
physical 13.5000 through 14.d000, which would certainly be suspicious in a Sun
piece of hardware.
From 0xf000.0000 through 0xf010.000 is a copy of the OBP prom binary (starts
with "OBMD"), followed by OBP expanding itself into memory. At f020.0000 is
(again, in our systems), the uncompressed binary - I recall f020.0000 is the
actual trap vector. On current Sun systems, trying to use f025.d000 would
overwrite OBP itself, currently in the middle of fb8 video interface code.
Looking at stuff in a real system, I don't see "unoccupied" virtual
addresses
until about f040.0000 - if you can figure out who is coming up with the
addresses of f025.d000 and (worse, earlier) f008.c000 , that might help figure
out what's going on.
9:58 p.m.
New subject: [PATCH 1/5] Fix alignment of memory allocated by OFMEM's ofmem_malloc().
On Tue, Dec 21, 2010 at 10:38 AM, Mark Cave-Ayland
<mark.cave-ayland(a)siriusit.co.uk> wrote:
A casual glance at the source code suggests that
pointers returned by ofmem_malloc() are aligned, but in fact they are not
because sizeof(alloc_desc_t) is added to the final pointer return value. Create some
additional padding after the previous value
of ofmem->next_malloc so we can store the alloc_desc_t underneath the final return
pointer in memory, thus ensuring that the
final pointer is aligned correctly.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland(a)siriusit.co.uk>
---
openbios-devel/libopenbios/ofmem_common.c | 21 +++++++++++++--------
1 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/openbios-devel/libopenbios/ofmem_common.c
b/openbios-devel/libopenbios/ofmem_common.c
index 20a4cba..a4e199c 100644
--- a/openbios-devel/libopenbios/ofmem_common.c
+++ b/openbios-devel/libopenbios/ofmem_common.c
@@ -33,6 +33,11 @@ static inline size_t ALIGN_SIZE(size_t x, size_t a)
return (x + a - 1) & ~(a-1);
}
+static inline void * ALIGN_PTR(uintptr_t x, size_t a)
Uppercase name suggests a macro, please use lowercase.
+{
+ return (void *)((x + a - 1) & ~(a-1));
I'd add spaces inside a-1.
+}
+
static ucell get_ram_size( void )
{
ofmem_t *ofmem = ofmem_arch_get_private();
@@ -102,29 +107,29 @@ void* ofmem_malloc( size_t size )
/* waste at most 4K by taking an entry from the freelist */
if( *pp && (**pp).size < size + 0x1000 ) {
- ret = (char*)*pp + sizeof(alloc_desc_t);
- memset( ret, 0, (**pp).size - sizeof(alloc_desc_t) );
+ ret = (char *)((uintptr_t)*pp + sizeof(alloc_desc_t));
+ memset( (char *)ret, 0, (**pp).size - sizeof(alloc_desc_t) );
There are a lot of char* and void* casts, perhaps type of ret should
be void * to avoid some of them. At least (char *)ret shouldn't be
necessary anyway.
*pp = (**pp).next;
- return ret;
+ return (void *)ret;
}
top = ofmem_arch_get_heap_top();
- if( pointer2cell(ofmem->next_malloc) + size > top ) {
+ ret = (char *)ALIGN_PTR((uintptr_t)ofmem->next_malloc + sizeof(alloc_desc_t),
CONFIG_OFMEM_MALLOC_ALIGN);
+ if( pointer2cell((void *)ret) + size > top ) {
printk("out of malloc memory (%x)!\n", size );
return NULL;
}
- d = (alloc_desc_t*) ofmem->next_malloc;
+ d = (alloc_desc_t*)((uintptr_t)ret - sizeof(alloc_desc_t));
ofmem->next_malloc += size;
d->next = NULL;
d->size = size;
- ret = (char*)d + sizeof(alloc_desc_t);
- memset( ret, 0, size - sizeof(alloc_desc_t) );
+ memset( (char *)ret, 0, size - sizeof(alloc_desc_t) );
- return ret;
+ return (void *)ret;
}
void ofmem_free( void *ptr )
--
1.7.1
--
OpenBIOS http://openbios.org/
Mailinglist: http://lists.openbios.org/mailman/listinfo
Free your System - May the Forth be with you
11:27 a.m.
New subject: [PATCH 1/5] Fix alignment of memory allocated by OFMEM's ofmem_malloc().
Blue Swirl wrote:
Yeah... this is a bit tricky as the style in OFMEM is different from
elsewhere. My thinking was to try and match the existing style and then
possibly attempt a tidy-up later after things are bedded in just to keep
the commits cleaner.
Okay - will do.
I did think about that, but wasn't sure if this would cause funny
pointer maths bugs. If you think that would be okay, I can give it a go.
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
+static inline
void * ALIGN_PTR(uintptr_t x, size_t a)
Uppercase name suggests a macro, please use lowercase. +{
+ return (void *)((x + a - 1) & ~(a-1));
I'd add spaces inside a-1. +}
+
static ucell get_ram_size( void )
{
ofmem_t *ofmem = ofmem_arch_get_private();
@@ -102,29 +107,29 @@ void* ofmem_malloc( size_t size )
/* waste at most 4K by taking an entry from the freelist */
if( *pp && (**pp).size < size + 0x1000 ) {
- ret = (char*)*pp + sizeof(alloc_desc_t);
- memset( ret, 0, (**pp).size - sizeof(alloc_desc_t) );
+ ret = (char *)((uintptr_t)*pp + sizeof(alloc_desc_t));
+ memset( (char *)ret, 0, (**pp).size - sizeof(alloc_desc_t) );
There are a lot of char* and void* casts, perhaps type of ret should
be void * to avoid some of them. At least (char *)ret shouldn't be
necessary anyway.
3482
days inactive
3489
days old
22 comments
4 participants
participants (4)
-
Andreas Färber -
Blue Swirl -
Mark Cave-Ayland -
Tarl Neustaedter