Il 19/02/2013 15:41, Fabien Chouteau ha scritto: > The current elf loader uses too much memory. For example, I have a > executable with a bss section of 400 MB and I set the ram size to 512 > MB. Qemu uses about 780MB of RAM (which is fine), but there's a peak at > 1.6 GB during initialization (this is not fine). > > This patch fixes two things: > 1) do not allocate each elf program twice. > 2) do not allocate memory for areas that are only zeros. > > For this we need a new field in Rom: "datasize" which is the size of the > allocated data. If datasize is less than romsize, it means that the area > from datasize to romsize is filled with zeros.
You haven't CCed the only two people that likely would review your patch. :) Paolo > Signed-off-by: Fabien Chouteau <chout...@adacore.com> > --- > hw/elf_ops.h | 19 ++++++++------- > hw/loader.c | 75 > ++++++++++++++++++++++++++++++++++++++++++++++++---------- > hw/loader.h | 2 ++ > 3 files changed, 75 insertions(+), 21 deletions(-) > > diff --git a/hw/elf_ops.h b/hw/elf_ops.h > index 531a425..acc701e 100644 > --- a/hw/elf_ops.h > +++ b/hw/elf_ops.h > @@ -197,7 +197,7 @@ static int glue(load_elf, SZ)(const char *name, int fd, > struct elfhdr ehdr; > struct elf_phdr *phdr = NULL, *ph; > int size, i, total_size; > - elf_word mem_size; > + elf_word mem_size, file_size; > uint64_t addr, low = (uint64_t)-1, high = 0; > uint8_t *data = NULL; > char label[128]; > @@ -252,14 +252,16 @@ static int glue(load_elf, SZ)(const char *name, int fd, > for(i = 0; i < ehdr.e_phnum; i++) { > ph = &phdr[i]; > if (ph->p_type == PT_LOAD) { > - mem_size = ph->p_memsz; > - /* XXX: avoid allocating */ > - data = g_malloc0(mem_size); > + mem_size = ph->p_memsz; /* Size of the ROM */ > + file_size = ph->p_filesz; /* Size of the allocated data */ > + data = g_malloc0(file_size); > if (ph->p_filesz > 0) { > - if (lseek(fd, ph->p_offset, SEEK_SET) < 0) > + if (lseek(fd, ph->p_offset, SEEK_SET) < 0) { > goto fail; > - if (read(fd, data, ph->p_filesz) != ph->p_filesz) > + } > + if (read(fd, data, file_size) != file_size) { > goto fail; > + } > } > /* address_offset is hack for kernel images that are > linked at the wrong physical address. */ > @@ -281,7 +283,9 @@ static int glue(load_elf, SZ)(const char *name, int fd, > } > > snprintf(label, sizeof(label), "phdr #%d: %s", i, name); > - rom_add_blob_fixed(label, data, mem_size, addr); > + > + /* rom_add_elf_program() seize the ownership of 'data' */ > + rom_add_elf_program(label, data, file_size, mem_size, addr); > > total_size += mem_size; > if (addr < low) > @@ -289,7 +293,6 @@ static int glue(load_elf, SZ)(const char *name, int fd, > if ((addr + mem_size) > high) > high = addr + mem_size; > > - g_free(data); > data = NULL; > } > } > diff --git a/hw/loader.c b/hw/loader.c > index 995edc3..bd2b52d 100644 > --- a/hw/loader.c > +++ b/hw/loader.c > @@ -533,7 +533,14 @@ typedef struct Rom Rom; > struct Rom { > char *name; > char *path; > + > + /* datasize is the amount of memory allocated in "data". If datasize is > less > + * than romsize, it means that the area from datasize to romsize is > filled > + * with zeros. > + */ > size_t romsize; > + size_t datasize; > + > uint8_t *data; > int isrom; > char *fw_dir; > @@ -589,14 +596,15 @@ int rom_add_file(const char *file, const char *fw_dir, > rom->fw_dir = g_strdup(fw_dir); > rom->fw_file = g_strdup(file); > } > - rom->addr = addr; > - rom->romsize = lseek(fd, 0, SEEK_END); > - rom->data = g_malloc0(rom->romsize); > + rom->addr = addr; > + rom->romsize = lseek(fd, 0, SEEK_END); > + rom->datasize = rom->romsize; > + rom->data = g_malloc0(rom->datasize); > lseek(fd, 0, SEEK_SET); > - rc = read(fd, rom->data, rom->romsize); > - if (rc != rom->romsize) { > + rc = read(fd, rom->data, rom->datasize); > + if (rc != rom->datasize) { > fprintf(stderr, "rom: file %-20s: read error: rc=%d (expected > %zd)\n", > - rom->name, rc, rom->romsize); > + rom->name, rc, rom->datasize); > goto err; > } > close(fd); > @@ -637,16 +645,37 @@ int rom_add_blob(const char *name, const void *blob, > size_t len, > { > Rom *rom; > > - rom = g_malloc0(sizeof(*rom)); > - rom->name = g_strdup(name); > - rom->addr = addr; > - rom->romsize = len; > - rom->data = g_malloc0(rom->romsize); > + rom = g_malloc0(sizeof(*rom)); > + rom->name = g_strdup(name); > + rom->addr = addr; > + rom->romsize = len; > + rom->datasize = len; > + rom->data = g_malloc0(rom->datasize); > memcpy(rom->data, blob, len); > rom_insert(rom); > return 0; > } > > +/* This function is specific for elf program because we don't need to > allocate > + * all the rom. We just allocate the first part and the rest is just zeros. > This > + * is why romsize and datasize are different. Also, this function seize the > + * memory ownership of "data", so we don't have to allocate and copy the > buffer. > + */ > +int rom_add_elf_program(const char *name, void *data, size_t datasize, > + size_t romsize, hwaddr addr) > +{ > + Rom *rom; > + > + rom = g_malloc0(sizeof(*rom)); > + rom->name = g_strdup(name); > + rom->addr = addr; > + rom->datasize = datasize; > + rom->romsize = romsize; > + rom->data = data; > + rom_insert(rom); > + return 0; > +} > + > int rom_add_vga(const char *file) > { > return rom_add_file(file, "vgaroms", 0, -1); > @@ -668,7 +697,7 @@ static void rom_reset(void *unused) > if (rom->data == NULL) { > continue; > } > - cpu_physical_memory_write_rom(rom->addr, rom->data, rom->romsize); > + cpu_physical_memory_write_rom(rom->addr, rom->data, rom->datasize); > if (rom->isrom) { > /* rom needs to be written only once */ > g_free(rom->data); > @@ -756,13 +785,33 @@ int rom_copy(uint8_t *dest, hwaddr addr, size_t size) > > d = dest + (rom->addr - addr); > s = rom->data; > - l = rom->romsize; > + l = rom->datasize; > > if ((d + l) > (dest + size)) { > l = dest - d; > } > > memcpy(d, s, l); > + > + if (rom->romsize > rom->datasize) { > + /* If datasize is less than romsize, it means that we didn't > + * allocate all the ROM because the trailing data are only zeros. > + */ > + > + d += l; > + l = rom->romsize - rom->datasize; > + > + if ((d + l) > (dest + size)) { > + /* Rom size doesn't fit in the destination area. Adjust to > avoid > + * overflow. > + */ > + l = dest - d; > + } > + > + if (l > 0) { > + memset(d, 0x0, l); > + } > + } > } > > return (d + l) - dest; > diff --git a/hw/loader.h b/hw/loader.h > index 5e61c95..0958f06 100644 > --- a/hw/loader.h > +++ b/hw/loader.h > @@ -27,6 +27,8 @@ int rom_add_file(const char *file, const char *fw_dir, > hwaddr addr, int32_t bootindex); > int rom_add_blob(const char *name, const void *blob, size_t len, > hwaddr addr); > +int rom_add_elf_program(const char *name, void *data, size_t datasize, > + size_t romsize, hwaddr addr); > int rom_load_all(void); > void rom_set_fw(void *f); > int rom_copy(uint8_t *dest, hwaddr addr, size_t size); >