[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-riscv] [Qemu-devel] [RFC v1 2/5] hw/riscv: Add support for loa
From: |
Bin Meng |
Subject: |
Re: [Qemu-riscv] [Qemu-devel] [RFC v1 2/5] hw/riscv: Add support for loading a firmware |
Date: |
Wed, 19 Jun 2019 23:30:00 +0800 |
Hi,
On Wed, Jun 19, 2019 at 11:26 PM Jonathan Behrens <address@hidden> wrote:
>
> I was actually just writing up the same thing. Shifting all the addresses in
> the ELF file by 2 or 4MB is somewhat surprising behavior, especially because
> this will apply to segments that are mapped even at much higher addresses. If
> you want a segment aligned to a 1GB superpage boundary you now need to place
> it slightly below so that it will be bumped up to the right place. Further,
> ANDing all addresses with 0x7fffffff makes it impossible to map anything
> beyond the first 2GB of RAM.
>
Yes, current kernel_translate() logic is tightly coupled to the kernel
entry VA, and if we link kernel at some other address it will just
fail.
> Jonathan
>
> On Wed, Jun 19, 2019 at 11:16 AM Bin Meng <address@hidden> wrote:
>>
>> On Wed, Jun 19, 2019 at 8:53 AM Alistair Francis
>> <address@hidden> wrote:
>> >
>> > Add support for loading a firmware file for the virt machine and the
>> > SiFive U. This can be run with the following command:
>> >
>> > qemu-system-riscv64 -machine virt -bios fw_jump.elf -kernel vmlinux
>> >
>> > Signed-off-by: Alistair Francis <address@hidden>
>> > ---
>> > hw/riscv/boot.c | 41 +++++++++++++++++++++++++++++++++++++++--
>> > hw/riscv/sifive_e.c | 2 +-
>> > hw/riscv/sifive_u.c | 6 +++++-
>> > hw/riscv/spike.c | 6 +++---
>> > hw/riscv/virt.c | 7 ++++++-
>> > include/hw/riscv/boot.h | 4 +++-
>> > 6 files changed, 57 insertions(+), 9 deletions(-)
>> >
>> > diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
>> > index 62f94aaf8a..392ca0cb2e 100644
>> > --- a/hw/riscv/boot.c
>> > +++ b/hw/riscv/boot.c
>> > @@ -23,13 +23,50 @@
>> > #include "exec/cpu-defs.h"
>> > #include "hw/loader.h"
>> > #include "hw/riscv/boot.h"
>> > +#include "hw/boards.h"
>> > #include "elf.h"
>> >
>> > -target_ulong riscv_load_kernel(const char *kernel_filename)
>> > +#if defined(TARGET_RISCV32)
>> > +# define KERNEL_BOOT_ADDRESS 0x80400000
>> > +#else
>> > +# define KERNEL_BOOT_ADDRESS 0x80200000
>> > +#endif
>> > +
>> > +static uint64_t kernel_translate(void *opaque, uint64_t addr)
>> > +{
>> > + MachineState *machine = opaque;
>> > +
>> > + /*
>> > + * If the user specified a firmware move the kernel to the offset
>> > + * start address.
>> > + */
>>
>> Why?
>>
>> > + if (machine->firmware) {
>> > + return (addr & 0x7fffffff) + KERNEL_BOOT_ADDRESS;
>>
>> So with both "-bios" and "-kernel", the kernel address will be moved
>> to another address other than 0x80200000 (for 64-bit). This does not
>> look good to me.
>>
So why not simply return KERNEL_BOOT_ADDRESS in kernel_translate()?
Regards,
Bin
[Qemu-riscv] [RFC v1 1/5] hw/riscv: Split out the boot functions, Alistair Francis, 2019/06/18
[Qemu-riscv] [RFC v1 4/5] roms: Add OpenSBI version 0.3, Alistair Francis, 2019/06/18