Re: [PATCH v7 3/3] hw/riscv: clear kernel_entry higher bits in load_elf_ram_sym()

[Bin Meng]

Hi Alistair,

On Mon, Jan 16, 2023 at 12:28 PM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Sat, Jan 14, 2023 at 11:41 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > On Sat, Jan 14, 2023 at 1:18 AM Daniel Henrique Barboza
> > <dbarboza@ventanamicro.com> wrote:
> > >
> > > Recent hw/risc/boot.c changes caused a regression in an use case with
> > > the Xvisor hypervisor. Running a 32 bit QEMU guest with '-kernel'
> > > stopped working. The reason seems to be that Xvisor is using 64 bit to
> > > encode the 32 bit addresses from the guest, and load_elf_ram_sym() is
> > > sign-extending the result with '1's [1].
> >
> > I would say it's not a regression of QEMU but something weird happened
> > to Alistair's 32-bit Xvisor image.
>
> I don't think it's a Xvisor issue.
>
> >
> > I just built a 32-bit Xvisor image from the latest Xvisor head
> > following the instructions provided in its source tree. With the
> > mainline QEMU only BIN file boots, but ELF does not. My 32-bit Xvisor
> > image has an address of 0x10000000. Apparently this address is not
> > correct, and the issue I saw is different from Alistair's. Alistair,
> > could you investigate why your 32-bit Xvisor ELF image has an address
> > of 0xffffffff80000000 set to kernel_load_base?
>
> Looking in load_elf() in include/hw/elf_ops.h at this line:
>
>     if (lowaddr)
>         *lowaddr = (uint64_t)(elf_sword)low;
>
> I can see that `low` is 0x80000000 but lowaddr is set to
> 0xffffffff80000000. So the address is being sign extended with 1s.
>

I don't understand the sign extension here. This seems intentional as
the codes does the signed extension then casted to unsigned 64-bit.

Do you know why?

> This patch seems to be the correct fix.
>

Regards,
Bin