[PATCH 0/3] Add OpenSBI dynamic firmware support

Atish Patra posted 3 patches 3 years, 11 months ago
Test docker-quick@centos7 passed
Test FreeBSD passed
Test asan failed
Test checkpatch passed
Test docker-mingw@fedora passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200616192700.1900260-1-atish.patra@wdc.com
Maintainers: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>, Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <Alistair.Francis@wdc.com>, Sagar Karandikar <sagark@eecs.berkeley.edu>
There is a newer version of this series
hw/riscv/boot.c                 | 95 +++++++++++++++++++++++++++++++++
hw/riscv/sifive_u.c             | 59 ++++++++------------
hw/riscv/spike.c                | 55 ++++++++-----------
hw/riscv/virt.c                 | 55 ++++++++-----------
include/hw/riscv/boot.h         |  5 ++
include/hw/riscv/boot_opensbi.h | 58 ++++++++++++++++++++
6 files changed, 223 insertions(+), 104 deletions(-)
create mode 100644 include/hw/riscv/boot_opensbi.h
[PATCH 0/3] Add OpenSBI dynamic firmware support
Posted by Atish Patra 3 years, 11 months ago
This series adds support OpenSBI dynamic firmware support to Qemu.
Qemu loader passes the information about the DT and next stage (i.e. kernel
or U-boot) via "a2" register. It allows the user to build bigger OS images
without worrying about overwriting DT. It also unifies the reset vector code
in rom and dt placement. Now, the DT is copied directly in DRAM instead of ROM.

The changes have been verified on following qemu machines.

64bit:
 - spike, sifive_u, virt
32bit:
 - virt

I have also verified fw_jump on all the above platforms to ensure that this
series doesn't break the existing setup.

Atish Patra (3):
riscv: Unify Qemu's reset vector code path
RISC-V: Copy the fdt in dram instead of ROM
riscv: Add opensbi firmware dynamic support

hw/riscv/boot.c                 | 95 +++++++++++++++++++++++++++++++++
hw/riscv/sifive_u.c             | 59 ++++++++------------
hw/riscv/spike.c                | 55 ++++++++-----------
hw/riscv/virt.c                 | 55 ++++++++-----------
include/hw/riscv/boot.h         |  5 ++
include/hw/riscv/boot_opensbi.h | 58 ++++++++++++++++++++
6 files changed, 223 insertions(+), 104 deletions(-)
create mode 100644 include/hw/riscv/boot_opensbi.h

--
2.26.2


Re: [PATCH 0/3] Add OpenSBI dynamic firmware support
Posted by no-reply@patchew.org 3 years, 11 months ago
Patchew URL: https://patchew.org/QEMU/20200616192700.1900260-1-atish.patra@wdc.com/



Hi,

