Port-hpcarm archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: More hpcboot memory/ELF loading errors?

Valeriy E. Ushakov wrote:
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,

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.

I thought this was the case too, but if you follow the code / chain, you'll note that they steal from the initial 'pvec' page allocated at the beginning (they just kidnap sizeof(PageTag) since there's no data to store). So I adjusted for reality.

@@ -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?

I haven't tried just that one -- though even if it works, it seems a little like a rubber-chickens and voodoo-sticks change ;). But I'll give it a try and report back in any case..

        sz += extra;
@@ -263,13 +260,17 @@ ElfLoader::symbol_block_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.

Oops, I thought I had done that; will update, thanks.

-       // 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 think this is the crux of the problem, and yeah, I think it's necessary. If you look at what ElfLoader::load_symbol_block() does, it loads (a) header, (b) symbol table, and (c) string table as individual segments, which means each will start on a new page in the tag chain.

So if you have e.g. 1320 + 1290 + 1350 bytes in those 3, before we'd return 1320 + ROUND4(1290) = 1292 + 1350 = 3962, which would look like it would fit in a single page (even in a single tagged page), but would actually require *3* tagged pages since each component would get it's own link (== tagged page) in the tag chain.

AAs for the 'partial tag' issue, I'm also not sure off the top of my head, but given the gyrations Architecture::allocateMemory goes through to convert bytes-needed-in-memory to tagged-pages-needed, I'd think as long as we rounded up all segments to the next unit of tagged-page-size (since after all that's what matters, not actual HW page size), there should be *no* page unaccounted for. Rounding up to HW page size is maybe a little sloppy, but should give more *conservative* estimates, which from our POV is OK (after all, it's going to get rounded up to the next chunk of 64k anyway when we allocate pages from WinCE).

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

I'm all for that, which is why I'm sending this to the list rather than checking in... but I also want to minimize the amount of damage my forehead takes from beating it against this too much, and in the end I want something which works at least semi-reliably -- and this hasn't been the case with the hpcboot built from unmodified sources.


Home | Main Index | Thread Index | Old Index