diff mbox series

[v13,6/7] target/riscv: Apply pointer masking for virtualized memory accesses

Message ID 20241216121907.660504-7-baturo.alexey@gmail.com (mailing list archive)
State New
Headers show
Series Pointer Masking update for Zjpm v1.0 | expand

Commit Message

Alexey Baturo Dec. 16, 2024, 12:19 p.m. UTC 
From: Alexey Baturo <baturo.alexey@gmail.com>

Signed-off-by: Alexey Baturo <baturo.alexey@gmail.com>
---
 target/riscv/cpu.h           |  1 +
 target/riscv/cpu_helper.c    | 18 +++++++++++++++
 target/riscv/internals.h     | 44 ++++++++++++++++++++++++++++++++++++
 target/riscv/op_helper.c     | 16 ++++++-------
 target/riscv/vector_helper.c | 21 -----------------
 5 files changed, 71 insertions(+), 29 deletions(-)

Comments

Daniel Henrique Barboza Dec. 16, 2024, 1:54 p.m. UTC | #1 
On 12/16/24 9:19 AM, baturo.alexey@gmail.com wrote:
> From: Alexey Baturo <baturo.alexey@gmail.com>
> 
> Signed-off-by: Alexey Baturo <baturo.alexey@gmail.com>
> ---

Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

