qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Jones <ajones@ventanamicro.com>
To: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Cc: qemu-devel@nongnu.org, qemu-riscv@nongnu.org,
	alistair.francis@wdc.com,  bmeng@tinylab.org,
	liweiwei@iscas.ac.cn, zhiwei_liu@linux.alibaba.com,
	 palmer@rivosinc.com
Subject: Re: [PATCH v8 04/19] target/riscv/cpu.c: set satp_mode_max MBARE during satp_finalize()
Date: Thu, 2 Nov 2023 10:32:22 +0100	[thread overview]
Message-ID: <20231102-30bfbfa066e5b09632fb068e@orel> (raw)
In-Reply-To: <20231101204204.345470-5-dbarboza@ventanamicro.com>

On Wed, Nov 01, 2023 at 05:41:49PM -0300, Daniel Henrique Barboza wrote:
> KVM CPUs can handle "cpu->cfg.satp_mode.supported == 0" because KVM will
> make it do internally, not requiring the current SATP support from TCG.
> 
> But other TCG CPUs doesn't deal well with it. We'll assert out before
> OpenSBI if the CPU doesn't set a default:
> 
> ERROR:../target/riscv/cpu.c:317:satp_mode_max_from_map: assertion failed: (map > 0)
> Bail out! ERROR:../target/riscv/cpu.c:317:satp_mode_max_from_map: assertion failed: (map > 0)
> 
> This will be thrown by target/riscv/csr.c, write_satp(), when stepping
> in validate_vm().
> 
> There's no current CPUs affected by it, but next patch will add a new
> CPU that doesn't have defaults and this assert will be hit.
> 
> Change riscv_cpu_satp_mode_finalize() to set satp_mode_max_supported()
> to MBARE if the CPU happens to not have a max mode set yet.
> 
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
>  target/riscv/cpu.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 9f6837ecb7..f7c1989d14 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -942,9 +942,19 @@ static void riscv_cpu_satp_mode_finalize(RISCVCPU *cpu, Error **errp)
>      bool rv32 = riscv_cpu_mxl(&cpu->env) == MXL_RV32;
>      uint8_t satp_mode_map_max, satp_mode_supported_max;
>  
> -    /* The CPU wants the OS to decide which satp mode to use */
>      if (cpu->cfg.satp_mode.supported == 0) {
> -        return;
> +        if (kvm_enabled()) {
> +            /* The CPU wants the OS to decide which satp mode to use */
> +            return;
> +        }
> +
> +        /*
> +         * We do not handle cpu->cfg.satp_mode.supported == 0
> +         * with TCG yet. Set to MBARE.
> +         */
> +        if (tcg_enabled()) {
> +            set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
> +        }
>      }
>  
>      satp_mode_supported_max =
> -- 
> 2.41.0
>

This patch should no longer be necessary if the suggestion I made in the
previous patch works the way I think it should. But, regarding this patch,
I do like that the "The CPU wants the OS to decide which satp mode to use"
comment became specific to KVM, which makes more sense to me. Maybe we
should just change that comment to something like

/*
 * With some accelerators, such as KVM, the OS dictates which satp mode to
 * use. For those cases, satp_mode.supported is zero and there's nothing
 * to do here.
 */

Thanks,
drew


  reply	other threads:[~2023-11-02  9:33 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-01 20:41 [PATCH v8 00/19] rv64i CPU, RVA22U64 profile support Daniel Henrique Barboza
2023-11-01 20:41 ` [PATCH v8 01/19] target/riscv: create TYPE_RISCV_VENDOR_CPU Daniel Henrique Barboza
2023-11-02  2:32   ` Alistair Francis
2023-11-01 20:41 ` [PATCH v8 02/19] target/riscv/tcg: do not use "!generic" CPU checks Daniel Henrique Barboza
2023-11-02  2:38   ` Alistair Francis
2023-11-01 20:41 ` [PATCH v8 03/19] target/riscv/cpu.c: set satp_max_supported in cpu_riscv_set_satp() Daniel Henrique Barboza
2023-11-02  9:24   ` Andrew Jones
2023-11-02 12:53     ` Daniel Henrique Barboza
2023-11-02 13:11       ` Andrew Jones
2023-11-01 20:41 ` [PATCH v8 04/19] target/riscv/cpu.c: set satp_mode_max MBARE during satp_finalize() Daniel Henrique Barboza
2023-11-02  9:32   ` Andrew Jones [this message]
2023-11-01 20:41 ` [PATCH v8 05/19] target/riscv/tcg: update priv_ver on user_set extensions Daniel Henrique Barboza
2023-11-02  9:47   ` Andrew Jones
2023-11-02 13:42     ` Daniel Henrique Barboza
2023-11-02 13:48       ` Andrew Jones
2023-11-01 20:41 ` [PATCH v8 06/19] target/riscv: add rv64i CPU Daniel Henrique Barboza
2023-11-02  9:59   ` Andrew Jones
2023-11-02 14:23     ` Daniel Henrique Barboza
2023-11-01 20:41 ` [PATCH v8 07/19] target/riscv: add zicbop extension flag Daniel Henrique Barboza
2023-11-01 20:41 ` [PATCH v8 08/19] target/riscv/tcg: add 'zic64b' support Daniel Henrique Barboza
2023-11-01 20:41 ` [PATCH v8 09/19] riscv-qmp-cmds.c: expose named features in cpu_model_expansion Daniel Henrique Barboza
2023-11-01 20:41 ` [PATCH v8 10/19] target/riscv: add rva22u64 profile definition Daniel Henrique Barboza
2023-11-01 20:41 ` [PATCH v8 11/19] target/riscv/kvm: add 'rva22u64' flag as unavailable Daniel Henrique Barboza
2023-11-01 20:41 ` [PATCH v8 12/19] target/riscv/tcg: add user flag for profile support Daniel Henrique Barboza
2023-11-01 20:41 ` [PATCH v8 13/19] target/riscv/tcg: add MISA user options hash Daniel Henrique Barboza
2023-11-01 20:41 ` [PATCH v8 14/19] target/riscv/tcg: add riscv_cpu_write_misa_bit() Daniel Henrique Barboza
2023-11-01 20:42 ` [PATCH v8 15/19] target/riscv/tcg: handle profile MISA bits Daniel Henrique Barboza
2023-11-01 20:42 ` [PATCH v8 16/19] target/riscv/tcg: add hash table insert helpers Daniel Henrique Barboza
2023-11-01 20:42 ` [PATCH v8 17/19] target/riscv/tcg: honor user choice for G MISA bits Daniel Henrique Barboza
2023-11-01 20:42 ` [PATCH v8 18/19] target/riscv/tcg: validate profiles during finalize Daniel Henrique Barboza
2023-11-01 20:42 ` [PATCH v8 19/19] riscv-qmp-cmds.c: add profile flags in cpu-model-expansion Daniel Henrique Barboza

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231102-30bfbfa066e5b09632fb068e@orel \
    --to=ajones@ventanamicro.com \
    --cc=alistair.francis@wdc.com \
    --cc=bmeng@tinylab.org \
    --cc=dbarboza@ventanamicro.com \
    --cc=liweiwei@iscas.ac.cn \
    --cc=palmer@rivosinc.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-riscv@nongnu.org \
    --cc=zhiwei_liu@linux.alibaba.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).