Re: [PATCH v2] target/riscv: add hgatp-specific WARL legalization

[Chao Liu <chao.liu@processmission.com>]

Hi Shunchao,
On Fri, Sep 25, 2026 at 10:07:09PM +0800, Shunchao Hu wrote:
> According to the Hypervisor Extension in RISC-V spec:
> - VMIDMAX is 7 bits for RV32 and 14 bits for RV64
> - the fields of hgatp are WARL in the normal way
> - the lowest two bits of the physical page number
>   (PPN) in hgatp always read as zeros
> 
> These behaviors differ from those of the satp CSR,
> so hgatp cannot reuse legalize_xatp(). Add a dedicated
> legalize_hgatp() helper to implement the hgatp-specific
> WARL semantics.
> 
> Co-authored-by: Tianze Wu <wutianze@ict.ac.cn>
> Signed-off-by: Tianze Wu <wutianze@ict.ac.cn>
> Signed-off-by: Shunchao Hu <hushunchao@bosc.ac.cn>
LGTM.

Reviewed-by: Chao Liu <chao.liu@processmission.com>

Thanks,
Chao

> ---
> Changes in v2:
> - Reword the comment to clarify that retaining the previous MODE is
>   QEMU's WARL legalization choice, not a requirement of the spec.
> - Link to v1: https://lore.kernel.org/qemu-devel/20260920-riscv-hgatp-warl-v1-1-2e52f78c8027@bosc.ac.cn
> 
> To: qemu-devel@nongnu.org
> Cc: Palmer Dabbelt <palmer@dabbelt.com>
> Cc: Alistair Francis <alistair.francis@wdc.com>
> Cc: Weiwei Li <liwei1518@gmail.com>
> Cc: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>
> Cc: Liu Zhiwei <zhiwei_liu@linux.alibaba.com>
> Cc: Chao Liu <chao.liu@processmission.com>
> Cc: qemu-riscv@nongnu.org
> ---
>  target/riscv/cpu_bits.h |  8 ++++++++
>  target/riscv/tcg/csr.c  | 46 ++++++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 52 insertions(+), 2 deletions(-)
> 
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index c01050ce2b..c4a914411b 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -711,6 +711,14 @@ typedef enum {
>  #define SATP64_ASID         0x0FFFF00000000000ULL
>  #define SATP64_PPN          0x00000FFFFFFFFFFFULL
>  
> +/* hgatp CSR field masks */
> +#define HGATP32_MODE        SATP32_MODE
> +#define HGATP32_VMID        0x1FC00000
> +#define HGATP32_PPN         SATP32_PPN
> +#define HGATP64_MODE        SATP64_MODE
> +#define HGATP64_VMID        0x03FFF00000000000ULL
> +#define HGATP64_PPN         SATP64_PPN
> +
>  /* RNMI mnstatus CSR mask */
>  #define MNSTATUS_NMIE       0x00000008
>  #define MNSTATUS_MNPV       0x00000080
> diff --git a/target/riscv/tcg/csr.c b/target/riscv/tcg/csr.c
> index 061bc9db77..fc7bd61073 100644
> --- a/target/riscv/tcg/csr.c
> +++ b/target/riscv/tcg/csr.c
> @@ -5050,17 +5050,59 @@ static RISCVException read_hgeip(CPURISCVState *env, int csrno,
>      return RISCV_EXCP_NONE;
>  }
>  
> +static target_ulong hgatp_mask(CPURISCVState *env)
> +{
> +    target_ulong mask;
> +
> +    if (riscv_cpu_mxl(env) == MXL_RV32) {
> +        mask = HGATP32_MODE | HGATP32_VMID | HGATP32_PPN;
> +    } else {
> +        mask = HGATP64_MODE | HGATP64_VMID | HGATP64_PPN;
> +    }
> +
> +    /* G-stage x4 root page tables are always 16 KiB aligned. */
> +    return mask & ~(target_ulong)3;
> +}
> +
> +static target_ulong legalize_hgatp(CPURISCVState *env,
> +                                   target_ulong old_hgatp,
> +                                   target_ulong val)
> +{
> +    target_ulong mode_mask = riscv_cpu_mxl(env) == MXL_RV32 ?
> +                             HGATP32_MODE : HGATP64_MODE;
> +    target_ulong hgatp = val & hgatp_mask(env);
> +    target_ulong mode = get_field(hgatp, mode_mask);
> +
> +    /*
> +     * hgatp.MODE is a WARL field, so an unsupported value must be
> +     * legalized rather than causing the entire write to be ignored.
> +     * The specification does not require a particular legal value,
> +     * QEMU here chooses to keep the previous mode while accepting
> +     * writes to the remaining WARL fields.
> +     */
> +    if (!validate_vm(env, mode)) {
> +        hgatp = set_field(hgatp, mode_mask,
> +                          get_field(old_hgatp, mode_mask));
> +    }
> +
> +    if (hgatp != old_hgatp) {
> +        tlb_flush(env_cpu(env));
> +    }
> +
> +    return hgatp;
> +}
> +
>  static RISCVException read_hgatp(CPURISCVState *env, int csrno,
>                                   target_ulong *val)
>  {
> -    *val = env->hgatp;
> +    *val = env->hgatp & hgatp_mask(env);
>      return RISCV_EXCP_NONE;
>  }
>  
>  static RISCVException write_hgatp(CPURISCVState *env, int csrno,
>                                    target_ulong val, uintptr_t ra)
>  {
> -    env->hgatp = legalize_xatp(env, env->hgatp, val);
> +    env->hgatp = legalize_hgatp(env, env->hgatp, val);
>      return RISCV_EXCP_NONE;
>  }
>  
> 
> ---
> base-commit: c1c18d1e640b64292859ce9f30f3c344edfb0294
> change-id: 20260920-riscv-hgatp-warl-1e48fd321188
> 
> Best regards,
> --  
> Shunchao Hu <hushunchao@bosc.ac.cn>
>