>   target/riscv/cpu.h           |  1 +
>   target/riscv/cpu_helper.c    | 18 +++++++++++++++
>   target/riscv/internals.h     | 44 ++++++++++++++++++++++++++++++++++++
>   target/riscv/op_helper.c     | 16 ++++++-------
>   target/riscv/vector_helper.c | 21 -----------------
>   5 files changed, 71 insertions(+), 29 deletions(-)
> 
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 38231fe21e..536ad20fdd 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -773,6 +773,7 @@ bool riscv_cpu_is_32bit(RISCVCPU *cpu);
>   
>   bool riscv_cpu_virt_mem_enabled(CPURISCVState *env);
>   RISCVPmPmm riscv_pm_get_pmm(CPURISCVState *env);
> +RISCVPmPmm riscv_pm_get_virt_pmm(CPURISCVState *env);
>   uint32_t riscv_pm_get_pmlen(RISCVPmPmm pmm);
>   
>   RISCVException riscv_csrr(CPURISCVState *env, int csrno,
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 480d2c2c8b..471d8d40a1 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -260,6 +260,24 @@ RISCVPmPmm riscv_pm_get_pmm(CPURISCVState *env)
>       return pmm;
>   }
>   
> +RISCVPmPmm riscv_pm_get_virt_pmm(CPURISCVState *env)
> +{
> +    RISCVPmPmm pmm = PMM_FIELD_DISABLED;
> +#ifndef CONFIG_USER_ONLY
> +    int priv_mode = cpu_address_mode(env);
> +    if (priv_mode == PRV_U) {
> +        pmm = get_field(env->hstatus, HSTATUS_HUPMM);
> +    } else {
> +        if (get_field(env->hstatus, HSTATUS_SPVP)) {
> +            pmm = get_field(env->henvcfg, HENVCFG_PMM);
> +        } else {
> +            pmm = get_field(env->senvcfg, SENVCFG_PMM);
> +        }
> +    }
> +#endif
> +    return pmm;
> +}
> +
>   bool riscv_cpu_virt_mem_enabled(CPURISCVState *env)
>   {
>       bool virt_mem_en = false;
> diff --git a/target/riscv/internals.h b/target/riscv/internals.h
> index ddbdee885b..017f33af1f 100644
> --- a/target/riscv/internals.h
> +++ b/target/riscv/internals.h
> @@ -142,4 +142,48 @@ static inline float16 check_nanbox_h(CPURISCVState *env, uint64_t f)
>   /* Our implementation of CPUClass::has_work */
>   bool riscv_cpu_has_work(CPUState *cs);
>   
> +/* Zjpm addr masking routine */
> +static inline target_ulong adjust_addr_body(CPURISCVState *env,
> +                                            target_ulong addr,
> +                                            bool is_virt)
> +{
> +    if (riscv_cpu_mxl(env) == MXL_RV32) {
> +        return addr;
> +    }
> +    RISCVPmPmm pmm = PMM_FIELD_DISABLED;
> +    if (is_virt) {
> +        pmm = riscv_pm_get_virt_pmm(env);
> +    } else {
> +        pmm = riscv_pm_get_pmm(env);
> +    }
> +    if (pmm == PMM_FIELD_DISABLED) {
> +        return addr;
> +    }
> +    uint32_t pmlen = riscv_pm_get_pmlen(pmm);
> +    bool signext = false;
> +    if (!is_virt) {
> +        signext = riscv_cpu_virt_mem_enabled(env);
> +    }
> +    addr = addr << pmlen;
> +    /* sign/zero extend masked address by N-1 bit */
> +    if (signext) {
> +        addr = (target_long)addr >> pmlen;
> +    } else {
> +        addr = addr >> pmlen;
> +    }
> +    return addr;
> +}
> +
> +static inline target_ulong adjust_addr(CPURISCVState *env,
> +                                       target_ulong addr)
> +{
> +    return adjust_addr_body(env, addr, false);
> +}
> +
> +static inline target_ulong adjust_addr_virt(CPURISCVState *env,
> +                                            target_ulong addr)
> +{
> +    return adjust_addr_body(env, addr, true);
> +}
> +
>   #endif
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index eddedacf4b..20e5bd5088 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -472,7 +472,7 @@ target_ulong helper_hyp_hlv_bu(CPURISCVState *env, target_ulong addr)
>       int mmu_idx = check_access_hlsv(env, false, ra);
>       MemOpIdx oi = make_memop_idx(MO_UB, mmu_idx);
>   
> -    return cpu_ldb_mmu(env, addr, oi, ra);
> +    return cpu_ldb_mmu(env, adjust_addr_virt(env, addr), oi, ra);
>   }
>   
>   target_ulong helper_hyp_hlv_hu(CPURISCVState *env, target_ulong addr)
> @@ -481,7 +481,7 @@ target_ulong helper_hyp_hlv_hu(CPURISCVState *env, target_ulong addr)
>       int mmu_idx = check_access_hlsv(env, false, ra);
>       MemOpIdx oi = make_memop_idx(MO_TEUW, mmu_idx);
>   
> -    return cpu_ldw_mmu(env, addr, oi, ra);
> +    return cpu_ldw_mmu(env, adjust_addr_virt(env, addr), oi, ra);
>   }
>   
>   target_ulong helper_hyp_hlv_wu(CPURISCVState *env, target_ulong addr)
> @@ -490,7 +490,7 @@ target_ulong helper_hyp_hlv_wu(CPURISCVState *env, target_ulong addr)
>       int mmu_idx = check_access_hlsv(env, false, ra);
>       MemOpIdx oi = make_memop_idx(MO_TEUL, mmu_idx);
>   
> -    return cpu_ldl_mmu(env, addr, oi, ra);
> +    return cpu_ldl_mmu(env, adjust_addr_virt(env, addr), oi, ra);
>   }
>   
>   target_ulong helper_hyp_hlv_d(CPURISCVState *env, target_ulong addr)
> @@ -499,7 +499,7 @@ target_ulong helper_hyp_hlv_d(CPURISCVState *env, target_ulong addr)
>       int mmu_idx = check_access_hlsv(env, false, ra);
>       MemOpIdx oi = make_memop_idx(MO_TEUQ, mmu_idx);
>   
> -    return cpu_ldq_mmu(env, addr, oi, ra);
> +    return cpu_ldq_mmu(env, adjust_addr_virt(env, addr), oi, ra);
>   }
>   
>   void helper_hyp_hsv_b(CPURISCVState *env, target_ulong addr, target_ulong val)
> @@ -508,7 +508,7 @@ void helper_hyp_hsv_b(CPURISCVState *env, target_ulong addr, target_ulong val)
>       int mmu_idx = check_access_hlsv(env, false, ra);
>       MemOpIdx oi = make_memop_idx(MO_UB, mmu_idx);
>   
> -    cpu_stb_mmu(env, addr, val, oi, ra);
> +    cpu_stb_mmu(env, adjust_addr_virt(env, addr), val, oi, ra);
>   }
>   
>   void helper_hyp_hsv_h(CPURISCVState *env, target_ulong addr, target_ulong val)
> @@ -517,7 +517,7 @@ void helper_hyp_hsv_h(CPURISCVState *env, target_ulong addr, target_ulong val)
>       int mmu_idx = check_access_hlsv(env, false, ra);
>       MemOpIdx oi = make_memop_idx(MO_TEUW, mmu_idx);
>   
> -    cpu_stw_mmu(env, addr, val, oi, ra);
> +    cpu_stw_mmu(env, adjust_addr_virt(env, addr), val, oi, ra);
>   }
>   
>   void helper_hyp_hsv_w(CPURISCVState *env, target_ulong addr, target_ulong val)
> @@ -526,7 +526,7 @@ void helper_hyp_hsv_w(CPURISCVState *env, target_ulong addr, target_ulong val)
>       int mmu_idx = check_access_hlsv(env, false, ra);
>       MemOpIdx oi = make_memop_idx(MO_TEUL, mmu_idx);
>   
> -    cpu_stl_mmu(env, addr, val, oi, ra);
> +    cpu_stl_mmu(env, adjust_addr_virt(env, addr), val, oi, ra);
>   }
>   
>   void helper_hyp_hsv_d(CPURISCVState *env, target_ulong addr, target_ulong val)
> @@ -535,7 +535,7 @@ void helper_hyp_hsv_d(CPURISCVState *env, target_ulong addr, target_ulong val)
>       int mmu_idx = check_access_hlsv(env, false, ra);
>       MemOpIdx oi = make_memop_idx(MO_TEUQ, mmu_idx);
>   
> -    cpu_stq_mmu(env, addr, val, oi, ra);
> +    cpu_stq_mmu(env, adjust_addr_virt(env, addr), val, oi, ra);
>   }
>   
>   /*
> diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
> index 37c6c198a5..a0093bcc9c 100644
> --- a/target/riscv/vector_helper.c
> +++ b/target/riscv/vector_helper.c
> @@ -105,27 +105,6 @@ static inline uint32_t vext_max_elems(uint32_t desc, uint32_t log2_esz)
>       return scale < 0 ? vlenb >> -scale : vlenb << scale;
>   }
>   
> -static inline target_ulong adjust_addr(CPURISCVState *env, target_ulong addr)
> -{
> -    if (riscv_cpu_mxl(env) == MXL_RV32) {
> -        return addr;
> -    }
> -    RISCVPmPmm pmm = riscv_pm_get_pmm(env);
> -    if (pmm == PMM_FIELD_DISABLED) {
> -        return addr;
> -    }
> -    int pmlen = riscv_pm_get_pmlen(pmm);
> -    bool signext = riscv_cpu_virt_mem_enabled(env);
> -    addr = addr << pmlen;
> -    /* sign/zero extend masked address by N-1 bit */
> -    if (signext) {
> -        addr = (target_long)addr >> pmlen;
> -    } else {
> -        addr = addr >> pmlen;
> -    }
> -    return addr;
> -}
> -
>   /*
>    * This function checks watchpoint before real load operation.
>    *
Alistair Francis Dec. 17, 2024, 6:13 a.m. UTC | #2 
On Mon, Dec 16, 2024 at 10:19 PM <baturo.alexey@gmail.com> wrote:
>
> From: Alexey Baturo <baturo.alexey@gmail.com>
>
> Signed-off-by: Alexey Baturo <baturo.alexey@gmail.com>
> ---
>  target/riscv/cpu.h           |  1 +
>  target/riscv/cpu_helper.c    | 18 +++++++++++++++
>  target/riscv/internals.h     | 44 ++++++++++++++++++++++++++++++++++++
>  target/riscv/op_helper.c     | 16 ++++++-------
>  target/riscv/vector_helper.c | 21 -----------------
>  5 files changed, 71 insertions(+), 29 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 38231fe21e..536ad20fdd 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -773,6 +773,7 @@ bool riscv_cpu_is_32bit(RISCVCPU *cpu);
>
>  bool riscv_cpu_virt_mem_enabled(CPURISCVState *env);
>  RISCVPmPmm riscv_pm_get_pmm(CPURISCVState *env);
> +RISCVPmPmm riscv_pm_get_virt_pmm(CPURISCVState *env);
>  uint32_t riscv_pm_get_pmlen(RISCVPmPmm pmm);
>
>  RISCVException riscv_csrr(CPURISCVState *env, int csrno,
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 480d2c2c8b..471d8d40a1 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -260,6 +260,24 @@ RISCVPmPmm riscv_pm_get_pmm(CPURISCVState *env)
>      return pmm;
>  }
>
> +RISCVPmPmm riscv_pm_get_virt_pmm(CPURISCVState *env)
> +{
> +    RISCVPmPmm pmm = PMM_FIELD_DISABLED;
> +#ifndef CONFIG_USER_ONLY
> +    int priv_mode = cpu_address_mode(env);
> +    if (priv_mode == PRV_U) {
> +        pmm = get_field(env->hstatus, HSTATUS_HUPMM);
> +    } else {
> +        if (get_field(env->hstatus, HSTATUS_SPVP)) {
> +            pmm = get_field(env->henvcfg, HENVCFG_PMM);
> +        } else {
> +            pmm = get_field(env->senvcfg, SENVCFG_PMM);
> +        }
> +    }
> +#endif
> +    return pmm;
> +}
> +
>  bool riscv_cpu_virt_mem_enabled(CPURISCVState *env)
>  {
>      bool virt_mem_en = false;
> diff --git a/target/riscv/internals.h b/target/riscv/internals.h
> index ddbdee885b..017f33af1f 100644
> --- a/target/riscv/internals.h
> +++ b/target/riscv/internals.h
> @@ -142,4 +142,48 @@ static inline float16 check_nanbox_h(CPURISCVState *env, uint64_t f)
>  /* Our implementation of CPUClass::has_work */
>  bool riscv_cpu_has_work(CPUState *cs);
>
> +/* Zjpm addr masking routine */
> +static inline target_ulong adjust_addr_body(CPURISCVState *env,
> +                                            target_ulong addr,
> +                                            bool is_virt)

