Re: [Qemu-devel] [PATCH 3/4] loader: Add rom_add_file_buf for adding roms from a buffer

[Blue Swirl]

On Mon, Oct 17, 2011 at 7:16 PM, Jordan Justen
<address@hidden> wrote:
> rom_add_file_buf is similar to rom_add_file, except the rom's
> contents are provided in a buffer.
>
> rom_add_file is modified to call rom_add_file_buf after
> reading the rom's contents from the file.
>
> Signed-off-by: Jordan Justen <address@hidden>
> ---
>  hw/loader.c |   71 +++++++++++++++++++++++++++++++++++++++-------------------
>  hw/loader.h |    5 ++++
>  2 files changed, 53 insertions(+), 23 deletions(-)
>
> diff --git a/hw/loader.c b/hw/loader.c
> index 5676c18..d1a4a98 100644
> --- a/hw/loader.c
> +++ b/hw/loader.c
> @@ -557,11 +557,11 @@ static void rom_insert(Rom *rom)
>     QTAILQ_INSERT_TAIL(&roms, rom, next);
>  }
>
> -int rom_add_file(const char *file, const char *fw_dir,
> -                 target_phys_addr_t addr, int32_t bootindex)
> +int rom_add_file_buf(const char *file, const void *data, size_t size,
> +                     const char *fw_dir,
> +                     target_phys_addr_t addr, int32_t bootindex)
>  {
>     Rom *rom;
> -    int rc, fd = -1;
>     char devpath[100];
>
>     rom = g_malloc0(sizeof(*rom));
> @@ -571,28 +571,16 @@ int rom_add_file(const char *file, const char *fw_dir,
>         rom->path = g_strdup(file);
>     }
>
> -    fd = open(rom->path, O_RDONLY | O_BINARY);
> -    if (fd == -1) {
> -        fprintf(stderr, "Could not open option rom '%s': %s\n",
> -                rom->path, strerror(errno));
> -        goto err;
> -    }
> -
>     if (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->romsize = size;
>     rom->data    = g_malloc0(rom->romsize);
> -    lseek(fd, 0, SEEK_SET);
> -    rc = read(fd, rom->data, rom->romsize);
> -    if (rc != rom->romsize) {
> -        fprintf(stderr, "rom: file %-20s: read error: rc=%d (expected 
> %zd)\n",
> -                rom->name, rc, rom->romsize);
> -        goto err;
> -    }
> -    close(fd);
> +
> +    memcpy(rom->data, data, rom->romsize);

This is not optimal, instead the data should be used directly. That
way also mmap()ed, deduplicated ROM files are possible.

> +
>     rom_insert(rom);
>     if (rom->fw_file && fw_cfg) {
>         const char *basename;
> @@ -614,14 +602,51 @@ int rom_add_file(const char *file, const char *fw_dir,
>
>     add_boot_device_path(bootindex, NULL, devpath);
>     return 0;
> +}
> +
> +int rom_add_file(const char *file, const char *fw_dir,
> +                 target_phys_addr_t addr, int32_t bootindex)
> +{
> +    char *filename;
> +    void *data = NULL;
> +    size_t size;
> +    int rc, fd = -1;
> +
> +    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, file);
> +    if (filename == NULL) {
> +        filename = g_strdup(file);
> +    }
> +
> +    fd = open(filename, O_RDONLY | O_BINARY);
> +    if (fd == -1) {
> +        fprintf(stderr, "Could not open option rom '%s': %s\n",
> +                filename, strerror(errno));
> +        goto err;
> +    }
> +
> +    size = lseek(fd, 0, SEEK_END);
> +    data = g_malloc0(size);
> +    lseek(fd, 0, SEEK_SET);
> +    rc = read(fd, data, size);

It should be easy to replace this with mmap(), maybe later.

> +    if (rc != size) {
> +        fprintf(stderr, "rom: file %-20s: read error: rc=%d (expected 
> %zd)\n",
> +                filename, rc, size);
> +        goto err;
> +    }
> +    close(fd);
> +
> +    rc = rom_add_file_buf(filename, data, size, fw_dir, addr, bootindex);
> +    if (rc != 0) {
> +        goto err;
> +    }
> +
> +    g_free(data);
> +    return 0;
>
>  err:
>     if (fd != -1)
>         close(fd);
> -    g_free(rom->data);
> -    g_free(rom->path);
> -    g_free(rom->name);
> -    g_free(rom);
> +    g_free(data);
>     return -1;
>  }
>
> diff --git a/hw/loader.h b/hw/loader.h
> index fc6bdff..9efe64a 100644
> --- a/hw/loader.h
> +++ b/hw/loader.h
> @@ -21,6 +21,9 @@ void pstrcpy_targphys(const char *name,
>                       const char *source);
>
>
> +int rom_add_file_buf(const char *file, const void *data, size_t size,
> +                     const char *fw_dir,
> +                     target_phys_addr_t addr, int32_t bootindex);
>  int rom_add_file(const char *file, const char *fw_dir,
>                  target_phys_addr_t addr, int32_t bootindex);
>  int rom_add_blob(const char *name, const void *blob, size_t len,
> @@ -31,6 +34,8 @@ int rom_copy(uint8_t *dest, target_phys_addr_t addr, size_t 
> size);
>  void *rom_ptr(target_phys_addr_t addr);
>  void do_info_roms(Monitor *mon);
>
> +#define rom_add_file_buf_fixed(_f, _d, _s, _a, _i)          \
> +    rom_add_file_buf(_f, _d, _s, NULL, _a, _i)
>  #define rom_add_file_fixed(_f, _a, _i)          \
>     rom_add_file(_f, NULL, _a, _i)
>  #define rom_add_blob_fixed(_f, _b, _l, _a)      \
> --
> 1.7.1
>
>
>