[Bug 1910696] Re: Qemu fails to start with error " There is no option group 'spice'"

[Anatol Pomozov]

Are there any downsides of compiling modules statically (like what
Debian folks do)?

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1910696

Title:
  Qemu fails to start with error " There is no option group 'spice'"

Status in QEMU:
  New

Bug description:
  After upgrade from 5.1.0 to 5.2.0, qemu fails on start with error:
  `
  /usr/bin/qemu-system-x86_64 -S -name trinti -uuid 
f8ad2ff6-8808-4f42-8f0b-9e23acd20f84 -daemonize -cpu host -nographic -serial 
chardev:console -nodefaults -no-reboot -no-user-config -sandbox 
on,obsolete=deny,elevateprivileges=allow,spawn=deny,resourcecontrol=deny 
-readconfig /var/log/lxd/trinti/qemu.conf -pidfile /var/log/lxd/trinti/qemu.pid 
-D /var/log/lxd/trinti/qemu.log -chroot /var/lib/lxd/virtual-machines/trinti 
-smbios type=2,manufacturer=Canonical Ltd.,product=LXD -runas nobody: 
  qemu-system-x86_64:/var/log/lxd/trinti/qemu.conf:27: There is no option group 
'spice'
  qemu-system-x86_64: -readconfig /var/log/lxd/trinti/qemu.conf: read config 
/var/log/lxd/trinti/qemu.conf: Invalid argument
  `
  Bisected to first bad commit: 
https://github.com/qemu/qemu/commit/cbe5fa11789035c43fd2108ac6f45848954954b5

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1910696/+subscriptions

[Bug 1907926] [NEW] Implement TPM2 configuration for emulators that provide TCP interface

[Anatol Pomozov]

Public bug reported:

swtpm provides several interfaces for its emulated device: unix socket
(can be used by qemu), chardev. swtpm also provides TCP interface for
the device which is very convenient for testing as it does not require
root permissions.

It would be very useful to have QEMU to work with TPM devices emulated
via TCP.

** Affects: qemu
 Importance: Undecided
 Status: New

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1907926

Title:
  Implement TPM2 configuration for emulators that provide TCP interface

Status in QEMU:
  New

Bug description:
  swtpm provides several interfaces for its emulated device: unix socket
  (can be used by qemu), chardev. swtpm also provides TCP interface for
  the device which is very convenient for testing as it does not require
  root permissions.

  It would be very useful to have QEMU to work with TPM devices emulated
  via TCP.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1907926/+subscriptions

[Bug 1877716] Re: Win10 guest unusable after a few minutes

[Anatol Pomozov]

Thank you Stefan for the fixes. Once these patches land the upstream
repo I'll pull it into the Arch package and reenable io_uring.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1877716

Title:
  Win10 guest unusable after a few minutes

Status in QEMU:
  New

Bug description:
  On Arch Linux, the recent qemu package update seems to misbehave on
  some systems. In my case, my Windows 10 guest runs fine for around 5
  minutes and then start to get really sluggish, even unresponsive. It
  needs to be forced off. I could reproduce this on a minimal VM with no
  passthrough, although my current testing setup involves an nvme pcie
  passthrough.

  I bisected it to the following commit which rapidly starts to run sluggishly 
on my setup:
  https://github.com/qemu/qemu/commit/73fd282e7b6dd4e4ea1c3bbb3d302c8db51e4ccf

  I've ran the previous commit (
  https://github.com/qemu/qemu/commit/b321051cf48ccc2d3d832af111d688f2282f089b
  ) for the entire night without an issue so far.

  I believe this might be a duplicate of
  https://bugs.launchpad.net/qemu/+bug/1873032 , although I'm not sure.

  Linux cc 5.6.10-arch1-1 #1 SMP PREEMPT Sat, 02 May 2020 19:11:54 + x86_64 
GNU/Linux
  AMD Ryzen 7 2700X Eight-Core Processor

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1877716/+subscriptions

[Bug 1877716] Re: Win10 guest unusable after a few minutes

[Anatol Pomozov]

Thank you Stefan for looking at this issue.

As Alexander and @postfactum mentioned Arch disabled io_uring feature
after this bug has been discovered. Here is an Arch Linux issue that
tracks it https://bugs.archlinux.org/task/66578

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1877716

Title:
  Win10 guest unusable after a few minutes

Status in QEMU:
  New

Bug description:
  On Arch Linux, the recent qemu package update seems to misbehave on
  some systems. In my case, my Windows 10 guest runs fine for around 5
  minutes and then start to get really sluggish, even unresponsive. It
  needs to be forced off. I could reproduce this on a minimal VM with no
  passthrough, although my current testing setup involves an nvme pcie
  passthrough.

  I bisected it to the following commit which rapidly starts to run sluggishly 
on my setup:
  https://github.com/qemu/qemu/commit/73fd282e7b6dd4e4ea1c3bbb3d302c8db51e4ccf

  I've ran the previous commit (
  https://github.com/qemu/qemu/commit/b321051cf48ccc2d3d832af111d688f2282f089b
  ) for the entire night without an issue so far.

  I believe this might be a duplicate of
  https://bugs.launchpad.net/qemu/+bug/1873032 , although I'm not sure.

  Linux cc 5.6.10-arch1-1 #1 SMP PREEMPT Sat, 02 May 2020 19:11:54 + x86_64 
GNU/Linux
  AMD Ryzen 7 2700X Eight-Core Processor

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1877716/+subscriptions

Re: [Qemu-devel] [PATCH 1/4] multiboot: Change multiboot_info from array of bytes to a C struct

[Anatol Pomozov]

Hello

Actually I just fetched recent chnages and tests/multiboot/run_test.sh
does not work for me anymore. I rebuilt 'master' branch without my
changes and see the same issue. It looks like debug console does not
print to stdio anymore.

Does anyone see the same issue?

Re: [Qemu-devel] [PATCH 1/4] multiboot: Change multiboot_info from array of bytes to a C struct

[Anatol Pomozov]

Hi Kevin

Is the patch series look good? Are there any other unresolved issues?

On Mon, Jan 29, 2018 at 12:02 PM, Kevin Wolf <kw...@redhat.com> wrote:
> Am 29.01.2018 um 19:21 hat Anatol Pomozov geschrieben:
>> Hi
>>
>> On Mon, Jan 15, 2018 at 6:49 AM, Kevin Wolf <kw...@redhat.com> wrote:
>> > Am 13.10.2017 um 01:54 hat Anatol Pomozov geschrieben:
>> >> Using C structs makes the code more readable and prevents type conversion
>> >> errors.
>> >>
>> >> Borrow multiboot1 header from GRUB project.
>> >>
>> >> Signed-off-by: Anatol Pomozov <anatol.pomo...@gmail.com>
>> >> ---
>> >>  hw/i386/multiboot.c | 124 +
>> >>  hw/i386/multiboot_header.h  | 254 
>> >> 
>> >>  tests/multiboot/mmap.c  |  14 +--
>> >>  tests/multiboot/mmap.out|  10 ++
>> >>  tests/multiboot/modules.c   |  12 ++-
>> >>  tests/multiboot/modules.out |  10 ++
>> >>  tests/multiboot/multiboot.h |  66 
>> >>  7 files changed, 339 insertions(+), 151 deletions(-)
>> >>  create mode 100644 hw/i386/multiboot_header.h
>> >>  delete mode 100644 tests/multiboot/multiboot.h
>> >
>> > This is a good change in general. However, I'm not sure if we should
>> > really replace the header used in the tests. After all, the tests also
>> > test whether things are at the right place, and if there is an error in
>> > the header file, we can only catch it if the tests keep using their own
>> > definitions.
>>
>> The added multibooh.h is from GRUB - the developers of multiboot. I
>> think we can trust it. Having a single header will make future tests
>> maintainence easier.
>>
>> But if you feel strongly that qemu tests should use it's own version
>> of multiboot header then I can add it back.
>
> No, I don't feel strongly, just wanted to mention that there is an
> advantage of having a separate header in case you had not thought of it.
> The decision is up to you.

I think it is better to use one standard trusted header. This way we
reduce maintenance cost.

Re: [Qemu-devel] [PATCH 1/4] multiboot: Change multiboot_info from array of bytes to a C struct

[Anatol Pomozov]

Hi

On Wed, Jan 31, 2018 at 1:12 AM, Kevin Wolf <kw...@redhat.com> wrote:
> Am 31.01.2018 um 00:15 hat Jack Schwartz geschrieben:
>> Hi Anatol and Kevin.
>>
>> Even though I am new to Qemu, I have a patch to deliver for
>> multiboot.c (as you know) and so I feel familiar enough to do a review
>> of this patch.  One comment is probably more for maintainers.
>
> The Multiboot code is essentially unmaintained. It's technically part of
> the PC/x86 subsystem, but their maintainers don't seem to care at all.
> So in order to make any progress here, I decided that I will send a
> pull request for Multiboot once we have the patches ready (as a one-time
> thing, without officially making myself the maintainer).
>
> So I am the closest thing to a maintainer that we have here, and while
> I'm familiar with some of the Multiboot-specific code, I don't really
> know the ELF code and don't have a lot of time to spend here.
>
> Therefore it's very welcome if you review the patches of each other,
> even if you're not perfectly familiar with the code, as there is
> probably noone else who could do a better review.
>
>> On 01/29/18 12:43, Anatol Pomozov wrote:
>> > @@ -253,11 +228,11 @@ int load_multiboot(FWCfgState *fw_cfg,
>> >   mb_load_size = mb_kernel_size;
>> >   }
>> > -/* Valid if mh_flags sets MULTIBOOT_HEADER_HAS_VBE.
>> > -uint32_t mh_mode_type = ldl_p(header+i+32);
>> > -uint32_t mh_width = ldl_p(header+i+36);
>> > -uint32_t mh_height = ldl_p(header+i+40);
>> > -uint32_t mh_depth = ldl_p(header+i+44); */
>> > +/* Valid if mh_flags sets MULTIBOOT_VIDEO_MODE.
>> > +uint32_t mh_mode_type = ldl_p(_header->mode_type);
>> > +uint32_t mh_width = ldl_p(_header->width);
>> > +uint32_t mh_height = ldl_p(_header->height);
>> > +uint32_t mh_depth = ldl_p(_header->depth); */
>> This question is probably more for maintainers...
>>
>> In the patch series I submitted, people were OK that I was going to delete
>> these lines since they were only comments anyway.  Your approach leaves
>> these lines in and updates them even though they are comments.  Which way is
>> preferred?
>
> As far as I am concerned, I honestly don't mind either way. It's
> trivial code, so we won't lose anything by removing it.

This change suppose to be a refactoring and tries to avoid functional
changes. I can remove it in a separate change.

