On Sun, Sep 20, 2026 at 10:39:39PM +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 <[email protected]>
> Signed-off-by: Tianze Wu <[email protected]>
> Signed-off-by: Shunchao Hu <[email protected]>
> ---
> target/riscv/cpu_bits.h | 8 ++++++++
> target/riscv/tcg/csr.c | 43 +++++++++++++++++++++++++++++++++++++++++--
> 2 files changed, 49 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..ef08600ca7 100644
> --- a/target/riscv/tcg/csr.c
> +++ b/target/riscv/tcg/csr.c
> @@ -5050,17 +5050,56 @@ 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);
> +
> + /*
> + * Unlike satp, an unsupported hgatp.MODE does not discard the whole
> + * write. Keep the old MODE while accepting the other WARL fields.
> + */
> + if (!validate_vm(env, mode)) {
> + hgatp = set_field(hgatp, mode_mask,
> + get_field(old_hgatp, mode_mask));
> + }
Looks good to me. One thing I want to note is that the RISC-V H Extension Spec
does not actually define how an unsupported hgatp.MODE write should be handled.
It only says that the write must not be ignored ad it is for satp, and similar
WARL, see:
A write to hgatp with an unsupported MODE value is not ignored as it is for
satp.
Instead, the fields of hgatp are WARL in the normal way, when so indicated
[1].
So keeping the old MODE is a valid choice, but not the only one. Could we make
the comment a bit more neutral?
[1]:
https://docs.riscv.org/reference/isa/v20260120/priv/hypervisor.html#norm:hgatp-mode_warl
Thanks,
Chao
> +
> + 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 <[email protected]>
>