Maybe is_virt_addr to be clear it's an address and not hypervisor virtulisation.

> +{
> +    if (riscv_cpu_mxl(env) == MXL_RV32) {
> +        return addr;
> +    }
> +    RISCVPmPmm pmm = PMM_FIELD_DISABLED;

Same comment about mixed code and declarations and some newlines would
be great :)

> +    if (is_virt) {
> +        pmm = riscv_pm_get_virt_pmm(env);
> +    } else {
> +        pmm = riscv_pm_get_pmm(env);
> +    }
> +    if (pmm == PMM_FIELD_DISABLED) {
> +        return addr;
> +    }
> +    uint32_t pmlen = riscv_pm_get_pmlen(pmm);
> +    bool signext = false;
> +    if (!is_virt) {
> +        signext = riscv_cpu_virt_mem_enabled(env);
> +    }
> +    addr = addr << pmlen;
> +    /* sign/zero extend masked address by N-1 bit */
> +    if (signext) {
> +        addr = (target_long)addr >> pmlen;
> +    } else {
> +        addr = addr >> pmlen;
> +    }
> +    return addr;
> +}
> +
> +static inline target_ulong adjust_addr(CPURISCVState *env,
> +                                       target_ulong addr)
> +{
> +    return adjust_addr_body(env, addr, false);
> +}
> +
> +static inline target_ulong adjust_addr_virt(CPURISCVState *env,
> +                                            target_ulong addr)
> +{
> +    return adjust_addr_body(env, addr, true);
> +}

