diff mbox series

[v3,3/3] hw/core/loader: Warn if we fail to load ROM regions at reset

Message ID 20210520051542.2378774-4-f4bug@amsat.org (mailing list archive)
State New, archived
Headers show
Series exec/memory: Enforce checking MemTxResult values | expand

Commit Message

Philippe Mathieu-Daudé May 20, 2021, 5:15 a.m. UTC
If the user provides an ELF file that's been linked to a wrong
address, we try to load it, fails, and keep going silently.
Instead,
Display a warning instead, but keep going to not disrupt users
accidentally relying on this 'continues-anyway' behaviour.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/core/loader.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

Comments

Peter Maydell May 27, 2021, 9:07 a.m. UTC | #1
On Thu, 20 May 2021 at 06:15, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> If the user provides an ELF file that's been linked to a wrong
> address, we try to load it, fails, and keep going silently.
> Instead,
> Display a warning instead, but keep going to not disrupt users
> accidentally relying on this 'continues-anyway' behaviour.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/core/loader.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/hw/core/loader.c b/hw/core/loader.c
> index b3c4a654b45..37a2f2c4959 100644
> --- a/hw/core/loader.c
> +++ b/hw/core/loader.c
> @@ -1147,8 +1147,16 @@ static void rom_reset(void *unused)
>              void *host = memory_region_get_ram_ptr(rom->mr);
>              memcpy(host, rom->data, rom->datasize);
>          } else {
> -            address_space_write_rom(rom->as, rom->addr, MEMTXATTRS_UNSPECIFIED,
> -                                    rom->data, rom->datasize);
> +            MemTxResult res;
> +
> +            res = address_space_write_rom(rom->as, rom->addr,
> +                                          MEMTXATTRS_UNSPECIFIED,
> +                                          rom->data, rom->datasize);
> +            if (res != MEMTX_OK) {
> +                warn_report("rom: unable to write data (file '%s', "
> +                            "addr=0x" TARGET_FMT_plx ", size=0x%zu)",
> +                            rom->name, rom->addr, rom->datasize);
> +            }
>          }
>          if (rom->isrom) {
>              /* rom needs to be written only once */

address_space_write_rom() as currently implemented cannot ever
fail: it always returns MEMTX_OK. (This is because it completely
ignores attempts to write to devices, and writing to devices backed
by host RAM always works.)

But perhaps this change is reasonable enough as future-proofing
in case we decide to allow address_space_write_rom() to write
rom blobs to devices in future?

-- PMM
diff mbox series

Patch

diff --git a/hw/core/loader.c b/hw/core/loader.c
index b3c4a654b45..37a2f2c4959 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -1147,8 +1147,16 @@  static void rom_reset(void *unused)
             void *host = memory_region_get_ram_ptr(rom->mr);
             memcpy(host, rom->data, rom->datasize);
         } else {
-            address_space_write_rom(rom->as, rom->addr, MEMTXATTRS_UNSPECIFIED,
-                                    rom->data, rom->datasize);
+            MemTxResult res;
+
+            res = address_space_write_rom(rom->as, rom->addr,
+                                          MEMTXATTRS_UNSPECIFIED,
+                                          rom->data, rom->datasize);
+            if (res != MEMTX_OK) {
+                warn_report("rom: unable to write data (file '%s', "
+                            "addr=0x" TARGET_FMT_plx ", size=0x%zu)",
+                            rom->name, rom->addr, rom->datasize);
+            }
         }
         if (rom->isrom) {
             /* rom needs to be written only once */