qemu-riscv
[Top][All Lists]
Advanced

[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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]