Patchwork [56/66] Save memory allocation in the elf loader

login
register
mail settings
Submitter Alexander Graf
Date March 8, 2013, 8:06 p.m.
Message ID <1362773228-1747-57-git-send-email-agraf@suse.de>
Download mbox | patch
Permalink /patch/226248/
State New
Headers show

Comments

Alexander Graf - March 8, 2013, 8:06 p.m.
From: Fabien Chouteau <chouteau@adacore.com>

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.

Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/elf_ops.h |   19 ++++++++------
 hw/loader.c  |   75 +++++++++++++++++++++++++++++++++++++++++++++++----------
 hw/loader.h  |    2 +
 3 files changed, 75 insertions(+), 21 deletions(-)

Patch

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);