RE: [PATCH v3] hw/core/loader: allow loading larger ROMs

2024-06-27 Thread Xingtao Yao (Fujitsu)



> -Original Message-
> From: Gregor Haas 
> Sent: Friday, June 28, 2024 9:51 AM
> To: qemu-devel@nongnu.org
> Cc: Yao, Xingtao/姚 幸涛 ; Gregor Haas
> 
> 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 
> ---
>  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 





[PATCH v3] hw/core/loader: allow loading larger ROMs

2024-06-27 Thread Gregor Haas
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 
---
 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