Re: [Qemu-devel] [PATCH v2 1/1] riscv: Ensure the kernel start address is correctly cast

[Alistair Francis]

On Mon, Jan 14, 2019 at 2:58 AM Philippe Mathieu-Daudé
<address@hidden> wrote:
>
> Hi Alistair,
>
> On 1/12/19 2:17 AM, Alistair Francis wrote:
> > Cast the kernel start address to the target bit length.
> >
> > This ensures that we calculate the initrd offset to a valid address for
> > the architecture.
>
> Can you add an example of the failure symptoms?

I can.

>
> >
> > Signed-off-by: Alistair Francis <address@hidden>
> > Suggested-by: Alexander Graf <address@hidden>
> > Reported-by: Alexander Graf <address@hidden>
> > ---
> > v2:
> >  - Remove old comment
> >  hw/riscv/sifive_e.c | 2 +-
> >  hw/riscv/sifive_u.c | 2 +-
> >  hw/riscv/spike.c    | 2 +-
> >  hw/riscv/virt.c     | 2 +-
> >  4 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
> > index 5d9d65ff29..e5d7fc548e 100644
> > --- a/hw/riscv/sifive_e.c
> > +++ b/hw/riscv/sifive_e.c
> > @@ -74,7 +74,7 @@ static const struct MemmapEntry {
> >      [SIFIVE_E_DTIM] =     { 0x80000000,     0x4000 }
> >  };
> >
> > -static uint64_t load_kernel(const char *kernel_filename)
> > +static target_ulong load_kernel(const char *kernel_filename)
> >  {
> >      uint64_t kernel_entry, kernel_high;
>
> Shouldn't you update load_elf() and co now to take target_ulong
> arguments? This would fix this error generically for all archs.

That is an option, but as load_elf() is called by every other machine
I don't want to break them. It's entirely possible that other machines
rely on this behaviour and changing it will break them.

Alistair

>
> >
> > diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> > index 3bd3b67507..3b3acec377 100644
> > --- a/hw/riscv/sifive_u.c
> > +++ b/hw/riscv/sifive_u.c
> > @@ -65,7 +65,7 @@ static const struct MemmapEntry {
> >
> >  #define GEM_REVISION        0x10070109
> >
> > -static uint64_t load_kernel(const char *kernel_filename)
> > +static target_ulong load_kernel(const char *kernel_filename)
> >  {
> >      uint64_t kernel_entry, kernel_high;
> >
> > diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
> > index 268df04c3c..79cb4c1282 100644
> > --- a/hw/riscv/spike.c
> > +++ b/hw/riscv/spike.c
> > @@ -53,7 +53,7 @@ static const struct MemmapEntry {
> >      [SPIKE_DRAM] =     { 0x80000000,        0x0 },
> >  };
> >
> > -static uint64_t load_kernel(const char *kernel_filename)
> > +static target_ulong load_kernel(const char *kernel_filename)
> >  {
> >      uint64_t kernel_entry, kernel_high;
> >
> > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> > index e7f0716fb6..648462b18c 100644
> > --- a/hw/riscv/virt.c
> > +++ b/hw/riscv/virt.c
> > @@ -62,7 +62,7 @@ static const struct MemmapEntry {
> >      [VIRT_PCIE_ECAM] =   { 0x30000000,    0x10000000 },
> >  };
> >
> > -static uint64_t load_kernel(const char *kernel_filename)
> > +static target_ulong load_kernel(const char *kernel_filename)
> >  {
> >      uint64_t kernel_entry, kernel_high;
> >
> >