Re: [Qemu-devel] [PATCH 2/4] multiboot: load elf sections and section headers
Hi Anatol.
I have one comment about sections.c headers (and didn't really look
further in that file), and start.S.
Comments inline...
On 01/29/18 12:43, Anatol Pomozov wrote:
Multiboot may load section headers and all sections (even those that are
not part of any segment) to target memory.
Tested with an ELF application that uses data from strings table
section.
Signed-off-by: Anatol Pomozov
---
hw/core/loader.c | 8 +--
hw/i386/multiboot.c | 17 +++--
hw/s390x/ipl.c | 2 +-
include/hw/elf_ops.h | 110 +--
include/hw/loader.h | 11 +++-
tests/multiboot/Makefile | 8 ++-
tests/multiboot/generate_sections_out.py | 33 ++
tests/multiboot/modules.out | 22 +++
tests/multiboot/run_test.sh | 6 +-
tests/multiboot/sections.c | 57
tests/multiboot/start.S | 2 +-
11 files changed, 248 insertions(+), 28 deletions(-)
create mode 100755 tests/multiboot/generate_sections_out.py
create mode 100644 tests/multiboot/sections.c
diff --git a/tests/multiboot/sections.c b/tests/multiboot/sections.c
new file mode 100644
index 00..64060510ce
--- /dev/null
+++ b/tests/multiboot/sections.c
@@ -0,0 +1,57 @@
+/*
+ * Copyright (c) 2017 Anatol Pomozov
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "libc.h"
+#include "../../hw/i386/multiboot_header.h"
As like the first patch, I suggest creating a "multiboot" subdir in the
"include" subtree and putting all multiboot header files there.
Including "multiboot/multiboot_header.h" would be cleaner.
+#include "../../include/elf.h"
+
+int test_main(uint32_t magic, struct multiboot_info *mbi)
+{
+void *p;
+unsigned int i;
+
+(void) magic;
+multiboot_elf_section_header_table_t shdr;
+
+printf("Multiboot header at %x, ELF section headers %s\n\n", mbi,
+mbi->flags & MULTIBOOT_INFO_ELF_SHDR ? "present" : "doesn't present");
+
+shdr = mbi->u.elf_sec;
+printf("Sections list with %d entries of size %d at %x, string index %d\n",
+shdr.num, shdr.size, shdr.addr, shdr.shndx);
+
+const char *string_table = (char *)((Elf32_Shdr *)(uintptr_t)(shdr.addr +
+shdr.shndx * shdr.size))->sh_addr;
+
+for (i = 0, p = (void *)shdr.addr;
+ i < shdr.num;
+ i++, p += shdr.size)
+{
+Elf32_Shdr *sec;
+
+sec = (Elf32_Shdr *)p;
+printf("Elf section name=%s addr=%lx size=%ld\n",
+string_table + sec->sh_name, sec->sh_addr, sec->sh_size);
+}
+
+return 0;
+}
diff --git a/tests/multiboot/start.S b/tests/multiboot/start.S
index 7d33959650..bd404100c2 100644
--- a/tests/multiboot/start.S
+++ b/tests/multiboot/start.S
@@ -23,7 +23,7 @@
.section multiboot
#define MB_MAGIC 0x1badb002
-#define MB_FLAGS 0x0
+#define MB_FLAGS 0x2
#define MB_CHECKSUM -(MB_MAGIC + MB_FLAGS)
C headers can be included in assembly files. Since you are bringing
over multiboot_header.h, why not include it and use its values instead
of re-defining them here?
Thanks,
Jack
.align 4
[Qemu-devel] [PATCH 2/4] multiboot: load elf sections and section headers
Multiboot may load section headers and all sections (even those that are
not part of any segment) to target memory.
Tested with an ELF application that uses data from strings table
section.
Signed-off-by: Anatol Pomozov
---
hw/core/loader.c | 8 +--
hw/i386/multiboot.c | 17 +++--
hw/s390x/ipl.c | 2 +-
include/hw/elf_ops.h | 110 +--
include/hw/loader.h | 11 +++-
tests/multiboot/Makefile | 8 ++-
tests/multiboot/generate_sections_out.py | 33 ++
tests/multiboot/modules.out | 22 +++
tests/multiboot/run_test.sh | 6 +-
tests/multiboot/sections.c | 57
tests/multiboot/start.S | 2 +-
11 files changed, 248 insertions(+), 28 deletions(-)
create mode 100755 tests/multiboot/generate_sections_out.py
create mode 100644 tests/multiboot/sections.c
diff --git a/hw/core/loader.c b/hw/core/loader.c
index 91669d65aa..3f0412eeef 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -439,7 +439,7 @@ int load_elf_as(const char *filename,
{
return load_elf_ram(filename, translate_fn, translate_opaque,
pentry, lowaddr, highaddr, big_endian, elf_machine,
-clear_lsb, data_swab, as, true);
+clear_lsb, data_swab, as, true, NULL);
}
/* return < 0 if error, otherwise the number of bytes loaded in memory */
@@ -448,7 +448,7 @@ int load_elf_ram(const char *filename,
void *translate_opaque, uint64_t *pentry, uint64_t *lowaddr,
uint64_t *highaddr, int big_endian, int elf_machine,
int clear_lsb, int data_swab, AddressSpace *as,
- bool load_rom)
+ bool load_rom, SectionsData *sections)
{
int fd, data_order, target_data_order, must_swab, ret = ELF_LOAD_FAILED;
uint8_t e_ident[EI_NIDENT];
@@ -488,11 +488,11 @@ int load_elf_ram(const char *filename,
if (e_ident[EI_CLASS] == ELFCLASS64) {
ret = load_elf64(filename, fd, translate_fn, translate_opaque,
must_swab,
pentry, lowaddr, highaddr, elf_machine, clear_lsb,
- data_swab, as, load_rom);
+ data_swab, as, load_rom, sections);
} else {
ret = load_elf32(filename, fd, translate_fn, translate_opaque,
must_swab,
pentry, lowaddr, highaddr, elf_machine, clear_lsb,
- data_swab, as, load_rom);
+ data_swab, as, load_rom, sections);
}
fail:
diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
index c6d05ca46b..79b89e4fee 100644
--- a/hw/i386/multiboot.c
+++ b/hw/i386/multiboot.c
@@ -125,7 +125,7 @@ int load_multiboot(FWCfgState *fw_cfg,
{
int i;
bool is_multiboot = false;
-uint32_t flags = 0;
+uint32_t flags = 0, bootinfo_flags = 0;
uint32_t mh_entry_addr;
uint32_t mh_load_addr;
uint32_t mb_kernel_size;
@@ -134,6 +134,7 @@ int load_multiboot(FWCfgState *fw_cfg,
uint8_t *mb_bootinfo_data;
uint32_t cmdline_len;
struct multiboot_header *multiboot_header;
+SectionsData sections;
/* Ok, let's see if it is a multiboot image.
The header is 12x32bit long, so the latest entry may be 8192 - 48. */
@@ -172,9 +173,9 @@ int load_multiboot(FWCfgState *fw_cfg,
exit(1);
}
-kernel_size = load_elf(kernel_filename, NULL, NULL, &elf_entry,
+kernel_size = load_elf_ram(kernel_filename, NULL, NULL, &elf_entry,
&elf_low, &elf_high, 0, I386_ELF_MACHINE,
- 0, 0);
+ 0, 0, NULL, true, §ions);
if (kernel_size < 0) {
fprintf(stderr, "Error while loading elf kernel\n");
exit(1);
@@ -191,6 +192,13 @@ int load_multiboot(FWCfgState *fw_cfg,
mb_debug("qemu: loading multiboot-elf kernel (%#x bytes) with entry
%#zx\n",
mb_kernel_size, (size_t)mh_entry_addr);
+
+stl_p(&bootinfo.u.elf_sec.num, sections.num);
+stl_p(&bootinfo.u.elf_sec.size, sections.size);
+stl_p(&bootinfo.u.elf_sec.addr, sections.addr);
+stl_p(&bootinfo.u.elf_sec.shndx, sections.shndx);
+
+bootinfo_flags |= MULTIBOOT_INFO_ELF_SHDR;
} else {
/* Valid if mh_flags sets MULTIBOOT_AOUT_KLUDGE. */
uint32_t mh_header_addr = ldl_p(&multiboot_header->header_addr);
@@ -332,7 +340,8 @@ int load_multiboot(FWCfgState *fw_cfg,
stl_p(&bootinfo.mods_count, mbs.mb_mods_count); /* mods_count */
/* the kernel is where we want it to be now */
-stl_p(&bootinfo.flags, MULTIBOOT_INFO_MEMORY
+stl_p(&bootinfo.flags, bootinfo_flags
+ | MULTIBOOT_INFO_MEMORY
| MULTIBOOT_INFO_BOO