Otherwise looks good

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> +
>  #endif
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index eddedacf4b..20e5bd5088 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -472,7 +472,7 @@ target_ulong helper_hyp_hlv_bu(CPURISCVState *env, target_ulong addr)
>      int mmu_idx = check_access_hlsv(env, false, ra);
>      MemOpIdx oi = make_memop_idx(MO_UB, mmu_idx);
>
> -    return cpu_ldb_mmu(env, addr, oi, ra);
> +    return cpu_ldb_mmu(env, adjust_addr_virt(env, addr), oi, ra);
>  }
>
>  target_ulong helper_hyp_hlv_hu(CPURISCVState *env, target_ulong addr)
> @@ -481,7 +481,7 @@ target_ulong helper_hyp_hlv_hu(CPURISCVState *env, target_ulong addr)
>      int mmu_idx = check_access_hlsv(env, false, ra);
>      MemOpIdx oi = make_memop_idx(MO_TEUW, mmu_idx);
>
> -    return cpu_ldw_mmu(env, addr, oi, ra);
> +    return cpu_ldw_mmu(env, adjust_addr_virt(env, addr), oi, ra);
>  }
>
>  target_ulong helper_hyp_hlv_wu(CPURISCVState *env, target_ulong addr)
> @@ -490,7 +490,7 @@ target_ulong helper_hyp_hlv_wu(CPURISCVState *env, target_ulong addr)
>      int mmu_idx = check_access_hlsv(env, false, ra);
>      MemOpIdx oi = make_memop_idx(MO_TEUL, mmu_idx);
>
> -    return cpu_ldl_mmu(env, addr, oi, ra);
> +    return cpu_ldl_mmu(env, adjust_addr_virt(env, addr), oi, ra);
>  }
>
>  target_ulong helper_hyp_hlv_d(CPURISCVState *env, target_ulong addr)
> @@ -499,7 +499,7 @@ target_ulong helper_hyp_hlv_d(CPURISCVState *env, target_ulong addr)
>      int mmu_idx = check_access_hlsv(env, false, ra);
>      MemOpIdx oi = make_memop_idx(MO_TEUQ, mmu_idx);
>
> -    return cpu_ldq_mmu(env, addr, oi, ra);
> +    return cpu_ldq_mmu(env, adjust_addr_virt(env, addr), oi, ra);
>  }
>
>  void helper_hyp_hsv_b(CPURISCVState *env, target_ulong addr, target_ulong val)
> @@ -508,7 +508,7 @@ void helper_hyp_hsv_b(CPURISCVState *env, target_ulong addr, target_ulong val)
>      int mmu_idx = check_access_hlsv(env, false, ra);
>      MemOpIdx oi = make_memop_idx(MO_UB, mmu_idx);
>
> -    cpu_stb_mmu(env, addr, val, oi, ra);
> +    cpu_stb_mmu(env, adjust_addr_virt(env, addr), val, oi, ra);
>  }
>
>  void helper_hyp_hsv_h(CPURISCVState *env, target_ulong addr, target_ulong val)
> @@ -517,7 +517,7 @@ void helper_hyp_hsv_h(CPURISCVState *env, target_ulong addr, target_ulong val)
>      int mmu_idx = check_access_hlsv(env, false, ra);
>      MemOpIdx oi = make_memop_idx(MO_TEUW, mmu_idx);
>
> -    cpu_stw_mmu(env, addr, val, oi, ra);
> +    cpu_stw_mmu(env, adjust_addr_virt(env, addr), val, oi, ra);
>  }
>
>  void helper_hyp_hsv_w(CPURISCVState *env, target_ulong addr, target_ulong val)
> @@ -526,7 +526,7 @@ void helper_hyp_hsv_w(CPURISCVState *env, target_ulong addr, target_ulong val)
>      int mmu_idx = check_access_hlsv(env, false, ra);
>      MemOpIdx oi = make_memop_idx(MO_TEUL, mmu_idx);
>
> -    cpu_stl_mmu(env, addr, val, oi, ra);
> +    cpu_stl_mmu(env, adjust_addr_virt(env, addr), val, oi, ra);
>  }
>
>  void helper_hyp_hsv_d(CPURISCVState *env, target_ulong addr, target_ulong val)
> @@ -535,7 +535,7 @@ void helper_hyp_hsv_d(CPURISCVState *env, target_ulong addr, target_ulong val)
>      int mmu_idx = check_access_hlsv(env, false, ra);
>      MemOpIdx oi = make_memop_idx(MO_TEUQ, mmu_idx);
>
> -    cpu_stq_mmu(env, addr, val, oi, ra);
> +    cpu_stq_mmu(env, adjust_addr_virt(env, addr), val, oi, ra);
>  }
>
>  /*
> diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
> index 37c6c198a5..a0093bcc9c 100644
> --- a/target/riscv/vector_helper.c
> +++ b/target/riscv/vector_helper.c
> @@ -105,27 +105,6 @@ static inline uint32_t vext_max_elems(uint32_t desc, uint32_t log2_esz)
>      return scale < 0 ? vlenb >> -scale : vlenb << scale;
>  }
>
> -static inline target_ulong adjust_addr(CPURISCVState *env, target_ulong addr)
> -{
> -    if (riscv_cpu_mxl(env) == MXL_RV32) {
> -        return addr;
> -    }
> -    RISCVPmPmm pmm = riscv_pm_get_pmm(env);
> -    if (pmm == PMM_FIELD_DISABLED) {
> -        return addr;
> -    }
> -    int pmlen = riscv_pm_get_pmlen(pmm);
> -    bool signext = riscv_cpu_virt_mem_enabled(env);
> -    addr = addr << pmlen;
> -    /* sign/zero extend masked address by N-1 bit */
> -    if (signext) {
> -        addr = (target_long)addr >> pmlen;
> -    } else {
> -        addr = addr >> pmlen;
> -    }
> -    return addr;
> -}
> -
>  /*
>   * This function checks watchpoint before real load operation.
>   *
> --
> 2.39.5
>
Alexey Baturo Dec. 17, 2024, 9 a.m. UTC | #3 
Hi Alistair,

Thanks for the review. I've tried to address your comments and submitted a
new version.
Could you please take a look if it's ok with you and if so, could you
please put these patches to the queue for the next qemu update?

Thanks!

вт, 17 дек. 2024 г. в 09:13, Alistair Francis <alistair23@gmail.com>:

> On Mon, Dec 16, 2024 at 10:19 PM <baturo.alexey@gmail.com> wrote:
> >
> > From: Alexey Baturo <baturo.alexey@gmail.com>
> >
> > Signed-off-by: Alexey Baturo <baturo.alexey@gmail.com>
> > ---
> >  target/riscv/cpu.h           |  1 +
> >  target/riscv/cpu_helper.c    | 18 +++++++++++++++
> >  target/riscv/internals.h     | 44 ++++++++++++++++++++++++++++++++++++
> >  target/riscv/op_helper.c     | 16 ++++++-------
> >  target/riscv/vector_helper.c | 21 -----------------
> >  5 files changed, 71 insertions(+), 29 deletions(-)
> >
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index 38231fe21e..536ad20fdd 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -773,6 +773,7 @@ bool riscv_cpu_is_32bit(RISCVCPU *cpu);
> >
> >  bool riscv_cpu_virt_mem_enabled(CPURISCVState *env);
> >  RISCVPmPmm riscv_pm_get_pmm(CPURISCVState *env);
> > +RISCVPmPmm riscv_pm_get_virt_pmm(CPURISCVState *env);
> >  uint32_t riscv_pm_get_pmlen(RISCVPmPmm pmm);
> >
> >  RISCVException riscv_csrr(CPURISCVState *env, int csrno,
> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > index 480d2c2c8b..471d8d40a1 100644
> > --- a/target/riscv/cpu_helper.c
> > +++ b/target/riscv/cpu_helper.c
> > @@ -260,6 +260,24 @@ RISCVPmPmm riscv_pm_get_pmm(CPURISCVState *env)
> >      return pmm;
> >  }
> >
> > +RISCVPmPmm riscv_pm_get_virt_pmm(CPURISCVState *env)
> > +{
> > +    RISCVPmPmm pmm = PMM_FIELD_DISABLED;
> > +#ifndef CONFIG_USER_ONLY
> > +    int priv_mode = cpu_address_mode(env);
> > +    if (priv_mode == PRV_U) {
> > +        pmm = get_field(env->hstatus, HSTATUS_HUPMM);
> > +    } else {
> > +        if (get_field(env->hstatus, HSTATUS_SPVP)) {
> > +            pmm = get_field(env->henvcfg, HENVCFG_PMM);
> > +        } else {
> > +            pmm = get_field(env->senvcfg, SENVCFG_PMM);
> > +        }
> > +    }
> > +#endif
> > +    return pmm;
> > +}
> > +
> >  bool riscv_cpu_virt_mem_enabled(CPURISCVState *env)
> >  {
> >      bool virt_mem_en = false;
> > diff --git a/target/riscv/internals.h b/target/riscv/internals.h
> > index ddbdee885b..017f33af1f 100644
> > --- a/target/riscv/internals.h
> > +++ b/target/riscv/internals.h
> > @@ -142,4 +142,48 @@ static inline float16 check_nanbox_h(CPURISCVState
> *env, uint64_t f)
> >  /* Our implementation of CPUClass::has_work */
> >  bool riscv_cpu_has_work(CPUState *cs);
> >
> > +/* Zjpm addr masking routine */
> > +static inline target_ulong adjust_addr_body(CPURISCVState *env,
> > +                                            target_ulong addr,
> > +                                            bool is_virt)
>
> Maybe is_virt_addr to be clear it's an address and not hypervisor
> virtulisation.
>
> > +{
> > +    if (riscv_cpu_mxl(env) == MXL_RV32) {
> > +        return addr;
> > +    }
> > +    RISCVPmPmm pmm = PMM_FIELD_DISABLED;
>
> Same comment about mixed code and declarations and some newlines would
> be great :)
>
> > +    if (is_virt) {
> > +        pmm = riscv_pm_get_virt_pmm(env);
> > +    } else {
> > +        pmm = riscv_pm_get_pmm(env);
> > +    }
> > +    if (pmm == PMM_FIELD_DISABLED) {
> > +        return addr;
> > +    }
> > +    uint32_t pmlen = riscv_pm_get_pmlen(pmm);
> > +    bool signext = false;
> > +    if (!is_virt) {
> > +        signext = riscv_cpu_virt_mem_enabled(env);
> > +    }
> > +    addr = addr << pmlen;
> > +    /* sign/zero extend masked address by N-1 bit */
> > +    if (signext) {
> > +        addr = (target_long)addr >> pmlen;
> > +    } else {
> > +        addr = addr >> pmlen;
> > +    }
> > +    return addr;
> > +}
> > +
> > +static inline target_ulong adjust_addr(CPURISCVState *env,
> > +                                       target_ulong addr)
> > +{
> > +    return adjust_addr_body(env, addr, false);
> > +}
> > +
> > +static inline target_ulong adjust_addr_virt(CPURISCVState *env,
> > +                                            target_ulong addr)
> > +{
> > +    return adjust_addr_body(env, addr, true);
> > +}
>
> Otherwise looks good
>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>
> Alistair
>
> > +
> >  #endif
> > diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> > index eddedacf4b..20e5bd5088 100644
> > --- a/target/riscv/op_helper.c
> > +++ b/target/riscv/op_helper.c
> > @@ -472,7 +472,7 @@ target_ulong helper_hyp_hlv_bu(CPURISCVState *env,
> target_ulong addr)
> >      int mmu_idx = check_access_hlsv(env, false, ra);
> >      MemOpIdx oi = make_memop_idx(MO_UB, mmu_idx);
> >
> > -    return cpu_ldb_mmu(env, addr, oi, ra);
> > +    return cpu_ldb_mmu(env, adjust_addr_virt(env, addr), oi, ra);
> >  }
> >
> >  target_ulong helper_hyp_hlv_hu(CPURISCVState *env, target_ulong addr)
> > @@ -481,7 +481,7 @@ target_ulong helper_hyp_hlv_hu(CPURISCVState *env,
> target_ulong addr)
> >      int mmu_idx = check_access_hlsv(env, false, ra);
> >      MemOpIdx oi = make_memop_idx(MO_TEUW, mmu_idx);
> >
> > -    return cpu_ldw_mmu(env, addr, oi, ra);
> > +    return cpu_ldw_mmu(env, adjust_addr_virt(env, addr), oi, ra);
> >  }
> >
> >  target_ulong helper_hyp_hlv_wu(CPURISCVState *env, target_ulong addr)
> > @@ -490,7 +490,7 @@ target_ulong helper_hyp_hlv_wu(CPURISCVState *env,
> target_ulong addr)
> >      int mmu_idx = check_access_hlsv(env, false, ra);
> >      MemOpIdx oi = make_memop_idx(MO_TEUL, mmu_idx);
> >
> > -    return cpu_ldl_mmu(env, addr, oi, ra);
> > +    return cpu_ldl_mmu(env, adjust_addr_virt(env, addr), oi, ra);
> >  }
> >
> >  target_ulong helper_hyp_hlv_d(CPURISCVState *env, target_ulong addr)
> > @@ -499,7 +499,7 @@ target_ulong helper_hyp_hlv_d(CPURISCVState *env,
> target_ulong addr)
> >      int mmu_idx = check_access_hlsv(env, false, ra);
> >      MemOpIdx oi = make_memop_idx(MO_TEUQ, mmu_idx);
> >
> > -    return cpu_ldq_mmu(env, addr, oi, ra);
> > +    return cpu_ldq_mmu(env, adjust_addr_virt(env, addr), oi, ra);
> >  }
> >
> >  void helper_hyp_hsv_b(CPURISCVState *env, target_ulong addr,
> target_ulong val)
> > @@ -508,7 +508,7 @@ void helper_hyp_hsv_b(CPURISCVState *env,
> target_ulong addr, target_ulong val)
> >      int mmu_idx = check_access_hlsv(env, false, ra);
> >      MemOpIdx oi = make_memop_idx(MO_UB, mmu_idx);
> >
> > -    cpu_stb_mmu(env, addr, val, oi, ra);
> > +    cpu_stb_mmu(env, adjust_addr_virt(env, addr), val, oi, ra);
> >  }
> >
> >  void helper_hyp_hsv_h(CPURISCVState *env, target_ulong addr,
> target_ulong val)
> > @@ -517,7 +517,7 @@ void helper_hyp_hsv_h(CPURISCVState *env,
> target_ulong addr, target_ulong val)
> >      int mmu_idx = check_access_hlsv(env, false, ra);
> >      MemOpIdx oi = make_memop_idx(MO_TEUW, mmu_idx);
> >
> > -    cpu_stw_mmu(env, addr, val, oi, ra);
> > +    cpu_stw_mmu(env, adjust_addr_virt(env, addr), val, oi, ra);
> >  }
> >
> >  void helper_hyp_hsv_w(CPURISCVState *env, target_ulong addr,
> target_ulong val)
> > @@ -526,7 +526,7 @@ void helper_hyp_hsv_w(CPURISCVState *env,
> target_ulong addr, target_ulong val)
> >      int mmu_idx = check_access_hlsv(env, false, ra);
> >      MemOpIdx oi = make_memop_idx(MO_TEUL, mmu_idx);
> >
> > -    cpu_stl_mmu(env, addr, val, oi, ra);
> > +    cpu_stl_mmu(env, adjust_addr_virt(env, addr), val, oi, ra);
> >  }
> >
> >  void helper_hyp_hsv_d(CPURISCVState *env, target_ulong addr,
> target_ulong val)
> > @@ -535,7 +535,7 @@ void helper_hyp_hsv_d(CPURISCVState *env,
> target_ulong addr, target_ulong val)
> >      int mmu_idx = check_access_hlsv(env, false, ra);
> >      MemOpIdx oi = make_memop_idx(MO_TEUQ, mmu_idx);
> >
> > -    cpu_stq_mmu(env, addr, val, oi, ra);
> > +    cpu_stq_mmu(env, adjust_addr_virt(env, addr), val, oi, ra);
> >  }
> >
> >  /*
> > diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
> > index 37c6c198a5..a0093bcc9c 100644
> > --- a/target/riscv/vector_helper.c
> > +++ b/target/riscv/vector_helper.c
> > @@ -105,27 +105,6 @@ static inline uint32_t vext_max_elems(uint32_t
> desc, uint32_t log2_esz)
> >      return scale < 0 ? vlenb >> -scale : vlenb << scale;
> >  }
> >
> > -static inline target_ulong adjust_addr(CPURISCVState *env, target_ulong
> addr)
> > -{
> > -    if (riscv_cpu_mxl(env) == MXL_RV32) {
> > -        return addr;
> > -    }
> > -    RISCVPmPmm pmm = riscv_pm_get_pmm(env);
> > -    if (pmm == PMM_FIELD_DISABLED) {
> > -        return addr;
> > -    }
> > -    int pmlen = riscv_pm_get_pmlen(pmm);
> > -    bool signext = riscv_cpu_virt_mem_enabled(env);
> > -    addr = addr << pmlen;
> > -    /* sign/zero extend masked address by N-1 bit */
> > -    if (signext) {
> > -        addr = (target_long)addr >> pmlen;
> > -    } else {
> > -        addr = addr >> pmlen;
> > -    }
> > -    return addr;
> > -}
> > -
> >  /*
> >   * This function checks watchpoint before real load operation.
> >   *
> > --
> > 2.39.5
> >
>
diff mbox series

Patch

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 38231fe21e..536ad20fdd 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -773,6 +773,7 @@  bool riscv_cpu_is_32bit(RISCVCPU *cpu);
 
 bool riscv_cpu_virt_mem_enabled(CPURISCVState *env);
 RISCVPmPmm riscv_pm_get_pmm(CPURISCVState *env);
+RISCVPmPmm riscv_pm_get_virt_pmm(CPURISCVState *env);
 uint32_t riscv_pm_get_pmlen(RISCVPmPmm pmm);
 
 RISCVException riscv_csrr(CPURISCVState *env, int csrno,
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 480d2c2c8b..471d8d40a1 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -260,6 +260,24 @@  RISCVPmPmm riscv_pm_get_pmm(CPURISCVState *env)
     return pmm;
 }
 
+RISCVPmPmm riscv_pm_get_virt_pmm(CPURISCVState *env)
+{
+    RISCVPmPmm pmm = PMM_FIELD_DISABLED;
+#ifndef CONFIG_USER_ONLY
+    int priv_mode = cpu_address_mode(env);
+    if (priv_mode == PRV_U) {
+        pmm = get_field(env->hstatus, HSTATUS_HUPMM);
+    } else {
+        if (get_field(env->hstatus, HSTATUS_SPVP)) {
+            pmm = get_field(env->henvcfg, HENVCFG_PMM);
+        } else {
+            pmm = get_field(env->senvcfg, SENVCFG_PMM);
+        }
+    }
+#endif
+    return pmm;
+}
+
 bool riscv_cpu_virt_mem_enabled(CPURISCVState *env)
 {
     bool virt_mem_en = false;
diff --git a/target/riscv/internals.h b/target/riscv/internals.h
index ddbdee885b..017f33af1f 100644
--- a/target/riscv/internals.h
+++ b/target/riscv/internals.h
@@ -142,4 +142,48 @@  static inline float16 check_nanbox_h(CPURISCVState *env, uint64_t f)
 /* Our implementation of CPUClass::has_work */
 bool riscv_cpu_has_work(CPUState *cs);
 
