diff mbox

[09/15] loader: use mmap for ROMs

Message ID 1467104499-27517-10-git-send-email-pl@kamp.de (mailing list archive)
State New, archived
Headers show

Commit Message

Peter Lieven June 28, 2016, 9:01 a.m. UTC 
a classic use for mmap here.

Signed-off-by: Peter Lieven <pl@kamp.de>
---
 hw/core/loader.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

Comments

Paolo Bonzini June 28, 2016, 10:41 a.m. UTC | #1 
On 28/06/2016 11:01, Peter Lieven wrote:
> a classic use for mmap here.
> 
> Signed-off-by: Peter Lieven <pl@kamp.de>

They are never freed, why does mmap help?

Paolo

> ---
>  hw/core/loader.c | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/core/loader.c b/hw/core/loader.c
> index 53e0e41..f217edc 100644
> --- a/hw/core/loader.c
> +++ b/hw/core/loader.c
> @@ -55,6 +55,7 @@
>  #include "exec/address-spaces.h"
>  #include "hw/boards.h"
>  #include "qemu/cutils.h"
> +#include "qemu/mmap-alloc.h"
>  
>  #include <zlib.h>
>  
> @@ -837,7 +838,7 @@ int rom_add_file(const char *file, const char *fw_dir,
>  {
>      MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
>      Rom *rom;
> -    int rc, fd = -1;
> +    int fd = -1;
>      char devpath[100];
>  
>      rom = g_malloc0(sizeof(*rom));
> @@ -867,12 +868,9 @@ int rom_add_file(const char *file, const char *fw_dir,
>      }
>  
>      rom->datasize = rom->romsize;
> -    rom->data     = g_malloc0(rom->datasize);
> -    lseek(fd, 0, SEEK_SET);
> -    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->datasize);
> +    rom->data     = mmap(NULL, rom->datasize, PROT_READ, MAP_SHARED, fd, 0);
> +    if (rom->data == MAP_FAILED) {
> +        fprintf(stderr, "rom: file %-20s: mmap error\n", rom->name);
>          goto err;
>      }
>      close(fd);
> @@ -915,7 +913,7 @@ err:
>      if (fd != -1)
>          close(fd);
>  
> -    g_free(rom->data);
> +    qemu_anon_ram_munmap(rom->data, rom->romsize);
>      g_free(rom->path);
>      g_free(rom->name);
>      if (fw_dir) {
> @@ -1013,7 +1011,7 @@ static void rom_reset(void *unused)
>          }
>          if (rom->isrom) {
>              /* rom needs to be written only once */
> -            g_free(rom->data);
> +            qemu_anon_ram_munmap(rom->data, rom->datasize);
>              rom->data = NULL;
>          }
>          /*
>
Peter Lieven June 28, 2016, 11:26 a.m. UTC | #2 
Am 28.06.2016 um 12:41 schrieb Paolo Bonzini:
>
> On 28/06/2016 11:01, Peter Lieven wrote:
>> a classic use for mmap here.
>>
>> Signed-off-by: Peter Lieven <pl@kamp.de>
> They are never freed, why does mmap help?

Actually it is. Adding some debug output to rom_add_file and rom_reset reveals the following:

rom load: /home/lieven/git/qemu/pc-bios/bios-256k.bin
rom load: /home/lieven/git/qemu/pc-bios/kvmvapic.bin
rom_reset

I think the rom is copied to system memory?

Peter
Peter Lieven July 4, 2016, 7:30 a.m. UTC | #3 
Am 28.06.2016 um 12:41 schrieb Paolo Bonzini:
>
> On 28/06/2016 11:01, Peter Lieven wrote:
>> a classic use for mmap here.
>>
>> Signed-off-by: Peter Lieven <pl@kamp.de>
> They are never freed, why does mmap help?

checked again. qemu_system_reset (which calls rom_reset) is invoked after pc_macine_init.
So the memory is indeed freed again.

Peter
diff mbox

Patch

diff --git a/hw/core/loader.c b/hw/core/loader.c
index 53e0e41..f217edc 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -55,6 +55,7 @@ 
 #include "exec/address-spaces.h"
 #include "hw/boards.h"
 #include "qemu/cutils.h"
+#include "qemu/mmap-alloc.h"
 
 #include <zlib.h>
 
@@ -837,7 +838,7 @@  int rom_add_file(const char *file, const char *fw_dir,
 {
     MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
     Rom *rom;
-    int rc, fd = -1;
+    int fd = -1;
     char devpath[100];
 
     rom = g_malloc0(sizeof(*rom));
@@ -867,12 +868,9 @@  int rom_add_file(const char *file, const char *fw_dir,
     }
 
     rom->datasize = rom->romsize;
-    rom->data     = g_malloc0(rom->datasize);
-    lseek(fd, 0, SEEK_SET);
-    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->datasize);
+    rom->data     = mmap(NULL, rom->datasize, PROT_READ, MAP_SHARED, fd, 0);
+    if (rom->data == MAP_FAILED) {
+        fprintf(stderr, "rom: file %-20s: mmap error\n", rom->name);
         goto err;
     }
     close(fd);
@@ -915,7 +913,7 @@  err:
     if (fd != -1)
         close(fd);
 
-    g_free(rom->data);
+    qemu_anon_ram_munmap(rom->data, rom->romsize);
     g_free(rom->path);
     g_free(rom->name);
     if (fw_dir) {
@@ -1013,7 +1011,7 @@  static void rom_reset(void *unused)
         }
         if (rom->isrom) {
             /* rom needs to be written only once */
-            g_free(rom->data);
+            qemu_anon_ram_munmap(rom->data, rom->datasize);
             rom->data = NULL;
         }
         /*