From patchwork Fri Feb 24 18:59:02 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Palmer Dabbelt X-Patchwork-Id: 13151698 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 6A635C6FA8E for ; Fri, 24 Feb 2023 19:03:38 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1pVdJn-0001ij-Fk; Fri, 24 Feb 2023 14:01:19 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pVdJj-0001en-Ge for qemu-devel@nongnu.org; Fri, 24 Feb 2023 14:01:15 -0500 Received: from mail-pl1-x62e.google.com ([2607:f8b0:4864:20::62e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1pVdJd-0002di-EB for qemu-devel@nongnu.org; Fri, 24 Feb 2023 14:01:15 -0500 Received: by mail-pl1-x62e.google.com with SMTP id u14so460944ple.7 for ; Fri, 24 Feb 2023 11:01:01 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rivosinc-com.20210112.gappssmtp.com; s=20210112; t=1677265261; h=to:from:cc:content-transfer-encoding:mime-version:references :in-reply-to:message-id:date:subject:from:to:cc:subject:date :message-id:reply-to; bh=X9n0Y9mrA9q11LFDbHArIq2i6N1FHNLTiYRxrxwVepg=; b=pAzb59jogfcV0Y/DGs9fIcvp0PqRhaATXUTyuSS6q2btO5+N78BDAwiMDxto0VRlxK MYJY91TlCfe5ol+SQT3lfK+Jl6H9oaNGltGpX8x6/pEclt3JeCCOoB9wQyFbtX5BAn8a 5IOKoxw79rE5UhUoQNpVEU+PtPix65rZyDo57MzQU1KxeC8Z8tYImxwjR+9AVPDT8Ekx L0nCGgxTJn8mgKUEAccEWmVOVuS5voK7FhYlPZGTGO9Ezrtz7XRT0PaAbg2ATdd8JGJ4 4n1p41+q2s3Rl/stKVjRiRsXAb1tQmwkzwcn6kPi71Xs3l+ybgVxFVw5I+N//5XtkeUK ySGg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1677265261; h=to:from:cc:content-transfer-encoding:mime-version:references :in-reply-to:message-id:date:subject:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=X9n0Y9mrA9q11LFDbHArIq2i6N1FHNLTiYRxrxwVepg=; b=bKWNZEzANrBqH/rbsIZH6yMez2OzhdslARyq2QDZVqBflSs9OXdgVRu9qegpWY5p6R xX/mA/V/lF2gS6SGPWPMjg77Fy0snAb0nnBN+zZzeMVgg6HVpkWGZ5XFLVWlDMVVdpZv nt2NfYwbyoBordrPZ0sZM0HKS8hur+vasxoMBD66remtGYMp/zxS1plwcktsRzt0PDEa RkZyOBrQyNL/eiBHiKuDn78DqOnf86IxAjuyWzAb5Velju9S9munHK+eLYumtJnxpIeK oUuF/lM7vxAOLmtnMQ+kWwOfZWqxZFfn6tkuyy0kObiD8FV+ts7bgo3dbSNgkBEuXSoN Yahg== X-Gm-Message-State: AO0yUKU0liN9k/ROlcRaaWDobiITJtPc61tJcYCo0CdoHiItw+R9VeLc sQntTfndfEiPCLEnCML5fB7W0Q== X-Google-Smtp-Source: AK7set+0ZXgq8FpdAx3KAnobRZu+gelRbLfBEx05Z123EaW5IDa8HG//PJV/5ZHNorZ3WoPOxOopCA== X-Received: by 2002:a17:90b:384d:b0:234:f77:d6d2 with SMTP id nl13-20020a17090b384d00b002340f77d6d2mr17882537pjb.45.1677265260788; Fri, 24 Feb 2023 11:01:00 -0800 (PST) Received: from localhost ([135.180.224.71]) by smtp.gmail.com with ESMTPSA id bh4-20020a170902a98400b0019cb6222698sm4257825plb.266.2023.02.24.11.01.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 24 Feb 2023 11:01:00 -0800 (PST) Subject: [PULL 1/8] hw/riscv: handle 32 bit CPUs kernel_entry in riscv_load_kernel() Date: Fri, 24 Feb 2023 10:59:02 -0800 Message-Id: <20230224185908.32706-2-palmer@rivosinc.com> X-Mailer: git-send-email 2.39.0 In-Reply-To: <20230224185908.32706-1-palmer@rivosinc.com> References: <20230224185908.32706-1-palmer@rivosinc.com> MIME-Version: 1.0 Cc: qemu-riscv@nongnu.org, qemu-devel@nongnu.org, Daniel Henrique Barboza , ilippe=20Mathieu-Daud=C3=A9?= , Alistair Francis , Palmer Dabbelt From: Palmer Dabbelt To: Peter Maydell Received-SPF: pass client-ip=2607:f8b0:4864:20::62e; envelope-from=palmer@rivosinc.com; helo=mail-pl1-x62e.google.com X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org From: Daniel Henrique Barboza 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 kernel_entry. 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 Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Alistair Francis Message-Id: <20230206140022.2748401-2-dbarboza@ventanamicro.com> Signed-off-by: Alistair Francis Signed-off-by: Palmer Dabbelt --- 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(-) diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c index c7e0e50bd8..df6b4a1fba 100644 --- a/hw/riscv/boot.c +++ b/hw/riscv/boot.c @@ -174,6 +174,7 @@ target_ulong riscv_load_firmware(const char *firmware_filename, } target_ulong riscv_load_kernel(MachineState *machine, + RISCVHartArrayState *harts, target_ulong kernel_start_addr, symbol_fn_t sym_cb) { @@ -192,21 +193,34 @@ target_ulong riscv_load_kernel(MachineState *machine, if (load_elf_ram_sym(kernel_filename, NULL, NULL, NULL, NULL, &kernel_load_base, NULL, NULL, 0, EM_RISCV, 1, 0, NULL, true, sym_cb) > 0) { - return kernel_load_base; + kernel_entry = kernel_load_base; + goto out; } if (load_uimage_as(kernel_filename, &kernel_entry, NULL, NULL, NULL, NULL, NULL) > 0) { - return kernel_entry; + goto out; } if (load_image_targphys_as(kernel_filename, kernel_start_addr, current_machine->ram_size, NULL) > 0) { - return kernel_start_addr; + kernel_entry = kernel_start_addr; + goto out; } error_report("could not load kernel '%s'", kernel_filename); exit(1); + +out: + /* + * For 32 bit CPUs 'kernel_entry' can be sign-extended by + * load_elf_ram_sym(). + */ + if (riscv_is_32bit(harts)) { + kernel_entry = extract64(kernel_entry, 0, 32); + } + + return kernel_entry; } void riscv_load_initrd(MachineState *machine, uint64_t kernel_entry) diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c index 2b91e49561..712625d2a4 100644 --- a/hw/riscv/microchip_pfsoc.c +++ b/hw/riscv/microchip_pfsoc.c @@ -629,7 +629,8 @@ static void microchip_icicle_kit_machine_init(MachineState *machine) kernel_start_addr = riscv_calc_kernel_start_addr(&s->soc.u_cpus, firmware_end_addr); - kernel_entry = riscv_load_kernel(machine, kernel_start_addr, NULL); + kernel_entry = riscv_load_kernel(machine, &s->soc.u_cpus, + kernel_start_addr, NULL); if (machine->initrd_filename) { riscv_load_initrd(machine, kernel_entry); diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c index 353f030d80..7fe4fb5628 100644 --- a/hw/riscv/opentitan.c +++ b/hw/riscv/opentitan.c @@ -101,7 +101,8 @@ static void opentitan_board_init(MachineState *machine) } if (machine->kernel_filename) { - riscv_load_kernel(machine, memmap[IBEX_DEV_RAM].base, NULL); + riscv_load_kernel(machine, &s->soc.cpus, + memmap[IBEX_DEV_RAM].base, NULL); } } diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c index 3e3f4b0088..1a7d381514 100644 --- a/hw/riscv/sifive_e.c +++ b/hw/riscv/sifive_e.c @@ -114,7 +114,8 @@ static void sifive_e_machine_init(MachineState *machine) memmap[SIFIVE_E_DEV_MROM].base, &address_space_memory); if (machine->kernel_filename) { - riscv_load_kernel(machine, memmap[SIFIVE_E_DEV_DTIM].base, NULL); + riscv_load_kernel(machine, &s->soc.cpus, + memmap[SIFIVE_E_DEV_DTIM].base, NULL); } } diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c index d3ab7a9cda..71be442a50 100644 --- a/hw/riscv/sifive_u.c +++ b/hw/riscv/sifive_u.c @@ -598,7 +598,8 @@ static void sifive_u_machine_init(MachineState *machine) kernel_start_addr = riscv_calc_kernel_start_addr(&s->soc.u_cpus, firmware_end_addr); - kernel_entry = riscv_load_kernel(machine, kernel_start_addr, NULL); + kernel_entry = riscv_load_kernel(machine, &s->soc.u_cpus, + kernel_start_addr, NULL); if (machine->initrd_filename) { riscv_load_initrd(machine, kernel_entry); diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c index cc3f6dac17..1fa91167ab 100644 --- a/hw/riscv/spike.c +++ b/hw/riscv/spike.c @@ -305,7 +305,8 @@ static void spike_board_init(MachineState *machine) kernel_start_addr = riscv_calc_kernel_start_addr(&s->soc[0], firmware_end_addr); - kernel_entry = riscv_load_kernel(machine, kernel_start_addr, + kernel_entry = riscv_load_kernel(machine, &s->soc[0], + kernel_start_addr, htif_symbol_callback); if (machine->initrd_filename) { diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c index b81081c70b..797c6084b6 100644 --- a/hw/riscv/virt.c +++ b/hw/riscv/virt.c @@ -1277,7 +1277,8 @@ static void virt_machine_done(Notifier *notifier, void *data) kernel_start_addr = riscv_calc_kernel_start_addr(&s->soc[0], firmware_end_addr); - kernel_entry = riscv_load_kernel(machine, kernel_start_addr, NULL); + kernel_entry = riscv_load_kernel(machine, &s->soc[0], + kernel_start_addr, NULL); if (machine->initrd_filename) { riscv_load_initrd(machine, kernel_entry); diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h index 511390f60e..6295316afb 100644 --- a/include/hw/riscv/boot.h +++ b/include/hw/riscv/boot.h @@ -44,6 +44,7 @@ target_ulong riscv_load_firmware(const char *firmware_filename, hwaddr firmware_load_addr, symbol_fn_t sym_cb); target_ulong riscv_load_kernel(MachineState *machine, + RISCVHartArrayState *harts, target_ulong firmware_end_addr, symbol_fn_t sym_cb); void riscv_load_initrd(MachineState *machine, uint64_t kernel_entry);