+/* Zjpm addr masking routine */
+static inline target_ulong adjust_addr_body(CPURISCVState *env,
+                                            target_ulong addr,
+                                            bool is_virt)
+{
+    if (riscv_cpu_mxl(env) == MXL_RV32) {
+        return addr;
+    }
+    RISCVPmPmm pmm = PMM_FIELD_DISABLED;
+    if (is_virt) {
+        pmm = riscv_pm_get_virt_pmm(env);
+    } else {
+        pmm = riscv_pm_get_pmm(env);
+    }
+    if (pmm == PMM_FIELD_DISABLED) {
+        return addr;
+    }
+    uint32_t pmlen = riscv_pm_get_pmlen(pmm);
+    bool signext = false;
+    if (!is_virt) {
+        signext = riscv_cpu_virt_mem_enabled(env);
+    }
+    addr = addr << pmlen;
+    /* sign/zero extend masked address by N-1 bit */
+    if (signext) {
+        addr = (target_long)addr >> pmlen;
+    } else {
+        addr = addr >> pmlen;
+    }
+    return addr;
+}
+
+static inline target_ulong adjust_addr(CPURISCVState *env,
+                                       target_ulong addr)
+{
+    return adjust_addr_body(env, addr, false);
+}
+
+static inline target_ulong adjust_addr_virt(CPURISCVState *env,
+                                            target_ulong addr)
+{
+    return adjust_addr_body(env, addr, true);
+}
+
 #endif
diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index eddedacf4b..20e5bd5088 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -472,7 +472,7 @@  target_ulong helper_hyp_hlv_bu(CPURISCVState *env, target_ulong addr)
     int mmu_idx = check_access_hlsv(env, false, ra);
     MemOpIdx oi = make_memop_idx(MO_UB, mmu_idx);
 
-    return cpu_ldb_mmu(env, addr, oi, ra);
+    return cpu_ldb_mmu(env, adjust_addr_virt(env, addr), oi, ra);
 }
 
 target_ulong helper_hyp_hlv_hu(CPURISCVState *env, target_ulong addr)
@@ -481,7 +481,7 @@  target_ulong helper_hyp_hlv_hu(CPURISCVState *env, target_ulong addr)
     int mmu_idx = check_access_hlsv(env, false, ra);
     MemOpIdx oi = make_memop_idx(MO_TEUW, mmu_idx);
 
-    return cpu_ldw_mmu(env, addr, oi, ra);
+    return cpu_ldw_mmu(env, adjust_addr_virt(env, addr), oi, ra);
 }
 
 target_ulong helper_hyp_hlv_wu(CPURISCVState *env, target_ulong addr)
@@ -490,7 +490,7 @@  target_ulong helper_hyp_hlv_wu(CPURISCVState *env, target_ulong addr)
     int mmu_idx = check_access_hlsv(env, false, ra);
     MemOpIdx oi = make_memop_idx(MO_TEUL, mmu_idx);
 
