I added parameter 'reset' into the original load_elf32/64() in patch 3/4.
Reserve the most lines and rename this function as elf_phy_loader32/64().
-static int glue(load_elf, SZ)(const char *name, int fd,
+static int glue(elf_phy_loader, SZ)(const char *name, int fd,
uint64_t (*translate_fn)(void *, uint64_t),
void *translate_opaque,
int must_swab, uint64_t *pentry,
uint64_t *lowaddr, uint64_t *highaddr,
- int elf_machine, int clear_lsb)
+ int elf_machine, int clear_lsb, int reset)
Then load_elf32/64() and elf_reset32/64() will call this function with
different reset values.
static void glue(elf_reset, SZ)(void *opaque)
{
ImageElf *elf = opaque;
int fd;
fd = open(elf->name, O_RDONLY | O_BINARY);
glue(elf_phy_loader, SZ)(elf->name, fd, elf->fn, elf->opaque, elf->swab,
NULL, NULL, NULL, elf->machine, elf->lsb, 1);
}
static int glue(load_elf, SZ)(const char *name, int fd,
uint64_t (*translate_fn)(void *, uint64_t),
void *translate_opaque,
int must_swab, uint64_t *pentry,
uint64_t *lowaddr, uint64_t *highaddr,
int elf_machine, int clear_lsb)
{
int ret;
ret = glue(elf_phy_loader, SZ)(name, fd, (*translate_fn), translate_opaque,
must_swab, pentry, lowaddr, highaddr,
elf_machine, clear_lsb, 0);
return ret;
}
Best Regards,
Olivia
> -----Original Message-----
> From: Alexander Graf [mailto:ag...@suse.de]
> Sent: Monday, November 26, 2012 9:03 PM
> To: Yin Olivia-R63875
> Cc: Stuart Yoder; qemu-devel@nongnu.org; qemu-...@nongnu.org
> Subject: Re: [Qemu-devel] [RFC PATCH v5 4/4] free the memory malloced by
> load_at()
>
>
> On 26.11.2012, at 02:53, Yin Olivia-R63875 wrote:
>
> > Hi Stuart & Alex,
> >
> > "syms" and "str" could not be free since "symbols" is a global
> > variable which need stay in the memory during the whole life of VM. So
> > it will not be free and reload when reset.
>
> Ah, that's used for the debug log output, right?
>
> > The only change is to the previous patch of elf loader (hw/elf_ops.h)
> as below:
> >
> > @@ -232,7 +235,9 @@ static int glue(load_elf, SZ)(const char *name, int
> fd,
> > if (pentry)
> > *pentry = (uint64_t)(elf_sword)ehdr.e_entry;
> >
> > - glue(load_symbols, SZ)(&ehdr, fd, must_swab, clear_lsb);
> > + if (!reset) {
> > + glue(load_symbols, SZ)(&ehdr, fd, must_swab, clear_lsb);
> > + }
> >
> > size = ehdr.e_phnum * sizeof(phdr[0]);
> > lseek(fd, ehdr.e_phoff, SEEK_SET);
> >
> > Do you think it is reasonable?
>
> I think semantically we want to only load the symbols the first time we
> load the binary (read: not on reset), yes. Where does the reset variable
> you're using there come from?
>
>
> Alex
>
> >
> > Best Regards,
> > Olivia
> >
> >> -----Original Message-----
> >> From: Alexander Graf [mailto:ag...@suse.de]
> >> Sent: Thursday, November 22, 2012 2:42 AM
> >> To: Stuart Yoder
> >> Cc: Yin Olivia-R63875; qemu-devel@nongnu.org; qemu-...@nongnu.org
> >> Subject: Re: [Qemu-devel] [RFC PATCH v5 4/4] free the memory malloced
> >> by
> >> load_at()
> >>
> >> On 11/21/2012 07:39 PM, Stuart Yoder wrote:
> >>> On Wed, Nov 21, 2012 at 8:38 AM, Olivia
> >>> Yin<hong-hua....@freescale.com>
> >> wrote:
> >>>> Signed-off-by: Olivia Yin<hong-hua....@freescale.com>
> >>>> ---
> >>>> hw/elf_ops.h | 2 ++
> >>>> 1 files changed, 2 insertions(+), 0 deletions(-)
> >>>>
> >>>> diff --git a/hw/elf_ops.h b/hw/elf_ops.h index b346861..9c76a75
> >>>> 100644
> >>>> --- a/hw/elf_ops.h
> >>>> +++ b/hw/elf_ops.h
> >>>> @@ -178,6 +178,8 @@ static int glue(load_symbols, SZ)(struct elfhdr
> >> *ehdr, int fd, int must_swab,
> >>>> s->disas_strtab = str;
> >>>> s->next = syminfos;
> >>>> syminfos = s;
> >>>> + g_free(syms);
> >>>> + g_free(str);
> >>>> g_free(shdr_table);
> >>>> return 0;
> >>>> fail:
> >>> Olivia, as Alex pointed out there are references to syms and str in
> >>> the struct "s"....so you can't just free those I don't think.
> >>>
> >>> The problem that leaves us with is that on every reset when we call
> >>> load_elf() that we re-load and re-malloc space for the symbols.
> >>>
> >>> I think the solution may be to factor out the call to load_symbols()
> >>> from load_elf(). It looks like what load_symbols does in the end is
> >>> set the variable syminfos to point to the loaded symbol info.
> >>>
> >>> If you factor load_symbols() out then in load_elf_32/64() you would
> >>> do something like:
> >>> elf_phy_loader_32/64()
> >>> load_symbols_32/64().
> >>>
> >>> We don't need to be reloading symbols on every reset.
> >>>
> >>> Alex, does that make sense?
> >>
> >> We can also mandate the caller of load_symbols to free the respective
> >> data :)
> >>
> >>
> >> Alex
> >>
> >
> >
>
