Subject: Re: KVA pitfall in initarm()
To: Toru Nishimura <locore32@gaea.ocn.ne.jp>
From: Richard Earnshaw <rearnsha@netbsd.org>
List: port-arm
Date: 01/04/2005 17:06:55
On Sun, 2004-12-26 at 11:53, Toru Nishimura wrote:
> Many ARM ports arrange KVA for text and data segments as two
> adjuscent address ranges in the following way;
> 
>         {
>                 extern char etext[], _end[];
>                 size_t textsize = (uintptr_t) etext - KERNEL_TEXT_BASE;
>                 size_t totalsize = (uintptr_t) _end - KERNEL_TEXT_BASE;
>                 u_int logical;
> 
>                 textsize = (textsize + PGOFSET) & ~PGOFSET;
>                 totalsize = (totalsize + PGOFSET) & ~PGOFSET;
>                 
>                 logical = boofoowoo;    /* offset of kernel in RAM */
>                 logical += pmap_map_chunk(l1pagetable, KERNEL_BASE + logical,
>                     physical_start + logical, textsize,
>                     VM_PROT_READ|VM_PROT_WRITE, PTE_CACHE);
>                 logical += pmap_map_chunk(l1pagetable, KERNEL_BASE + logical,
>                     physical_start + logical, totalsize - textsize,
>                     VM_PROT_READ|VM_PROT_WRITE, PTE_CACHE);
>         }
> 
> This is a potetial pitfall for the case when there is a hole (segment alignment)
> between text segment and data segment.  Data segment would result in
> be mapped incorrect KVA (it badly bited me, indeed).  The right way is to map
> data segment at its own offset, not the end of text.  

There certainly is an assumption that the pages of the kernel text and
data have a 1:1 correspondence between the image loaded at boot and the
pages after mapping.  That doesn't mean there can be no alignment of the
data segment, just that any space must be zero-filled in the boot image.

> It, however, doesn't make
> sense to map two since they have an idential pmap attributes.  So, I propose
> here to have a single pmap_map_chunk() call to cover text+data instead of two
> calls.  
I'm not sure why we would want anything different in the way of
pmap_map_chunk in it's current incarnation.  

I think I see a bug in the above code (I'll need to run some checks to
be sure), the text size should be rounded *down* to the nearest page and
mapped VM_PROT_READ only; the data segment would then be rounded up and
mapped VM_PROT_READ|VM_PROT_WRITE.  It might also make sense to use
__data_start rather than _etext to avoid marking code unnecessarily
writable.

> Note that the code above also does short for the case when ELF symbol
> table is available next to _end address.

I think most evbarm kernels use "options SYMTAB_SPACE=..." to put the
kernel symbols into the data section.

R.