qemu-arm
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RFC PATCH v2 7/7] hw/core/loader: Assert loading ROM regions succee


From: Philippe Mathieu-Daudé
Subject: Re: [RFC PATCH v2 7/7] hw/core/loader: Assert loading ROM regions succeeds at reset
Date: Mon, 18 May 2020 18:25:18 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0

On 5/18/20 6:12 PM, Peter Maydell wrote:
On Mon, 18 May 2020 at 16:53, Philippe Mathieu-Daudé <address@hidden> wrote:

If we are unable to load a blob in a ROM region, we should not
ignore it and let the machine boot.

Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
---
RFC: Maybe more polite with user to use hw_error()?
---
  hw/core/loader.c | 8 ++++++--
  1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/core/loader.c b/hw/core/loader.c
index 8bbb1797a4..4e046388b4 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -1146,8 +1146,12 @@ 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);
+            assert(res == MEMTX_OK);

We shouln't assert(), because this is easy for a user to trigger
by loading an ELF file that's been linked to the wrong address.
Something helpful that ideally includes the name of the ELF file
and perhaps the address might be nice.

(But overall I'm a bit wary of this and other patches in the series
just because they add checks that were previously not there, and
I'm not sure whether users might be accidentally relying on
the continues-anyway behaviour.)

I understand. Thanks for reviewing, I'll rework this one and the previous set_kernel_args().


thanks
-- PMM




reply via email to

[Prev in Thread] Current Thread [Next in Thread]