-    return cpu_ldl_mmu(env, addr, oi, ra);
+    return cpu_ldl_mmu(env, adjust_addr_virt(env, addr), oi, ra);
 }
 
 target_ulong helper_hyp_hlv_d(CPURISCVState *env, target_ulong addr)
@@ -499,7 +499,7 @@  target_ulong helper_hyp_hlv_d(CPURISCVState *env, target_ulong addr)
     int mmu_idx = check_access_hlsv(env, false, ra);
     MemOpIdx oi = make_memop_idx(MO_TEUQ, mmu_idx);
 
-    return cpu_ldq_mmu(env, addr, oi, ra);
+    return cpu_ldq_mmu(env, adjust_addr_virt(env, addr), oi, ra);
 }
 
 void helper_hyp_hsv_b(CPURISCVState *env, target_ulong addr, target_ulong val)
@@ -508,7 +508,7 @@  void helper_hyp_hsv_b(CPURISCVState *env, target_ulong addr, target_ulong val)
     int mmu_idx = check_access_hlsv(env, false, ra);
     MemOpIdx oi = make_memop_idx(MO_UB, mmu_idx);
 
-    cpu_stb_mmu(env, addr, val, oi, ra);
+    cpu_stb_mmu(env, adjust_addr_virt(env, addr), val, oi, ra);
 }
 
 void helper_hyp_hsv_h(CPURISCVState *env, target_ulong addr, target_ulong val)
