| Message ID | 20240628015036.1815838-1-gregorhaas1997@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] hw/core/loader: allow loading larger ROMs | expand |
> -----Original Message----- > From: Gregor Haas <gregorhaas1997@gmail.com> > Sent: Friday, June 28, 2024 9:51 AM > To: qemu-devel@nongnu.org > Cc: Yao, Xingtao/姚 幸涛 <yaoxt.fnst@fujitsu.com>; Gregor Haas > <gregorhaas1997@gmail.com> > Subject: [PATCH v3] hw/core/loader: allow loading larger ROMs > > The read() syscall is not guaranteed to return all data from a file. The > default ROM loader implementation currently does not take this into account, > instead failing if all bytes are not read at once. This change loads the ROM > using load_image_size() instead, which correctly reads all data using > multiple calls to read(). Also, the ROM size is now determined using the > get_image_size() function rather than using manual lseek(). > > Signed-off-by: Gregor Haas <gregorhaas1997@gmail.com> > --- > hw/core/loader.c | 17 ++--------------- > 1 file changed, 2 insertions(+), 15 deletions(-) > > diff --git a/hw/core/loader.c b/hw/core/loader.c > index 2f8105d7de..c2c61158f1 100644 > --- a/hw/core/loader.c > +++ b/hw/core/loader.c > @@ -1076,7 +1076,6 @@ ssize_t rom_add_file(const char *file, const char *fw_dir, > MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine()); > Rom *rom; > ssize_t rc; > - int fd = -1; > char devpath[100]; > > if (as && mr) { > @@ -1094,19 +1093,12 @@ ssize_t 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 = get_image_size(rom->path); > if (rom->romsize == -1) { > fprintf(stderr, "rom: file %-20s: get size error: %s\n", > rom->name, strerror(errno)); > @@ -1115,14 +1107,12 @@ ssize_t 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); > + rc = load_image_size(rom->path, rom->data, rom->datasize); > if (rc != rom->datasize) { > fprintf(stderr, "rom: file %-20s: read error: rc=%zd (expected %zd)\n", > rom->name, rc, rom->datasize); > goto err; > } > - close(fd); > rom_insert(rom); > if (rom->fw_file && fw_cfg) { > const char *basename; > @@ -1159,9 +1149,6 @@ ssize_t rom_add_file(const char *file, const char *fw_dir, > return 0; > > err: > - if (fd != -1) > - close(fd); > - > rom_free(rom); > return -1; > } > -- > 2.45.2 Reviewed-by: Xingtao Yao <yaoxt.fnst@fujitsu.com >
diff --git a/hw/core/loader.c b/hw/core/loader.c index 2f8105d7de..c2c61158f1 100644 --- a/hw/core/loader.c +++ b/hw/core/loader.c @@ -1076,7 +1076,6 @@ ssize_t rom_add_file(const char *file, const char *fw_dir, MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine()); Rom *rom; ssize_t rc; - int fd = -1; char devpath[100]; if (as && mr) { @@ -1094,19 +1093,12 @@ ssize_t 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 = get_image_size(rom->path); if (rom->romsize == -1) { fprintf(stderr, "rom: file %-20s: get size error: %s\n", rom->name, strerror(errno)); @@ -1115,14 +1107,12 @@ ssize_t 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); + rc = load_image_size(rom->path, rom->data, rom->datasize); if (rc != rom->datasize) { fprintf(stderr, "rom: file %-20s: read error: rc=%zd (expected %zd)\n", rom->name, rc, rom->datasize); goto err; } - close(fd); rom_insert(rom); if (rom->fw_file && fw_cfg) { const char *basename; @@ -1159,9 +1149,6 @@ ssize_t rom_add_file(const char *file, const char *fw_dir, return 0; err: - if (fd != -1) - close(fd); - rom_free(rom); return -1; }
The read() syscall is not guaranteed to return all data from a file. The default ROM loader implementation currently does not take this into account, instead failing if all bytes are not read at once. This change loads the ROM using load_image_size() instead, which correctly reads all data using multiple calls to read(). Also, the ROM size is now determined using the get_image_size() function rather than using manual lseek(). Signed-off-by: Gregor Haas <gregorhaas1997@gmail.com> --- hw/core/loader.c | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-)