This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC      qga/commands.o
  CC      qga/guest-agent-command-state.o
  CC      qga/main.o
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `  CC      qga/commands-posix.o
__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  CC      qga/channel-posix.o
  CC      qga/qapi-generated/qga-qapi-types.o
---
  GEN     docs/interop/qemu-ga-ref.html
  GEN     docs/interop/qemu-ga-ref.txt
  GEN     docs/interop/qemu-ga-ref.7
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  LINK    qemu-ga
  LINK    qemu-keymap
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  LINK    ivshmem-client
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  LINK    ivshmem-server
  LINK    qemu-nbd
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  LINK    qemu-storage-daemon
  AS      pc-bios/optionrom/multiboot.o
  AS      pc-bios/optionrom/linuxboot.o
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  CC      pc-bios/optionrom/linuxboot_dma.o
  AS      pc-bios/optionrom/kvmvapic.o
  AS      pc-bios/optionrom/pvh.o
---
  BUILD   pc-bios/optionrom/multiboot.raw
  BUILD   pc-bios/optionrom/linuxboot.raw
  BUILD   pc-bios/optionrom/kvmvapic.raw
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  SIGN    pc-bios/optionrom/multiboot.bin
  SIGN    pc-bios/optionrom/linuxboot.bin
  SIGN    pc-bios/optionrom/kvmvapic.bin
  LINK    fsdev/virtfs-proxy-helper
  LINK    scsi/qemu-pr-helper
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  BUILD   pc-bios/optionrom/linuxboot_dma.img
  BUILD   pc-bios/optionrom/linuxboot_dma.raw
  SIGN    pc-bios/optionrom/linuxboot_dma.bin
  LINK    qemu-bridge-helper
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  BUILD   pc-bios/optionrom/pvh.img
  LINK    virtiofsd
  BUILD   pc-bios/optionrom/pvh.raw
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  SIGN    pc-bios/optionrom/pvh.bin
  LINK    vhost-user-input
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  GEN     x86_64-softmmu/hmp-commands.h
  GEN     x86_64-softmmu/hmp-commands-info.h
  GEN     x86_64-softmmu/config-devices.h
---
  CC      x86_64-softmmu/hw/display/virtio-gpu.o
  CC      x86_64-softmmu/hw/display/virtio-gpu-3d.o
  CC      x86_64-softmmu/hw/display/vhost-user-gpu.o
/tmp/qemu-test/src/fpu/softfloat.c:3365:13: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation]
    absZ &= ~ ( ( ( roundBits ^ 0x40 ) == 0 ) & roundNearestEven );
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            !
/tmp/qemu-test/src/fpu/softfloat.c:3423:18: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation]
        absZ0 &= ~ ( ( (uint64_t) ( absZ1<<1 ) == 0 ) & roundNearestEven );
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                 !
/tmp/qemu-test/src/fpu/softfloat.c:3483:18: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation]
        absZ0 &= ~(((uint64_t)(absZ1<<1) == 0) & roundNearestEven);
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                 !
/tmp/qemu-test/src/fpu/softfloat.c:3606:13: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation]
    zSig &= ~ ( ( ( roundBits ^ 0x40 ) == 0 ) & roundNearestEven );
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            !
/tmp/qemu-test/src/fpu/softfloat.c:3760:13: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation]
    zSig &= ~ ( ( ( roundBits ^ 0x200 ) == 0 ) & roundNearestEven );
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            !
/tmp/qemu-test/src/fpu/softfloat.c:3987:21: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation]
                    ~ ( ( (uint64_t) ( zSig1<<1 ) == 0 ) & roundNearestEven );
                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                    !
/tmp/qemu-test/src/fpu/softfloat.c:4003:22: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation]
            zSig0 &= ~ ( ( (uint64_t) ( zSig1<<1 ) == 0 ) & roundNearestEven );
                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                     !
/tmp/qemu-test/src/fpu/softfloat.c:4273:18: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation]
        zSig1 &= ~ ( ( zSig2 + zSig2 == 0 ) & roundNearestEven );
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                 !
8 errors generated.
make[1]: *** [/tmp/qemu-test/src/rules.mak:69: fpu/softfloat.o] Error 1
make[1]: *** Waiting for unfinished jobs....
/tmp/qemu-test/src/migration/ram.c:919:45: error: implicit conversion from 'unsigned long' to 'double' changes value from 18446744073709551615 to 18446744073709551616 [-Werror,-Wimplicit-int-float-conversion]
            xbzrle_counters.encoding_rate = UINT64_MAX;
                                          ~ ^~~~~~~~~~
/usr/include/stdint.h:130:23: note: expanded from macro 'UINT64_MAX'
---
18446744073709551615UL
^~~~~~~~~~~~~~~~~~~~~~
1 error generated.
make[1]: *** [/tmp/qemu-test/src/rules.mak:69: migration/ram.o] Error 1
make: *** [Makefile:527: x86_64-softmmu/all] Error 2
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 669, in <module>
    sys.exit(main())
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=87c0cd21d942481f9c8b0deaf8f61398', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=x86_64-softmmu', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-yoca3tbl/src/docker-src.2020-06-16-18.35.31.23981:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-debug']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=87c0cd21d942481f9c8b0deaf8f61398
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-yoca3tbl/src'
make: *** [docker-run-test-debug@fedora] Error 2

real    4m15.706s
user    0m9.061s


The full log is available at
http://patchew.org/logs/20200616192700.1900260-1-atish.patra@wdc.com/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Re: [PATCH 0/3] Add OpenSBI dynamic firmware support
Posted by Bin Meng 3 years, 11 months ago
On Wed, Jun 17, 2020 at 3:29 AM Atish Patra <atish.patra@wdc.com> wrote:
>
> This series adds support OpenSBI dynamic firmware support to Qemu.
> Qemu loader passes the information about the DT and next stage (i.e. kernel
> or U-boot) via "a2" register. It allows the user to build bigger OS images
> without worrying about overwriting DT. It also unifies the reset vector code

I am not sure in what situation overwriting DT could happen. Could you
please elaborate?

> in rom and dt placement. Now, the DT is copied directly in DRAM instead of ROM.
>
> The changes have been verified on following qemu machines.
>
> 64bit:
>  - spike, sifive_u, virt
> 32bit:
>  - virt

Any test instructions?

>
> I have also verified fw_jump on all the above platforms to ensure that this
> series doesn't break the existing setup.
>

Regards,
Bin

Re: [PATCH 0/3] Add OpenSBI dynamic firmware support
Posted by Atish Patra 3 years, 11 months ago
On Thu, Jun 18, 2020 at 1:56 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Wed, Jun 17, 2020 at 3:29 AM Atish Patra <atish.patra@wdc.com> wrote:
> >
> > This series adds support OpenSBI dynamic firmware support to Qemu.
> > Qemu loader passes the information about the DT and next stage (i.e. kernel
> > or U-boot) via "a2" register. It allows the user to build bigger OS images
> > without worrying about overwriting DT. It also unifies the reset vector code
>
> I am not sure in what situation overwriting DT could happen. Could you
> please elaborate?
>

Currently, the DT is loaded 0x82200000 (34MB offset) for fw_jump.
Thus, a bigger kernel image
would overwrite the DT. In fact, it was reported by FreeBSD folks.
https://github.com/riscv/opensbi/issues/169

There are temporary solutions that can put DT a little bit further or
put it within 2MB offset. But that's
just delaying the inevitable.

> > in rom and dt placement. Now, the DT is copied directly in DRAM instead of ROM.
> >
> > The changes have been verified on following qemu machines.
> >
> > 64bit:
> >  - spike, sifive_u, virt
> > 32bit:
> >  - virt
>
> Any test instructions?
>

you just need to provide fw_dynamic instead of fw_jump in bios argument.

For example: Here is my qemu commandline for testing

qemu-system-riscv64 -M virt -smp 4 -m 2g -display none -serial
mon:stdio -bios
~/workspace/opensbi/build/platform/generic/firmware/fw_dynamic.bin \
   -kernel /home/atish/workspace/linux/arch/riscv/boot/Image -initrd
/home/atish/workspace/rootfs_images/riscv64_busybox_rootfs.img -object
rng-random,filename=/dev/urandom,id=rng0 \
   -device virtio-rng-device,rng=rng0 -device
virtio-net-device,netdev=usernet -netdev
user,id=usernet,hostfwd=tcp::10000-:22 -d in_asm -D log -append 'rw
console=ttyS0 earlycon'

> >
> > I have also verified fw_jump on all the above platforms to ensure that this
> > series doesn't break the existing setup.
> >
>
> Regards,
> Bin
>


-- 
Regards,
Atish

Re: [PATCH 0/3] Add OpenSBI dynamic firmware support
Posted by Alexander Richardson 3 years, 11 months ago
On Thu, 18 Jun 2020 at 19:22, Atish Patra <atishp@atishpatra.org> wrote:
>
> On Thu, Jun 18, 2020 at 1:56 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > On Wed, Jun 17, 2020 at 3:29 AM Atish Patra <atish.patra@wdc.com> wrote:
> > >
> > > This series adds support OpenSBI dynamic firmware support to Qemu.
> > > Qemu loader passes the information about the DT and next stage (i.e. kernel
> > > or U-boot) via "a2" register. It allows the user to build bigger OS images
> > > without worrying about overwriting DT. It also unifies the reset vector code
> >
> > I am not sure in what situation overwriting DT could happen. Could you
> > please elaborate?
> >
>
> Currently, the DT is loaded 0x82200000 (34MB offset) for fw_jump.
> Thus, a bigger kernel image
> would overwrite the DT. In fact, it was reported by FreeBSD folks.
> https://github.com/riscv/opensbi/issues/169
>
The problem is that the DT overwrites the kernel image. Usually this
is not noticeable since it's so small and rarely overwrites something
useful, but in my case it was overwriting program memory which
resulted in invalid instruction crashes.
Since this is quite awkward to debug, I added a kernel assertion to
FreeBSD to abort boot in that case.

> There are temporary solutions that can put DT a little bit further or
> put it within 2MB offset. But that's
> just delaying the inevitable.
>
I've changed OpenSBI locally to use a 1MB offset (i.e. place the DT
between OpenSBI and the kernel), but I think the fw_dynamic approach
is much nicer.

Thanks,
Alex