@@ -517,7 +517,7 @@  void helper_hyp_hsv_h(CPURISCVState *env, target_ulong addr, target_ulong val)
     int mmu_idx = check_access_hlsv(env, false, ra);
     MemOpIdx oi = make_memop_idx(MO_TEUW, mmu_idx);
 
-    cpu_stw_mmu(env, addr, val, oi, ra);
+    cpu_stw_mmu(env, adjust_addr_virt(env, addr), val, oi, ra);
 }
 
 void helper_hyp_hsv_w(CPURISCVState *env, target_ulong addr, target_ulong val)
@@ -526,7 +526,7 @@  void helper_hyp_hsv_w(CPURISCVState *env, target_ulong addr, target_ulong val)
     int mmu_idx = check_access_hlsv(env, false, ra);
     MemOpIdx oi = make_memop_idx(MO_TEUL, mmu_idx);
 
-    cpu_stl_mmu(env, addr, val, oi, ra);
+    cpu_stl_mmu(env, adjust_addr_virt(env, addr), val, oi, ra);
 }
 
 void helper_hyp_hsv_d(CPURISCVState *env, target_ulong addr, target_ulong val)
@@ -535,7 +535,7 @@  void helper_hyp_hsv_d(CPURISCVState *env, target_ulong addr, target_ulong val)
     int mmu_idx = check_access_hlsv(env, false, ra);
     MemOpIdx oi = make_memop_idx(MO_TEUQ, mmu_idx);
 
-    cpu_stq_mmu(env, addr, val, oi, ra);
+    cpu_stq_mmu(env, adjust_addr_virt(env, addr), val, oi, ra);
 }
 
 /*
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index 37c6c198a5..a0093bcc9c 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -105,27 +105,6 @@  static inline uint32_t vext_max_elems(uint32_t desc, uint32_t log2_esz)
     return scale < 0 ? vlenb >> -scale : vlenb << scale;
 }
 
-static inline target_ulong adjust_addr(CPURISCVState *env, target_ulong addr)
-{
-    if (riscv_cpu_mxl(env) == MXL_RV32) {
-        return addr;
-    }
-    RISCVPmPmm pmm = riscv_pm_get_pmm(env);
-    if (pmm == PMM_FIELD_DISABLED) {
-        return addr;
-    }
-    int pmlen = riscv_pm_get_pmlen(pmm);
-    bool signext = riscv_cpu_virt_mem_enabled(env);
-    addr = addr << pmlen;
-    /* sign/zero extend masked address by N-1 bit */
-    if (signext) {
-        addr = (target_long)addr >> pmlen;
-    } else {
-        addr = addr >> pmlen;
-    }
-    return addr;
-}
-
 /*
  * This function checks watchpoint before real load operation.
  *