>
> The ideal solution would be just implementing support for it, of course.
> If we wanted to do that, I think we would have to pass the values
> through fw_cfg and then set the VBE mode in the Mutiboot option rom.
>
>> >   mb_debug("multiboot: mh_header_addr = %#x\n", mh_header_addr);
>> >   mb_debug("multiboot: mh_load_addr = %#x\n", mh_load_addr);
>> > @@ -295,14 +270,15 @@ int load_multiboot(FWCfgState *fw_cfg,
>> >   }
>> >   mbs.mb_buf_size += cmdline_len;
>> > -mbs.mb_buf_size += MB_MOD_SIZE * mbs.mb_mods_avail;
>> > +mbs.mb_buf_size += sizeof(multiboot_module_t) * mbs.mb_mods_avail;
>> >   mbs.mb_buf_size += strlen(bootloader_name) + 1;
>> >   mbs.mb_buf_size = TARGET_PAGE_ALIGN(mbs.mb_buf_size);
>> >   /* enlarge mb_buf to hold cmdlines, bootloader, mb-info structs */
>> >   mbs.mb_buf= g_realloc(mbs.mb_buf, mbs.mb_buf_size);
>> > -mbs.offset_cmdlines   = mbs.offset_mbinfo + mbs.mb_mods_avail * 
>> > MB_MOD_SIZE;
>> > +mbs.offset_cmdlines   = mbs.offset_mbinfo +
>> > +mbs.mb_mods_avail * sizeof(multiboot_module_t);
>> >   mbs.offset_bootloader = mbs.offset_cmdlines + cmdline_len;
>> >   if (initrd_filename) {
>> > @@ -348,22 +324,22 @@ int load_multiboot(FWCfgState *fw_cfg,
>> >   char kcmdline[strlen(kernel_filename) + strlen(kernel_cmdline) + 2];
>> >   snprintf(kcmdline, sizeof(kcmdline), "%s %s",
>> >kernel_filename, kernel_cmdline);
>> > -stl_p(bootinfo + MBI_CMDLINE, mb_add_cmdline(, kcmdline));
>> > +stl_p(, mb_add_cmdline(, kcmdline));
>> > -stl_p(bootinfo + MBI_BOOTLOADER, mb_add_bootloader(, 
>> > bootloader_name));
>> > +stl_p(_loader_name, mb_add_bootloader(, 
>> > bootloader_name));
>> > -stl_p(bootinfo + MBI_MODS_ADDR,  mbs.mb_buf_phys + mbs.offset_mbinfo);
>> > -stl_p(bootinfo + MBI_MODS_COUNT, mbs.mb_mods_count); /* mods_count */
>> > +stl_p(_addr,  mbs.mb_buf_phys + mbs.offset_mbin

[Qemu-devel] [PATCH 3/4] multiboot: make tests work with clang

[Anatol Pomozov]

 * clang 3.8 enables SSE even for 32bit code. Generate code for pentium
   CPU to make sure no new instructions are used.
 * add memset() implementation. Clang implements array zeroing in
   print_num() via memset() function call.

Signed-off-by: Anatol Pomozov <anatol.pomo...@gmail.com>
---
 tests/multiboot/Makefile| 2 +-
 tests/multiboot/libc.c  | 9 +
 tests/multiboot/libc.h  | 2 ++
 tests/multiboot/run_test.sh | 1 +
 4 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/tests/multiboot/Makefile b/tests/multiboot/Makefile
index b6a5056347..79dfe85afc 100644
--- a/tests/multiboot/Makefile
+++ b/tests/multiboot/Makefile
@@ -1,5 +1,5 @@
 CC=gcc
-CCFLAGS=-m32 -Wall -Wextra -Werror -fno-stack-protector -nostdinc -fno-builtin
+CCFLAGS=-m32 -Wall -Wextra -Werror -fno-stack-protector -nostdinc -fno-builtin 
-march=pentium
 ASFLAGS=-m32
 
 LD=ld
diff --git a/tests/multiboot/libc.c b/tests/multiboot/libc.c
index 6df9bda96d..512fccd7fa 100644
--- a/tests/multiboot/libc.c
+++ b/tests/multiboot/libc.c
@@ -33,6 +33,15 @@ void* memcpy(void *dest, const void *src, int n)
 
 return dest;
 }
+void *memset(void *s, int c, size_t n)
+{
+size_t i;
+char *d = s;
+for (i = 0; i < n; i++) {
+*d++ = c;
+}
+return s;
+}
 
 static void print_char(char c)
 {
diff --git a/tests/multiboot/libc.h b/tests/multiboot/libc.h
index 04c9922c27..bdf7c34287 100644
--- a/tests/multiboot/libc.h
+++ b/tests/multiboot/libc.h
@@ -36,6 +36,7 @@ typedef signed short int16_t;
 typedef signed char int8_t;
 
 typedef uint32_t uintptr_t;
+typedef uint32_t size_t;
 
 
 /* stdarg.h */
@@ -58,5 +59,6 @@ static inline void outb(uint16_t port, uint8_t data)
 
 void printf(const char *fmt, ...);
 void* memcpy(void *dest, const void *src, int n);
+void* memset(void *s, int c, size_t n);
 
 #endif
diff --git a/tests/multiboot/run_test.sh b/tests/multiboot/run_test.sh
index f04e35cbf0..38dcfef42c 100755
--- a/tests/multiboot/run_test.sh
+++ b/tests/multiboot/run_test.sh
@@ -29,6 +29,7 @@ run_qemu() {
 printf %b "\n\n=== Running test case: $kernel $* ===\n\n" >> test.log
 
 $QEMU \
+-cpu pentium \
 -kernel $kernel \
 -display none \
 -device isa-debugcon,chardev=stdio \
-- 
2.16.0.rc1.238.g530d649a79-goog

[Qemu-devel] [PATCH 4/4] multiboot: Make elf64 loading functionality compatible with GRUB

[Anatol Pomozov]

GRUB is a reference multiboot implementation and supports loading elf64
binaries. Make QEMU to work similar was as GRUB.
---
 hw/i386/multiboot.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
index 79b89e4fee..676ac7c48d 100644
--- a/hw/i386/multiboot.c
+++ b/hw/i386/multiboot.c
@@ -168,11 +168,6 @@ int load_multiboot(FWCfgState *fw_cfg,
 int kernel_size;
 fclose(f);
 
-if (((struct elf64_hdr*)header)->e_machine == EM_X86_64) {
-fprintf(stderr, "Cannot load x86-64 image, give a 32bit one.\n");
-exit(1);
-}
-
 kernel_size = load_elf_ram(kernel_filename, NULL, NULL, _entry,
_low, _high, 0, I386_ELF_MACHINE,
0, 0, NULL, true, );
-- 
2.16.0.rc1.238.g530d649a79-goog

[Qemu-devel] [PATCH 2/4] multiboot: load elf sections and section headers

[Anatol Pomozov]

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 <anatol.pomo...@gmail.com>
---
 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, _entry,
+kernel_size = load_elf_ram(kernel_filename, NULL, NULL, _entry,
_low, _high, 0, I386_ELF_MACHINE,
-   0, 0);
+   0, 0, NULL, true, );
 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(_sec.num, sections.num);
+stl_p(_sec.size, sections.size);
+stl_p(_sec.addr, sections.addr);
+stl_p(_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(_header->header_addr);
@@ -332,7 +340,8 @@ int load_multiboot(FWCfgState *fw_cfg,
 stl_p(_count, mbs.mb_mods_count); /* mods_count */
 
 /* the kernel is where we want it to be now */
-stl_p(, MULTIBOOT_INFO_MEMORY
+stl_p(, bootinfo_flags
+   | MULTIBOOT_INFO_MEMORY
| MULTIBOOT_INFO_BOOTDEV
| MULTIBOOT_INFO_CMDLINE
  

[Qemu-devel] [PATCH 1/4] multiboot: Change multiboot_info from array of bytes to a C struct

[Anatol Pomozov]

Using C structs makes the code more readable and prevents type conversion
errors.

Borrow multiboot1 header from GRUB project.

Signed-off-by: Anatol Pomozov <anatol.pomo...@gmail.com>
---
 hw/i386/multiboot.c | 124 +
 hw/i386/multiboot_header.h  | 254 
 tests/multiboot/mmap.c  |  14 +--
 tests/multiboot/mmap.out|  10 ++
 tests/multiboot/modules.c   |  12 ++-
 tests/multiboot/modules.out |  10 ++
 tests/multiboot/multiboot.h |  66 
 7 files changed, 339 insertions(+), 151 deletions(-)
 create mode 100644 hw/i386/multiboot_header.h
 delete mode 100644 tests/multiboot/multiboot.h

diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
index c7b70c91d5..c6d05ca46b 100644
--- a/hw/i386/multiboot.c
+++ b/hw/i386/multiboot.c
@@ -28,6 +28,7 @@
 #include "hw/hw.h"
 #include "hw/nvram/fw_cfg.h"
 #include "multiboot.h"
+#include "multiboot_header.h"
 #include "hw/loader.h"
 #include "elf.h"
 #include "sysemu/sysemu.h"
@@ -47,39 +48,9 @@
 #error multiboot struct needs to fit in 16 bit real mode
 #endif
 
-enum {
-/* Multiboot info */
-MBI_FLAGS   = 0,
-MBI_MEM_LOWER   = 4,
-MBI_MEM_UPPER   = 8,
-MBI_BOOT_DEVICE = 12,
-MBI_CMDLINE = 16,
-MBI_MODS_COUNT  = 20,
-MBI_MODS_ADDR   = 24,
-MBI_MMAP_ADDR   = 48,
-MBI_BOOTLOADER  = 64,
-
-MBI_SIZE= 88,
-
-/* Multiboot modules */
-MB_MOD_START= 0,
-MB_MOD_END  = 4,
-MB_MOD_CMDLINE  = 8,
-
-MB_MOD_SIZE = 16,
-
-/* Region offsets */
-ADDR_E820_MAP = MULTIBOOT_STRUCT_ADDR + 0,
-ADDR_MBI  = ADDR_E820_MAP + 0x500,
-
-/* Multiboot flags */
-MULTIBOOT_FLAGS_MEMORY  = 1 << 0,
-MULTIBOOT_FLAGS_BOOT_DEVICE = 1 << 1,
-MULTIBOOT_FLAGS_CMDLINE = 1 << 2,
-MULTIBOOT_FLAGS_MODULES = 1 << 3,
-MULTIBOOT_FLAGS_MMAP= 1 << 6,
-MULTIBOOT_FLAGS_BOOTLOADER  = 1 << 9,
-};
+/* Region offsets */
+#define ADDR_E820_MAP MULTIBOOT_STRUCT_ADDR
+#define ADDR_MBI  (ADDR_E820_MAP + 0x500)
 
 typedef struct {
 /* buffer holding kernel, cmdlines and mb_infos */
@@ -128,14 +99,15 @@ static void mb_add_mod(MultibootState *s,
hwaddr start, hwaddr end,
hwaddr cmdline_phys)
 {
-char *p;
+multiboot_module_t *mod;
 assert(s->mb_mods_count < s->mb_mods_avail);
 
-p = (char *)s->mb_buf + s->offset_mbinfo + MB_MOD_SIZE * s->mb_mods_count;
+mod = s->mb_buf + s->offset_mbinfo +
+  sizeof(multiboot_module_t) * s->mb_mods_count;
 
-stl_p(p + MB_MOD_START,   start);
-stl_p(p + MB_MOD_END, end);
-stl_p(p + MB_MOD_CMDLINE, cmdline_phys);
+stl_p(>mod_start, start);
+stl_p(>mod_end,   end);
+stl_p(>cmdline,   cmdline_phys);
 
 mb_debug("mod%02d: "TARGET_FMT_plx" - "TARGET_FMT_plx"\n",
  s->mb_mods_count, start, end);
@@ -151,26 +123,29 @@ int load_multiboot(FWCfgState *fw_cfg,
int kernel_file_size,
uint8_t *header)
 {
-int i, is_multiboot = 0;
+int i;
+bool is_multiboot = false;
 uint32_t flags = 0;
 uint32_t mh_entry_addr;
 uint32_t mh_load_addr;
 uint32_t mb_kernel_size;
 MultibootState mbs;
-uint8_t bootinfo[MBI_SIZE];
+multiboot_info_t bootinfo;
 uint8_t *mb_bootinfo_data;
 uint32_t cmdline_len;
+struct multiboot_header *multiboot_header;
 
 /* Ok, let's see if it is a multiboot image.
The header is 12x32bit long, so the latest entry may be 8192 - 48. */
 for (i = 0; i < (8192 - 48); i += 4) {
-if (ldl_p(header+i) == 0x1BADB002) {
-uint32_t checksum = ldl_p(header+i+8);
-flags = ldl_p(header+i+4);
+multiboot_header = (struct multiboot_header *)(header + i);
+if (ldl_p(_header->magic) == MULTIBOOT_HEADER_MAGIC) {
+uint32_t checksum = ldl_p(_header->checksum);
+flags = ldl_p(_header->flags);
 checksum += flags;
-checksum += (uint32_t)0x1BADB002;
+checksum += (uint32_t)MULTIBOOT_HEADER_MAGIC;
 if (!checksum) {
-is_multiboot = 1;
+is_multiboot = true;
 break;
 }
 }
@@ -180,13 +155,13 @@ int load_multiboot(FWCfgState *fw_cfg,
 return 0; /* no multiboot */
 
 mb_debug("qemu: I believe we found a multiboot image!\n");
-memset(bootinfo, 0, sizeof(bootinfo));
+memset(, 0, sizeof(bootinfo));
 memset(, 0, sizeof(mbs));
 
-if (flags & 0x0004) { /* MULTIBOOT_HEADER_HAS_VBE */
+if (flags & MULTIBOOT_VIDEO_MODE) {
 fprintf(stderr, "qemu: multiboot knows VBE. we don't.\n");
 }
-if (!(flags & 0x0001)) { /* 

Re: [Qemu-devel] [PATCH 3/4] multiboot: load elf sections and section headers

[Anatol Pomozov]

Hi

On Mon, Jan 15, 2018 at 7:41 AM, Kevin Wolf <kw...@redhat.com> wrote:
> Am 13.10.2017 um 01:54 hat Anatol Pomozov geschrieben:
>> 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 <anatol.pomo...@gmail.com>
>> ---
>>  hw/core/loader.c |   8 +--
>>  hw/i386/multiboot.c  |  83 +---
>>  hw/s390x/ipl.c   |   2 +-
>>  include/hw/elf_ops.h | 107 
>> ++-
>>  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   |  55 
>>  tests/multiboot/start.S  |   2 +-
>>  11 files changed, 276 insertions(+), 61 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 4593061445..a8787f7685 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 7dacd6d827..841d05160a 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. */
>> @@ -161,37 +162,8 @@ int load_multiboot(FWCfgState *fw_cfg,
>>  if (flags & MULTIBOOT_VIDEO_MODE) {
>>  fprintf(stderr, "qemu: multiboot knows VBE. we don't.\n");
>>  }
>> -if (!(flags & MULTIBOOT_AOUT_KLUDGE)) {
>> -uint64_t elf_entry;
>> -uint64_t elf_low, elf_high;
>> -int kernel_size;
>> -fclose(f);
>>
>> -if (((struct elf64_hdr*)header)->e_machine == EM_X86_64) {
>> -fprintf(stderr, "Cannot load x86-64 image, give a 32bit

Re: [Qemu-devel] [PATCH 2/4] multiboot: load any machine type of ELF

[Anatol Pomozov]

Hi

On Mon, Jan 15, 2018 at 6:52 AM, Kevin Wolf <kw...@redhat.com> wrote:
> Am 16.10.2017 um 20:38 hat Anatol Pomozov geschrieben:
>> Hi
>>
>> On Mon, Oct 16, 2017 at 1:27 AM, Kevin Wolf <kw...@redhat.com> wrote:
>> > Am 14.10.2017 um 15:41 hat Peter Maydell geschrieben:
>> >> On 14 October 2017 at 00:21, Eduardo Habkost <ehabk...@redhat.com> wrote:
>> >> > I don't believe the spec restricts that, but I don't see why it
>> >> > would be useful to load an ELF file that doesn't match the target
>> >> > architecture (e.g. loading non-x86 ELF files on a x86 machine
>> >> > like PC).
>>
>> I see what do you mean Eduardo. Yes it makes sense to restrict ELF to
>> the currently emulated platform.
>>
>> On a second thought adding multiboot support for non x86 needs to go
>> with other changes, e.g. multiboot.c should be moved out of hw/i386 to
>> some platform-independent location. It probably worth to postpone this
>> change until after Qemu gets multiboot2 support that explicitly states
>> MIPS support, thus it will be easier to test this codepath on multiple
>> platforms.
>>
>> So if you don't mind I'll remove this patch from the current patch
>> series and put it into later one.
>
> I can't find a new version of this patch series with this patch dropped.
> Are you still planning to send one?

Yes I dropped it. It belongs to future multiboot refactoring patch series.

> The spec isn't really explicit about it being a requirement, but it does
> say that its target are 32-bit OSes on PCs, and it defines the boot
> state in terms of i386 registers, so it doesn't make sense for non-x86.

>From my reading of multiboot specification it says that boot state is
32-bit. Thus OS image should make sure its entry point is 32bit code.
But specification does not say that ELF should not contain 64bit code.
GRUB project, the reference multiboot implementation, supports 64bit
ELF.

> From my interpretation of the spec, even support for 64-bit ELFs seems
> to be (implicitly) out of spec (there is one place where it even says
> "refer to the i386 ELF documentation for details"), but if GRUB
> implements it...

Yes, GRUB supports it from its early days. One reason why I sent this
patch is to make QEMU multiboot functionality compatible with GRUB. So
the same binary can work in emulator and at real hardware.

Re: [Qemu-devel] [PATCH 1/4] multiboot: Change multiboot_info from array of bytes to a C struct

[Anatol Pomozov]

Hi

On Mon, Jan 15, 2018 at 6:49 AM, Kevin Wolf <kw...@redhat.com> wrote:
> Am 13.10.2017 um 01:54 hat Anatol Pomozov geschrieben:
>> Using C structs makes the code more readable and prevents type conversion
>> errors.
>>
>> Borrow multiboot1 header from GRUB project.
>>
>> Signed-off-by: Anatol Pomozov <anatol.pomo...@gmail.com>
>> ---
>>  hw/i386/multiboot.c | 124 +
>>  hw/i386/multiboot_header.h  | 254 
>> 
>>  tests/multiboot/mmap.c  |  14 +--
>>  tests/multiboot/mmap.out|  10 ++
>>  tests/multiboot/modules.c   |  12 ++-
>>  tests/multiboot/modules.out |  10 ++
>>  tests/multiboot/multiboot.h |  66 
>>  7 files changed, 339 insertions(+), 151 deletions(-)
>>  create mode 100644 hw/i386/multiboot_header.h
>>  delete mode 100644 tests/multiboot/multiboot.h
>
> This is a good change in general. However, I'm not sure if we should
> really replace the header used in the tests. After all, the tests also
> test whether things are at the right place, and if there is an error in
> the header file, we can only catch it if the tests keep using their own
> definitions.

The added multibooh.h is from GRUB - the developers of multiboot. I
think we can trust it. Having a single header will make future tests
maintainence easier.

But if you feel strongly that qemu tests should use it's own version
of multiboot header then I can add it back.

Re: [Qemu-devel] [PATCH QEMU v1 0/4] multiboot: bss_end_addr can be zero / cleanup

[Anatol Pomozov]

Hello Jack

On Wed, Jan 17, 2018 at 12:06 PM, Jack Schwartz
 wrote:
> Hi Kevin and Anatol.
>
> Kevin, thanks for your review.
>
> More inline below...
>
> On 01/15/18 07:54, Kevin Wolf wrote:
>>
>> Am 21.12.2017 um 18:25 hat Jack Schwartz geschrieben:
>>>
>>> Properly account for the possibility of multiboot kernels with a zero
>>> bss_end_addr.  The Multiboot Specification, section 3.1.3 allows for
>>> kernels without a bss section, by allowing a zeroed bss_end_addr
>>> multiboot
>>> header field.
>>>
>>> Do some cleanup to multiboot.c as well:
>>> - Remove some unused variables.
>>> - Use more intuitive header names when displaying fields in messages.
>>> - Change fprintf(stderr...) to error_report
>>
>> There are some conflicts with Anatol's (CCed) multiboot series:
>> https://lists.nongnu.org/archive/html/qemu-devel/2017-10/msg03003.html
>>
>> None if these should be hard to resolve, but it would be good if you
>> could agree with each other whose patch series should come first, and
>> then the other one should be rebased on top of that.
>
> Anatol,
>
> from my side, there are pros and cons to either patch set going in first,
> but advantages to either are pretty negligible.  Pro for you going first: I
> can use the constants you will define in header files.  Pro for me going
> first: your merge should be about the same as if you went first (since my
> changes are small, more localized and affect only multiboot.c) and my merge
> will be easier.
>
> What are your thoughts?

Please move ahead with your patches. I'll rebase my changes on top of yours.

>>
>> Testing:
>>1) Ran the "make check" test suite.
>>2) Booted multiboot kernel with bss_end_addr=0.  (I rolled my own
>>   grub multiboot.elf test "kernel" by modifying source.)  Verified
>>   with gdb that new code that reads addresses/offsets from multiboot
>>   header was accessed.
>>3) Booted multiboot kernel with non-zero bss_end_addr.
>>4) Uncommented DEBUG_MULTIBOOT in multiboot.c and verified messages
>> worked.
>>5) Code has soaked in an internal repo for two months.
>> Can you integrate your test kernel from 2) in tests/multiboot/ so we can
>> keep this as a regression test?
>
> Kevin and alias,
>
> Before I proceed with adding my multiboot test file, I'll clarify here that
> I started with a version from the grub2 tree.  In that file I expanded a
> header file, also from the same tree.  Neither file had any license header,
> though the tree I got them from (Dated October 2017) contains the GNU GPLv3
> license file.
>
> I'll need to check with my company before I can say I can deliver this file.
> If I deliver it, I'll add a header stating the GPL license, that it came
> from grub2 and to check its repository for contributors.
>>>
>>> Jack Schwartz (4):
>>>multiboot: bss_end_addr can be zero
>>>multiboot: Remove unused variables from multiboot.c
>>>multiboot: Use header names when displaying fields
>>>multiboot: fprintf(stderr...) -> error_report()
>>
>> Apart from the conflicts, the patches look good to me.
>
> Thanks,
> Jack
>>
>>
>> Kevin
>>
>

Re: [Qemu-devel] [PATCH 3/4] multiboot: load elf sections and section headers

[Anatol Pomozov]

Hello

On Tue, Oct 31, 2017 at 11:38 AM, Anatol Pomozov
<anatol.pomo...@gmail.com> wrote:
> Hi
>
> On Thu, Oct 19, 2017 at 2:36 AM, Kevin Wolf <kw...@redhat.com> wrote:
>> Am 18.10.2017 um 19:22 hat Anatol Pomozov geschrieben:
>>> Hello qemu-devs,
>>>
>>> This patchset has been posted a while ago. I would like to move
>>> forward with it and look at the next task (e.g. multiboot2 support in
>>> QEMU). Who is the multiboot code maintainer who can help to review
>>> this patchset and give go/no-go answer?
>>
>> I think that's exactly your problem, there is nobody who feels
>> responsible for Multiboot support. Officially, it is part of the PC/x86
>> subsystems:
>
> In this case I would like to become the official Qemu/Multiboot
> maintainer. I do development related to x86 osdev and actively use
> qemu for my projects. I am interested in having a feature-rich and
> robust Multiboot implementation.

It is very unfortunate to see lack of interest in improving
qemu/multiboot functionality. I guess my best option for now is to
avoid using qemu multiboot implementation and use grub instead.

Re: [Qemu-devel] [PATCH 3/4] multiboot: load elf sections and section headers

[Anatol Pomozov]

Hi

On Thu, Oct 19, 2017 at 2:36 AM, Kevin Wolf <kw...@redhat.com> wrote:
> Am 18.10.2017 um 19:22 hat Anatol Pomozov geschrieben:
>> Hello qemu-devs,
>>
>> This patchset has been posted a while ago. I would like to move
>> forward with it and look at the next task (e.g. multiboot2 support in
>> QEMU). Who is the multiboot code maintainer who can help to review
>> this patchset and give go/no-go answer?
>
> I think that's exactly your problem, there is nobody who feels
> responsible for Multiboot support. Officially, it is part of the PC/x86
> subsystems:

In this case I would like to become the official Qemu/Multiboot
maintainer. I do development related to x86 osdev and actively use
qemu for my projects. I am interested in having a feature-rich and
robust Multiboot implementation. In mid term I see several tasks for
me:
 - finish and merge my cleanup + MULTIBOOT_INFO_ELF_SHDR patch series
 - add Multiboot2 support
 - (optional) look at MIPS support for Multiboot. I do not have/use
MIPS but it is beneficial to add multiplatform support to Multiboot
 - reviewing incoming patches


A bit about me. My name is Anatol Pomozov and at my spare time I
participate at several open sources projects. I am an Arch developer
since 2013 where I maintain a bunch of packages (including Arch qemu
package [1]). I have ~40 patches in the Linux kernel tree, where the
biggest contribution is a driver for audio codec NAU8825. I
contributed a lot of small patches to numerous projects (compilers,
toolchains, ...)

[1] https://www.archlinux.org/packages/extra/i686/qemu/

Re: [Qemu-devel] [PATCH 3/4] multiboot: load elf sections and section headers

[Anatol Pomozov]

Hello qemu-devs,

This patchset has been posted a while ago. I would like to move
forward with it and look at the next task (e.g. multiboot2 support in
QEMU). Who is the multiboot code maintainer who can help to review
this patchset and give go/no-go answer?

On Thu, Oct 12, 2017 at 4:54 PM, Anatol Pomozov
<anatol.pomo...@gmail.com> 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 <anatol.pomo...@gmail.com>
> ---
>  hw/core/loader.c |   8 +--
>  hw/i386/multiboot.c  |  83 +---
>  hw/s390x/ipl.c   |   2 +-
>  include/hw/elf_ops.h | 107 
> ++-
>  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   |  55 
>  tests/multiboot/start.S  |   2 +-
>  11 files changed, 276 insertions(+), 61 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 4593061445..a8787f7685 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 7dacd6d827..841d05160a 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. */
> @@ -161,37 +162,8 @@ int load_multiboot(FWCfgState *fw_cfg,
>  if (flags & MULTIBOOT_VIDEO_MODE) {
>  fprintf(stderr, "qemu: multiboot knows VBE. we don't.\n");
>  }
> -if (!(flags & MULTIBOOT_AOUT_KLUDGE)) {
> -uint64_t elf_entry;
> -uint64_t elf_low, elf_high;
> -int kernel_size;
> -fclose(f);
>
> -if (((struct elf64_hdr*)header)->e_machine == EM_X86_64) {
> -fprintf(stderr, "Cannot load x86-64 image, give a 32bit one.\n");
> -exit(1);
> -}
> -
> -kernel_size = load_elf(kernel_filename, NULL, NULL, _entry,
> -   _low,

Re: [Qemu-devel] [PATCH 2/4] multiboot: load any machine type of ELF

[Anatol Pomozov]

Hi

On Mon, Oct 16, 2017 at 1:27 AM, Kevin Wolf  wrote:
> Am 14.10.2017 um 15:41 hat Peter Maydell geschrieben:
>> On 14 October 2017 at 00:21, Eduardo Habkost  wrote:
>> > I don't believe the spec restricts that, but I don't see why it
>> > would be useful to load an ELF file that doesn't match the target
>> > architecture (e.g. loading non-x86 ELF files on a x86 machine
>> > like PC).

I see what do you mean Eduardo. Yes it makes sense to restrict ELF to
the currently emulated platform.

On a second thought adding multiboot support for non x86 needs to go
with other changes, e.g. multiboot.c should be moved out of hw/i386 to
some platform-independent location. It probably worth to postpone this
change until after Qemu gets multiboot2 support that explicitly states
MIPS support, thus it will be easier to test this codepath on multiple
platforms.

So if you don't mind I'll remove this patch from the current patch
series and put it into later one.

>>
>> Agreed. If we have non i386 boards that want to use multiboot
>> we should probably move the common code out of hw/i386...
>
> Impossible with Multiboot 1, it's a spec that is really made for i386.
>
> The spec isn't really explicit about it being a requirement, but it does
> say that its target are 32-bit OSes on PCs, and it defines the boot
> state in terms of i386 registers, so it doesn't make sense for non-x86.
>
> From my interpretation of the spec, even support for 64-bit ELFs seems
> to be (implicitly) out of spec (there is one place where it even says
> "refer to the i386 ELF documentation for details"), but if GRUB
> implements it...

Yes GRUB has support for ELF64 multiboot loading and it successfully
used by many projects in osdev community.

This functionality has been added to GRUB 12 years ago
https://github.com/coreos/grub/commit/ea4097134fbd834d2f688363f9a8208bf6a49ecd

Re: [Qemu-devel] [PATCH 2/4] multiboot: load any machine type of ELF

[Anatol Pomozov]

Hi

On Fri, Oct 13, 2017 at 12:25 PM, Eduardo Habkost <ehabk...@redhat.com> wrote:
> On Thu, Oct 12, 2017 at 04:54:37PM -0700, Anatol Pomozov wrote:
>> x86 is not the only architecture supported by multiboot.
>> For example GRUB supports MIPS architecture as well.
>>
>> Signed-off-by: Anatol Pomozov <anatol.pomo...@gmail.com>
>> ---
>>  hw/i386/multiboot.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
>> index c9254f313e..7dacd6d827 100644
>> --- a/hw/i386/multiboot.c
>> +++ b/hw/i386/multiboot.c
>> @@ -173,7 +173,7 @@ int load_multiboot(FWCfgState *fw_cfg,
>>  }
>>
>>  kernel_size = load_elf(kernel_filename, NULL, NULL, _entry,
>> -   _low, _high, 0, I386_ELF_MACHINE,
>> +   _low, _high, 0, EM_NONE,
>> 0, 0);
>
> I assume we still want PC to reject non-x86 ELF files.

Does multiboot spec states this restriction? I've heard that there are
attempts to implement multiboot at ARM [1] [2]. Also multiboot2 spec
mentions MIPS as one of the target architectures.

[1] https://github.com/jncronin/rpi-boot/blob/master/MULTIBOOT-ARM
[2] https://wiki.linaro.org/AndrePrzywara/Multiboot

>  Isn't it
> better to add a elf_machine argument to load_multiboot() so each
> load_multiboot() caller can specify what's the expected
> architecture?
>
>
>>  if (kernel_size < 0) {
>>  fprintf(stderr, "Error while loading elf kernel\n");
>> --
>> 2.15.0.rc0.271.g36b669edcc-goog
>>
>
> --
> Eduardo

[Qemu-devel] [PATCH 3/4] multiboot: load elf sections and section headers

[Anatol Pomozov]

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 <anatol.pomo...@gmail.com>
---
 hw/core/loader.c |   8 +--
 hw/i386/multiboot.c  |  83 +---
 hw/s390x/ipl.c   |   2 +-
 include/hw/elf_ops.h | 107 ++-
 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   |  55 
 tests/multiboot/start.S  |   2 +-
 11 files changed, 276 insertions(+), 61 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 4593061445..a8787f7685 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 7dacd6d827..841d05160a 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. */
@@ -161,37 +162,8 @@ int load_multiboot(FWCfgState *fw_cfg,
 if (flags & MULTIBOOT_VIDEO_MODE) {
 fprintf(stderr, "qemu: multiboot knows VBE. we don't.\n");
 }
-if (!(flags & MULTIBOOT_AOUT_KLUDGE)) {
-uint64_t elf_entry;
-uint64_t elf_low, elf_high;
-int kernel_size;
-fclose(f);
 
-if (((struct elf64_hdr*)header)->e_machine == EM_X86_64) {
-fprintf(stderr, "Cannot load x86-64 image, give a 32bit one.\n");
-exit(1);
-}
-
-kernel_size = load_elf(kernel_filename, NULL, NULL, _entry,
-   _low, _high, 0, EM_NONE,
-   0, 0);
-if (kernel_size < 0) {
-fprintf(stderr, "Error while loading elf kernel\n");
-exit(1);
-}
-mh_load_addr = elf_low;
-mb_kernel_size = elf_high - elf_low;
-mh_entry_addr = elf_entry;
-
-mbs.mb_buf = g_malloc(mb_kernel_size);
-if (rom_copy(mbs.mb_buf, mh_load_addr, mb_kernel_size) != 
mb_kernel_size) {
-fprintf(stderr, "Error while fetching elf kernel from rom\n");
-exit(1);
-}
-
-mb_debug("qemu: loading multiboot-elf kernel (%#x bytes) with entry 
%#zx\n",
-  mb_kernel_size, (size_t)mh_entry_addr);
-} else {
+if (flags & MULTIBOOT_AOUT_KLUDGE) {
 /* Valid if mh_flags sets MULTIBOOT_

[Qemu-devel] [PATCH 2/4] multiboot: load any machine type of ELF

[Anatol Pomozov]

x86 is not the only architecture supported by multiboot.
For example GRUB supports MIPS architecture as well.

Signed-off-by: Anatol Pomozov <anatol.pomo...@gmail.com>
---
 hw/i386/multiboot.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
index c9254f313e..7dacd6d827 100644
--- a/hw/i386/multiboot.c
+++ b/hw/i386/multiboot.c
@@ -173,7 +173,7 @@ int load_multiboot(FWCfgState *fw_cfg,
 }
 
 kernel_size = load_elf(kernel_filename, NULL, NULL, _entry,
-   _low, _high, 0, I386_ELF_MACHINE,
+   _low, _high, 0, EM_NONE,
0, 0);
 if (kernel_size < 0) {
 fprintf(stderr, "Error while loading elf kernel\n");
-- 
2.15.0.rc0.271.g36b669edcc-goog

[Qemu-devel] [PATCH 4/4] multiboot: make tests work with clang

[Anatol Pomozov]

 * clang 3.8 enables SSE even for 32bit code. Generate code for pentium
   CPU to make sure no new instructions are used.
 * add memset() implementation. Clang implements array zeroing in
   print_num() via memset() function call.
---
 tests/multiboot/Makefile| 2 +-
 tests/multiboot/libc.c  | 9 +
 tests/multiboot/libc.h  | 2 ++
 tests/multiboot/run_test.sh | 1 +
 4 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/tests/multiboot/Makefile b/tests/multiboot/Makefile
index b6a5056347..79dfe85afc 100644
--- a/tests/multiboot/Makefile
+++ b/tests/multiboot/Makefile
@@ -1,5 +1,5 @@
 CC=gcc
-CCFLAGS=-m32 -Wall -Wextra -Werror -fno-stack-protector -nostdinc -fno-builtin
+CCFLAGS=-m32 -Wall -Wextra -Werror -fno-stack-protector -nostdinc -fno-builtin 
-march=pentium
 ASFLAGS=-m32
 
 LD=ld
diff --git a/tests/multiboot/libc.c b/tests/multiboot/libc.c
index 6df9bda96d..512fccd7fa 100644
--- a/tests/multiboot/libc.c
+++ b/tests/multiboot/libc.c
@@ -33,6 +33,15 @@ void* memcpy(void *dest, const void *src, int n)
 
 return dest;
 }
+void *memset(void *s, int c, size_t n)
+{
+size_t i;
+char *d = s;
+for (i = 0; i < n; i++) {
+*d++ = c;
+}
+return s;
+}
 
 static void print_char(char c)
 {
diff --git a/tests/multiboot/libc.h b/tests/multiboot/libc.h
index 04c9922c27..bdf7c34287 100644
--- a/tests/multiboot/libc.h
+++ b/tests/multiboot/libc.h
@@ -36,6 +36,7 @@ typedef signed short int16_t;
 typedef signed char int8_t;
 
 typedef uint32_t uintptr_t;
+typedef uint32_t size_t;
 
 
 /* stdarg.h */
@@ -58,5 +59,6 @@ static inline void outb(uint16_t port, uint8_t data)
 
 void printf(const char *fmt, ...);
 void* memcpy(void *dest, const void *src, int n);
+void* memset(void *s, int c, size_t n);
 
 #endif
diff --git a/tests/multiboot/run_test.sh b/tests/multiboot/run_test.sh
index f04e35cbf0..38dcfef42c 100755
--- a/tests/multiboot/run_test.sh
+++ b/tests/multiboot/run_test.sh
@@ -29,6 +29,7 @@ run_qemu() {
 printf %b "\n\n=== Running test case: $kernel $* ===\n\n" >> test.log
 
 $QEMU \
+-cpu pentium \
 -kernel $kernel \
 -display none \
 -device isa-debugcon,chardev=stdio \
-- 
2.15.0.rc0.271.g36b669edcc-goog

[Qemu-devel] (no subject)

[Anatol Pomozov]

It is V3 of multiboot improvements to Qemu

Changes made sinse V2:
 - rebase on top of qemu master changes
 - make multiboot/sections test more reliable
 Add generate_sections_out.py script that generates ELF sections information
 - rename 'struct section_data' to 'struct SectionData' to match naming
 convention in include/hw/loader.h

[Qemu-devel] [PATCH 1/4] multiboot: Change multiboot_info from array of bytes to a C struct

[Anatol Pomozov]

Using C structs makes the code more readable and prevents type conversion
errors.

Borrow multiboot1 header from GRUB project.

Signed-off-by: Anatol Pomozov <anatol.pomo...@gmail.com>
---
 hw/i386/multiboot.c | 124 +
 hw/i386/multiboot_header.h  | 254 
 tests/multiboot/mmap.c  |  14 +--
 tests/multiboot/mmap.out|  10 ++
 tests/multiboot/modules.c   |  12 ++-
 tests/multiboot/modules.out |  10 ++
 tests/multiboot/multiboot.h |  66 
 7 files changed, 339 insertions(+), 151 deletions(-)
 create mode 100644 hw/i386/multiboot_header.h
 delete mode 100644 tests/multiboot/multiboot.h

diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
index c7b70c91d5..c9254f313e 100644
--- a/hw/i386/multiboot.c
+++ b/hw/i386/multiboot.c
@@ -28,6 +28,7 @@
 #include "hw/hw.h"
 #include "hw/nvram/fw_cfg.h"
 #include "multiboot.h"
+#include "multiboot_header.h"
 #include "hw/loader.h"
 #include "elf.h"
 #include "sysemu/sysemu.h"
@@ -47,39 +48,9 @@
 #error multiboot struct needs to fit in 16 bit real mode
 #endif
 
-enum {
-/* Multiboot info */
-MBI_FLAGS   = 0,
-MBI_MEM_LOWER   = 4,
-MBI_MEM_UPPER   = 8,
-MBI_BOOT_DEVICE = 12,
-MBI_CMDLINE = 16,
-MBI_MODS_COUNT  = 20,
-MBI_MODS_ADDR   = 24,
-MBI_MMAP_ADDR   = 48,
-MBI_BOOTLOADER  = 64,
-
-MBI_SIZE= 88,
-
-/* Multiboot modules */
-MB_MOD_START= 0,
-MB_MOD_END  = 4,
-MB_MOD_CMDLINE  = 8,
-
-MB_MOD_SIZE = 16,
-
-/* Region offsets */
-ADDR_E820_MAP = MULTIBOOT_STRUCT_ADDR + 0,
-ADDR_MBI  = ADDR_E820_MAP + 0x500,
-
-/* Multiboot flags */
-MULTIBOOT_FLAGS_MEMORY  = 1 << 0,
-MULTIBOOT_FLAGS_BOOT_DEVICE = 1 << 1,
-MULTIBOOT_FLAGS_CMDLINE = 1 << 2,
-MULTIBOOT_FLAGS_MODULES = 1 << 3,
-MULTIBOOT_FLAGS_MMAP= 1 << 6,
-MULTIBOOT_FLAGS_BOOTLOADER  = 1 << 9,
-};
+/* Region offsets */
+#define ADDR_E820_MAP MULTIBOOT_STRUCT_ADDR
+#define ADDR_MBI  (ADDR_E820_MAP + 0x500)
 
 typedef struct {
 /* buffer holding kernel, cmdlines and mb_infos */
@@ -128,14 +99,15 @@ static void mb_add_mod(MultibootState *s,
hwaddr start, hwaddr end,
hwaddr cmdline_phys)
 {
-char *p;
+multiboot_module_t *mod;
 assert(s->mb_mods_count < s->mb_mods_avail);
 
-p = (char *)s->mb_buf + s->offset_mbinfo + MB_MOD_SIZE * s->mb_mods_count;
+mod = s->mb_buf + s->offset_mbinfo +
+  sizeof(multiboot_module_t) * s->mb_mods_count;
 
-stl_p(p + MB_MOD_START,   start);
-stl_p(p + MB_MOD_END, end);
-stl_p(p + MB_MOD_CMDLINE, cmdline_phys);
+stl_p(>mod_start, start);
+stl_p(>mod_end,   end);
+stl_p(>cmdline,   cmdline_phys);
 
 mb_debug("mod%02d: "TARGET_FMT_plx" - "TARGET_FMT_plx"\n",
  s->mb_mods_count, start, end);
@@ -151,26 +123,29 @@ int load_multiboot(FWCfgState *fw_cfg,
int kernel_file_size,
uint8_t *header)
 {
-int i, is_multiboot = 0;
+int i;
+bool is_multiboot = false;
 uint32_t flags = 0;
 uint32_t mh_entry_addr;
 uint32_t mh_load_addr;
 uint32_t mb_kernel_size;
 MultibootState mbs;
-uint8_t bootinfo[MBI_SIZE];
+multiboot_info_t bootinfo;
 uint8_t *mb_bootinfo_data;
 uint32_t cmdline_len;
+struct multiboot_header *multiboot_header;
 
 /* Ok, let's see if it is a multiboot image.
The header is 12x32bit long, so the latest entry may be 8192 - 48. */
 for (i = 0; i < (8192 - 48); i += 4) {
-if (ldl_p(header+i) == 0x1BADB002) {
-uint32_t checksum = ldl_p(header+i+8);
-flags = ldl_p(header+i+4);
+multiboot_header = (struct multiboot_header *)(header + i);
+if (ldl_p(_header->magic) == MULTIBOOT_HEADER_MAGIC) {
+uint32_t checksum = ldl_p(_header->checksum);
+flags = ldl_p(_header->flags);
 checksum += flags;
-checksum += (uint32_t)0x1BADB002;
+checksum += (uint32_t)MULTIBOOT_HEADER_MAGIC;
 if (!checksum) {
-is_multiboot = 1;
+is_multiboot = true;
 break;
 }
 }
@@ -180,13 +155,13 @@ int load_multiboot(FWCfgState *fw_cfg,
 return 0; /* no multiboot */
 
 mb_debug("qemu: I believe we found a multiboot image!\n");
-memset(bootinfo, 0, sizeof(bootinfo));
+memset(, 0, sizeof(bootinfo));
 memset(, 0, sizeof(mbs));
 
-if (flags & 0x0004) { /* MULTIBOOT_HEADER_HAS_VBE */
+if (flags & MULTIBOOT_VIDEO_MODE) {
 fprintf(stderr, "qemu: multiboot knows VBE. we don't.\n");
 }
-if (!(flags & 0x0001)) { /* 

Re: [Qemu-devel] How to make ELF headers/symbol sections available for multiboot?

[Anatol Pomozov]

Hello

On Thu, Aug 17, 2017 at 1:54 PM, Anatol Pomozov
<anatol.pomo...@gmail.com> wrote:
> Hi
>
> On Tue, Aug 8, 2017 at 8:04 AM, Kevin Wolf <kw...@redhat.com> wrote:
>> Am 04.08.2017 um 06:53 hat Anatol Pomozov geschrieben:
>>> Hi Kevin
>>>
>>> Thanks for the information.
>>>
>>> So I sounds like we do want multiboot to load all sections regardless
>>> of its segments info. To achieve it we need to read sections headers
>>> and load all section that were not loaded yet.
>>>
>>> I have a working implementation here
>>> https://github.com/anatol/qemu/commit/26773cf4f1f30b2d0d3fd89cce024f8e9c5603c5
>>> Tested it with my multiboot OS image. The target iterates over all
>>> sections and prints their names/address. The output result is the same
>>> for QEMU and for VmWare+GRUB.
>>>
>>> Let me know if this idea looks good so I can send this patch to qemu 
>>> maillist.
>>
>> I'm not sure if I'll find the time to review this in detail, but I'll
>> just give it a quick look.
>>
>> You seem to attempt to add support for 64 bit ELF binaries. The
>> Multiboot standard doesn't explicitly say that this is forbidden, but
>> everything in it expects 32 bit binaries and I don't think GRUB (as the
>> reference implementation for Multiboot) accepts 64 bit ELFs either. The
>> boot state is 32 bit protected mode in any case. So unless I'm mistaken
>> on GRUB's behaviour, I'd rather not support 64 bit ELFs.
>
> Grub actually does support ELF64 loading with multiboot. Here is the
> GRUB code that implements it
> https://github.com/coreos/grub/blob/master/grub-core/loader/multiboot.c#L210
>
> It would be great if QEMU behaved similar way.
>
> Actually I've been using qemu with ELF64 multiboot for a while and it
> works great.
>
>>
>> You shouldn't look at ELF headers at all if MULTIBOOT_AOUT_KLUDGE is
>> given. In this case, the kernel image is to be treated as a flat binary
>> and section headers can't be expected. I think your code breaks non-ELF
>> kernels.


I have these changes ready now
http://lists.nongnu.org/archive/html/qemu-devel/2017-08/msg03265.html
It added ELF parser functionality as discussed above. Are there any
open questions? What is the best way to proceed forward with these
patches?

I also have interest in adding Multiboot2 support to qemu. But before
going with it I would like to finish working on my current patchset.

Re: [Qemu-devel] How to make ELF headers/symbol sections available for multiboot?

[Anatol Pomozov]

Hello

I have these changes ready now
http://lists.nongnu.org/archive/html/qemu-devel/2017-08/msg03265.html
It added ELF parser functionality as discussed above. Are there any
open questions? What is the best way to proceed forward with these
patches?

I also have interest in adding Multiboot2 support to qemu. But before
going with it I would like to finish working on my current patchset.

On Thu, Aug 17, 2017 at 1:54 PM, Anatol Pomozov
<anatol.pomo...@gmail.com> wrote:
> Hi
>
> On Tue, Aug 8, 2017 at 8:04 AM, Kevin Wolf <kw...@redhat.com> wrote:
>> Am 04.08.2017 um 06:53 hat Anatol Pomozov geschrieben:
>>> Hi Kevin
>>>
>>> Thanks for the information.
>>>
>>> So I sounds like we do want multiboot to load all sections regardless
>>> of its segments info. To achieve it we need to read sections headers
>>> and load all section that were not loaded yet.
>>>
>>> I have a working implementation here
>>> https://github.com/anatol/qemu/commit/26773cf4f1f30b2d0d3fd89cce024f8e9c5603c5
>>> Tested it with my multiboot OS image. The target iterates over all
>>> sections and prints their names/address. The output result is the same
>>> for QEMU and for VmWare+GRUB.
>>>
>>> Let me know if this idea looks good so I can send this patch to qemu 
>>> maillist.
>>
>> I'm not sure if I'll find the time to review this in detail, but I'll
>> just give it a quick look.
>>
>> You seem to attempt to add support for 64 bit ELF binaries. The
>> Multiboot standard doesn't explicitly say that this is forbidden, but
>> everything in it expects 32 bit binaries and I don't think GRUB (as the
>> reference implementation for Multiboot) accepts 64 bit ELFs either. The
>> boot state is 32 bit protected mode in any case. So unless I'm mistaken
>> on GRUB's behaviour, I'd rather not support 64 bit ELFs.
>
> Grub actually does support ELF64 loading with multiboot. Here is the
> GRUB code that implements it
> https://github.com/coreos/grub/blob/master/grub-core/loader/multiboot.c#L210
>
> It would be great if QEMU behaved similar way.
>
> Actually I've been using qemu with ELF64 multiboot for a while and it
> works great.
>
>>
>> You shouldn't look at ELF headers at all if MULTIBOOT_AOUT_KLUDGE is
>> given. In this case, the kernel image is to be treated as a flat binary
>> and section headers can't be expected. I think your code breaks non-ELF
>> kernels.
>
> Ok, will revert this part back.
>
>>
>> As for loading the section headers, duplicating ELF parser code may be
>> functionally correct, but probably not the best way to implement things.
>> As I wrote in an earlier email, load_elf() already parses all the
>> information that you need. You really just need to store it somewhere,
>> which would probably just mean adding a new parameter to load_elf() and
>> the parser could then store a copy of the data there if it's non-NULL.
>
> Loading section headers is actually a small part of my change. It also:
>   - calculates memory needed to load all sections into memory
>   - allocates memory
>   - loads sections
>
> This "load all sections into memory" functionality is very multiboot
> specific. But if you think if load_elf() is better place for it then I
> can look at moving it to load_elf().

Re: [Qemu-devel] [PATCH] multiboot: make tests work with clang

[Anatol Pomozov]

On Wed, Aug 23, 2017 at 4:44 PM, Philippe Mathieu-Daudé <f4...@amsat.org> wrote:
> Hi Anatol,
>
> On 08/23/2017 04:22 PM, Anatol Pomozov wrote:
>>
>>   * explicitly disable SSE as clang enables it by default for 32bit code
>>   * add memset() implementation. Clang complains that the function is
>> absent, gcc seems provides default built-in.
>> ---
>>   tests/multiboot/Makefile | 2 +-
>>   tests/multiboot/libc.c   | 9 +
>>   tests/multiboot/libc.h   | 2 ++
>>   3 files changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/multiboot/Makefile b/tests/multiboot/Makefile
>> index 856e76682a..7fadbca33a 100644
>> --- a/tests/multiboot/Makefile
>> +++ b/tests/multiboot/Makefile
>> @@ -1,5 +1,5 @@
>>   CC=gcc
>> -CCFLAGS=-m32 -Wall -Wextra -Werror -fno-stack-protector -nostdinc
>> -fno-builtin
>> +CCFLAGS=-m32 -Wall -Wextra -Werror -fno-stack-protector -nostdinc
>> -fno-builtin -mno-sse
>
>
> What about using -march=i586 instead? so no need to disable each next
> features clang decide to default enable.
>
> Even cleaner IMHO, use -march=pentium in CFLAGS and add "-cpu pentium" in
> run_test.sh so we are sure the generated code matches, what do you think?

It sounds reasonable. Just sent an updated patch. Works with gcc 6.3
and clang 3.8.

[Qemu-devel] [PATCH] multiboot: make tests work with clang

[Anatol Pomozov]

 * clang 3.8 enables SSE even for 32bit code. Generate code for pentium
   CPU to make sure no new instructions are used.
 * add memset() implementation. Clang implements array zeroing in
   print_num() via memset() function call.
---
 tests/multiboot/Makefile| 2 +-
 tests/multiboot/libc.c  | 9 +
 tests/multiboot/libc.h  | 2 ++
 tests/multiboot/run_test.sh | 1 +
 4 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/tests/multiboot/Makefile b/tests/multiboot/Makefile
index 856e76682a..2e015409dc 100644
--- a/tests/multiboot/Makefile
+++ b/tests/multiboot/Makefile
@@ -1,5 +1,5 @@
 CC=gcc
-CCFLAGS=-m32 -Wall -Wextra -Werror -fno-stack-protector -nostdinc -fno-builtin
+CCFLAGS=-m32 -Wall -Wextra -Werror -fno-stack-protector -nostdinc -fno-builtin 
-march=pentium
 ASFLAGS=-m32
 
 LD=ld
diff --git a/tests/multiboot/libc.c b/tests/multiboot/libc.c
index 6df9bda96d..512fccd7fa 100644
--- a/tests/multiboot/libc.c
+++ b/tests/multiboot/libc.c
@@ -33,6 +33,15 @@ void* memcpy(void *dest, const void *src, int n)
 
 return dest;
 }
+void *memset(void *s, int c, size_t n)
+{
+size_t i;
+char *d = s;
+for (i = 0; i < n; i++) {
+*d++ = c;
+}
+return s;
+}
 
 static void print_char(char c)
 {
diff --git a/tests/multiboot/libc.h b/tests/multiboot/libc.h
index 04c9922c27..44b71350dd 100644
--- a/tests/multiboot/libc.h
+++ b/tests/multiboot/libc.h
@@ -36,6 +36,7 @@ typedef signed short int16_t;
 typedef signed char int8_t;
 
 typedef uint32_t uintptr_t;
+typedef uint32_t size_t;
 
 
 /* stdarg.h */
@@ -58,5 +59,6 @@ static inline void outb(uint16_t port, uint8_t data)
 
 void printf(const char *fmt, ...);
 void* memcpy(void *dest, const void *src, int n);
+void *memset(void *s, int c, size_t n);
 
 #endif
diff --git a/tests/multiboot/run_test.sh b/tests/multiboot/run_test.sh
index f04e35cbf0..38dcfef42c 100755
--- a/tests/multiboot/run_test.sh
+++ b/tests/multiboot/run_test.sh
@@ -29,6 +29,7 @@ run_qemu() {
 printf %b "\n\n=== Running test case: $kernel $* ===\n\n" >> test.log
 
 $QEMU \
+-cpu pentium \
 -kernel $kernel \
 -display none \
 -device isa-debugcon,chardev=stdio \
-- 
2.14.1.342.g6490525c54-goog

[Qemu-devel] [PATCH] multiboot: make tests work with clang

[Anatol Pomozov]

 * explicitly disable SSE as clang enables it by default for 32bit code
 * add memset() implementation. Clang complains that the function is
   absent, gcc seems provides default built-in.
---
 tests/multiboot/Makefile | 2 +-
 tests/multiboot/libc.c   | 9 +
 tests/multiboot/libc.h   | 2 ++
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/tests/multiboot/Makefile b/tests/multiboot/Makefile
index 856e76682a..7fadbca33a 100644
--- a/tests/multiboot/Makefile
+++ b/tests/multiboot/Makefile
@@ -1,5 +1,5 @@
 CC=gcc
-CCFLAGS=-m32 -Wall -Wextra -Werror -fno-stack-protector -nostdinc -fno-builtin
+CCFLAGS=-m32 -Wall -Wextra -Werror -fno-stack-protector -nostdinc -fno-builtin 
-mno-sse
 ASFLAGS=-m32
 
 LD=ld
diff --git a/tests/multiboot/libc.c b/tests/multiboot/libc.c
index 6df9bda96d..512fccd7fa 100644
--- a/tests/multiboot/libc.c
+++ b/tests/multiboot/libc.c
@@ -33,6 +33,15 @@ void* memcpy(void *dest, const void *src, int n)
 
 return dest;
 }
+void *memset(void *s, int c, size_t n)
+{
+size_t i;
+char *d = s;
+for (i = 0; i < n; i++) {
+*d++ = c;
+}
+return s;
+}
 
 static void print_char(char c)
 {
diff --git a/tests/multiboot/libc.h b/tests/multiboot/libc.h
index 04c9922c27..44b71350dd 100644
--- a/tests/multiboot/libc.h
+++ b/tests/multiboot/libc.h
@@ -36,6 +36,7 @@ typedef signed short int16_t;
 typedef signed char int8_t;
 
 typedef uint32_t uintptr_t;
+typedef uint32_t size_t;
 
 
 /* stdarg.h */
@@ -58,5 +59,6 @@ static inline void outb(uint16_t port, uint8_t data)
 
 void printf(const char *fmt, ...);
 void* memcpy(void *dest, const void *src, int n);
+void *memset(void *s, int c, size_t n);
 
 #endif
-- 
2.14.1.342.g6490525c54-goog

Re: [Qemu-devel] [PATCH 3/3] multiboot: load elf sections and section headers

[Anatol Pomozov]

Hi

On Wed, Aug 23, 2017 at 10:22 AM, Anatol Pomozov
<anatol.pomo...@gmail.com> 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 <anatol.pomo...@gmail.com>
> ---
>  hw/core/loader.c |   8 ++--
>  hw/i386/multiboot.c  |  83 +
>  hw/s390x/ipl.c   |   2 +-
>  include/hw/elf_ops.h | 107 
> ++-
>  include/hw/loader.h  |  11 -
>  tests/multiboot/Makefile |   5 +-
>  tests/multiboot/modules.out  |  22 -
>  tests/multiboot/run_test.sh  |   6 ++-
>  tests/multiboot/sections.c   |  55 ++
>  tests/multiboot/sections.out |  21 +
>  tests/multiboot/start.S  |   2 +-
>  11 files changed, 261 insertions(+), 61 deletions(-)
>  create mode 100644 tests/multiboot/sections.c
>  create mode 100644 tests/multiboot/sections.out
>
> diff --git a/hw/core/loader.c b/hw/core/loader.c
> index ebe574c7ea..c7e3608e7a 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, struct sections_data *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 a49217ac62..9ced0bfe30 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;
> +struct sections_data sections;
>
>  /* Ok, let's see if it is a multiboot image.
> The header is 12x32bit long, so the latest entry may be 8192 - 48. */
> @@ -161,37 +162,8 @@ int load_multiboot(FWCfgState *fw_cfg,
>  if (flags & MULTIBOOT_VIDEO_MODE) {
>  fprintf(stderr, "qemu: multiboot knows VBE. we don't.\n");
>  }
> -if (!(flags & MULTIBOOT_AOUT_KLUDGE)) {
> -uint64_t elf_entry;
> -uint64_t elf_low, elf_high;
> -int kernel_size;
> -fclose(f);
>
> -if (((struct elf64_hdr*)header)->e_machine == EM_X86_64) {
> -fprintf(stderr, "Cannot load x86-64 image, give a 32bit one.\n");
> -exit(1);
> -}
> -
> -kernel_size = load_elf(kernel_filename, NULL, NULL, _entry,
> -   _low, _high, 0, EM_NONE,
> -   0, 0);
> -if (kernel_size < 0) {
> -fprintf(stderr, "Error while loading elf kernel\n");
> -exit(1);
> -}
> -mh_load_addr = elf_low;
> -mb_kernel_size = elf_high - elf_low;
> -   

[Qemu-devel] [PATCH 1/3] multiboot: Change multiboot_info from array of bytes to a C struct

[Anatol Pomozov]

Using C structs makes the code more readable and prevents type conversion
errors.

Borrow multiboot1 header from GRUB project.

Signed-off-by: Anatol Pomozov <anatol.pomo...@gmail.com>
---
 hw/i386/multiboot.c | 124 +
 hw/i386/multiboot_header.h  | 254 
 tests/multiboot/mmap.c  |  14 +--
 tests/multiboot/mmap.out|  10 ++
 tests/multiboot/modules.c   |  12 ++-
 tests/multiboot/modules.out |  10 ++
 tests/multiboot/multiboot.h |  66 
 7 files changed, 339 insertions(+), 151 deletions(-)
 create mode 100644 hw/i386/multiboot_header.h
 delete mode 100644 tests/multiboot/multiboot.h

diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
index f13e23139b..19113c0fce 100644
--- a/hw/i386/multiboot.c
+++ b/hw/i386/multiboot.c
@@ -28,6 +28,7 @@
 #include "hw/hw.h"
 #include "hw/nvram/fw_cfg.h"
 #include "multiboot.h"
+#include "multiboot_header.h"
 #include "hw/loader.h"
 #include "elf.h"
 #include "sysemu/sysemu.h"
@@ -47,39 +48,9 @@
 #error multiboot struct needs to fit in 16 bit real mode
 #endif
 
-enum {
-/* Multiboot info */
-MBI_FLAGS   = 0,
-MBI_MEM_LOWER   = 4,
-MBI_MEM_UPPER   = 8,
-MBI_BOOT_DEVICE = 12,
-MBI_CMDLINE = 16,
-MBI_MODS_COUNT  = 20,
-MBI_MODS_ADDR   = 24,
-MBI_MMAP_ADDR   = 48,
-MBI_BOOTLOADER  = 64,
-
-MBI_SIZE= 88,
-
-/* Multiboot modules */
-MB_MOD_START= 0,
-MB_MOD_END  = 4,
-MB_MOD_CMDLINE  = 8,
-
-MB_MOD_SIZE = 16,
-
-/* Region offsets */
-ADDR_E820_MAP = MULTIBOOT_STRUCT_ADDR + 0,
-ADDR_MBI  = ADDR_E820_MAP + 0x500,
-
-/* Multiboot flags */
-MULTIBOOT_FLAGS_MEMORY  = 1 << 0,
-MULTIBOOT_FLAGS_BOOT_DEVICE = 1 << 1,
-MULTIBOOT_FLAGS_CMDLINE = 1 << 2,
-MULTIBOOT_FLAGS_MODULES = 1 << 3,
-MULTIBOOT_FLAGS_MMAP= 1 << 6,
-MULTIBOOT_FLAGS_BOOTLOADER  = 1 << 9,
-};
+/* Region offsets */
+#define ADDR_E820_MAP MULTIBOOT_STRUCT_ADDR
+#define ADDR_MBI  (ADDR_E820_MAP + 0x500)
 
 typedef struct {
 /* buffer holding kernel, cmdlines and mb_infos */
@@ -128,14 +99,15 @@ static void mb_add_mod(MultibootState *s,
hwaddr start, hwaddr end,
hwaddr cmdline_phys)
 {
-char *p;
+multiboot_module_t *mod;
 assert(s->mb_mods_count < s->mb_mods_avail);
 
-p = (char *)s->mb_buf + s->offset_mbinfo + MB_MOD_SIZE * s->mb_mods_count;
+mod = s->mb_buf + s->offset_mbinfo +
+  sizeof(multiboot_module_t) * s->mb_mods_count;
 
-stl_p(p + MB_MOD_START,   start);
-stl_p(p + MB_MOD_END, end);
-stl_p(p + MB_MOD_CMDLINE, cmdline_phys);
+stl_p(>mod_start, start);
+stl_p(>mod_end,   end);
+stl_p(>cmdline,   cmdline_phys);
 
 mb_debug("mod%02d: "TARGET_FMT_plx" - "TARGET_FMT_plx"\n",
  s->mb_mods_count, start, end);
@@ -151,26 +123,29 @@ int load_multiboot(FWCfgState *fw_cfg,
int kernel_file_size,
uint8_t *header)
 {
-int i, is_multiboot = 0;
+int i;
+bool is_multiboot = false;
 uint32_t flags = 0;
 uint32_t mh_entry_addr;
 uint32_t mh_load_addr;
 uint32_t mb_kernel_size;
 MultibootState mbs;
-uint8_t bootinfo[MBI_SIZE];
+multiboot_info_t bootinfo;
 uint8_t *mb_bootinfo_data;
 uint32_t cmdline_len;
+struct multiboot_header *multiboot_header;
 
 /* Ok, let's see if it is a multiboot image.
The header is 12x32bit long, so the latest entry may be 8192 - 48. */
 for (i = 0; i < (8192 - 48); i += 4) {
-if (ldl_p(header+i) == 0x1BADB002) {
-uint32_t checksum = ldl_p(header+i+8);
-flags = ldl_p(header+i+4);
+multiboot_header = (struct multiboot_header *)(header + i);
+if (ldl_p(_header->magic) == MULTIBOOT_HEADER_MAGIC) {
+uint32_t checksum = ldl_p(_header->checksum);
+flags = ldl_p(_header->flags);
 checksum += flags;
-checksum += (uint32_t)0x1BADB002;
+checksum += (uint32_t)MULTIBOOT_HEADER_MAGIC;
 if (!checksum) {
-is_multiboot = 1;
+is_multiboot = true;
 break;
 }
 }
@@ -180,13 +155,13 @@ int load_multiboot(FWCfgState *fw_cfg,
 return 0; /* no multiboot */
 
 mb_debug("qemu: I believe we found a multiboot image!\n");
-memset(bootinfo, 0, sizeof(bootinfo));
+memset(, 0, sizeof(bootinfo));
 memset(, 0, sizeof(mbs));
 
-if (flags & 0x0004) { /* MULTIBOOT_HEADER_HAS_VBE */
+if (flags & MULTIBOOT_VIDEO_MODE) {
 fprintf(stderr, "qemu: multiboot knows VBE. we don't.\n");
 }
-if (!(flags & 0x0001)) { /* 

[Qemu-devel] [PATCH 3/3] multiboot: load elf sections and section headers

[Anatol Pomozov]

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 <anatol.pomo...@gmail.com>
---
 hw/core/loader.c |   8 ++--
 hw/i386/multiboot.c  |  83 +
 hw/s390x/ipl.c   |   2 +-
 include/hw/elf_ops.h | 107 ++-
 include/hw/loader.h  |  11 -
 tests/multiboot/Makefile |   5 +-
 tests/multiboot/modules.out  |  22 -
 tests/multiboot/run_test.sh  |   6 ++-
 tests/multiboot/sections.c   |  55 ++
 tests/multiboot/sections.out |  21 +
 tests/multiboot/start.S  |   2 +-
 11 files changed, 261 insertions(+), 61 deletions(-)
 create mode 100644 tests/multiboot/sections.c
 create mode 100644 tests/multiboot/sections.out

diff --git a/hw/core/loader.c b/hw/core/loader.c
index ebe574c7ea..c7e3608e7a 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, struct sections_data *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 a49217ac62..9ced0bfe30 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;
+struct sections_data sections;
 
 /* Ok, let's see if it is a multiboot image.
The header is 12x32bit long, so the latest entry may be 8192 - 48. */
@@ -161,37 +162,8 @@ int load_multiboot(FWCfgState *fw_cfg,
 if (flags & MULTIBOOT_VIDEO_MODE) {
 fprintf(stderr, "qemu: multiboot knows VBE. we don't.\n");
 }
-if (!(flags & MULTIBOOT_AOUT_KLUDGE)) {
-uint64_t elf_entry;
-uint64_t elf_low, elf_high;
-int kernel_size;
-fclose(f);
 
-if (((struct elf64_hdr*)header)->e_machine == EM_X86_64) {
-fprintf(stderr, "Cannot load x86-64 image, give a 32bit one.\n");
-exit(1);
-}
-
-kernel_size = load_elf(kernel_filename, NULL, NULL, _entry,
-   _low, _high, 0, EM_NONE,
-   0, 0);
-if (kernel_size < 0) {
-fprintf(stderr, "Error while loading elf kernel\n");
-exit(1);
-}
-mh_load_addr = elf_low;
-mb_kernel_size = elf_high - elf_low;
-mh_entry_addr = elf_entry;
-
-mbs.mb_buf = g_malloc(mb_kernel_size);
-if (rom_copy(mbs.mb_buf, mh_load_addr, mb_kernel_size) != 
mb_kernel_size) {
-fprintf(stderr, "Error while fetching elf kernel from rom\n");
-exit(1);
-}
-
-mb_debug("qemu: loading multiboot-elf kernel (%#x bytes) with entry 
%#zx\n",
-  mb_kernel_size, (size_t)mh_entry_addr);
-} else {
+if (flags & MULTIBOOT_AOUT_KLUDGE) {
 /* Valid if mh_flags sets MULTIBOOT_AOUT_KLUDGE. */
 uint32_t mh_header_addr = ldl_p(_header->header_addr);
 uin

[Qemu-devel] [PATCH 2/3] multiboot: load any machine type of ELF

[Anatol Pomozov]

x86 is not the only architecture supported by multiboot.
For example GRUB supports MIPS architecture as well.

Signed-off-by: Anatol Pomozov <anatol.pomo...@gmail.com>
---
 hw/i386/multiboot.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
index 19113c0fce..a49217ac62 100644
--- a/hw/i386/multiboot.c
+++ b/hw/i386/multiboot.c
@@ -173,7 +173,7 @@ int load_multiboot(FWCfgState *fw_cfg,
 }
 
 kernel_size = load_elf(kernel_filename, NULL, NULL, _entry,
-   _low, _high, 0, I386_ELF_MACHINE,
+   _low, _high, 0, EM_NONE,
0, 0);
 if (kernel_size < 0) {
 fprintf(stderr, "Error while loading elf kernel\n");
-- 
2.14.1.342.g6490525c54-goog

[Qemu-devel] [PATCH 2/3] multiboot: load any machine type of ELF

[Anatol Pomozov]

x86 is not the only architecture supported by multiboot.
For example GRUB supports MIPS architecture as well.

Signed-off-by: Anatol Pomozov <anatol.pomo...@gmail.com>
---
 hw/i386/multiboot.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
index 19113c0fce..a49217ac62 100644
--- a/hw/i386/multiboot.c
+++ b/hw/i386/multiboot.c
@@ -173,7 +173,7 @@ int load_multiboot(FWCfgState *fw_cfg,
 }
 
 kernel_size = load_elf(kernel_filename, NULL, NULL, _entry,
-   _low, _high, 0, I386_ELF_MACHINE,
+   _low, _high, 0, EM_NONE,
0, 0);
 if (kernel_size < 0) {
 fprintf(stderr, "Error while loading elf kernel\n");
-- 
2.14.1.480.gb18f417b89-goog

[Qemu-devel] [PATCH 3/3] multiboot: load elf sections and section headers

[Anatol Pomozov]

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 <anatol.pomo...@gmail.com>
---
 hw/core/loader.c |   8 ++--
 hw/i386/multiboot.c  |  83 ---
 hw/s390x/ipl.c   |   2 +-
 include/hw/elf_ops.h | 107 ++-
 include/hw/loader.h  |  11 +-
 5 files changed, 164 insertions(+), 47 deletions(-)

diff --git a/hw/core/loader.c b/hw/core/loader.c
index ebe574c7ea..c7e3608e7a 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, struct sections_data *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 a49217ac62..7f7630f92d 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;
+struct sections_data sections;
 
 /* Ok, let's see if it is a multiboot image.
The header is 12x32bit long, so the latest entry may be 8192 - 48. */
@@ -161,37 +162,8 @@ int load_multiboot(FWCfgState *fw_cfg,
 if (flags & MULTIBOOT_VIDEO_MODE) {
 fprintf(stderr, "qemu: multiboot knows VBE. we don't.\n");
 }
-if (!(flags & MULTIBOOT_AOUT_KLUDGE)) {
-uint64_t elf_entry;
-uint64_t elf_low, elf_high;
-int kernel_size;
-fclose(f);
 
-if (((struct elf64_hdr*)header)->e_machine == EM_X86_64) {
-fprintf(stderr, "Cannot load x86-64 image, give a 32bit one.\n");
-exit(1);
-}
-
-kernel_size = load_elf(kernel_filename, NULL, NULL, _entry,
-   _low, _high, 0, EM_NONE,
-   0, 0);
-if (kernel_size < 0) {
-fprintf(stderr, "Error while loading elf kernel\n");
-exit(1);
-}
-mh_load_addr = elf_low;
-mb_kernel_size = elf_high - elf_low;
-mh_entry_addr = elf_entry;
-
-mbs.mb_buf = g_malloc(mb_kernel_size);
-if (rom_copy(mbs.mb_buf, mh_load_addr, mb_kernel_size) != 
mb_kernel_size) {
-fprintf(stderr, "Error while fetching elf kernel from rom\n");
-exit(1);
-}
-
-mb_debug("qemu: loading multiboot-elf kernel (%#x bytes) with entry 
%#zx\n",
-  mb_kernel_size, (size_t)mh_entry_addr);
-} else {
+if (flags & MULTIBOOT_AOUT_KLUDGE) {
 /* Valid if mh_flags sets MULTIBOOT_AOUT_KLUDGE. */
 uint32_t mh_header_addr = ldl_p(_header->header_addr);
 uint32_t mh_load_end_addr = ldl_p(_header->load_end_addr);
@@ -209,12 +181,6 @@ int load_multiboot(FWCfgState *fw_cfg,
 mb_load_size = mb_kernel_size;
 }
 
-/* Valid if mh_flags sets MULTIBOOT_VIDEO_MODE.
-uint32_t mh_mode_type = ldl_p(_header->mode_type);
-uint32_t mh_width = ldl_p(_header->width);
-uint32_t mh_height = ldl_p(_header->he

[Qemu-devel] [PATCH 1/3] multiboot: Change multiboot_info from array of bytes to a C struct

[Anatol Pomozov]

Using C structs makes the code more readable and prevents type conversion
errors.

Borrow multiboot1 header from GRUB project.

Signed-off-by: Anatol Pomozov <anatol.pomo...@gmail.com>
---
 hw/i386/multiboot.c| 124 +-
 hw/i386/multiboot_header.h | 254 +
 2 files changed, 304 insertions(+), 74 deletions(-)
 create mode 100644 hw/i386/multiboot_header.h

diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
index f13e23139b..19113c0fce 100644
--- a/hw/i386/multiboot.c
+++ b/hw/i386/multiboot.c
@@ -28,6 +28,7 @@
 #include "hw/hw.h"
 #include "hw/nvram/fw_cfg.h"
 #include "multiboot.h"
+#include "multiboot_header.h"
 #include "hw/loader.h"
 #include "elf.h"
 #include "sysemu/sysemu.h"
@@ -47,39 +48,9 @@
 #error multiboot struct needs to fit in 16 bit real mode
 #endif
 
-enum {
-/* Multiboot info */
-MBI_FLAGS   = 0,
-MBI_MEM_LOWER   = 4,
-MBI_MEM_UPPER   = 8,
-MBI_BOOT_DEVICE = 12,
-MBI_CMDLINE = 16,
-MBI_MODS_COUNT  = 20,
-MBI_MODS_ADDR   = 24,
-MBI_MMAP_ADDR   = 48,
-MBI_BOOTLOADER  = 64,
-
-MBI_SIZE= 88,
-
-/* Multiboot modules */
-MB_MOD_START= 0,
-MB_MOD_END  = 4,
-MB_MOD_CMDLINE  = 8,
-
-MB_MOD_SIZE = 16,
-
-/* Region offsets */
-ADDR_E820_MAP = MULTIBOOT_STRUCT_ADDR + 0,
-ADDR_MBI  = ADDR_E820_MAP + 0x500,
-
-/* Multiboot flags */
-MULTIBOOT_FLAGS_MEMORY  = 1 << 0,
-MULTIBOOT_FLAGS_BOOT_DEVICE = 1 << 1,
-MULTIBOOT_FLAGS_CMDLINE = 1 << 2,
-MULTIBOOT_FLAGS_MODULES = 1 << 3,
-MULTIBOOT_FLAGS_MMAP= 1 << 6,
-MULTIBOOT_FLAGS_BOOTLOADER  = 1 << 9,
-};
+/* Region offsets */
+#define ADDR_E820_MAP MULTIBOOT_STRUCT_ADDR
+#define ADDR_MBI  (ADDR_E820_MAP + 0x500)
 
 typedef struct {
 /* buffer holding kernel, cmdlines and mb_infos */
@@ -128,14 +99,15 @@ static void mb_add_mod(MultibootState *s,
hwaddr start, hwaddr end,
hwaddr cmdline_phys)
 {
-char *p;
+multiboot_module_t *mod;
 assert(s->mb_mods_count < s->mb_mods_avail);
 
-p = (char *)s->mb_buf + s->offset_mbinfo + MB_MOD_SIZE * s->mb_mods_count;
+mod = s->mb_buf + s->offset_mbinfo +
+  sizeof(multiboot_module_t) * s->mb_mods_count;
 
-stl_p(p + MB_MOD_START,   start);
-stl_p(p + MB_MOD_END, end);
-stl_p(p + MB_MOD_CMDLINE, cmdline_phys);
+stl_p(>mod_start, start);
+stl_p(>mod_end,   end);
+stl_p(>cmdline,   cmdline_phys);
 
 mb_debug("mod%02d: "TARGET_FMT_plx" - "TARGET_FMT_plx"\n",
  s->mb_mods_count, start, end);
@@ -151,26 +123,29 @@ int load_multiboot(FWCfgState *fw_cfg,
int kernel_file_size,
uint8_t *header)
 {
-int i, is_multiboot = 0;
+int i;
+bool is_multiboot = false;
 uint32_t flags = 0;
 uint32_t mh_entry_addr;
 uint32_t mh_load_addr;
 uint32_t mb_kernel_size;
 MultibootState mbs;
-uint8_t bootinfo[MBI_SIZE];
+multiboot_info_t bootinfo;
 uint8_t *mb_bootinfo_data;
 uint32_t cmdline_len;
+struct multiboot_header *multiboot_header;
 
 /* Ok, let's see if it is a multiboot image.
The header is 12x32bit long, so the latest entry may be 8192 - 48. */
 for (i = 0; i < (8192 - 48); i += 4) {
-if (ldl_p(header+i) == 0x1BADB002) {
-uint32_t checksum = ldl_p(header+i+8);
-flags = ldl_p(header+i+4);
+multiboot_header = (struct multiboot_header *)(header + i);
+if (ldl_p(_header->magic) == MULTIBOOT_HEADER_MAGIC) {
+uint32_t checksum = ldl_p(_header->checksum);
+flags = ldl_p(_header->flags);
 checksum += flags;
-checksum += (uint32_t)0x1BADB002;
+checksum += (uint32_t)MULTIBOOT_HEADER_MAGIC;
 if (!checksum) {
-is_multiboot = 1;
+is_multiboot = true;
 break;
 }
 }
@@ -180,13 +155,13 @@ int load_multiboot(FWCfgState *fw_cfg,
 return 0; /* no multiboot */
 
 mb_debug("qemu: I believe we found a multiboot image!\n");
-memset(bootinfo, 0, sizeof(bootinfo));
+memset(, 0, sizeof(bootinfo));
 memset(, 0, sizeof(mbs));
 
-if (flags & 0x0004) { /* MULTIBOOT_HEADER_HAS_VBE */
+if (flags & MULTIBOOT_VIDEO_MODE) {
 fprintf(stderr, "qemu: multiboot knows VBE. we don't.\n");
 }
-if (!(flags & 0x0001)) { /* MULTIBOOT_HEADER_HAS_ADDR */
+if (!(flags & MULTIBOOT_AOUT_KLUDGE)) {
 uint64_t elf_entry;
 uint64_t elf_low, elf_high;
 int kernel_size;
@@ -217,14 +192,14 @@ int load_multiboot(FWCfgState *fw_cfg,
 mb_d

Re: [Qemu-devel] How to make ELF headers/symbol sections available for multiboot?

[Anatol Pomozov]

Hi

On Tue, Aug 8, 2017 at 8:04 AM, Kevin Wolf <kw...@redhat.com> wrote:
> Am 04.08.2017 um 06:53 hat Anatol Pomozov geschrieben:
>> Hi Kevin
>>
>> Thanks for the information.
>>
>> So I sounds like we do want multiboot to load all sections regardless
>> of its segments info. To achieve it we need to read sections headers
>> and load all section that were not loaded yet.
>>
>> I have a working implementation here
>> https://github.com/anatol/qemu/commit/26773cf4f1f30b2d0d3fd89cce024f8e9c5603c5
>> Tested it with my multiboot OS image. The target iterates over all
>> sections and prints their names/address. The output result is the same
>> for QEMU and for VmWare+GRUB.
>>
>> Let me know if this idea looks good so I can send this patch to qemu 
>> maillist.
>
> I'm not sure if I'll find the time to review this in detail, but I'll
> just give it a quick look.
>
> You seem to attempt to add support for 64 bit ELF binaries. The
> Multiboot standard doesn't explicitly say that this is forbidden, but
> everything in it expects 32 bit binaries and I don't think GRUB (as the
> reference implementation for Multiboot) accepts 64 bit ELFs either. The
> boot state is 32 bit protected mode in any case. So unless I'm mistaken
> on GRUB's behaviour, I'd rather not support 64 bit ELFs.

Grub actually does support ELF64 loading with multiboot. Here is the
GRUB code that implements it
https://github.com/coreos/grub/blob/master/grub-core/loader/multiboot.c#L210

It would be great if QEMU behaved similar way.

Actually I've been using qemu with ELF64 multiboot for a while and it
works great.

>
> You shouldn't look at ELF headers at all if MULTIBOOT_AOUT_KLUDGE is
> given. In this case, the kernel image is to be treated as a flat binary
> and section headers can't be expected. I think your code breaks non-ELF
> kernels.

Ok, will revert this part back.

>
> As for loading the section headers, duplicating ELF parser code may be
> functionally correct, but probably not the best way to implement things.
> As I wrote in an earlier email, load_elf() already parses all the
> information that you need. You really just need to store it somewhere,
> which would probably just mean adding a new parameter to load_elf() and
> the parser could then store a copy of the data there if it's non-NULL.

Loading section headers is actually a small part of my change. It also:
  - calculates memory needed to load all sections into memory
  - allocates memory
  - loads sections

This "load all sections into memory" functionality is very multiboot
specific. But if you think if load_elf() is better place for it then I
can look at moving it to load_elf().

Re: [Qemu-devel] How to make ELF headers/symbol sections available for multiboot?

[Anatol Pomozov]

Hi Kevin

Thanks for the information.

So I sounds like we do want multiboot to load all sections regardless
of its segments info. To achieve it we need to read sections headers
and load all section that were not loaded yet.

I have a working implementation here
https://github.com/anatol/qemu/commit/26773cf4f1f30b2d0d3fd89cce024f8e9c5603c5
Tested it with my multiboot OS image. The target iterates over all
sections and prints their names/address. The output result is the same
for QEMU and for VmWare+GRUB.

Let me know if this idea looks good so I can send this patch to qemu maillist.


On Thu, Aug 3, 2017 at 1:39 AM, Kevin Wolf <kw...@redhat.com> wrote:
> Am 03.08.2017 um 00:00 hat Anatol Pomozov geschrieben:
>> Hello Richard
>>
>> Thank you for this useful information. I still learning about ELF and
>> a lot of things are still unclear for me.
>>
>> On Mon, Jul 31, 2017 at 11:20 AM, Richard Henderson <r...@twiddle.net> wrote:
>> > On 07/31/2017 10:21 AM, Anatol Pomozov wrote:
>> >> ELF sections info is needed for an OS to map address space properly.
>> >
>> > No, ELF *program header* info is needed for an OS to map the address space
>> > properly.  For example:
>> >
>> > $ readelf -hl vmlinux-4.9.0-3-5kc-malta
>>
>> Thanks for this information about program headers. I reread elf_ops.h
>> and now I see that QEMU loads all PT_LOAD segments. I wonder why GRUB
>> bootloader loads also sections that are not in this segment (e.g. GRUB
>> loads content of .shstrtab into target's memory despite my elf does
>> not keep it in PT_LOAD segment).
>
> Remember that what we're talking about isn't primarily an ELF loader,
> but a Multiboot loader. Multiboot can work without ELF (e.g. with flat
> binaries) if bit 16 in the Multiboot header flags is set and the
> additional header fields are correctly provided. However, if you are
> loading an ELF binary, then Multiboot specifies that the bootloader
> makes use of the ELF headers and you don't have to set bit 16 in the
> header.
>
> If ELF says that some things have to be loaded (and bit 16 is clear),
> then the bootloader will load them. If Multiboot says that additional
> things can or have to be loaded, it will load those, too. The section
> headers are only one example of additional data that Multiboot provides.
> It also loads additional modules, provides the e820 memory map, etc.,
> none of which are described by ELF.
>
>> What ELF specification says about it? Does it tell a loader to load
>> only PT_LOAD segments? In this case bootloaders should follow it as
>> well and current QEMU behavior is correct.
>
> The current behaviour of QEMU is correct because it doesn't set bit 5 in
> the flags field of the Multiboot info struct, i.e. it doesn't advertise
> that the section headers are available, so it doesn't have to provide
> it.
>
> It is also not feature complete, because a full Multiboot implementation
> would implement this and advertise bit 5. Adding the feature would be
> useful because there are guest kernels that can make use of it.
>
>> But multiboot expects section headers info and .shstrtab section to be
>> loaded to the target memory. I believe I need to modify my linker
>> script and add this information into the loadable segment.
>
> No. First of all, you don't even need this information to find the start
> and end of the kernel code. Second, if a Multiboot loader loads this, it
> doesn't have to be in a loadable segment. This is more like debug
> information, not like an inherent part of the program.
>
>> Here is my current linker script:
>>
>> ==
>> ENTRY(start)
>>
>> SECTIONS {
>> . = 1M;
>>
>> .text : {
>> KEEP(*(.data.multiboot))
>> *(.text .text*)
>> }
>>
>> .rodata : {
>> *(.rodata .rodata*)
>> }
>>
>> .data : {
>> *(.data .data.*)
>> }
>>
>> .bss : {
>> __bss_start = .;
>> *(.bss .bss*)
>> . = ALIGN(8);
>> __bss_end = .;
>> }
>> }
>> ===
>>
>> With my linker script only .text .rodata .data and .bss are included
>> into the loadable segment.
>>
>>
>> So my questions: how to tell the linker to include "section headers"
>> and ".shstrtab" section into loadable segment? Once it is done I can
>> try to modify QEMU to pass its addresses to the target.
>
> Requiring section headers to be contained in a loadable segment wouldn't
> be a correct implementation of the Multiboot feature.
>
> Kevin

Re: [Qemu-devel] How to make ELF headers/symbol sections available for multiboot?

[Anatol Pomozov]

Hello Richard

Thank you for this useful information. I still learning about ELF and
a lot of things are still unclear for me.

On Mon, Jul 31, 2017 at 11:20 AM, Richard Henderson <r...@twiddle.net> wrote:
> On 07/31/2017 10:21 AM, Anatol Pomozov wrote:
>> ELF sections info is needed for an OS to map address space properly.
>
> No, ELF *program header* info is needed for an OS to map the address space
> properly.  For example:
>
> $ readelf -hl vmlinux-4.9.0-3-5kc-malta

Thanks for this information about program headers. I reread elf_ops.h
and now I see that QEMU loads all PT_LOAD segments. I wonder why GRUB
bootloader loads also sections that are not in this segment (e.g. GRUB
loads content of .shstrtab into target's memory despite my elf does
not keep it in PT_LOAD segment).

What ELF specification says about it? Does it tell a loader to load
only PT_LOAD segments? In this case bootloaders should follow it as
well and current QEMU behavior is correct.

But multiboot expects section headers info and .shstrtab section to be
loaded to the target memory. I believe I need to modify my linker
script and add this information into the loadable segment.

Here is my current linker script:

==
ENTRY(start)

SECTIONS {
. = 1M;

.text : {
KEEP(*(.data.multiboot))
*(.text .text*)
}

.rodata : {
*(.rodata .rodata*)
}

.data : {
*(.data .data.*)
}

.bss : {
__bss_start = .;
*(.bss .bss*)
. = ALIGN(8);
__bss_end = .;
}
}
===

With my linker script only .text .rodata .data and .bss are included
into the loadable segment.


So my questions: how to tell the linker to include "section headers"
and ".shstrtab" section into loadable segment? Once it is done I can
try to modify QEMU to pass its addresses to the target.

>
> Using a mips kernel binary I happend to have handy; it would be the same for
> x86_64, prior to being bundled in the (imo superfluous bzImage) format.
>
> ELF Header:
>   Magic:   7f 45 4c 46 02 01 01 00 00 00 00 00 00 00 00 00
>   Class: ELF64
>   Data:  2's complement, little endian
>   Version:   1 (current)
>   OS/ABI:UNIX - System V
>   ABI Version:   0
>   Type:  EXEC (Executable file)
>   Machine:   MIPS R3000
>   Version:   0x1
>   Entry point address:   0x806ed590
>   Start of program headers:  64 (bytes into file)
>   Start of section headers:  13351328 (bytes into file)
>   Flags: 0x8001, noreorder, mips64r2
>   Size of this header:   64 (bytes)
>   Size of program headers:   56 (bytes)
>   Number of program headers: 2
>   Size of section headers:   64 (bytes)
>   Number of section headers: 28
>   Section header string table index: 27
>
> The ELF file header, always at file offset 0.  The relevant info is the
> encoding (64-bit little-endian), cpu (mips), and start of program headers.
>
> Program Headers:
>   Type   Offset VirtAddr   PhysAddr
>  FileSizMemSiz  Flags  Align
>   LOAD   0x1000 0x8010 0x8010
>  0x009fefb4 0x00a5a150  RWE1000
>   NOTE   0x005fecb0 0x806fdcb0 0x806fdcb0
>  0x0024 0x0024  R  4
>
> The ELF program header.  There is one segment to be loaded, at a given 
> physical
> address (which happens to match the virtual address, but that need not have
> been so).
>
> The segment consists of 0x9fefb4 bytes of data to be loaded from the file, and
> occupies 0xa5a150 in memory.  The difference between the two sizes is "bss",
> and should be zeroed.
>
> A proper ELF loader will process *all* LOAD segments, however many are 
> required
> by the binary.  Though in practice there will probably only be 1 or 2.
>
> Section to Segment mapping:
>   Segment Sections...
>   00 .text __ex_table __dbe_table .notes .rodata .pci_fixup __ksymtab
> __ksymtab_gpl __kcrctab __kcrctab_gpl __ksymtab_strings __param __modver .data
> .data..page_aligned .init.text .init.data .exit.text .data.reloc .sbss .bss
>   01 __dbe_table .notes
>
> This mapping is provided by readelf for convenience, not actually present in
> the ELF file.  But it is handy when debugging a linker script.
>
>> Quoting Multiboot specification
>> https://www.gnu.org/software/grub/manual/multiboot/multiboot.html
>
&g

Re: [Qemu-devel] How to make ELF headers/symbol sections available for multiboot?

[Anatol Pomozov]

Hi

On Sun, Jul 30, 2017 at 2:42 PM, Eduardo Habkost <ehabk...@redhat.com> wrote:
>
> CCing Alex, the original author of load_multiboot(), and Kevin,
> who touched multiboot code recently.
>
>
> On Fri, Jul 28, 2017 at 02:28:34PM -0700, Anatol Pomozov wrote:
>> Hi
>>
>> I am looking at x86 multiboot code and trying to add "ELF section
>> header" info feature. This will let target to learn more about booted
>> binary and its sections.
>
> Are there existing OSes that use that information?

ELF sections info is needed for an OS to map address space properly.

I do not know much about production-grade OS but multiboot protocol is
quite popular among newly created and hobby OS. Multiboot provide some
useful information that need to be found somewhere else (e.g. by
requesting info from BIOS).

Here is an example of Phil's Rust OS that uses multiboot to read ELF
section information. https://os.phil-opp.com/allocating-frames/ They
use GRUB that loads all ELF sections into memory and provides ELF info
via multiboot structure.

It is interesting to see how LittleKernel tries to find the sections
information by using _start/_end markers in their linker script
https://github.com/littlekernel/lk/blob/master/arch/x86/64/kernel.ld
And while it works, in fact it just tries to recover ELF sections
information. And that is why I think it would be more useful if qemu
implemented ELF sections feature from Multiboot.

> Do you have a pointer to what that feature does exactly?

Quoting Multiboot specification
https://www.gnu.org/software/grub/manual/multiboot/multiboot.html

===BEGIN=

If bit 5 in the ‘flags’ word is set, then the following fields in the
Multiboot information structure starting at byte 28 are valid:

 +---+
 28  | num   |
 32  | size  |
 36  | addr  |
 40  | shndx |
 +---+

These indicate where the section header table from an ELF kernel is,
the size of each entry, number of entries, and the string table used
as the index of names. They correspond to the ‘shdr_*’ entries
(‘shdr_num’, etc.) in the Executable and Linkable Format (elf)
specification in the program header. All sections are loaded, and the
physical address fields of the elf section header then refer to where
the sections are in memory (refer to the i386 elf documentation for
details as to how to read the section header(s)). Note that ‘shdr_num’
may be 0, indicating no symbols, even if bit 5 in the ‘flags’
===END===

Basically it makes some info from ELF header and section headers
available for target.

[Qemu-devel] How to make ELF headers/symbol sections available for multiboot?

[Anatol Pomozov]

Hi

I am looking at x86 multiboot code and trying to add "ELF section
header" info feature. This will let target to learn more about booted
binary and its sections.

I have a draft here
https://github.com/anatol/qemu/commit/ad943a6eb78feee048b6bb2a1e5f49f5b686e24c

My understanding is that qemu multiboot loads only TEXT/BSS/DATA
sections. Other stuff like symbols sections and ELF headers are not
available for target.

So I need to perform 2 things:

 - Load ELF section headers into target's memory. I did by appending
additional space to mbs.mb_buf and copying header data. Is it the best
way to do?

 - Next I need to load other ELF sections such as symbols (e.g.
.shstrtab) that store section names. What is the best way to do in
multiboo.c code? Would it make sense to load all ELF sections?

Thanks in advance.

[Qemu-devel] [PATCH] multiboot: Change multiboot_info from array of bytes to a C struct

[Anatol Pomozov]

Using C structs makes the code more readable and prevents type conversion
errors.

Borrow multiboot1 header from GRUB project.

Signed-off-by: Anatol Pomozov <anatol.pomo...@gmail.com>
---
 hw/i386/multiboot.c| 124 +-
 hw/i386/multiboot_header.h | 254 +
 2 files changed, 304 insertions(+), 74 deletions(-)
 create mode 100644 hw/i386/multiboot_header.h

diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
index f13e23139b..19113c0fce 100644
--- a/hw/i386/multiboot.c
+++ b/hw/i386/multiboot.c
@@ -28,6 +28,7 @@
 #include "hw/hw.h"
 #include "hw/nvram/fw_cfg.h"
 #include "multiboot.h"
+#include "multiboot_header.h"
 #include "hw/loader.h"
 #include "elf.h"
 #include "sysemu/sysemu.h"
@@ -47,39 +48,9 @@
 #error multiboot struct needs to fit in 16 bit real mode
 #endif
 
-enum {
-/* Multiboot info */
-MBI_FLAGS   = 0,
-MBI_MEM_LOWER   = 4,
-MBI_MEM_UPPER   = 8,
-MBI_BOOT_DEVICE = 12,
-MBI_CMDLINE = 16,
-MBI_MODS_COUNT  = 20,
-MBI_MODS_ADDR   = 24,
-MBI_MMAP_ADDR   = 48,
-MBI_BOOTLOADER  = 64,
-
-MBI_SIZE= 88,
-
-/* Multiboot modules */
-MB_MOD_START= 0,
-MB_MOD_END  = 4,
-MB_MOD_CMDLINE  = 8,
-
-MB_MOD_SIZE = 16,
-
-/* Region offsets */
-ADDR_E820_MAP = MULTIBOOT_STRUCT_ADDR + 0,
-ADDR_MBI  = ADDR_E820_MAP + 0x500,
-
-/* Multiboot flags */
-MULTIBOOT_FLAGS_MEMORY  = 1 << 0,
-MULTIBOOT_FLAGS_BOOT_DEVICE = 1 << 1,
-MULTIBOOT_FLAGS_CMDLINE = 1 << 2,
-MULTIBOOT_FLAGS_MODULES = 1 << 3,
-MULTIBOOT_FLAGS_MMAP= 1 << 6,
-MULTIBOOT_FLAGS_BOOTLOADER  = 1 << 9,
-};
+/* Region offsets */
+#define ADDR_E820_MAP MULTIBOOT_STRUCT_ADDR
+#define ADDR_MBI  (ADDR_E820_MAP + 0x500)
 
 typedef struct {
 /* buffer holding kernel, cmdlines and mb_infos */
@@ -128,14 +99,15 @@ static void mb_add_mod(MultibootState *s,
hwaddr start, hwaddr end,
hwaddr cmdline_phys)
 {
-char *p;
+multiboot_module_t *mod;
 assert(s->mb_mods_count < s->mb_mods_avail);
 
-p = (char *)s->mb_buf + s->offset_mbinfo + MB_MOD_SIZE * s->mb_mods_count;
+mod = s->mb_buf + s->offset_mbinfo +
+  sizeof(multiboot_module_t) * s->mb_mods_count;
 
-stl_p(p + MB_MOD_START,   start);
-stl_p(p + MB_MOD_END, end);
-stl_p(p + MB_MOD_CMDLINE, cmdline_phys);
+stl_p(>mod_start, start);
+stl_p(>mod_end,   end);
+stl_p(>cmdline,   cmdline_phys);
 
 mb_debug("mod%02d: "TARGET_FMT_plx" - "TARGET_FMT_plx"\n",
  s->mb_mods_count, start, end);
@@ -151,26 +123,29 @@ int load_multiboot(FWCfgState *fw_cfg,
int kernel_file_size,
uint8_t *header)
 {
-int i, is_multiboot = 0;
+int i;
+bool is_multiboot = false;
 uint32_t flags = 0;
 uint32_t mh_entry_addr;
 uint32_t mh_load_addr;
 uint32_t mb_kernel_size;
 MultibootState mbs;
-uint8_t bootinfo[MBI_SIZE];
+multiboot_info_t bootinfo;
 uint8_t *mb_bootinfo_data;
 uint32_t cmdline_len;
+struct multiboot_header *multiboot_header;
 
 /* Ok, let's see if it is a multiboot image.
The header is 12x32bit long, so the latest entry may be 8192 - 48. */
 for (i = 0; i < (8192 - 48); i += 4) {
-if (ldl_p(header+i) == 0x1BADB002) {
-uint32_t checksum = ldl_p(header+i+8);
-flags = ldl_p(header+i+4);
+multiboot_header = (struct multiboot_header *)(header + i);
+if (ldl_p(_header->magic) == MULTIBOOT_HEADER_MAGIC) {
+uint32_t checksum = ldl_p(_header->checksum);
+flags = ldl_p(_header->flags);
 checksum += flags;
-checksum += (uint32_t)0x1BADB002;
+checksum += (uint32_t)MULTIBOOT_HEADER_MAGIC;
 if (!checksum) {
-is_multiboot = 1;
+is_multiboot = true;
 break;
 }
 }
@@ -180,13 +155,13 @@ int load_multiboot(FWCfgState *fw_cfg,
 return 0; /* no multiboot */
 
 mb_debug("qemu: I believe we found a multiboot image!\n");
-memset(bootinfo, 0, sizeof(bootinfo));
+memset(, 0, sizeof(bootinfo));
 memset(, 0, sizeof(mbs));
 
-if (flags & 0x0004) { /* MULTIBOOT_HEADER_HAS_VBE */
+if (flags & MULTIBOOT_VIDEO_MODE) {
 fprintf(stderr, "qemu: multiboot knows VBE. we don't.\n");
 }
-if (!(flags & 0x0001)) { /* MULTIBOOT_HEADER_HAS_ADDR */
+if (!(flags & MULTIBOOT_AOUT_KLUDGE)) {
 uint64_t elf_entry;
 uint64_t elf_low, elf_high;
 int kernel_size;
@@ -217,14 +192,14 @@ int load_multiboot(FWCfgState *fw_cfg,
 mb_d

[Qemu-devel] [PATCH] multiboot: Change multiboot_info from array of bytes to a C struct

[Anatol Pomozov]

Using C structs makes the code more readable and prevents type conversion
errors.

Borrow multiboot1 header from GRUB project.
---
 hw/i386/multiboot.c| 122 -
 hw/i386/multiboot_header.h | 265 +
 2 files changed, 313 insertions(+), 74 deletions(-)
 create mode 100644 hw/i386/multiboot_header.h

diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
index f13e23139b..596c7435d5 100644
--- a/hw/i386/multiboot.c
+++ b/hw/i386/multiboot.c
@@ -28,6 +28,7 @@
 #include "hw/hw.h"
 #include "hw/nvram/fw_cfg.h"
 #include "multiboot.h"
+#include "multiboot_header.h"
 #include "hw/loader.h"
 #include "elf.h"
 #include "sysemu/sysemu.h"
@@ -47,39 +48,9 @@
 #error multiboot struct needs to fit in 16 bit real mode
 #endif
 
-enum {
-/* Multiboot info */
-MBI_FLAGS   = 0,
-MBI_MEM_LOWER   = 4,
-MBI_MEM_UPPER   = 8,
-MBI_BOOT_DEVICE = 12,
-MBI_CMDLINE = 16,
-MBI_MODS_COUNT  = 20,
-MBI_MODS_ADDR   = 24,
-MBI_MMAP_ADDR   = 48,
-MBI_BOOTLOADER  = 64,
-
-MBI_SIZE= 88,
-
-/* Multiboot modules */
-MB_MOD_START= 0,
-MB_MOD_END  = 4,
-MB_MOD_CMDLINE  = 8,
-
-MB_MOD_SIZE = 16,
-
-/* Region offsets */
-ADDR_E820_MAP = MULTIBOOT_STRUCT_ADDR + 0,
-ADDR_MBI  = ADDR_E820_MAP + 0x500,
-
-/* Multiboot flags */
-MULTIBOOT_FLAGS_MEMORY  = 1 << 0,
-MULTIBOOT_FLAGS_BOOT_DEVICE = 1 << 1,
-MULTIBOOT_FLAGS_CMDLINE = 1 << 2,
-MULTIBOOT_FLAGS_MODULES = 1 << 3,
-MULTIBOOT_FLAGS_MMAP= 1 << 6,
-MULTIBOOT_FLAGS_BOOTLOADER  = 1 << 9,
-};
+/* Region offsets */
+#define ADDR_E820_MAP MULTIBOOT_STRUCT_ADDR
+#define ADDR_MBI  (ADDR_E820_MAP + 0x500)
 
 typedef struct {
 /* buffer holding kernel, cmdlines and mb_infos */
@@ -128,14 +99,14 @@ static void mb_add_mod(MultibootState *s,
hwaddr start, hwaddr end,
hwaddr cmdline_phys)
 {
-char *p;
+multiboot_module_t *mod;
 assert(s->mb_mods_count < s->mb_mods_avail);
 
-p = (char *)s->mb_buf + s->offset_mbinfo + MB_MOD_SIZE * s->mb_mods_count;
+mod = s->mb_buf + s->offset_mbinfo + sizeof(multiboot_module_t) * 
s->mb_mods_count;
 
-stl_p(p + MB_MOD_START,   start);
-stl_p(p + MB_MOD_END, end);
-stl_p(p + MB_MOD_CMDLINE, cmdline_phys);
+stl_p(>mod_start, start);
+stl_p(>mod_end,   end);
+stl_p(>cmdline,   cmdline_phys);
 
 mb_debug("mod%02d: "TARGET_FMT_plx" - "TARGET_FMT_plx"\n",
  s->mb_mods_count, start, end);
@@ -151,26 +122,29 @@ int load_multiboot(FWCfgState *fw_cfg,
int kernel_file_size,
uint8_t *header)
 {
-int i, is_multiboot = 0;
+int i;
+bool is_multiboot = false;
 uint32_t flags = 0;
 uint32_t mh_entry_addr;
 uint32_t mh_load_addr;
 uint32_t mb_kernel_size;
 MultibootState mbs;
-uint8_t bootinfo[MBI_SIZE];
+multiboot_info_t bootinfo;
 uint8_t *mb_bootinfo_data;
 uint32_t cmdline_len;
+struct multiboot_header *multiboot_header;
 
 /* Ok, let's see if it is a multiboot image.
The header is 12x32bit long, so the latest entry may be 8192 - 48. */
 for (i = 0; i < (8192 - 48); i += 4) {
-if (ldl_p(header+i) == 0x1BADB002) {
-uint32_t checksum = ldl_p(header+i+8);
-flags = ldl_p(header+i+4);
+multiboot_header = (struct multiboot_header *)(header + i);
+if (ldl_p(_header->magic) == MULTIBOOT_HEADER_MAGIC) {
+uint32_t checksum = ldl_p(_header->checksum);
+flags = ldl_p(_header->flags);
 checksum += flags;
-checksum += (uint32_t)0x1BADB002;
+checksum += (uint32_t)MULTIBOOT_HEADER_MAGIC;
 if (!checksum) {
-is_multiboot = 1;
+is_multiboot = true;
 break;
 }
 }
@@ -180,13 +154,13 @@ int load_multiboot(FWCfgState *fw_cfg,
 return 0; /* no multiboot */
 
 mb_debug("qemu: I believe we found a multiboot image!\n");
-memset(bootinfo, 0, sizeof(bootinfo));
+memset(, 0, sizeof(bootinfo));
 memset(, 0, sizeof(mbs));
 
-if (flags & 0x0004) { /* MULTIBOOT_HEADER_HAS_VBE */
+if (flags & MULTIBOOT_VIDEO_MODE) {
 fprintf(stderr, "qemu: multiboot knows VBE. we don't.\n");
 }
-if (!(flags & 0x0001)) { /* MULTIBOOT_HEADER_HAS_ADDR */
+if (!(flags & MULTIBOOT_AOUT_KLUDGE)) {
 uint64_t elf_entry;
 uint64_t elf_low, elf_high;
 int kernel_size;
@@ -217,14 +191,14 @@ 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);
 } else {
-/* Valid if mh_flags sets MULTIBOOT_HEADER_HAS_ADDR. */
-uint32_t mh_header_addr = ldl_p(header+i+12);
-uint32_t mh_load_end_addr = 

[Qemu-devel] [Bug 1699867] [NEW] x86_64 qemu crashes at far call into long-mode

[Anatol Pomozov]

Public bug reported:

I am experimenting with this OS https://github.com/phil-opp/blog_os and
spotted a weird behavior with qemu.

I am looking at code that does transition from 32bit mode to 64bit mode.
Right now it does 'jmp $SELECTOR,64bitfunction'. https://github.com
/phil-
opp/blog_os/blob/557c6a58ea11e31685b9d014d307002d64df5c22/src/arch/x86_64/boot.asm#L32

This code works fine with qemu/kvm/vmware.

To transition from 32 to 64 bit code it is possible to use 'call'
operation. So I am trying to replace that code with 'call
$SELECTOR,64bitfunction'. It works fine with kvm and wmware. But it
fails with Qemu emulation. See the interrup debug log attached. qemu
crashes at 10302c (far call mnemonic).


  103016:   e8 18 00 00 00  callq  103033 
  10301b:   e8 5c 00 00 00  callq  10307c 
  103020:   e8 ec 00 00 00  callq  103111 
  103025:   0f 01 15 28 00 10 00lgdt   0x100028(%rip)# 203054 

  10302c:   9a  (bad)  
  10302d:   40 31 10rex xor %edx,(%rax)
  103030:   00 08   add%cl,(%rax)


As the code works at hardware I expect it to work with qemu. Thus current qemu 
behavior looks like a bug.

** Affects: qemu
 Importance: Undecided
 Status: New

** Attachment added: "Debug log for qemu crash at far call"
   https://bugs.launchpad.net/bugs/1699867/+attachment/4901095/+files/debug.log

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1699867

Title:
  x86_64 qemu crashes at far call into long-mode

Status in QEMU:
  New

Bug description:
  I am experimenting with this OS https://github.com/phil-opp/blog_os
  and spotted a weird behavior with qemu.

  I am looking at code that does transition from 32bit mode to 64bit
  mode. Right now it does 'jmp $SELECTOR,64bitfunction'.
  https://github.com/phil-
  
opp/blog_os/blob/557c6a58ea11e31685b9d014d307002d64df5c22/src/arch/x86_64/boot.asm#L32

  This code works fine with qemu/kvm/vmware.

  To transition from 32 to 64 bit code it is possible to use 'call'
  operation. So I am trying to replace that code with 'call
  $SELECTOR,64bitfunction'. It works fine with kvm and wmware. But it
  fails with Qemu emulation. See the interrup debug log attached. qemu
  crashes at 10302c (far call mnemonic).

  
103016:   e8 18 00 00 00  callq  103033 
10301b:   e8 5c 00 00 00  callq  10307c 
103020:   e8 ec 00 00 00  callq  103111 
103025:   0f 01 15 28 00 10 00lgdt   0x100028(%rip)# 203054 

10302c:   9a  (bad)  
10302d:   40 31 10rex xor %edx,(%rax)
103030:   00 08   add%cl,(%rax)

  
  As the code works at hardware I expect it to work with qemu. Thus current 
qemu behavior looks like a bug.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1699867/+subscriptions

Re: [Qemu-devel] [PATCH] Remove restriction that prevents bootimg elf64 images

[Anatol Pomozov]

Hi Adam, thank you for your reply.

On Mon, Jun 19, 2017 at 2:08 PM, Adam Lackorzynski
<a...@os.inf.tu-dresden.de> wrote:
> Hi,
>
> On Tue Jun 13, 2017 at 17:05:41 -0700, Anatol Pomozov wrote:
>> Do these arguments sound reasonable to apply the patch?
>
> I'm not really convinced.
>
>> On Thu, Jun 8, 2017 at 2:07 PM, Anatol Pomozov <anatol.pomo...@gmail.com> 
>> wrote:
>> > +reply-all
>> >
>> > On Thu, Jun 8, 2017 at 1:41 PM, Adam Lackorzynski
>> > <a...@os.inf.tu-dresden.de> wrote:
>> >>
>> >> On Tue Jun 06, 2017 at 21:41:48 -0700, Anatol Pomozov wrote:
>> >>> It is possible to create a 64 bit elf image that has valid multiboot 
>> >>> header.
>> >>> qemu should be able to boot such images.
>> >>
>> >> But this 64bit image actually starts with 32bit code, right?
>> >
>> > Correct. The very first part of the startup code has to be 32bit.
>> > After it sets "long mode" it can use 64bit instructions. To make sure
>> > that the preamble has only 32bit instructions one have to use asm
>> > directive such as ".code32".
>> >
>> > Here is an example from LitleKernel sturtup code:
>> >
>> > https://github.com/littlekernel/lk/blob/master/arch/x86/64/start.S#L50
>> >
>> > .code32 tells assembler to treat following text as 32 bit code. And
>> > later when it jumps into "long mode"
>> >
>> > https://github.com/littlekernel/lk/blob/master/arch/x86/64/start.S#L214
>> > one can use 64bit code.
>> >
>> >> So it's a 32bit program and the check verifies that this is the case.
>> >
>> > While preamble have to contain 32 only instructions the rest of the
>> > image can perfectly contain 64bit code. Right now 64bit binary cannot
>> > be run with "qemu-system-x86_64 -kernel". But the same binary runs
>> > fine if packed with GRUB as iso.
>> >
>> > I tried to hack around this restriction by adding
>> > "OUTPUT_FORMAT(elf32-i386)" to the linker file and compiling project
>> > with 64bit support. But GNU ld program crashed at Ubuntu 14.04. It
>> > means not that many people use this code path. GNU ld compiled from
>> > HEAD does not have this problem but now GDB is confused by the fact
>> > that ELF contains 64bit code while header reports i386.
>
> That's unfortunate.
>
>> > Practically there is no reason for this check as it prevents running
>> > 64bit binaries with "qemu-system-x86_64 -kernel".
>
> One reason for the check is that it prevents that one loads a 64bit ELF
> binary that then fails strangely because it does not have the magic
> 32bit code to set up things.

I would not call the 32bit preamble a magic - it is fairly well
documented; it is just a result of backward compatibility that
AMD/Intel carry for a long time. Every single 64-bit operation system
(Linux, Windows, FreeBSD, ...) has such 32bit init sequence.

And while I understand your concerns about bugs in 64-bit mode
initialization I do not think that preventing elf64 images altogether
is a right fix for it. Loading 64bit OS with multiboot is a totally
valid use-case, you can find plenty of examples/OS that show how to do
it. Adding the restriction is just another hoop that users need to
jump over.

It is a good idea to check how other multiboot-compliant bootloaders
deal with such situation. GRUB has the best support for multiboot and
it works great both with elf64 bit images. You can see its code here
http://git.savannah.gnu.org/cgit/grub.git/tree/grub-core/loader/multiboot.c#n208

/* Load ELF32 or ELF64.  */
grub_err_t
grub_multiboot_load_elf (mbi_load_data_t *mld)
{
  if (grub_multiboot_is_elf32 (mld->buffer))
return grub_multiboot_load_elf32 (mld);
  else if (grub_multiboot_is_elf64 (mld->buffer))
return grub_multiboot_load_elf64 (mld);

  return grub_error (GRUB_ERR_UNKNOWN_OS, N_("invalid arch-dependent
ELF magic"));
}


I tried my elf64 OS in following configuration:
  Qemu + GRUB = works
  VMWare + GRUB = works
  GRUB at bare hardware = works

Qemu with -kernel loading elf64 directly - works flawlessly if the
patch above applied.

So currently QEMU breaks compatibility with GRUB. elf64 images boot
fine with GRUB while fail to boot with QEMU.

> At least there needs to be an override
> (could also be in the ELF info).
>
> Doing a proper 32bit wrapper is also possible, although that would need
> a little ELF loader, or a custom loader, probably. We've done it this
> way but that also requires some more lines.

No need for a wrapper. Current QEMU code works fine with

[Qemu-devel] [Bug 1699567] [NEW] Qemu does not force SSE data alignment

[Anatol Pomozov]

Public bug reported:

I have an OS that tries to use SSE operations. It works fine in qemu.
But it crashes when I try to run the OS at the host cpu using KVM.

The instruction that crahes with #GP(0) is
 movaps ADDR,%xmm0

The documentation says ADDR has to be 16-bytes alignment otherwise #GP
is generated. And indeed the problem was with the data alignment. After
adjusting it at my side the OS works fine both with Qemu and KVM.

It would be great if QEMU followed specification more closely and forced
SSE data alignment requirements. It will help to catch alignment issues
early and debug it easier.


$ qemu-system-x86_64 -version
QEMU emulator version 2.9.50 (v2.9.0-1363-g95eef1c68b)
Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers

** Affects: qemu
 Importance: Undecided
 Status: New

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1699567

Title:
  Qemu does not force SSE data alignment

Status in QEMU:
  New

Bug description:
  I have an OS that tries to use SSE operations. It works fine in qemu.
  But it crashes when I try to run the OS at the host cpu using KVM.

  The instruction that crahes with #GP(0) is
   movaps ADDR,%xmm0

  The documentation says ADDR has to be 16-bytes alignment otherwise #GP
  is generated. And indeed the problem was with the data alignment.
  After adjusting it at my side the OS works fine both with Qemu and
  KVM.

  It would be great if QEMU followed specification more closely and
  forced SSE data alignment requirements. It will help to catch
  alignment issues early and debug it easier.

  
  $ qemu-system-x86_64 -version
  QEMU emulator version 2.9.50 (v2.9.0-1363-g95eef1c68b)
  Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1699567/+subscriptions

[Qemu-devel] [Bug 1248959] Re: pdpe1gb flag is missing in guest running on Intel h/w

[Anatol Pomozov]

I observe the same situation. My host CPU (Intel Xeon CPU E5-2690)
supports 1gb pages but qemu keeps it disabled by default. I have to use
either '-cpu phenom' or '-cpu host' with KVM.

It makes me wondering what is the default CPU for QEMU? Is it possible
to set qemu CPU featureset closer to what host CPU has?

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1248959

Title:
  pdpe1gb flag is missing in guest running on Intel h/w

Status in QEMU:
  New

Bug description:
  I need to utilize 1G hugepages on my guest system. But this is not
  possible as long as there is no pdpe1gb support in guest system.  The
  latest source code contains pdpe1gb  support for AMD but not for
  Intel.

  Are there any obstacles that does not allow to implement it for modern
  Intel chips?

  My configuration:
  Host:
  ---
  uname -a
  Linux tripel.salab.cic.nsn-rdnet.net 2.6.32-358.14.1.el6.x86_64 #1 SMP Tue 
Jul 16 23:51:20 UTC 2013 x86_64 x86_64 x86_64 GNU/Linux

  cat /etc/*-release
  CentOS release 6.4 (Final)

  yum list installed | grep qemu
  gpxe-roms-qemu.noarch0.9.7-6.9.el6 @base
  qemu-img.x86_64  2:0.12.1.2-2.355.0.1.el6.centos.5
  qemu-kvm.x86_64  2:0.12.1.2-2.355.0.1.el6.centos.5

  cat /proc/cpuinfo
  processor   : 0
  vendor_id   : GenuineIntel
  cpu family  : 6
  model   : 45
  model name  : Intel(R) Xeon(R) CPU E5-2680 0 @ 2.70GHz
  stepping: 7
  cpu MHz : 2700.000
  cache size  : 20480 KB
  physical id : 0
  siblings: 16
  core id : 0
  cpu cores   : 8
  apicid  : 0
  initial apicid  : 0
  fpu : yes
  fpu_exception   : yes
  cpuid level : 13
  wp  : yes
  flags   : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca 
cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx 
pdpe1gb rdtscp lm constant_tsc arch_perfmon pebs bts rep_good xtopology 
nonstop_tsc aperfmperf pni pclmulqdq dtes64 monitor ds_cpl vmx smx est tm2 
ssse3 cx16 xtpr pdcm pcid dca sse4_1 sse4_2 x2apic popcnt tsc_deadline_timer 
aes xsave avx lahf_lm ida arat epb xsaveopt pln pts dts tpr_shadow vnmi 
flexpriority ept vpid
  bogomips: 5387.09
  clflush size: 64
  cache_alignment : 64
  address sizes   : 46 bits physical, 48 bits virtual

  /usr/libexec/qemu-kvm -cpu ?
  Recognized CPUID flags:
f_edx: pbe ia64 tm ht ss sse2 sse fxsr mmx acpi ds clflush pn pse36 pat 
cmov mca pge mtrr sep apic cx8 mce pae msr tsc pse de vme fpu
f_ecx: hypervisor rdrand f16c avx osxsave xsave aes tsc-deadline popcnt 
movbe x2apic sse4.2|sse4_2 sse4.1|sse4_1 dca pcid pdcm xtpr cx16 fma cid ssse3 
tm2 est smx vmx ds_cpl monitor dtes64 pclmulqdq|pclmuldq pni|sse3
extf_edx: 3dnow 3dnowext lm|i64 rdtscp pdpe1gb fxsr_opt|ffxsr fxsr mmx 
mmxext nx|xd pse36 pat cmov mca pge mtrr syscall apic cx8 mce pae msr tsc pse 
de vme fpu
extf_ecx: perfctr_nb perfctr_core topoext tbm nodeid_msr tce fma4 lwp wdt 
skinit xop ibs osvw 3dnowprefetch misalignsse sse4a abm cr8legacy extapic svm 
cmp_legacy lahf_lm

  ps ax | grep qemu
   7197 ?Sl 0:15 /usr/libexec/qemu-kvm -name vladimir.AS-0 -S -M 
rhel6.4.0 -cpu 
SandyBridge,+pdpe1gb,+osxsave,+dca,+pcid,+pdcm,+xtpr,+tm2,+est,+smx,+vmx,+ds_cpl,+monitor,+dtes64,+pbe,+tm,+ht,+ss,+acpi,+ds,+vme
 -enable-kvm -m 8192 -mem-prealloc -mem-path 
/var/lib/hugetlbfs/pagesize-1GB/libvirt/qemu -smp 4,sockets=4,cores=1,threads=1 
-uuid ec2d3c58-a7f0-fdbd-9de5-b547a5b3130f -nographic -nodefconfig -nodefaults 
-chardev 
socket,id=charmonitor,path=/var/lib/libvirt/qemu/vladimir.AS-0.monitor,server,nowait
 -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc -no-shutdown 
-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 -netdev 
tap,fd=28,id=hostnet0 -device 
e1000,netdev=hostnet0,id=net0,mac=52:54:00:81:5b:df,bus=pci.0,addr=0x3,bootindex=1
 -chardev pty,id=charserial0 -device isa-serial,chardev=charserial0,id=serial0 
-device pci-assign,host=02:00.0,id=hostdev0,configfd=29,bus=pci.0,addr=0x4 
-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5

  Guest:
  -
  # uname -a
  Linux AS-0 2.6.34.13-WR4.3.fp_x86_64_standard-00019-g052bb3e #1 SMP Wed May 8 
12:21:02 EEST 2013 x86_64 x86_64 x86_64 GNU/Linux

  #  cat /etc/*-release
  Wind River Linux 4.3 glibc_cgl

  # cat /proc/cpuinfo
  processor   : 0
  vendor_id   : GenuineIntel
  cpu family  : 6
  model   : 42
  model name  : Intel Xeon E312xx (Sandy Bridge)
  stepping: 1
  cpu MHz : 2693.893
  cache size  : 4096 KB
  fpu : yes
  fpu_exception   : yes
  cpuid level : 13
  wp  : yes
  flags   : fpu vme de pse tsc msr pae mce cx8 apic mtrr pge mca cmov 
pat pse36 clflush mmx fxsr sse sse2 ss syscall nx lm constant_tsc rep_good pni 
pclmulqdq ssse3 cx16 sse4_1 

Re: [Qemu-devel] [PATCH] Remove restriction that prevents bootimg elf64 images

[Anatol Pomozov]

Hi Adam

Do these arguments sound reasonable to apply the patch?

On Thu, Jun 8, 2017 at 2:07 PM, Anatol Pomozov <anatol.pomo...@gmail.com> wrote:
> +reply-all
>
> On Thu, Jun 8, 2017 at 1:41 PM, Adam Lackorzynski
> <a...@os.inf.tu-dresden.de> wrote:
>>
>> On Tue Jun 06, 2017 at 21:41:48 -0700, Anatol Pomozov wrote:
>>> It is possible to create a 64 bit elf image that has valid multiboot header.
>>> qemu should be able to boot such images.
>>
>> But this 64bit image actually starts with 32bit code, right?
>
> Correct. The very first part of the startup code has to be 32bit.
> After it sets "long mode" it can use 64bit instructions. To make sure
> that the preamble has only 32bit instructions one have to use asm
> directive such as ".code32".
>
> Here is an example from LitleKernel sturtup code:
>
> https://github.com/littlekernel/lk/blob/master/arch/x86/64/start.S#L50
>
> .code32 tells assembler to treat following text as 32 bit code. And
> later when it jumps into "long mode"
>
> https://github.com/littlekernel/lk/blob/master/arch/x86/64/start.S#L214
> one can use 64bit code.
>
>> So it's a 32bit program and the check verifies that this is the case.
>
> While preamble have to contain 32 only instructions the rest of the
> image can perfectly contain 64bit code. Right now 64bit binary cannot
> be run with "qemu-system-x86_64 -kernel". But the same binary runs
> fine if packed with GRUB as iso.
>
> I tried to hack around this restriction by adding
> "OUTPUT_FORMAT(elf32-i386)" to the linker file and compiling project
> with 64bit support. But GNU ld program crashed at Ubuntu 14.04. It
> means not that many people use this code path. GNU ld compiled from
> HEAD does not have this problem but now GDB is confused by the fact
> that ELF contains 64bit code while header reports i386.
>
> Practically there is no reason for this check as it prevents running
> 64bit binaries with "qemu-system-x86_64 -kernel".

Re: [Qemu-devel] [PATCH] Remove restriction that prevents bootimg elf64 images

[Anatol Pomozov]

+reply-all

On Thu, Jun 8, 2017 at 1:41 PM, Adam Lackorzynski
<a...@os.inf.tu-dresden.de> wrote:
>
> On Tue Jun 06, 2017 at 21:41:48 -0700, Anatol Pomozov wrote:
>> It is possible to create a 64 bit elf image that has valid multiboot header.
>> qemu should be able to boot such images.
>
> But this 64bit image actually starts with 32bit code, right?

Correct. The very first part of the startup code has to be 32bit.
After it sets "long mode" it can use 64bit instructions. To make sure
that the preamble has only 32bit instructions one have to use asm
directive such as ".code32".

Here is an example from LitleKernel sturtup code:

https://github.com/littlekernel/lk/blob/master/arch/x86/64/start.S#L50

.code32 tells assembler to treat following text as 32 bit code. And
later when it jumps into "long mode"

https://github.com/littlekernel/lk/blob/master/arch/x86/64/start.S#L214
one can use 64bit code.

> So it's a 32bit program and the check verifies that this is the case.

While preamble have to contain 32 only instructions the rest of the
image can perfectly contain 64bit code. Right now 64bit binary cannot
be run with "qemu-system-x86_64 -kernel". But the same binary runs
fine if packed with GRUB as iso.

I tried to hack around this restriction by adding
"OUTPUT_FORMAT(elf32-i386)" to the linker file and compiling project
with 64bit support. But GNU ld program crashed at Ubuntu 14.04. It
means not that many people use this code path. GNU ld compiled from
HEAD does not have this problem but now GDB is confused by the fact
that ELF contains 64bit code while header reports i386.

Practically there is no reason for this check as it prevents running
64bit binaries with "qemu-system-x86_64 -kernel".

Re: [Qemu-devel] [PATCH] Remove restriction that prevents bootimg elf64 images

[Anatol Pomozov]

+ more folks who made changes to hw/i386/multiboot.c

On Tue, Jun 6, 2017 at 9:41 PM, Anatol Pomozov <anatol.pomo...@gmail.com> wrote:
> It is possible to create a 64 bit elf image that has valid multiboot header.
> qemu should be able to boot such images.
>
> Tested with homemade 64bit OS - now it boots fine with 'qemu -kernel'
> and as a grub image.
>
> Signed-off-by: Anatol Pomozov <anatol.pomo...@gmail.com>
> ---
>  hw/i386/multiboot.c | 5 -
>  1 file changed, 5 deletions(-)
>
> diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
> index 663f35a658..cf1b4f5fb3 100644
> --- a/hw/i386/multiboot.c
> +++ b/hw/i386/multiboot.c
> @@ -192,11 +192,6 @@ int load_multiboot(FWCfgState *fw_cfg,
>  int kernel_size;
>  fclose(f);
>
> -if (((struct elf64_hdr*)header)->e_machine == EM_X86_64) {
> -fprintf(stderr, "Cannot load x86-64 image, give a 32bit one.\n");
> -exit(1);
> -}
> -
>  kernel_size = load_elf(kernel_filename, NULL, NULL, _entry,
> _low, _high, 0, I386_ELF_MACHINE,
> 0, 0);
> --
> 2.13.1
>

[Qemu-devel] [PATCH] Remove restriction that prevents bootimg elf64 images

[Anatol Pomozov]

It is possible to create a 64 bit elf image that has valid multiboot header.
qemu should be able to boot such images.

Tested with homemade 64bit OS - now it boots fine with 'qemu -kernel'
and as a grub image.

Signed-off-by: Anatol Pomozov <anatol.pomo...@gmail.com>
---
 hw/i386/multiboot.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
index 663f35a658..cf1b4f5fb3 100644
--- a/hw/i386/multiboot.c
+++ b/hw/i386/multiboot.c
@@ -192,11 +192,6 @@ int load_multiboot(FWCfgState *fw_cfg,
 int kernel_size;
 fclose(f);
 
-if (((struct elf64_hdr*)header)->e_machine == EM_X86_64) {
-fprintf(stderr, "Cannot load x86-64 image, give a 32bit one.\n");
-exit(1);
-}
-
 kernel_size = load_elf(kernel_filename, NULL, NULL, _entry,
_low, _high, 0, I386_ELF_MACHINE,
0, 0);
-- 
2.13.1

[Qemu-devel] [Bug 1695286] [NEW] Add multiboot2 support

[Anatol Pomozov]

Public bug reported:

multiboot2 is a recent specification that resolves some of the issues of
multiboot. Multiboot2 is supported by some tools already (e.g. grub).

It would be great if one can run OS with multiboot2 using '-kernel'
option, similar as it is done now with multiboot images.

Quick googling shows there is a Debian bug and patch that adds multiboot
support https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=621529 Would
it be possible to integrate it to QEMU upstream?

** Affects: qemu
 Importance: Undecided
 Status: New

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1695286

Title:
  Add multiboot2 support

Status in QEMU:
  New

Bug description:
  multiboot2 is a recent specification that resolves some of the issues
  of multiboot. Multiboot2 is supported by some tools already (e.g.
  grub).

  It would be great if one can run OS with multiboot2 using '-kernel'
  option, similar as it is done now with multiboot images.

  Quick googling shows there is a Debian bug and patch that adds
  multiboot support https://bugs.debian.org/cgi-
  bin/bugreport.cgi?bug=621529 Would it be possible to integrate it to
  QEMU upstream?

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1695286/+subscriptions