[PATCH 4/4] rx: Support loading of ELF files too

Keith Packard via posted 4 patches 1 month, 3 weeks ago
[PATCH 4/4] rx: Support loading of ELF files too
Posted by Keith Packard via 1 month, 3 weeks ago
The existing loader supports raw binary blobs with the entry point
defined as the start of the blob. Add support for loading ELF files by
first checking if the provided filename has a valid ELF header,
falling back to the existing loader code when that fails.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 hw/rx/rx-gdbsim.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/hw/rx/rx-gdbsim.c b/hw/rx/rx-gdbsim.c
index 9e395ae345..58492b713f 100644
--- a/hw/rx/rx-gdbsim.c
+++ b/hw/rx/rx-gdbsim.c
@@ -25,6 +25,7 @@
 #include "exec/cpu_ldst.h"
 #include "hw/loader.h"
 #include "hw/rx/rx62n.h"
+#include "elf.h"
 #include "system/qtest.h"
 #include "system/device_tree.h"
 #include "system/reset.h"
@@ -85,6 +86,37 @@ static void rx_cpu_reset(void *opaque)
     }
 }
 
+static bool rx_load_elf(RXCPU *cpu, const char *filename)
+{
+    CPUState *cs = CPU(cpu);
+    Elf32_Ehdr elf_header;
+    bool elf_is64;
+    Error *err = NULL;
+    uint64_t lowaddr, highaddr, entry;
+    ssize_t ret;
+
+    load_elf_hdr(filename, &elf_header, &elf_is64, &err);
+    if (err) {
+        error_free(err);
+        return false;
+    }
+
+    ret = load_elf_as(filename, NULL, NULL, NULL,
+                      &entry, &lowaddr, &highaddr, NULL, false,
+                      EM_RX, 1, 0, cs->as);
+
+    if (ret <= 0) {
+        /* The header loaded but the image didn't */
+        error_report("qemu: could not load elf '%s': %s",
+                     filename, load_elf_strerror(ret));
+        exit(1);
+    }
+
+    cpu->env.reset_pc = entry;
+    cpu->env.use_reset_pc = true;
+    return true;
+}
+
 static void rx_load_image(RXCPU *cpu, const char *filename,
                           uint32_t start, uint32_t size)
 {
@@ -92,6 +124,10 @@ static void rx_load_image(RXCPU *cpu, const char *filename,
     long kernel_size;
     int i;
 
+    if (rx_load_elf(cpu, filename)) {
+        return;
+    }
+
     kernel_size = load_image_targphys(filename, start, size);
     if (kernel_size < 0) {
         fprintf(stderr, "qemu: could not load kernel '%s'\n", filename);
-- 
2.47.2
Re: [PATCH 4/4] rx: Support loading of ELF files too
Posted by Peter Maydell 1 month ago
On Tue, 18 Feb 2025 at 21:23, Keith Packard via <qemu-devel@nongnu.org> wrote:
>
> The existing loader supports raw binary blobs with the entry point
> defined as the start of the blob. Add support for loading ELF files by
> first checking if the provided filename has a valid ELF header,
> falling back to the existing loader code when that fails.
>
> Signed-off-by: Keith Packard <keithp@keithp.com>

Ths code does what it intends to, and I'm not saying we should
definitely *not* have it. I do think it's worth considering whether
we need it, given that you can already load an ELF image via the
generic loader (-device loader).

-kernel is a very "do what I mean" option that differs from target to
target, but what it's ideally supposed to mean is "this is a Linux
kernel, boot it like a Linux kernel". Some of our targets also
overload it to additionally have the behaviour "but if it's an ELF
file, load it as an ELF file, not like a kernel" (notably at least
arm M-profile). But I think that's something I'd consider a mistake
in retrospect, which we can't change now for backwards compatibility
reasons. (In particular it means we can't have "-kernel kernel.elf"
mean "this is a Linux kernel which is an ELF file and it wants
all the usual CPU setup that Linux's booting requirements mandate".)

So for target/rx, I guess what I'm saying is that if Linux
kernels for this architecture really are usually floating around
as ELF files then we should support that, but if this is just
"some users would like a way to load an ELF file, start at the
ELF entry point, and do no other setup of the CPU or system before
running code" then it might be better to instead point those
users at the generic loader (which does exactly that job
and does the same thing on all target architectures),
rather than adding a second mechanism for doing that.

thanks
-- PMM