[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v11 1/3] hw/riscv: handle 32 bit CPUs kernel_entry in riscv_l

From: Philippe Mathieu-Daudé
Subject: Re: [PATCH v11 1/3] hw/riscv: handle 32 bit CPUs kernel_entry in riscv_load_kernel()
Date: Mon, 6 Feb 2023 16:12:12 +0100
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.6.1

On 6/2/23 15:00, Daniel Henrique Barboza wrote:
Next patch will move all calls to riscv_load_initrd() to
riscv_load_kernel(). Machines that want to load initrd will be able to
do via an extra flag to riscv_load_kernel().

This change will expose a sign-extend behavior that is happening in
load_elf_ram_sym() when running 32 bit guests [1]. This is currently
obscured by the fact that riscv_load_initrd() is using the return of
riscv_load_kernel(), defined as target_ulong, and this return type will
crop the higher 32 bits that would be padded with 1s by the sign
extension when running in 32 bit targets. The changes to be done will
force riscv_load_initrd() to use an uint64_t instead, exposing it to the
padding when dealing with 32 bit CPUs.

There is a discussion about whether load_elf_ram_sym() should or should
not sign extend the value returned by 'lowaddr'. What we can do is to
prevent the behavior change that the next patch will end up doing.
riscv_load_initrd() wasn't dealing with 64 bit kernel entries when
running 32 bit CPUs, and we want to keep it that way.

One way of doing it is to use target_ulong in 'kernel_entry' in
riscv_load_kernel() and rely on the fact that this var will not be sign
extended for 32 bit targets. Another way is to explictly clear the
higher 32 bits when running 32 bit CPUs for all possibilities of

We opted for the later. This will allow us to be clear about the design
choices made in the function, while also allowing us to add a small
comment about what load_elf_ram_sym() is doing. With this change, the
consolation patch can do its job without worrying about unintended
behavioral changes.

[1] https://lists.gnu.org/archive/html/qemu-devel/2023-01/msg02281.html

Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
  hw/riscv/boot.c            | 20 +++++++++++++++++---
  hw/riscv/microchip_pfsoc.c |  3 ++-
  hw/riscv/opentitan.c       |  3 ++-
  hw/riscv/sifive_e.c        |  3 ++-
  hw/riscv/sifive_u.c        |  3 ++-
  hw/riscv/spike.c           |  3 ++-
  hw/riscv/virt.c            |  3 ++-
  include/hw/riscv/boot.h    |  1 +
  8 files changed, 30 insertions(+), 9 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

reply via email to

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