Re: More hpcboot memory/ELF loading errors?

["Valeriy E. Ushakov" <uwe%stderr.spb.ru@localhost>]

On Wed, Mar 05, 2008 at 21:33:19 -0500, Rafal Boni wrote:

> The issue appears to be that while the symbol header, symbol table and 
> string table are all loaded in 3 separate passes, the size-estimation 
> code returned the size of all 3 items as a single datum, ignoring the 
> fact that each of those 3 components would need at least one page (and 
> maybe more) to store the contents in the 'load chain'.
> 
> Diff attached... comments welcome,
> --rafal

> Index: hpcboot/load_elf.cpp
> ===================================================================
> RCS file: /cvsroot/src/sys/arch/hpc/stand/hpcboot/load_elf.cpp,v
> retrieving revision 1.17
> diff -u -p -u -p -r1.17 load_elf.cpp
> --- hpcboot/load_elf.cpp      5 Mar 2006 04:04:13 -0000       1.17
> +++ hpcboot/load_elf.cpp      6 Mar 2008 02:01:36 -0000
> @@ -123,10 +123,6 @@ ElfLoader::memorySize()
>                       sz += _mem->roundPage(filesz);
>                       // compensate for partial last tag
>                       extra += _mem->getTaggedPageSize();
> -                     if (filesz < ph->p_memsz)
> -                             // compensate for zero clear
> -                             extra += _mem->getTaggedPageSize();
> -
>               }
>       }

Why have you removed this?  Zero-clear chunk occupies an extra tagged
page in the chain.


> @@ -135,8 +131,9 @@ ElfLoader::memorySize()
>       if (symblk_sz) {
>               sz += symblk_sz;
>               DPRINTF((TEXT(" = 0x%x]"), symblk_sz));
> -             // XXX: compensate for partial tags after ELF header and symtab
> -             extra += 2 * _mem->getTaggedPageSize();
> +             // XXX: compensate for partial tags after ELF header, symtab
> +             // and strtab
> +             extra += 3 * _mem->getTaggedPageSize();
>       }

Does it work for you with this hunk alone?


>       sz += extra;
> @@ -263,13 +260,17 @@ ElfLoader::symbol_block_size()
>           ROUND4(_sym_blk.shsym->sh_size);
>       _sym_blk.enable = TRUE;
>  
> -     DPRINTF((TEXT("+[ksyms: header 0x%x, symtab 0x%x, strtab 0x%x"),
> +     DPRINTF((TEXT("+[ksyms: header 0x%x, symtab 0x%x, strtab 0x%x = 0x%x]"),
>           _sym_blk.header_size, _sym_blk.shsym->sh_size,
> -         _sym_blk.shstr->sh_size));
> +         _sym_blk.shstr->sh_size, _sym_blk.header_size + 
> +         ROUND4(_sym_blk.shsym->sh_size) + _sym_blk.shstr->sh_size));

The sum is printed in the caller.  If you print it here, you need to
remove the printf in the caller.


> -     // return total amount of symbol block
> -     return (_sym_blk.header_size + ROUND4(_sym_blk.shsym->sh_size) +
> -         _sym_blk.shstr->sh_size);
> +     // Round each of the three components to page_size since they're
> +     // loaded as 3 different segments.
> +     return (_mem->roundPage(_sym_blk.header_size) +
> +             _mem->roundPage(ROUND4(_sym_blk.shsym->sh_size)) +
> +             _mem->roundPage(_sym_blk.shstr->sh_size));
> +         
>  }

Is these roundPage necessary?  I really don't want to inflict on
myself the pain of reading and understanding this code again, but IIRC
extra pages added in the caller are supposed to take care of this.
Moreover, IIRC, doing roundPage doesn't actually help the "partial
tag" problem.


I'd prefer we really understand were the overflow occurs before we
commit anything to this code that is already quite hairy.

SY, Uwe
-- 
uwe%stderr.spb.ru@localhost                       |       Zu Grunde kommen
http://snark.ptc.spbu.ru/~uwe/          |       Ist zu Grunde gehen