[PATCH v9 0/6] Pointer Masking update for Zjpm v1.0

[PATCH v9 0/6] Pointer Masking update for Zjpm v1.0

[Alexey Baturo]

From: Alexey Baturo <baturo.alexey@gmail.com>

Hi,

It looks like Pointer Masking spec has reached v1.0 and been frozen,
rebasing on riscv-to-apply.next branch and resubmitting patches. 

Thanks.

[v8]:
Rebasing patches on current qemu branch and resubmitting them.


[v7]:
I'm terribly sorry, but previous rebase went wrong and somehow I missed it.
This time I double-checked rebased version.
This patch series is properly rebased on https://github.com/alistair23/qemu/tree/riscv-to-apply.next 

[v6]:
This patch series is rebased on https://github.com/alistair23/qemu/tree/riscv-to-apply.next 

[v5]:
This patch series targets Zjpm v0.8 extension.
The spec itself could be found here: https://github.com/riscv/riscv-j-extension/blob/8088461d8d66a7676872b61c908cbeb7cf5c5d1d/zjpm-spec.pdf
This patch series is updated after the suggested comments:
- add "x-" to the extension names to indicate experimental

[v4]:
Patch series updated after the suggested comments:
- removed J-letter extension as it's unused
- renamed and fixed function to detect if address should be sign-extended
- zeroed unused context variables and moved computation logic to another patch
- bumped pointer masking version_id and minimum_version_id by 1

[v3]:
There patches are updated after Richard's comments:
- moved new tb flags to the end
- used tcg_gen_(s)extract to get the final address
- properly handle CONFIG_USER_ONLY

[v2]:
As per Richard's suggestion I made pmm field part of tb_flags.
It allowed to get rid of global variable to store pmlen.
Also it allowed to simplify all the machinery around it.

[v1]:
It looks like Zjpm v0.8 is almost frozen and we don't expect it change drastically anymore.
Compared to the original implementation with explicit base and mask CSRs, we now only have
several fixed options for number of masked bits which are set using existing CSRs.
The changes have been tested with handwritten assembly tests and LLVM HWASAN
test suite.

Alexey Baturo (6):
  target/riscv: Remove obsolete pointer masking  extension code.
  target/riscv: Add new CSR fields for S{sn,mn,m}pm extensions as part
    of Zjpm v0.8
  target/riscv: Add helper functions to calculate current number of
    masked bits for pointer masking
  target/riscv: Add pointer masking tb flags
  target/riscv: Update address modify functions to take into account
    pointer masking
  target/riscv: Enable updates for pointer masking variables and thus
    enable pointer masking extension

 target/riscv/cpu.c           |  21 +--
 target/riscv/cpu.h           |  46 +++--
 target/riscv/cpu_bits.h      |  90 +---------
 target/riscv/cpu_cfg.h       |   3 +
 target/riscv/cpu_helper.c    |  97 +++++-----
 target/riscv/csr.c           | 337 ++---------------------------------
 target/riscv/machine.c       |  20 +--
 target/riscv/pmp.c           |  13 +-
 target/riscv/pmp.h           |  11 +-
 target/riscv/tcg/tcg-cpu.c   |   5 +-
 target/riscv/translate.c     |  46 ++---
 target/riscv/vector_helper.c |  15 +-
 12 files changed, 158 insertions(+), 546 deletions(-)

-- 
2.34.1

[PATCH v9 1/6] target/riscv: Remove obsolete pointer masking extension code.

[Alexey Baturo]

From: Alexey Baturo <baturo.alexey@gmail.com>

Zjpm v0.8 is almost frozen and it's much simplier compared to the existing one:
The newer version doesn't allow to specify custom mask or base for masking.
Instead it allows only certain options for masking top bits.

Signed-off-by: Alexey Baturo <baturo.alexey@gmail.com>

Acked-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/cpu.c           |  13 +-
 target/riscv/cpu.h           |  30 +---
 target/riscv/cpu_bits.h      |  87 ----------
 target/riscv/cpu_helper.c    |  52 ------
 target/riscv/csr.c           | 326 -----------------------------------
 target/riscv/machine.c       |  14 +-
 target/riscv/tcg/tcg-cpu.c   |   5 +-
 target/riscv/translate.c     |  27 +--
 target/riscv/vector_helper.c |   2 +-
 9 files changed, 13 insertions(+), 543 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index a74f0eb29c..1e350e9bd8 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -42,7 +42,7 @@
 /* RISC-V CPU definitions */
 static const char riscv_single_letter_exts[] = "IEMAFDQCBPVH";
 const uint32_t misa_bits[] = {RVI, RVE, RVM, RVA, RVF, RVD, RVV,
-                              RVC, RVS, RVU, RVH, RVJ, RVG, RVB, 0};
+                              RVC, RVS, RVU, RVH, RVG, RVB, 0};
 
 /*
  * From vector_helper.c
@@ -795,13 +795,6 @@ static void riscv_cpu_dump_state(CPUState *cs, FILE *f, int flags)
             CSR_MSCRATCH,
             CSR_SSCRATCH,
             CSR_SATP,
-            CSR_MMTE,
-            CSR_UPMBASE,
-            CSR_UPMMASK,
-            CSR_SPMBASE,
-            CSR_SPMMASK,
-            CSR_MPMBASE,
-            CSR_MPMMASK,
         };
 
         for (i = 0; i < ARRAY_SIZE(dump_csrs); ++i) {
@@ -981,8 +974,6 @@ static void riscv_cpu_reset_hold(Object *obj, ResetType type)
         }
         i++;
     }
-    /* mmte is supposed to have pm.current hardwired to 1 */
-    env->mmte |= (EXT_STATUS_INITIAL | MMTE_M_PM_CURRENT);
 
     /*
      * Bits 10, 6, 2 and 12 of mideleg are read only 1 when the Hypervisor
@@ -1004,7 +995,6 @@ static void riscv_cpu_reset_hold(Object *obj, ResetType type)
     pmp_unlock_entries(env);
 #endif
     env->xl = riscv_cpu_mxl(env);
-    riscv_cpu_update_mask(env);
     cs->exception_index = RISCV_EXCP_NONE;
     env->load_res = -1;
     set_default_nan_mode(1, &env->fp_status);
@@ -1395,7 +1385,6 @@ static const MISAExtInfo misa_ext_info_arr[] = {
     MISA_EXT_INFO(RVS, "s", "Supervisor-level instructions"),
     MISA_EXT_INFO(RVU, "u", "User-level instructions"),
     MISA_EXT_INFO(RVH, "h", "Hypervisor"),
-    MISA_EXT_INFO(RVJ, "x-j", "Dynamic translated languages"),
     MISA_EXT_INFO(RVV, "v", "Vector operations"),
     MISA_EXT_INFO(RVG, "g", "General purpose (IMAFD_Zicsr_Zifencei)"),
     MISA_EXT_INFO(RVB, "x-b", "Bit manipulation (Zba_Zbb_Zbs)")
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index e0dd1828b5..232521bb87 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -66,7 +66,6 @@ typedef struct CPUArchState CPURISCVState;
 #define RVS RV('S')
 #define RVU RV('U')
 #define RVH RV('H')
-#define RVJ RV('J')
 #define RVG RV('G')
 #define RVB RV('B')
 
@@ -393,17 +392,6 @@ struct CPUArchState {
     /* True if in debugger mode.  */
     bool debugger;
 
-    /*
-     * CSRs for PointerMasking extension
-     */
-    target_ulong mmte;
-    target_ulong mpmmask;
-    target_ulong mpmbase;
-    target_ulong spmmask;
-    target_ulong spmbase;
-    target_ulong upmmask;
-    target_ulong upmbase;
-
     /* CSRs for execution environment configuration */
     uint64_t menvcfg;
     uint64_t mstateen[SMSTATEEN_MAX_COUNT];
@@ -412,9 +400,6 @@ struct CPUArchState {
     target_ulong senvcfg;
     uint64_t henvcfg;
 #endif
-    target_ulong cur_pmmask;
-    target_ulong cur_pmbase;
-
     /* Fields from here on are preserved across CPU reset. */
     QEMUTimer *stimer; /* Internal timer for S-mode interrupt */
     QEMUTimer *vstimer; /* Internal timer for VS-mode interrupt */
@@ -563,16 +548,14 @@ FIELD(TB_FLAGS, VSTART_EQ_ZERO, 15, 1)
 /* The combination of MXL/SXL/UXL that applies to the current cpu mode. */
 FIELD(TB_FLAGS, XL, 16, 2)
 /* If PointerMasking should be applied */
-FIELD(TB_FLAGS, PM_MASK_ENABLED, 18, 1)
-FIELD(TB_FLAGS, PM_BASE_ENABLED, 19, 1)
-FIELD(TB_FLAGS, VTA, 20, 1)
-FIELD(TB_FLAGS, VMA, 21, 1)
+FIELD(TB_FLAGS, VTA, 18, 1)
+FIELD(TB_FLAGS, VMA, 19, 1)
 /* Native debug itrigger */
-FIELD(TB_FLAGS, ITRIGGER, 22, 1)
+FIELD(TB_FLAGS, ITRIGGER, 20, 1)
 /* Virtual mode enabled */
-FIELD(TB_FLAGS, VIRT_ENABLED, 23, 1)
-FIELD(TB_FLAGS, PRIV, 24, 2)
-FIELD(TB_FLAGS, AXL, 26, 2)
+FIELD(TB_FLAGS, VIRT_ENABLED, 21, 1)
+FIELD(TB_FLAGS, PRIV, 22, 2)
+FIELD(TB_FLAGS, AXL, 24, 2)
 
 #ifdef TARGET_RISCV32
 #define riscv_cpu_mxl(env)  ((void)(env), MXL_RV32)
@@ -705,7 +688,6 @@ static inline uint32_t vext_get_vlmax(uint32_t vlenb, uint32_t vsew,
 void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
                           uint64_t *cs_base, uint32_t *pflags);
 
-void riscv_cpu_update_mask(CPURISCVState *env);
 bool riscv_cpu_is_32bit(RISCVCPU *cpu);
 
 RISCVException riscv_csrr(CPURISCVState *env, int csrno,
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 74318a925c..da16ba236a 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -484,37 +484,6 @@
 #define CSR_MHPMCOUNTER30H  0xb9e
 #define CSR_MHPMCOUNTER31H  0xb9f
 
-/*
- * User PointerMasking registers
- * NB: actual CSR numbers might be changed in future
- */
-#define CSR_UMTE            0x4c0
-#define CSR_UPMMASK         0x4c1
-#define CSR_UPMBASE         0x4c2
-
-/*
- * Machine PointerMasking registers
- * NB: actual CSR numbers might be changed in future
- */
-#define CSR_MMTE            0x3c0
-#define CSR_MPMMASK         0x3c1
-#define CSR_MPMBASE         0x3c2
-
-/*
- * Supervisor PointerMaster registers
- * NB: actual CSR numbers might be changed in future
- */
-#define CSR_SMTE            0x1c0
-#define CSR_SPMMASK         0x1c1
-#define CSR_SPMBASE         0x1c2
-
-/*
- * Hypervisor PointerMaster registers
- * NB: actual CSR numbers might be changed in future
- */
-#define CSR_VSMTE           0x2c0
-#define CSR_VSPMMASK        0x2c1
-#define CSR_VSPMBASE        0x2c2
 #define CSR_SCOUNTOVF       0xda0
 
 /* Crypto Extension */
@@ -734,11 +703,6 @@ typedef enum RISCVException {
 #define VS_MODE_INTERRUPTS ((uint64_t)(MIP_VSSIP | MIP_VSTIP | MIP_VSEIP))
 #define HS_MODE_INTERRUPTS ((uint64_t)(MIP_SGEIP | VS_MODE_INTERRUPTS))
 
-/* General PointerMasking CSR bits */
-#define PM_ENABLE       0x00000001ULL
-#define PM_CURRENT      0x00000002ULL
-#define PM_INSN         0x00000004ULL
-
 /* Execution environment configuration bits */
 #define MENVCFG_FIOM                       BIT(0)
 #define MENVCFG_CBIE                       (3UL << 4)
@@ -771,57 +735,6 @@ typedef enum RISCVException {
 #define HENVCFGH_PBMTE                      MENVCFGH_PBMTE
 #define HENVCFGH_STCE                       MENVCFGH_STCE
 
-/* Offsets for every pair of control bits per each priv level */
-#define XS_OFFSET    0ULL
-#define U_OFFSET     2ULL
-#define S_OFFSET     5ULL
-#define M_OFFSET     8ULL
-
-#define PM_XS_BITS   (EXT_STATUS_MASK << XS_OFFSET)
-#define U_PM_ENABLE  (PM_ENABLE  << U_OFFSET)
-#define U_PM_CURRENT (PM_CURRENT << U_OFFSET)
-#define U_PM_INSN    (PM_INSN    << U_OFFSET)
-#define S_PM_ENABLE  (PM_ENABLE  << S_OFFSET)
-#define S_PM_CURRENT (PM_CURRENT << S_OFFSET)
-#define S_PM_INSN    (PM_INSN    << S_OFFSET)
-#define M_PM_ENABLE  (PM_ENABLE  << M_OFFSET)
-#define M_PM_CURRENT (PM_CURRENT << M_OFFSET)
-#define M_PM_INSN    (PM_INSN    << M_OFFSET)
-
-/* mmte CSR bits */
-#define MMTE_PM_XS_BITS     PM_XS_BITS
-#define MMTE_U_PM_ENABLE    U_PM_ENABLE
-#define MMTE_U_PM_CURRENT   U_PM_CURRENT
-#define MMTE_U_PM_INSN      U_PM_INSN
-#define MMTE_S_PM_ENABLE    S_PM_ENABLE
-#define MMTE_S_PM_CURRENT   S_PM_CURRENT
-#define MMTE_S_PM_INSN      S_PM_INSN
-#define MMTE_M_PM_ENABLE    M_PM_ENABLE
-#define MMTE_M_PM_CURRENT   M_PM_CURRENT
-#define MMTE_M_PM_INSN      M_PM_INSN
-#define MMTE_MASK    (MMTE_U_PM_ENABLE | MMTE_U_PM_CURRENT | MMTE_U_PM_INSN | \
-                      MMTE_S_PM_ENABLE | MMTE_S_PM_CURRENT | MMTE_S_PM_INSN | \
-                      MMTE_M_PM_ENABLE | MMTE_M_PM_CURRENT | MMTE_M_PM_INSN | \
-                      MMTE_PM_XS_BITS)
-
-/* (v)smte CSR bits */
-#define SMTE_PM_XS_BITS     PM_XS_BITS
-#define SMTE_U_PM_ENABLE    U_PM_ENABLE
-#define SMTE_U_PM_CURRENT   U_PM_CURRENT
-#define SMTE_U_PM_INSN      U_PM_INSN
-#define SMTE_S_PM_ENABLE    S_PM_ENABLE
-#define SMTE_S_PM_CURRENT   S_PM_CURRENT
-#define SMTE_S_PM_INSN      S_PM_INSN
-#define SMTE_MASK    (SMTE_U_PM_ENABLE | SMTE_U_PM_CURRENT | SMTE_U_PM_INSN | \
-                      SMTE_S_PM_ENABLE | SMTE_S_PM_CURRENT | SMTE_S_PM_INSN | \
-                      SMTE_PM_XS_BITS)
-
-/* umte CSR bits */
-#define UMTE_U_PM_ENABLE    U_PM_ENABLE
-#define UMTE_U_PM_CURRENT   U_PM_CURRENT
-#define UMTE_U_PM_INSN      U_PM_INSN
-#define UMTE_MASK     (UMTE_U_PM_ENABLE | MMTE_U_PM_CURRENT | UMTE_U_PM_INSN)
-
 /* MISELECT, SISELECT, and VSISELECT bits (AIA) */
 #define ISELECT_IPRIO0                     0x30
 #define ISELECT_IPRIO15                    0x3f
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 2097a26ca0..bf58350669 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -138,61 +138,10 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
     flags = FIELD_DP32(flags, TB_FLAGS, VS, vs);
     flags = FIELD_DP32(flags, TB_FLAGS, XL, env->xl);
     flags = FIELD_DP32(flags, TB_FLAGS, AXL, cpu_address_xl(env));
-    if (env->cur_pmmask != 0) {
-        flags = FIELD_DP32(flags, TB_FLAGS, PM_MASK_ENABLED, 1);
-    }
-    if (env->cur_pmbase != 0) {
-        flags = FIELD_DP32(flags, TB_FLAGS, PM_BASE_ENABLED, 1);
-    }
 
     *pflags = flags;
 }
 
-void riscv_cpu_update_mask(CPURISCVState *env)
-{
-    target_ulong mask = 0, base = 0;
-    RISCVMXL xl = env->xl;
-    /*
-     * TODO: Current RVJ spec does not specify
-     * how the extension interacts with XLEN.
-     */
-#ifndef CONFIG_USER_ONLY
-    int mode = cpu_address_mode(env);
-    xl = cpu_get_xl(env, mode);
-    if (riscv_has_ext(env, RVJ)) {
-        switch (mode) {
-        case PRV_M:
-            if (env->mmte & M_PM_ENABLE) {
-                mask = env->mpmmask;
-                base = env->mpmbase;
-            }
-            break;
-        case PRV_S:
-            if (env->mmte & S_PM_ENABLE) {
-                mask = env->spmmask;
-                base = env->spmbase;
-            }
-            break;
-        case PRV_U:
-            if (env->mmte & U_PM_ENABLE) {
-                mask = env->upmmask;
-                base = env->upmbase;
-            }
-            break;
-        default:
-            g_assert_not_reached();
-        }
-    }
-#endif
-    if (xl == MXL_RV32) {
-        env->cur_pmmask = mask & UINT32_MAX;
-        env->cur_pmbase = base & UINT32_MAX;
-    } else {
-        env->cur_pmmask = mask;
-        env->cur_pmbase = base;
-    }
-}
-
 #ifndef CONFIG_USER_ONLY
 
 /*
@@ -724,7 +673,6 @@ void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv)
     /* tlb_flush is unnecessary as mode is contained in mmu_idx */
     env->priv = newpriv;
     env->xl = cpu_recompute_xl(env);
-    riscv_cpu_update_mask(env);
 
     /*
      * Clear the load reservation - otherwise a reservation placed in one
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 6b460ee0e8..4b2c932564 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -486,16 +486,6 @@ static RISCVException hgatp(CPURISCVState *env, int csrno)
     return hmode(env, csrno);
 }
 
-/* Checks if PointerMasking registers could be accessed */
-static RISCVException pointer_masking(CPURISCVState *env, int csrno)
-{
-    /* Check if j-ext is present */
-    if (riscv_has_ext(env, RVJ)) {
-        return RISCV_EXCP_NONE;
-    }
-    return RISCV_EXCP_ILLEGAL_INST;
-}
-
 static RISCVException aia_hmode(CPURISCVState *env, int csrno)
 {
     if (!riscv_cpu_cfg(env)->ext_ssaia) {
@@ -1404,7 +1394,6 @@ static RISCVException write_mstatus(CPURISCVState *env, int csrno,
         env->xl = cpu_recompute_xl(env);
     }
 
-    riscv_cpu_update_mask(env);
     return RISCV_EXCP_NONE;
 }
 
@@ -3971,302 +3960,6 @@ static RISCVException write_mcontext(CPURISCVState *env, int csrno,
     return RISCV_EXCP_NONE;
 }
 
-/*
- * Functions to access Pointer Masking feature registers
- * We have to check if current priv lvl could modify
- * csr in given mode
- */
-static bool check_pm_current_disabled(CPURISCVState *env, int csrno)
-{
-    int csr_priv = get_field(csrno, 0x300);
-    int pm_current;
-
-    if (env->debugger) {
-        return false;
-    }
-    /*
-     * If priv lvls differ that means we're accessing csr from higher priv lvl,
-     * so allow the access
-     */
-    if (env->priv != csr_priv) {
-        return false;
-    }
-    switch (env->priv) {
-    case PRV_M:
-        pm_current = get_field(env->mmte, M_PM_CURRENT);
-        break;
-    case PRV_S:
-        pm_current = get_field(env->mmte, S_PM_CURRENT);
-        break;
-    case PRV_U:
-        pm_current = get_field(env->mmte, U_PM_CURRENT);
-        break;
-    default:
-        g_assert_not_reached();
-    }
-    /* It's same priv lvl, so we allow to modify csr only if pm.current==1 */
-    return !pm_current;
-}
-
-static RISCVException read_mmte(CPURISCVState *env, int csrno,
-                                target_ulong *val)
-{
-    *val = env->mmte & MMTE_MASK;
-    return RISCV_EXCP_NONE;
-}
-
-static RISCVException write_mmte(CPURISCVState *env, int csrno,
-                                 target_ulong val)
-{
-    uint64_t mstatus;
-    target_ulong wpri_val = val & MMTE_MASK;
-
-    if (val != wpri_val) {
-        qemu_log_mask(LOG_GUEST_ERROR, "%s" TARGET_FMT_lx " %s"
-                      TARGET_FMT_lx "\n", "MMTE: WPRI violation written 0x",
-                      val, "vs expected 0x", wpri_val);
-    }
-    /* for machine mode pm.current is hardwired to 1 */
-    wpri_val |= MMTE_M_PM_CURRENT;
-
-    /* hardwiring pm.instruction bit to 0, since it's not supported yet */
-    wpri_val &= ~(MMTE_M_PM_INSN | MMTE_S_PM_INSN | MMTE_U_PM_INSN);
-    env->mmte = wpri_val | EXT_STATUS_DIRTY;
-    riscv_cpu_update_mask(env);
-
-    /* Set XS and SD bits, since PM CSRs are dirty */
-    mstatus = env->mstatus | MSTATUS_XS;
-    write_mstatus(env, csrno, mstatus);
-    return RISCV_EXCP_NONE;
-}
-
-static RISCVException read_smte(CPURISCVState *env, int csrno,
-                                target_ulong *val)
-{
-    *val = env->mmte & SMTE_MASK;
-    return RISCV_EXCP_NONE;
-}
-
-static RISCVException write_smte(CPURISCVState *env, int csrno,
-                                 target_ulong val)
-{
-    target_ulong wpri_val = val & SMTE_MASK;
-
-    if (val != wpri_val) {
-        qemu_log_mask(LOG_GUEST_ERROR, "%s" TARGET_FMT_lx " %s"
-                      TARGET_FMT_lx "\n", "SMTE: WPRI violation written 0x",
-                      val, "vs expected 0x", wpri_val);
-    }
-
-    /* if pm.current==0 we can't modify current PM CSRs */
-    if (check_pm_current_disabled(env, csrno)) {
-        return RISCV_EXCP_NONE;
-    }
-
-    wpri_val |= (env->mmte & ~SMTE_MASK);
-    write_mmte(env, csrno, wpri_val);
-    return RISCV_EXCP_NONE;
-}
-
-static RISCVException read_umte(CPURISCVState *env, int csrno,
-                                target_ulong *val)
-{
-    *val = env->mmte & UMTE_MASK;
-    return RISCV_EXCP_NONE;
-}
-
-static RISCVException write_umte(CPURISCVState *env, int csrno,
-                                 target_ulong val)
-{
-    target_ulong wpri_val = val & UMTE_MASK;
-
-    if (val != wpri_val) {
-        qemu_log_mask(LOG_GUEST_ERROR, "%s" TARGET_FMT_lx " %s"
-                      TARGET_FMT_lx "\n", "UMTE: WPRI violation written 0x",
-                      val, "vs expected 0x", wpri_val);
-    }
-
-    if (check_pm_current_disabled(env, csrno)) {
-        return RISCV_EXCP_NONE;
-    }
-
-    wpri_val |= (env->mmte & ~UMTE_MASK);
-    write_mmte(env, csrno, wpri_val);
-    return RISCV_EXCP_NONE;
-}
-
-static RISCVException read_mpmmask(CPURISCVState *env, int csrno,
-                                   target_ulong *val)
-{
-    *val = env->mpmmask;
-    return RISCV_EXCP_NONE;
-}
-
-static RISCVException write_mpmmask(CPURISCVState *env, int csrno,
-                                    target_ulong val)
-{
-    uint64_t mstatus;
-
-    env->mpmmask = val;
-    if ((cpu_address_mode(env) == PRV_M) && (env->mmte & M_PM_ENABLE)) {
-        env->cur_pmmask = val;
-    }
-    env->mmte |= EXT_STATUS_DIRTY;
-
-    /* Set XS and SD bits, since PM CSRs are dirty */
-    mstatus = env->mstatus | MSTATUS_XS;
-    write_mstatus(env, csrno, mstatus);
-    return RISCV_EXCP_NONE;
-}
-
-static RISCVException read_spmmask(CPURISCVState *env, int csrno,
-                                   target_ulong *val)
-{
-    *val = env->spmmask;
-    return RISCV_EXCP_NONE;
-}
-
-static RISCVException write_spmmask(CPURISCVState *env, int csrno,
-                                    target_ulong val)
-{
-    uint64_t mstatus;
-
-    /* if pm.current==0 we can't modify current PM CSRs */
-    if (check_pm_current_disabled(env, csrno)) {
-        return RISCV_EXCP_NONE;
-    }
-    env->spmmask = val;
-    if ((cpu_address_mode(env) == PRV_S) && (env->mmte & S_PM_ENABLE)) {
-        env->cur_pmmask = val;
-        if (cpu_get_xl(env, PRV_S) == MXL_RV32) {
-            env->cur_pmmask &= UINT32_MAX;
-        }
-    }
-    env->mmte |= EXT_STATUS_DIRTY;
-
-    /* Set XS and SD bits, since PM CSRs are dirty */
-    mstatus = env->mstatus | MSTATUS_XS;
-    write_mstatus(env, csrno, mstatus);
-    return RISCV_EXCP_NONE;
-}
-
-static RISCVException read_upmmask(CPURISCVState *env, int csrno,
-                                   target_ulong *val)
-{
-    *val = env->upmmask;
-    return RISCV_EXCP_NONE;
-}
-
-static RISCVException write_upmmask(CPURISCVState *env, int csrno,
-                                    target_ulong val)
-{
-    uint64_t mstatus;
-
-    /* if pm.current==0 we can't modify current PM CSRs */
-    if (check_pm_current_disabled(env, csrno)) {
-        return RISCV_EXCP_NONE;
-    }
-    env->upmmask = val;
-    if ((cpu_address_mode(env) == PRV_U) && (env->mmte & U_PM_ENABLE)) {
-        env->cur_pmmask = val;
-        if (cpu_get_xl(env, PRV_U) == MXL_RV32) {
-            env->cur_pmmask &= UINT32_MAX;
-        }
-    }
-    env->mmte |= EXT_STATUS_DIRTY;
-
-    /* Set XS and SD bits, since PM CSRs are dirty */
-    mstatus = env->mstatus | MSTATUS_XS;
-    write_mstatus(env, csrno, mstatus);
-    return RISCV_EXCP_NONE;
-}
-
-static RISCVException read_mpmbase(CPURISCVState *env, int csrno,
-                                   target_ulong *val)
-{
-    *val = env->mpmbase;
-    return RISCV_EXCP_NONE;
-}
-
-static RISCVException write_mpmbase(CPURISCVState *env, int csrno,
-                                    target_ulong val)
-{
-    uint64_t mstatus;
-
-    env->mpmbase = val;
-    if ((cpu_address_mode(env) == PRV_M) && (env->mmte & M_PM_ENABLE)) {
-        env->cur_pmbase = val;
-    }
-    env->mmte |= EXT_STATUS_DIRTY;
-
-    /* Set XS and SD bits, since PM CSRs are dirty */
-    mstatus = env->mstatus | MSTATUS_XS;
-    write_mstatus(env, csrno, mstatus);
-    return RISCV_EXCP_NONE;
-}
-
-static RISCVException read_spmbase(CPURISCVState *env, int csrno,
-                                   target_ulong *val)
-{
-    *val = env->spmbase;
-    return RISCV_EXCP_NONE;
-}
-
-static RISCVException write_spmbase(CPURISCVState *env, int csrno,
-                                    target_ulong val)
-{
-    uint64_t mstatus;
-
-    /* if pm.current==0 we can't modify current PM CSRs */
-    if (check_pm_current_disabled(env, csrno)) {
-        return RISCV_EXCP_NONE;
-    }
-    env->spmbase = val;
-    if ((cpu_address_mode(env) == PRV_S) && (env->mmte & S_PM_ENABLE)) {
-        env->cur_pmbase = val;
-        if (cpu_get_xl(env, PRV_S) == MXL_RV32) {
-            env->cur_pmbase &= UINT32_MAX;
-        }
-    }
-    env->mmte |= EXT_STATUS_DIRTY;
-
-    /* Set XS and SD bits, since PM CSRs are dirty */
-    mstatus = env->mstatus | MSTATUS_XS;
-    write_mstatus(env, csrno, mstatus);
-    return RISCV_EXCP_NONE;
-}
-
-static RISCVException read_upmbase(CPURISCVState *env, int csrno,
-                                   target_ulong *val)
-{
-    *val = env->upmbase;
-    return RISCV_EXCP_NONE;
-}
-
-static RISCVException write_upmbase(CPURISCVState *env, int csrno,
-                                    target_ulong val)
-{
-    uint64_t mstatus;
-
-    /* if pm.current==0 we can't modify current PM CSRs */
-    if (check_pm_current_disabled(env, csrno)) {
-        return RISCV_EXCP_NONE;
-    }
-    env->upmbase = val;
-    if ((cpu_address_mode(env) == PRV_U) && (env->mmte & U_PM_ENABLE)) {
-        env->cur_pmbase = val;
-        if (cpu_get_xl(env, PRV_U) == MXL_RV32) {
-            env->cur_pmbase &= UINT32_MAX;
-        }
-    }
-    env->mmte |= EXT_STATUS_DIRTY;
-
-    /* Set XS and SD bits, since PM CSRs are dirty */
-    mstatus = env->mstatus | MSTATUS_XS;
-    write_mstatus(env, csrno, mstatus);
-    return RISCV_EXCP_NONE;
-}
-
 #endif
 
 /* Crypto Extension */
@@ -4925,25 +4618,6 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
     [CSR_TINFO]     =  { "tinfo",    debug, read_tinfo,    write_ignore   },
     [CSR_MCONTEXT]  =  { "mcontext", debug, read_mcontext, write_mcontext },
 
-    /* User Pointer Masking */
-    [CSR_UMTE]    =    { "umte",    pointer_masking, read_umte,  write_umte },
-    [CSR_UPMMASK] =    { "upmmask", pointer_masking, read_upmmask,
-                         write_upmmask                                      },
-    [CSR_UPMBASE] =    { "upmbase", pointer_masking, read_upmbase,
-                         write_upmbase                                      },
-    /* Machine Pointer Masking */
-    [CSR_MMTE]    =    { "mmte",    pointer_masking, read_mmte,  write_mmte },
-    [CSR_MPMMASK] =    { "mpmmask", pointer_masking, read_mpmmask,
-                         write_mpmmask                                      },
-    [CSR_MPMBASE] =    { "mpmbase", pointer_masking, read_mpmbase,
-                         write_mpmbase                                      },
-    /* Supervisor Pointer Masking */
-    [CSR_SMTE]    =    { "smte",    pointer_masking, read_smte,  write_smte },
-    [CSR_SPMMASK] =    { "spmmask", pointer_masking, read_spmmask,
-                         write_spmmask                                      },
-    [CSR_SPMBASE] =    { "spmbase", pointer_masking, read_spmbase,
-                         write_spmbase                                      },
-
     /* Performance Counters */
     [CSR_HPMCOUNTER3]    = { "hpmcounter3",    ctr,    read_hpmcounter },
     [CSR_HPMCOUNTER4]    = { "hpmcounter4",    ctr,    read_hpmcounter },
diff --git a/target/riscv/machine.c b/target/riscv/machine.c
index 76f2150f78..64ab66e332 100644
--- a/target/riscv/machine.c
+++ b/target/riscv/machine.c
@@ -152,10 +152,7 @@ static const VMStateDescription vmstate_vector = {
 
 static bool pointermasking_needed(void *opaque)
 {
-    RISCVCPU *cpu = opaque;
-    CPURISCVState *env = &cpu->env;
-
-    return riscv_has_ext(env, RVJ);
+    return false;
 }
 
 static const VMStateDescription vmstate_pointermasking = {
@@ -164,14 +161,6 @@ static const VMStateDescription vmstate_pointermasking = {
     .minimum_version_id = 1,
     .needed = pointermasking_needed,
     .fields = (const VMStateField[]) {
-        VMSTATE_UINTTL(env.mmte, RISCVCPU),
-        VMSTATE_UINTTL(env.mpmmask, RISCVCPU),
-        VMSTATE_UINTTL(env.mpmbase, RISCVCPU),
-        VMSTATE_UINTTL(env.spmmask, RISCVCPU),
-        VMSTATE_UINTTL(env.spmbase, RISCVCPU),
-        VMSTATE_UINTTL(env.upmmask, RISCVCPU),
-        VMSTATE_UINTTL(env.upmbase, RISCVCPU),
-
         VMSTATE_END_OF_LIST()
     }
 };
@@ -266,7 +255,6 @@ static int riscv_cpu_post_load(void *opaque, int version_id)
     CPURISCVState *env = &cpu->env;
 
     env->xl = cpu_recompute_xl(env);
-    riscv_cpu_update_mask(env);
     return 0;
 }
 
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index 4ebebebe09..f64316ef17 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -998,7 +998,6 @@ static const RISCVCPUMisaExtConfig misa_ext_cfgs[] = {
     MISA_CFG(RVS, true),
     MISA_CFG(RVU, true),
     MISA_CFG(RVH, true),
-    MISA_CFG(RVJ, false),
     MISA_CFG(RVV, false),
     MISA_CFG(RVG, false),
     MISA_CFG(RVB, false),
@@ -1285,8 +1284,8 @@ static void riscv_init_max_cpu_extensions(Object *obj)
     CPURISCVState *env = &cpu->env;
     const RISCVCPUMultiExtConfig *prop;
 
-    /* Enable RVG, RVJ and RVV that are disabled by default */
-    riscv_cpu_set_misa_ext(env, env->misa_ext | RVG | RVJ | RVV);
+    /* Enable RVG and RVV that are disabled by default */
+    riscv_cpu_set_misa_ext(env, env->misa_ext | RVG | RVV);
 
     for (prop = riscv_cpu_extensions; prop && prop->name; prop++) {
         isa_ext_update_enabled(cpu, prop->offset, true);
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 9ff09ebdb6..e5b339b1fa 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -42,9 +42,6 @@ static TCGv cpu_gpr[32], cpu_gprh[32], cpu_pc, cpu_vl, cpu_vstart;
 static TCGv_i64 cpu_fpr[32]; /* assume F and D extensions */
 static TCGv load_res;
 static TCGv load_val;
-/* globals for PM CSRs */
-static TCGv pm_mask;
-static TCGv pm_base;
 
 /*
  * If an operation is being performed on less than TARGET_LONG_BITS,
@@ -106,9 +103,6 @@ typedef struct DisasContext {
     bool vl_eq_vlmax;
     CPUState *cs;
     TCGv zero;
-    /* PointerMasking extension */
-    bool pm_mask_enabled;
-    bool pm_base_enabled;
     /* Ztso */
     bool ztso;
     /* Use icount trigger for native debug */
@@ -583,14 +577,9 @@ static TCGv get_address(DisasContext *ctx, int rs1, int imm)
     TCGv src1 = get_gpr(ctx, rs1, EXT_NONE);
 
     tcg_gen_addi_tl(addr, src1, imm);
-    if (ctx->pm_mask_enabled) {
-        tcg_gen_andc_tl(addr, addr, pm_mask);
-    } else if (get_address_xl(ctx) == MXL_RV32) {
+    if (get_address_xl(ctx) == MXL_RV32) {
         tcg_gen_ext32u_tl(addr, addr);
     }
-    if (ctx->pm_base_enabled) {
-        tcg_gen_or_tl(addr, addr, pm_base);
-    }
 
     return addr;
 }
@@ -602,14 +591,9 @@ static TCGv get_address_indexed(DisasContext *ctx, int rs1, TCGv offs)
     TCGv src1 = get_gpr(ctx, rs1, EXT_NONE);
 
     tcg_gen_add_tl(addr, src1, offs);
-    if (ctx->pm_mask_enabled) {
-        tcg_gen_andc_tl(addr, addr, pm_mask);
-    } else if (get_xl(ctx) == MXL_RV32) {
+    if (get_xl(ctx) == MXL_RV32) {
         tcg_gen_ext32u_tl(addr, addr);
     }
-    if (ctx->pm_base_enabled) {
-        tcg_gen_or_tl(addr, addr, pm_base);
-    }
     return addr;
 }
 
@@ -1201,8 +1185,6 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
     ctx->xl = FIELD_EX32(tb_flags, TB_FLAGS, XL);
     ctx->address_xl = FIELD_EX32(tb_flags, TB_FLAGS, AXL);
     ctx->cs = cs;
-    ctx->pm_mask_enabled = FIELD_EX32(tb_flags, TB_FLAGS, PM_MASK_ENABLED);
-    ctx->pm_base_enabled = FIELD_EX32(tb_flags, TB_FLAGS, PM_BASE_ENABLED);
     ctx->ztso = cpu->cfg.ext_ztso;
     ctx->itrigger = FIELD_EX32(tb_flags, TB_FLAGS, ITRIGGER);
     ctx->zero = tcg_constant_tl(0);
@@ -1335,9 +1317,4 @@ void riscv_translate_init(void)
                              "load_res");
     load_val = tcg_global_mem_new(tcg_env, offsetof(CPURISCVState, load_val),
                              "load_val");
-    /* Assign PM CSRs to tcg globals */
-    pm_mask = tcg_global_mem_new(tcg_env, offsetof(CPURISCVState, cur_pmmask),
-                                 "pmmask");
-    pm_base = tcg_global_mem_new(tcg_env, offsetof(CPURISCVState, cur_pmbase),
-                                 "pmbase");
 }
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index fa139040f8..39ba2a09dd 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -104,7 +104,7 @@ static inline uint32_t vext_max_elems(uint32_t desc, uint32_t log2_esz)
 
 static inline target_ulong adjust_addr(CPURISCVState *env, target_ulong addr)
 {
-    return (addr & ~env->cur_pmmask) | env->cur_pmbase;
+    return addr;
 }
 
 /*
-- 
2.34.1

[PATCH v9 2/6] target/riscv: Add new CSR fields for S{sn, mn, m}pm extensions as part of Zjpm v0.8

[Alexey Baturo]

From: Alexey Baturo <baturo.alexey@gmail.com>

Signed-off-by: Alexey Baturo <baturo.alexey@gmail.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/cpu.h      |  8 ++++++++
 target/riscv/cpu_bits.h |  3 +++
 target/riscv/cpu_cfg.h  |  3 +++
 target/riscv/csr.c      | 11 +++++++++++
 target/riscv/machine.c  | 10 +++++++---
 target/riscv/pmp.c      | 13 ++++++++++---
 target/riscv/pmp.h      | 11 ++++++-----
 7 files changed, 48 insertions(+), 11 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 232521bb87..52b6ba73c8 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -121,6 +121,14 @@ typedef enum {
     EXT_STATUS_DIRTY,
 } RISCVExtStatus;
 
+/* Enum holds PMM field values for Zjpm v0.8 extension */
+typedef enum {
+    PMM_FIELD_DISABLED = 0,
+    PMM_FIELD_RESERVED = 1,
+    PMM_FIELD_PMLEN7   = 2,
+    PMM_FIELD_PMLEN16  = 3,
+} RISCVPmPmm;
+
 #define MMU_USER_IDX 3
 
 #define MAX_RISCV_PMPS (16)
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index da16ba236a..13ce2218d1 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -708,6 +708,7 @@ typedef enum RISCVException {
 #define MENVCFG_CBIE                       (3UL << 4)
 #define MENVCFG_CBCFE                      BIT(6)
 #define MENVCFG_CBZE                       BIT(7)
+#define MENVCFG_PMM                        (3ULL << 32)
 #define MENVCFG_ADUE                       (1ULL << 61)
 #define MENVCFG_PBMTE                      (1ULL << 62)
 #define MENVCFG_STCE                       (1ULL << 63)
@@ -721,11 +722,13 @@ typedef enum RISCVException {
 #define SENVCFG_CBIE                       MENVCFG_CBIE
 #define SENVCFG_CBCFE                      MENVCFG_CBCFE
 #define SENVCFG_CBZE                       MENVCFG_CBZE
+#define SENVCFG_PMM                        MENVCFG_PMM
 
 #define HENVCFG_FIOM                       MENVCFG_FIOM
 #define HENVCFG_CBIE                       MENVCFG_CBIE
 #define HENVCFG_CBCFE                      MENVCFG_CBCFE
 #define HENVCFG_CBZE                       MENVCFG_CBZE
+#define HENVCFG_PMM                        MENVCFG_PMM
 #define HENVCFG_ADUE                       MENVCFG_ADUE
 #define HENVCFG_PBMTE                      MENVCFG_PBMTE
 #define HENVCFG_STCE                       MENVCFG_STCE
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index e1e4f32698..9ecdc792c5 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -120,6 +120,9 @@ struct RISCVCPUConfig {
     bool ext_ssaia;
     bool ext_sscofpmf;
     bool ext_smepmp;
+    bool ext_ssnpm;
+    bool ext_smnpm;
+    bool ext_smmpm;
     bool rvv_ta_all_1s;
     bool rvv_ma_all_1s;
 
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 4b2c932564..45b548eb0b 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -530,6 +530,9 @@ static RISCVException have_mseccfg(CPURISCVState *env, int csrno)
     if (riscv_cpu_cfg(env)->ext_zkr) {
         return RISCV_EXCP_NONE;
     }
+    if (riscv_cpu_cfg(env)->ext_smmpm) {
+        return RISCV_EXCP_NONE;
+    }
 
     return RISCV_EXCP_ILLEGAL_INST;
 }
@@ -2083,6 +2086,10 @@ static RISCVException write_menvcfg(CPURISCVState *env, int csrno,
                 (cfg->ext_sstc ? MENVCFG_STCE : 0) |
                 (cfg->ext_svadu ? MENVCFG_ADUE : 0);
     }
+    /* Update PMM field only if the value is valid according to Zjpm v0.8 */
+    if (((val & MENVCFG_PMM) >> 32) != PMM_FIELD_RESERVED) {
+        mask |= MENVCFG_PMM;
+    }
     env->menvcfg = (env->menvcfg & ~mask) | (val & mask);
 
     return RISCV_EXCP_NONE;
@@ -2127,6 +2134,10 @@ static RISCVException write_senvcfg(CPURISCVState *env, int csrno,
                                     target_ulong val)
 {
     uint64_t mask = SENVCFG_FIOM | SENVCFG_CBIE | SENVCFG_CBCFE | SENVCFG_CBZE;
+    /* Update PMM field only if the value is valid according to Zjpm v0.8 */
+    if (((val & SENVCFG_PMM) >> 32) != PMM_FIELD_RESERVED) {
+        mask |= SENVCFG_PMM;
+    }
     RISCVException ret;
 
     ret = smstateen_acc_ok(env, 0, SMSTATEEN0_HSENVCFG);
diff --git a/target/riscv/machine.c b/target/riscv/machine.c
index 64ab66e332..bbbb28f373 100644
--- a/target/riscv/machine.c
+++ b/target/riscv/machine.c
@@ -152,15 +152,19 @@ static const VMStateDescription vmstate_vector = {
 
 static bool pointermasking_needed(void *opaque)
 {
-    return false;
+    RISCVCPU *cpu = opaque;
+    return cpu->cfg.ext_ssnpm || cpu->cfg.ext_smnpm || cpu->cfg.ext_smmpm;
 }
 
 static const VMStateDescription vmstate_pointermasking = {
     .name = "cpu/pointer_masking",
-    .version_id = 1,
-    .minimum_version_id = 1,
+    .version_id = 2,
+    .minimum_version_id = 2,
     .needed = pointermasking_needed,
     .fields = (const VMStateField[]) {
+        VMSTATE_UINTTL(env.mseccfg, RISCVCPU),
+        VMSTATE_UINTTL(env.senvcfg, RISCVCPU),
+        VMSTATE_UINTTL(env.menvcfg, RISCVCPU),
         VMSTATE_END_OF_LIST()
     }
 };
diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
index 2a76b611a0..7ddb9dbf0b 100644
--- a/target/riscv/pmp.c
+++ b/target/riscv/pmp.c
@@ -574,6 +574,12 @@ target_ulong pmpaddr_csr_read(CPURISCVState *env, uint32_t addr_index)
 void mseccfg_csr_write(CPURISCVState *env, target_ulong val)
 {
     int i;
+    uint64_t mask = MSECCFG_MMWP | MSECCFG_MML;
+
+    /* Update PMM field only if the value is valid according to Zjpm v0.8 */
+    if (((val & MSECCFG_PMM) >> 32) != PMM_FIELD_RESERVED) {
+        mask |= MSECCFG_PMM;
+    }
 
     trace_mseccfg_csr_write(env->mhartid, val);
 
@@ -589,12 +595,13 @@ void mseccfg_csr_write(CPURISCVState *env, target_ulong val)
 
     if (riscv_cpu_cfg(env)->ext_smepmp) {
         /* Sticky bits */
-        val |= (env->mseccfg & (MSECCFG_MMWP | MSECCFG_MML));
-        if ((val ^ env->mseccfg) & (MSECCFG_MMWP | MSECCFG_MML)) {
+        val |= (env->mseccfg & mask);
+        if ((val ^ env->mseccfg) & mask) {
             tlb_flush(env_cpu(env));
         }
     } else {
-        val &= ~(MSECCFG_MMWP | MSECCFG_MML | MSECCFG_RLB);
+        mask |= MSECCFG_RLB;
+        val &= ~(mask);
     }
 
     env->mseccfg = val;
diff --git a/target/riscv/pmp.h b/target/riscv/pmp.h
index f5c10ce85c..ccff0eb9b6 100644
--- a/target/riscv/pmp.h
+++ b/target/riscv/pmp.h
@@ -40,11 +40,12 @@ typedef enum {
 } pmp_am_t;
 
 typedef enum {
-    MSECCFG_MML   = 1 << 0,
-    MSECCFG_MMWP  = 1 << 1,
-    MSECCFG_RLB   = 1 << 2,
-    MSECCFG_USEED = 1 << 8,
-    MSECCFG_SSEED = 1 << 9
+    MSECCFG_MML       = 1 << 0,
+    MSECCFG_MMWP      = 1 << 1,
+    MSECCFG_RLB       = 1 << 2,
+    MSECCFG_USEED     = 1 << 8,
+    MSECCFG_SSEED     = 1 << 9,
+    MSECCFG_PMM       = 3UL << 32,
 } mseccfg_field_t;
 
 typedef struct {
-- 
2.34.1

Re: [PATCH v9 2/6] target/riscv: Add new CSR fields for S{sn, mn, m}pm extensions as part of Zjpm v0.8

[liwei]

On 2024/5/11 18:10, Alexey Baturo wrote:
> From: Alexey Baturo <baturo.alexey@gmail.com>
>
> Signed-off-by: Alexey Baturo <baturo.alexey@gmail.com>
>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>   target/riscv/cpu.h      |  8 ++++++++
>   target/riscv/cpu_bits.h |  3 +++
>   target/riscv/cpu_cfg.h  |  3 +++
>   target/riscv/csr.c      | 11 +++++++++++
>   target/riscv/machine.c  | 10 +++++++---
>   target/riscv/pmp.c      | 13 ++++++++++---
>   target/riscv/pmp.h      | 11 ++++++-----
>   7 files changed, 48 insertions(+), 11 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 232521bb87..52b6ba73c8 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -121,6 +121,14 @@ typedef enum {
>       EXT_STATUS_DIRTY,
>   } RISCVExtStatus;
>   
> +/* Enum holds PMM field values for Zjpm v0.8 extension */
> +typedef enum {
> +    PMM_FIELD_DISABLED = 0,
> +    PMM_FIELD_RESERVED = 1,
> +    PMM_FIELD_PMLEN7   = 2,
> +    PMM_FIELD_PMLEN16  = 3,
> +} RISCVPmPmm;
> +
>   #define MMU_USER_IDX 3
>   
>   #define MAX_RISCV_PMPS (16)
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index da16ba236a..13ce2218d1 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -708,6 +708,7 @@ typedef enum RISCVException {
>   #define MENVCFG_CBIE                       (3UL << 4)
>   #define MENVCFG_CBCFE                      BIT(6)
>   #define MENVCFG_CBZE                       BIT(7)
> +#define MENVCFG_PMM                        (3ULL << 32)
>   #define MENVCFG_ADUE                       (1ULL << 61)
>   #define MENVCFG_PBMTE                      (1ULL << 62)
>   #define MENVCFG_STCE                       (1ULL << 63)
> @@ -721,11 +722,13 @@ typedef enum RISCVException {
>   #define SENVCFG_CBIE                       MENVCFG_CBIE
>   #define SENVCFG_CBCFE                      MENVCFG_CBCFE
>   #define SENVCFG_CBZE                       MENVCFG_CBZE
> +#define SENVCFG_PMM                        MENVCFG_PMM
>   
>   #define HENVCFG_FIOM                       MENVCFG_FIOM
>   #define HENVCFG_CBIE                       MENVCFG_CBIE
>   #define HENVCFG_CBCFE                      MENVCFG_CBCFE
>   #define HENVCFG_CBZE                       MENVCFG_CBZE
> +#define HENVCFG_PMM                        MENVCFG_PMM
>   #define HENVCFG_ADUE                       MENVCFG_ADUE
>   #define HENVCFG_PBMTE                      MENVCFG_PBMTE
>   #define HENVCFG_STCE                       MENVCFG_STCE
> diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
> index e1e4f32698..9ecdc792c5 100644
> --- a/target/riscv/cpu_cfg.h
> +++ b/target/riscv/cpu_cfg.h
> @@ -120,6 +120,9 @@ struct RISCVCPUConfig {
>       bool ext_ssaia;
>       bool ext_sscofpmf;
>       bool ext_smepmp;
> +    bool ext_ssnpm;
> +    bool ext_smnpm;
> +    bool ext_smmpm;
>       bool rvv_ta_all_1s;
>       bool rvv_ma_all_1s;
>   
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 4b2c932564..45b548eb0b 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -530,6 +530,9 @@ static RISCVException have_mseccfg(CPURISCVState *env, int csrno)
>       if (riscv_cpu_cfg(env)->ext_zkr) {
>           return RISCV_EXCP_NONE;
>       }
> +    if (riscv_cpu_cfg(env)->ext_smmpm) {
> +        return RISCV_EXCP_NONE;
> +    }
>   
>       return RISCV_EXCP_ILLEGAL_INST;
>   }
> @@ -2083,6 +2086,10 @@ static RISCVException write_menvcfg(CPURISCVState *env, int csrno,
>                   (cfg->ext_sstc ? MENVCFG_STCE : 0) |
>                   (cfg->ext_svadu ? MENVCFG_ADUE : 0);
>       }
> +    /* Update PMM field only if the value is valid according to Zjpm v0.8 */
> +    if (((val & MENVCFG_PMM) >> 32) != PMM_FIELD_RESERVED) {
> +        mask |= MENVCFG_PMM;
> +    }

Extension and RV64 check seems missed here.This field is added by smnpm, 
So I think it's can only be changed only when smnpm is enabled.

>       env->menvcfg = (env->menvcfg & ~mask) | (val & mask);
>   
>       return RISCV_EXCP_NONE;
> @@ -2127,6 +2134,10 @@ static RISCVException write_senvcfg(CPURISCVState *env, int csrno,
>                                       target_ulong val)
>   {
>       uint64_t mask = SENVCFG_FIOM | SENVCFG_CBIE | SENVCFG_CBCFE | SENVCFG_CBZE;
> +    /* Update PMM field only if the value is valid according to Zjpm v0.8 */
> +    if (((val & SENVCFG_PMM) >> 32) != PMM_FIELD_RESERVED) {
> +        mask |= SENVCFG_PMM;
> +    }
>       RISCVException ret
similar to above.
> ;
>   
>       ret = smstateen_acc_ok(env, 0, SMSTATEEN0_HSENVCFG);
> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> index 64ab66e332..bbbb28f373 100644
> --- a/target/riscv/machine.c
> +++ b/target/riscv/machine.c
> @@ -152,15 +152,19 @@ static const VMStateDescription vmstate_vector = {
>   
>   static bool pointermasking_needed(void *opaque)
>   {
> -    return false;
> +    RISCVCPU *cpu = opaque;
> +    return cpu->cfg.ext_ssnpm || cpu->cfg.ext_smnpm || cpu->cfg.ext_smmpm;
>   }
>   
>   static const VMStateDescription vmstate_pointermasking = {
>       .name = "cpu/pointer_masking",
> -    .version_id = 1,
> -    .minimum_version_id = 1,
> +    .version_id = 2,
> +    .minimum_version_id = 2,
>       .needed = pointermasking_needed,
>       .fields = (const VMStateField[]) {
> +        VMSTATE_UINTTL(env.mseccfg, RISCVCPU),
> +        VMSTATE_UINTTL(env.senvcfg, RISCVCPU),
> +        VMSTATE_UINTTL(env.menvcfg, RISCVCPU),
>           VMSTATE_END_OF_LIST()
>       }
>   };
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index 2a76b611a0..7ddb9dbf0b 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -574,6 +574,12 @@ target_ulong pmpaddr_csr_read(CPURISCVState *env, uint32_t addr_index)
>   void mseccfg_csr_write(CPURISCVState *env, target_ulong val)
>   {
>       int i;
> +    uint64_t mask = MSECCFG_MMWP | MSECCFG_MML;
> +
> +    /* Update PMM field only if the value is valid according to Zjpm v0.8 */
> +    if (((val & MSECCFG_PMM) >> 32) != PMM_FIELD_RESERVED) {
> +        mask |= MSECCFG_PMM;
> +    }
>   

similar to above too.

Regards,

Weiwei Li

>       trace_mseccfg_csr_write(env->mhartid, val);
>   
> @@ -589,12 +595,13 @@ void mseccfg_csr_write(CPURISCVState *env, target_ulong val)
>   
>       if (riscv_cpu_cfg(env)->ext_smepmp) {
>           /* Sticky bits */
> -        val |= (env->mseccfg & (MSECCFG_MMWP | MSECCFG_MML));
> -        if ((val ^ env->mseccfg) & (MSECCFG_MMWP | MSECCFG_MML)) {
> +        val |= (env->mseccfg & mask);
> +        if ((val ^ env->mseccfg) & mask) {
>               tlb_flush(env_cpu(env));
>           }
>       } else {
> -        val &= ~(MSECCFG_MMWP | MSECCFG_MML | MSECCFG_RLB);
> +        mask |= MSECCFG_RLB;
> +        val &= ~(mask);
>       }
>   
>       env->mseccfg = val;
> diff --git a/target/riscv/pmp.h b/target/riscv/pmp.h
> index f5c10ce85c..ccff0eb9b6 100644
> --- a/target/riscv/pmp.h
> +++ b/target/riscv/pmp.h
> @@ -40,11 +40,12 @@ typedef enum {
>   } pmp_am_t;
>   
>   typedef enum {
> -    MSECCFG_MML   = 1 << 0,
> -    MSECCFG_MMWP  = 1 << 1,
> -    MSECCFG_RLB   = 1 << 2,
> -    MSECCFG_USEED = 1 << 8,
> -    MSECCFG_SSEED = 1 << 9
> +    MSECCFG_MML       = 1 << 0,
> +    MSECCFG_MMWP      = 1 << 1,
> +    MSECCFG_RLB       = 1 << 2,
> +    MSECCFG_USEED     = 1 << 8,
> +    MSECCFG_SSEED     = 1 << 9,
> +    MSECCFG_PMM       = 3UL << 32,
>   } mseccfg_field_t;
>   
>   typedef struct {

Re: [PATCH v9 2/6] target/riscv: Add new CSR fields for S{sn, mn, m}pm extensions as part of Zjpm v0.8

[Alistair Francis]

On Sat, May 11, 2024 at 8:12 PM Alexey Baturo <baturo.alexey@gmail.com> wrote:
>
> From: Alexey Baturo <baturo.alexey@gmail.com>
>
> Signed-off-by: Alexey Baturo <baturo.alexey@gmail.com>
>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  target/riscv/cpu.h      |  8 ++++++++
>  target/riscv/cpu_bits.h |  3 +++
>  target/riscv/cpu_cfg.h  |  3 +++
>  target/riscv/csr.c      | 11 +++++++++++
>  target/riscv/machine.c  | 10 +++++++---
>  target/riscv/pmp.c      | 13 ++++++++++---
>  target/riscv/pmp.h      | 11 ++++++-----
>  7 files changed, 48 insertions(+), 11 deletions(-)

This patch generates warnings/errors

include/qemu/compiler.h:70:35: error: invalid operands to binary -
(have ‘uint32_t *’ {aka ‘unsigned int *’} and ‘uint64_t *’ {aka ‘long
unsigned int *’})
   70 | #define type_check(t1,t2) ((t1*)0 - (t2*)0)
      |                                   ^
...
../target/riscv/machine.c:167:9: note: in expansion of macro ‘VMSTATE_UINTTL’
  167 |         VMSTATE_UINTTL(env.menvcfg, RISCVCPU),
      |         ^~~~~~~~~~~~~~

Alistair

Re: [PATCH v9 2/6] target/riscv: Add new CSR fields for S{sn, mn, m}pm extensions as part of Zjpm v0.8

[Daniel Henrique Barboza]

On 5/11/24 7:10 AM, Alexey Baturo wrote:
> From: Alexey Baturo <baturo.alexey@gmail.com>
> 
> Signed-off-by: Alexey Baturo <baturo.alexey@gmail.com>
> 
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>   target/riscv/cpu.h      |  8 ++++++++
>   target/riscv/cpu_bits.h |  3 +++
>   target/riscv/cpu_cfg.h  |  3 +++
>   target/riscv/csr.c      | 11 +++++++++++
>   target/riscv/machine.c  | 10 +++++++---
>   target/riscv/pmp.c      | 13 ++++++++++---
>   target/riscv/pmp.h      | 11 ++++++-----
>   7 files changed, 48 insertions(+), 11 deletions(-)
> 
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 232521bb87..52b6ba73c8 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -121,6 +121,14 @@ typedef enum {
>       EXT_STATUS_DIRTY,
>   } RISCVExtStatus;
>   
> +/* Enum holds PMM field values for Zjpm v0.8 extension */
> +typedef enum {
> +    PMM_FIELD_DISABLED = 0,
> +    PMM_FIELD_RESERVED = 1,
> +    PMM_FIELD_PMLEN7   = 2,
> +    PMM_FIELD_PMLEN16  = 3,
> +} RISCVPmPmm;
> +
>   #define MMU_USER_IDX 3
>   
>   #define MAX_RISCV_PMPS (16)
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index da16ba236a..13ce2218d1 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -708,6 +708,7 @@ typedef enum RISCVException {
>   #define MENVCFG_CBIE                       (3UL << 4)
>   #define MENVCFG_CBCFE                      BIT(6)
>   #define MENVCFG_CBZE                       BIT(7)
> +#define MENVCFG_PMM                        (3ULL << 32)
>   #define MENVCFG_ADUE                       (1ULL << 61)
>   #define MENVCFG_PBMTE                      (1ULL << 62)
>   #define MENVCFG_STCE                       (1ULL << 63)
> @@ -721,11 +722,13 @@ typedef enum RISCVException {
>   #define SENVCFG_CBIE                       MENVCFG_CBIE
>   #define SENVCFG_CBCFE                      MENVCFG_CBCFE
>   #define SENVCFG_CBZE                       MENVCFG_CBZE
> +#define SENVCFG_PMM                        MENVCFG_PMM
>   
>   #define HENVCFG_FIOM                       MENVCFG_FIOM
>   #define HENVCFG_CBIE                       MENVCFG_CBIE
>   #define HENVCFG_CBCFE                      MENVCFG_CBCFE
>   #define HENVCFG_CBZE                       MENVCFG_CBZE
> +#define HENVCFG_PMM                        MENVCFG_PMM
>   #define HENVCFG_ADUE                       MENVCFG_ADUE
>   #define HENVCFG_PBMTE                      MENVCFG_PBMTE
>   #define HENVCFG_STCE                       MENVCFG_STCE
> diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
> index e1e4f32698..9ecdc792c5 100644
> --- a/target/riscv/cpu_cfg.h
> +++ b/target/riscv/cpu_cfg.h
> @@ -120,6 +120,9 @@ struct RISCVCPUConfig {
>       bool ext_ssaia;
>       bool ext_sscofpmf;
>       bool ext_smepmp;
> +    bool ext_ssnpm;
> +    bool ext_smnpm;
> +    bool ext_smmpm;
>       bool rvv_ta_all_1s;
>       bool rvv_ma_all_1s;
>   
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 4b2c932564..45b548eb0b 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -530,6 +530,9 @@ static RISCVException have_mseccfg(CPURISCVState *env, int csrno)
>       if (riscv_cpu_cfg(env)->ext_zkr) {
>           return RISCV_EXCP_NONE;
>       }
> +    if (riscv_cpu_cfg(env)->ext_smmpm) {
> +        return RISCV_EXCP_NONE;
> +    }
>   
>       return RISCV_EXCP_ILLEGAL_INST;
>   }
> @@ -2083,6 +2086,10 @@ static RISCVException write_menvcfg(CPURISCVState *env, int csrno,
>                   (cfg->ext_sstc ? MENVCFG_STCE : 0) |
>                   (cfg->ext_svadu ? MENVCFG_ADUE : 0);
>       }
> +    /* Update PMM field only if the value is valid according to Zjpm v0.8 */
> +    if (((val & MENVCFG_PMM) >> 32) != PMM_FIELD_RESERVED) {
> +        mask |= MENVCFG_PMM;
> +    }
>       env->menvcfg = (env->menvcfg & ~mask) | (val & mask);
>   
>       return RISCV_EXCP_NONE;
> @@ -2127,6 +2134,10 @@ static RISCVException write_senvcfg(CPURISCVState *env, int csrno,
>                                       target_ulong val)
>   {
>       uint64_t mask = SENVCFG_FIOM | SENVCFG_CBIE | SENVCFG_CBCFE | SENVCFG_CBZE;
> +    /* Update PMM field only if the value is valid according to Zjpm v0.8 */
> +    if (((val & SENVCFG_PMM) >> 32) != PMM_FIELD_RESERVED) {
> +        mask |= SENVCFG_PMM;
> +    }
>       RISCVException ret;
>   
>       ret = smstateen_acc_ok(env, 0, SMSTATEEN0_HSENVCFG);
> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> index 64ab66e332..bbbb28f373 100644
> --- a/target/riscv/machine.c
> +++ b/target/riscv/machine.c
> @@ -152,15 +152,19 @@ static const VMStateDescription vmstate_vector = {
>   
>   static bool pointermasking_needed(void *opaque)
>   {
> -    return false;
> +    RISCVCPU *cpu = opaque;
> +    return cpu->cfg.ext_ssnpm || cpu->cfg.ext_smnpm || cpu->cfg.ext_smmpm;
>   }
>   
>   static const VMStateDescription vmstate_pointermasking = {
>       .name = "cpu/pointer_masking",
> -    .version_id = 1,
> -    .minimum_version_id = 1,
> +    .version_id = 2,
> +    .minimum_version_id = 2,
>       .needed = pointermasking_needed,
>       .fields = (const VMStateField[]) {
> +        VMSTATE_UINTTL(env.mseccfg, RISCVCPU),
> +        VMSTATE_UINTTL(env.senvcfg, RISCVCPU),
> +        VMSTATE_UINTTL(env.menvcfg, RISCVCPU),
>           VMSTATE_END_OF_LIST()
>       }


These fields are already being added in the machine vmstate via vmstate_envcfg,
added via commit 29a9ec9b, "target/riscv: Add *envcfg* CSRs support":


static const VMStateDescription vmstate_envcfg = {
     .name = "cpu/envcfg",
     .version_id = 1,
     .minimum_version_id = 1,
     .needed = envcfg_needed,
     .fields = (const VMStateField[]) {
         VMSTATE_UINT64(env.menvcfg, RISCVCPU),
         VMSTATE_UINTTL(env.senvcfg, RISCVCPU),
         VMSTATE_UINT64(env.henvcfg, RISCVCPU),
         VMSTATE_END_OF_LIST()
     }
};

You need to keep both pointermasking_needed() and vmstate_pointermasking
untouched. Doing that will also fix the build problem that Alistair mentioned
in his reply.


Thanks,

Daniel

>   };
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index 2a76b611a0..7ddb9dbf0b 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -574,6 +574,12 @@ target_ulong pmpaddr_csr_read(CPURISCVState *env, uint32_t addr_index)
>   void mseccfg_csr_write(CPURISCVState *env, target_ulong val)
>   {
>       int i;
> +    uint64_t mask = MSECCFG_MMWP | MSECCFG_MML;
> +
> +    /* Update PMM field only if the value is valid according to Zjpm v0.8 */
> +    if (((val & MSECCFG_PMM) >> 32) != PMM_FIELD_RESERVED) {
> +        mask |= MSECCFG_PMM;
> +    }
>   
>       trace_mseccfg_csr_write(env->mhartid, val);
>   
> @@ -589,12 +595,13 @@ void mseccfg_csr_write(CPURISCVState *env, target_ulong val)
>   
>       if (riscv_cpu_cfg(env)->ext_smepmp) {
>           /* Sticky bits */
> -        val |= (env->mseccfg & (MSECCFG_MMWP | MSECCFG_MML));
> -        if ((val ^ env->mseccfg) & (MSECCFG_MMWP | MSECCFG_MML)) {
> +        val |= (env->mseccfg & mask);
> +        if ((val ^ env->mseccfg) & mask) {
>               tlb_flush(env_cpu(env));
>           }
>       } else {
> -        val &= ~(MSECCFG_MMWP | MSECCFG_MML | MSECCFG_RLB);
> +        mask |= MSECCFG_RLB;
> +        val &= ~(mask);
>       }
>   
>       env->mseccfg = val;
> diff --git a/target/riscv/pmp.h b/target/riscv/pmp.h
> index f5c10ce85c..ccff0eb9b6 100644
> --- a/target/riscv/pmp.h
> +++ b/target/riscv/pmp.h
> @@ -40,11 +40,12 @@ typedef enum {
>   } pmp_am_t;
>   
>   typedef enum {
> -    MSECCFG_MML   = 1 << 0,
> -    MSECCFG_MMWP  = 1 << 1,
> -    MSECCFG_RLB   = 1 << 2,
> -    MSECCFG_USEED = 1 << 8,
> -    MSECCFG_SSEED = 1 << 9
> +    MSECCFG_MML       = 1 << 0,
> +    MSECCFG_MMWP      = 1 << 1,
> +    MSECCFG_RLB       = 1 << 2,
> +    MSECCFG_USEED     = 1 << 8,
> +    MSECCFG_SSEED     = 1 << 9,
> +    MSECCFG_PMM       = 3UL << 32,
>   } mseccfg_field_t;
>   
>   typedef struct {

Re: [PATCH v9 2/6] target/riscv: Add new CSR fields for S{sn, mn, m}pm extensions as part of Zjpm v0.8

[Frank Chang]

Alexey Baturo <baturo.alexey@gmail.com> 於 2024年5月11日 週六 下午6:12寫道：
>
> From: Alexey Baturo <baturo.alexey@gmail.com>
>
> Signed-off-by: Alexey Baturo <baturo.alexey@gmail.com>
>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  target/riscv/cpu.h      |  8 ++++++++
>  target/riscv/cpu_bits.h |  3 +++
>  target/riscv/cpu_cfg.h  |  3 +++
>  target/riscv/csr.c      | 11 +++++++++++
>  target/riscv/machine.c  | 10 +++++++---
>  target/riscv/pmp.c      | 13 ++++++++++---
>  target/riscv/pmp.h      | 11 ++++++-----
>  7 files changed, 48 insertions(+), 11 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 232521bb87..52b6ba73c8 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -121,6 +121,14 @@ typedef enum {
>      EXT_STATUS_DIRTY,
>  } RISCVExtStatus;
>
> +/* Enum holds PMM field values for Zjpm v0.8 extension */
> +typedef enum {
> +    PMM_FIELD_DISABLED = 0,
> +    PMM_FIELD_RESERVED = 1,
> +    PMM_FIELD_PMLEN7   = 2,
> +    PMM_FIELD_PMLEN16  = 3,
> +} RISCVPmPmm;
> +
>  #define MMU_USER_IDX 3
>
>  #define MAX_RISCV_PMPS (16)
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index da16ba236a..13ce2218d1 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -708,6 +708,7 @@ typedef enum RISCVException {
>  #define MENVCFG_CBIE                       (3UL << 4)
>  #define MENVCFG_CBCFE                      BIT(6)
>  #define MENVCFG_CBZE                       BIT(7)
> +#define MENVCFG_PMM                        (3ULL << 32)
>  #define MENVCFG_ADUE                       (1ULL << 61)
>  #define MENVCFG_PBMTE                      (1ULL << 62)
>  #define MENVCFG_STCE                       (1ULL << 63)
> @@ -721,11 +722,13 @@ typedef enum RISCVException {
>  #define SENVCFG_CBIE                       MENVCFG_CBIE
>  #define SENVCFG_CBCFE                      MENVCFG_CBCFE
>  #define SENVCFG_CBZE                       MENVCFG_CBZE
> +#define SENVCFG_PMM                        MENVCFG_PMM
>
>  #define HENVCFG_FIOM                       MENVCFG_FIOM
>  #define HENVCFG_CBIE                       MENVCFG_CBIE
>  #define HENVCFG_CBCFE                      MENVCFG_CBCFE
>  #define HENVCFG_CBZE                       MENVCFG_CBZE
> +#define HENVCFG_PMM                        MENVCFG_PMM
>  #define HENVCFG_ADUE                       MENVCFG_ADUE
>  #define HENVCFG_PBMTE                      MENVCFG_PBMTE
>  #define HENVCFG_STCE                       MENVCFG_STCE
> diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
> index e1e4f32698..9ecdc792c5 100644
> --- a/target/riscv/cpu_cfg.h
> +++ b/target/riscv/cpu_cfg.h
> @@ -120,6 +120,9 @@ struct RISCVCPUConfig {
>      bool ext_ssaia;
>      bool ext_sscofpmf;
>      bool ext_smepmp;
> +    bool ext_ssnpm;
> +    bool ext_smnpm;
> +    bool ext_smmpm;
>      bool rvv_ta_all_1s;
>      bool rvv_ma_all_1s;
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 4b2c932564..45b548eb0b 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -530,6 +530,9 @@ static RISCVException have_mseccfg(CPURISCVState *env, int csrno)
>      if (riscv_cpu_cfg(env)->ext_zkr) {
>          return RISCV_EXCP_NONE;
>      }
> +    if (riscv_cpu_cfg(env)->ext_smmpm) {
> +        return RISCV_EXCP_NONE;
> +    }
>
>      return RISCV_EXCP_ILLEGAL_INST;
>  }
> @@ -2083,6 +2086,10 @@ static RISCVException write_menvcfg(CPURISCVState *env, int csrno,
>                  (cfg->ext_sstc ? MENVCFG_STCE : 0) |
>                  (cfg->ext_svadu ? MENVCFG_ADUE : 0);
>      }
> +    /* Update PMM field only if the value is valid according to Zjpm v0.8 */
> +    if (((val & MENVCFG_PMM) >> 32) != PMM_FIELD_RESERVED) {
> +        mask |= MENVCFG_PMM;
> +    }
>      env->menvcfg = (env->menvcfg & ~mask) | (val & mask);
>
>      return RISCV_EXCP_NONE;
> @@ -2127,6 +2134,10 @@ static RISCVException write_senvcfg(CPURISCVState *env, int csrno,
>                                      target_ulong val)
>  {
>      uint64_t mask = SENVCFG_FIOM | SENVCFG_CBIE | SENVCFG_CBCFE | SENVCFG_CBZE;
> +    /* Update PMM field only if the value is valid according to Zjpm v0.8 */
> +    if (((val & SENVCFG_PMM) >> 32) != PMM_FIELD_RESERVED) {
> +        mask |= SENVCFG_PMM;
> +    }
>      RISCVException ret;
>
>      ret = smstateen_acc_ok(env, 0, SMSTATEEN0_HSENVCFG);
> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> index 64ab66e332..bbbb28f373 100644
> --- a/target/riscv/machine.c
> +++ b/target/riscv/machine.c
> @@ -152,15 +152,19 @@ static const VMStateDescription vmstate_vector = {
>
>  static bool pointermasking_needed(void *opaque)
>  {
> -    return false;
> +    RISCVCPU *cpu = opaque;
> +    return cpu->cfg.ext_ssnpm || cpu->cfg.ext_smnpm || cpu->cfg.ext_smmpm;
>  }
>
>  static const VMStateDescription vmstate_pointermasking = {
>      .name = "cpu/pointer_masking",
> -    .version_id = 1,
> -    .minimum_version_id = 1,
> +    .version_id = 2,
> +    .minimum_version_id = 2,
>      .needed = pointermasking_needed,
>      .fields = (const VMStateField[]) {
> +        VMSTATE_UINTTL(env.mseccfg, RISCVCPU),
> +        VMSTATE_UINTTL(env.senvcfg, RISCVCPU),
> +        VMSTATE_UINTTL(env.menvcfg, RISCVCPU),
>          VMSTATE_END_OF_LIST()
>      }
>  };
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index 2a76b611a0..7ddb9dbf0b 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -574,6 +574,12 @@ target_ulong pmpaddr_csr_read(CPURISCVState *env, uint32_t addr_index)
>  void mseccfg_csr_write(CPURISCVState *env, target_ulong val)
>  {
>      int i;
> +    uint64_t mask = MSECCFG_MMWP | MSECCFG_MML;
> +
> +    /* Update PMM field only if the value is valid according to Zjpm v0.8 */
> +    if (((val & MSECCFG_PMM) >> 32) != PMM_FIELD_RESERVED) {
> +        mask |= MSECCFG_PMM;
> +    }
>
>      trace_mseccfg_csr_write(env->mhartid, val);
>
> @@ -589,12 +595,13 @@ void mseccfg_csr_write(CPURISCVState *env, target_ulong val)
>
>      if (riscv_cpu_cfg(env)->ext_smepmp) {
>          /* Sticky bits */
> -        val |= (env->mseccfg & (MSECCFG_MMWP | MSECCFG_MML));
> -        if ((val ^ env->mseccfg) & (MSECCFG_MMWP | MSECCFG_MML)) {
> +        val |= (env->mseccfg & mask);
> +        if ((val ^ env->mseccfg) & mask) {
>              tlb_flush(env_cpu(env));
>          }
>      } else {
> -        val &= ~(MSECCFG_MMWP | MSECCFG_MML | MSECCFG_RLB);
> +        mask |= MSECCFG_RLB;
> +        val &= ~(mask);
>      }
>
>      env->mseccfg = val;
> diff --git a/target/riscv/pmp.h b/target/riscv/pmp.h
> index f5c10ce85c..ccff0eb9b6 100644
> --- a/target/riscv/pmp.h
> +++ b/target/riscv/pmp.h
> @@ -40,11 +40,12 @@ typedef enum {
>  } pmp_am_t;
>
>  typedef enum {
> -    MSECCFG_MML   = 1 << 0,
> -    MSECCFG_MMWP  = 1 << 1,
> -    MSECCFG_RLB   = 1 << 2,
> -    MSECCFG_USEED = 1 << 8,
> -    MSECCFG_SSEED = 1 << 9
> +    MSECCFG_MML       = 1 << 0,
> +    MSECCFG_MMWP      = 1 << 1,
> +    MSECCFG_RLB       = 1 << 2,
> +    MSECCFG_USEED     = 1 << 8,
> +    MSECCFG_SSEED     = 1 << 9,
> +    MSECCFG_PMM       = 3UL << 32,

Should use '3ULL' to left shift bits more than 31 bits.
Otherwise, it would generate the error:
"error: left shift count >= width of type [-Werror=shift-count-overflow]"

Regards,
Frank Chang

>  } mseccfg_field_t;
>
>  typedef struct {
> --
> 2.34.1
>
>

Re: [PATCH v9 2/6] target/riscv: Add new CSR fields for S{sn, mn, m}pm extensions as part of Zjpm v0.8

[Frank Chang]

Alexey Baturo <baturo.alexey@gmail.com> 於 2024年5月11日 週六 下午6:12寫道：
>
> From: Alexey Baturo <baturo.alexey@gmail.com>
>
> Signed-off-by: Alexey Baturo <baturo.alexey@gmail.com>
>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  target/riscv/cpu.h      |  8 ++++++++
>  target/riscv/cpu_bits.h |  3 +++
>  target/riscv/cpu_cfg.h  |  3 +++
>  target/riscv/csr.c      | 11 +++++++++++
>  target/riscv/machine.c  | 10 +++++++---
>  target/riscv/pmp.c      | 13 ++++++++++---
>  target/riscv/pmp.h      | 11 ++++++-----
>  7 files changed, 48 insertions(+), 11 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 232521bb87..52b6ba73c8 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -121,6 +121,14 @@ typedef enum {
>      EXT_STATUS_DIRTY,
>  } RISCVExtStatus;
>
> +/* Enum holds PMM field values for Zjpm v0.8 extension */
> +typedef enum {
> +    PMM_FIELD_DISABLED = 0,
> +    PMM_FIELD_RESERVED = 1,
> +    PMM_FIELD_PMLEN7   = 2,
> +    PMM_FIELD_PMLEN16  = 3,
> +} RISCVPmPmm;
> +
>  #define MMU_USER_IDX 3
>
>  #define MAX_RISCV_PMPS (16)
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index da16ba236a..13ce2218d1 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -708,6 +708,7 @@ typedef enum RISCVException {
>  #define MENVCFG_CBIE                       (3UL << 4)
>  #define MENVCFG_CBCFE                      BIT(6)
>  #define MENVCFG_CBZE                       BIT(7)
> +#define MENVCFG_PMM                        (3ULL << 32)
>  #define MENVCFG_ADUE                       (1ULL << 61)
>  #define MENVCFG_PBMTE                      (1ULL << 62)
>  #define MENVCFG_STCE                       (1ULL << 63)
> @@ -721,11 +722,13 @@ typedef enum RISCVException {
>  #define SENVCFG_CBIE                       MENVCFG_CBIE
>  #define SENVCFG_CBCFE                      MENVCFG_CBCFE
>  #define SENVCFG_CBZE                       MENVCFG_CBZE
> +#define SENVCFG_PMM                        MENVCFG_PMM
>
>  #define HENVCFG_FIOM                       MENVCFG_FIOM
>  #define HENVCFG_CBIE                       MENVCFG_CBIE
>  #define HENVCFG_CBCFE                      MENVCFG_CBCFE
>  #define HENVCFG_CBZE                       MENVCFG_CBZE
> +#define HENVCFG_PMM                        MENVCFG_PMM
>  #define HENVCFG_ADUE                       MENVCFG_ADUE
>  #define HENVCFG_PBMTE                      MENVCFG_PBMTE
>  #define HENVCFG_STCE                       MENVCFG_STCE
> diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
> index e1e4f32698..9ecdc792c5 100644
> --- a/target/riscv/cpu_cfg.h
> +++ b/target/riscv/cpu_cfg.h
> @@ -120,6 +120,9 @@ struct RISCVCPUConfig {
>      bool ext_ssaia;
>      bool ext_sscofpmf;
>      bool ext_smepmp;
> +    bool ext_ssnpm;
> +    bool ext_smnpm;
> +    bool ext_smmpm;
>      bool rvv_ta_all_1s;
>      bool rvv_ma_all_1s;
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 4b2c932564..45b548eb0b 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -530,6 +530,9 @@ static RISCVException have_mseccfg(CPURISCVState *env, int csrno)
>      if (riscv_cpu_cfg(env)->ext_zkr) {
>          return RISCV_EXCP_NONE;
>      }
> +    if (riscv_cpu_cfg(env)->ext_smmpm) {
> +        return RISCV_EXCP_NONE;
> +    }
>
>      return RISCV_EXCP_ILLEGAL_INST;
>  }
> @@ -2083,6 +2086,10 @@ static RISCVException write_menvcfg(CPURISCVState *env, int csrno,
>                  (cfg->ext_sstc ? MENVCFG_STCE : 0) |
>                  (cfg->ext_svadu ? MENVCFG_ADUE : 0);
>      }
> +    /* Update PMM field only if the value is valid according to Zjpm v0.8 */
> +    if (((val & MENVCFG_PMM) >> 32) != PMM_FIELD_RESERVED) {

It's also possible that val is 32-bit for RV32 QEMU.
Right shifting more than 31 bits would generate the error:
'error: right shift count >= width of type [-Werror=shift-count-overflow]'

Regards,
Frank Chang

> +        mask |= MENVCFG_PMM;
> +    }
>      env->menvcfg = (env->menvcfg & ~mask) | (val & mask);
>
>      return RISCV_EXCP_NONE;
> @@ -2127,6 +2134,10 @@ static RISCVException write_senvcfg(CPURISCVState *env, int csrno,
>                                      target_ulong val)
>  {
>      uint64_t mask = SENVCFG_FIOM | SENVCFG_CBIE | SENVCFG_CBCFE | SENVCFG_CBZE;
> +    /* Update PMM field only if the value is valid according to Zjpm v0.8 */
> +    if (((val & SENVCFG_PMM) >> 32) != PMM_FIELD_RESERVED) {
> +        mask |= SENVCFG_PMM;
> +    }
>      RISCVException ret;
>
>      ret = smstateen_acc_ok(env, 0, SMSTATEEN0_HSENVCFG);
> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> index 64ab66e332..bbbb28f373 100644
> --- a/target/riscv/machine.c
> +++ b/target/riscv/machine.c
> @@ -152,15 +152,19 @@ static const VMStateDescription vmstate_vector = {
>
>  static bool pointermasking_needed(void *opaque)
>  {
> -    return false;
> +    RISCVCPU *cpu = opaque;
> +    return cpu->cfg.ext_ssnpm || cpu->cfg.ext_smnpm || cpu->cfg.ext_smmpm;
>  }
>
>  static const VMStateDescription vmstate_pointermasking = {
>      .name = "cpu/pointer_masking",
> -    .version_id = 1,
> -    .minimum_version_id = 1,
> +    .version_id = 2,
> +    .minimum_version_id = 2,
>      .needed = pointermasking_needed,
>      .fields = (const VMStateField[]) {
> +        VMSTATE_UINTTL(env.mseccfg, RISCVCPU),
> +        VMSTATE_UINTTL(env.senvcfg, RISCVCPU),
> +        VMSTATE_UINTTL(env.menvcfg, RISCVCPU),
>          VMSTATE_END_OF_LIST()
>      }
>  };
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index 2a76b611a0..7ddb9dbf0b 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -574,6 +574,12 @@ target_ulong pmpaddr_csr_read(CPURISCVState *env, uint32_t addr_index)
>  void mseccfg_csr_write(CPURISCVState *env, target_ulong val)
>  {
>      int i;
> +    uint64_t mask = MSECCFG_MMWP | MSECCFG_MML;
> +
> +    /* Update PMM field only if the value is valid according to Zjpm v0.8 */
> +    if (((val & MSECCFG_PMM) >> 32) != PMM_FIELD_RESERVED) {
> +        mask |= MSECCFG_PMM;
> +    }
>
>      trace_mseccfg_csr_write(env->mhartid, val);
>
> @@ -589,12 +595,13 @@ void mseccfg_csr_write(CPURISCVState *env, target_ulong val)
>
>      if (riscv_cpu_cfg(env)->ext_smepmp) {
>          /* Sticky bits */
> -        val |= (env->mseccfg & (MSECCFG_MMWP | MSECCFG_MML));
> -        if ((val ^ env->mseccfg) & (MSECCFG_MMWP | MSECCFG_MML)) {
> +        val |= (env->mseccfg & mask);
> +        if ((val ^ env->mseccfg) & mask) {
>              tlb_flush(env_cpu(env));
>          }
>      } else {
> -        val &= ~(MSECCFG_MMWP | MSECCFG_MML | MSECCFG_RLB);
> +        mask |= MSECCFG_RLB;
> +        val &= ~(mask);
>      }
>
>      env->mseccfg = val;
> diff --git a/target/riscv/pmp.h b/target/riscv/pmp.h
> index f5c10ce85c..ccff0eb9b6 100644
> --- a/target/riscv/pmp.h
> +++ b/target/riscv/pmp.h
> @@ -40,11 +40,12 @@ typedef enum {
>  } pmp_am_t;
>
>  typedef enum {
> -    MSECCFG_MML   = 1 << 0,
> -    MSECCFG_MMWP  = 1 << 1,
> -    MSECCFG_RLB   = 1 << 2,
> -    MSECCFG_USEED = 1 << 8,
> -    MSECCFG_SSEED = 1 << 9
> +    MSECCFG_MML       = 1 << 0,
> +    MSECCFG_MMWP      = 1 << 1,
> +    MSECCFG_RLB       = 1 << 2,
> +    MSECCFG_USEED     = 1 << 8,
> +    MSECCFG_SSEED     = 1 << 9,
> +    MSECCFG_PMM       = 3UL << 32,
>  } mseccfg_field_t;
>
>  typedef struct {
> --
> 2.34.1
>
>

[PATCH v9 3/6] target/riscv: Add helper functions to calculate current number of masked bits for pointer masking

[Alexey Baturo]

From: Alexey Baturo <baturo.alexey@gmail.com>

Signed-off-by: Alexey Baturo <baturo.alexey@gmail.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/cpu.h        |  5 ++++
 target/riscv/cpu_helper.c | 58 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 52b6ba73c8..9cac723b19 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -698,8 +698,13 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
 
 bool riscv_cpu_is_32bit(RISCVCPU *cpu);
 
+bool riscv_cpu_virt_mem_enabled(CPURISCVState *env);
+RISCVPmPmm riscv_pm_get_pmm(CPURISCVState *env);
+int riscv_pm_get_pmlen(RISCVPmPmm pmm);
+
 RISCVException riscv_csrr(CPURISCVState *env, int csrno,
                           target_ulong *ret_value);
+
 RISCVException riscv_csrrw(CPURISCVState *env, int csrno,
                            target_ulong *ret_value,
                            target_ulong new_value, target_ulong write_mask);
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index bf58350669..e4a127ca84 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -142,6 +142,64 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
     *pflags = flags;
 }
 
+RISCVPmPmm riscv_pm_get_pmm(CPURISCVState *env)
+{
+    int pmm = 0;
+#ifndef CONFIG_USER_ONLY
+    int priv_mode = cpu_address_mode(env);
+    /* Get current PMM field */
+    switch (priv_mode) {
+    case PRV_M:
+        pmm = riscv_cpu_cfg(env)->ext_smmpm ?
+                  get_field(env->mseccfg, MSECCFG_PMM) : PMM_FIELD_DISABLED;
+        break;
+    case PRV_S:
+        pmm = riscv_cpu_cfg(env)->ext_smnpm ?
+                  get_field(env->menvcfg, MENVCFG_PMM) : PMM_FIELD_DISABLED;
+        break;
+    case PRV_U:
+        pmm = riscv_cpu_cfg(env)->ext_ssnpm ?
+                  get_field(env->senvcfg, SENVCFG_PMM) : PMM_FIELD_DISABLED;
+        break;
+    default:
+        g_assert_not_reached();
+    }
+#endif
+    return pmm;
+}
+
+bool riscv_cpu_virt_mem_enabled(CPURISCVState *env)
+{
+    bool virt_mem_en = false;
+#ifndef CONFIG_USER_ONLY
+    int satp_mode = 0;
+    int priv_mode = cpu_address_mode(env);
+    /* Get current PMM field */
+    if (riscv_cpu_mxl(env) == MXL_RV32) {
+        satp_mode = get_field(env->satp, SATP32_MODE);
+    } else {
+        satp_mode = get_field(env->satp, SATP64_MODE);
+    }
+    virt_mem_en = ((satp_mode != VM_1_10_MBARE) && (priv_mode != PRV_M));
+#endif
+    return virt_mem_en;
+}
+
+int riscv_pm_get_pmlen(RISCVPmPmm pmm)
+{
+    switch (pmm) {
+    case PMM_FIELD_DISABLED:
+        return 0;
+    case PMM_FIELD_PMLEN7:
+        return 7;
+    case PMM_FIELD_PMLEN16:
+        return 16;
+    default:
+        g_assert_not_reached();
+    }
+    return -1;
+}
+
 #ifndef CONFIG_USER_ONLY
 
 /*
-- 
2.34.1

Re: [PATCH v9 3/6] target/riscv: Add helper functions to calculate current number of masked bits for pointer masking

[LIU Zhiwei]

On 2024/5/11 18:10, Alexey Baturo wrote:
> From: Alexey Baturo <baturo.alexey@gmail.com>
>
> Signed-off-by: Alexey Baturo <baturo.alexey@gmail.com>
>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>   target/riscv/cpu.h        |  5 ++++
>   target/riscv/cpu_helper.c | 58 +++++++++++++++++++++++++++++++++++++++
>   2 files changed, 63 insertions(+)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 52b6ba73c8..9cac723b19 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -698,8 +698,13 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
>   
>   bool riscv_cpu_is_32bit(RISCVCPU *cpu);
>   
> +bool riscv_cpu_virt_mem_enabled(CPURISCVState *env);
> +RISCVPmPmm riscv_pm_get_pmm(CPURISCVState *env);
> +int riscv_pm_get_pmlen(RISCVPmPmm pmm);
> +
>   RISCVException riscv_csrr(CPURISCVState *env, int csrno,
>                             target_ulong *ret_value);
> +
>   RISCVException riscv_csrrw(CPURISCVState *env, int csrno,
>                              target_ulong *ret_value,
>                              target_ulong new_value, target_ulong write_mask);
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index bf58350669..e4a127ca84 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -142,6 +142,64 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
>       *pflags = flags;
>   }
>   
> +RISCVPmPmm riscv_pm_get_pmm(CPURISCVState *env)
> +{
> +    int pmm = 0;
> +#ifndef CONFIG_USER_ONLY
> +    int priv_mode = cpu_address_mode(env);

This takes care of the MPRV and MPP, but is not enough.

When MPRV == 1 and MXR == 1, we should ignore the address. In this case, 
we should return PMM_FIELD_DISABLED.

We should also consider the MPV  field, see the  comments below.

> +    /* Get current PMM field */
> +    switch (priv_mode) {
> +    case PRV_M:
> +        pmm = riscv_cpu_cfg(env)->ext_smmpm ?
> +                  get_field(env->mseccfg, MSECCFG_PMM) : PMM_FIELD_DISABLED;
> +        break;
> +    case PRV_S:
> +        pmm = riscv_cpu_cfg(env)->ext_smnpm ?
> +                  get_field(env->menvcfg, MENVCFG_PMM) : PMM_FIELD_DISABLED;
When in virtualization mode, we should use henvcfg instead of menvcfg .
> +        break;
> +    case PRV_U:
> +        pmm = riscv_cpu_cfg(env)->ext_ssnpm ?
> +                  get_field(env->senvcfg, SENVCFG_PMM) : PMM_FIELD_DISABLED;

When Smode is not implemented, we should use smnpm here.

> +        break;
> +    default:
> +        g_assert_not_reached();
> +    }
> +#endif
> +    return pmm;
> +}
> +
> +bool riscv_cpu_virt_mem_enabled(CPURISCVState *env)
> +{
> +    bool virt_mem_en = false;
> +#ifndef CONFIG_USER_ONLY
> +    int satp_mode = 0;
> +    int priv_mode = cpu_address_mode(env);

Or S mode is not implemented.

Zhiwei

> +    /* Get current PMM field */
> +    if (riscv_cpu_mxl(env) == MXL_RV32) {
> +        satp_mode = get_field(env->satp, SATP32_MODE);
> +    } else {
> +        satp_mode = get_field(env->satp, SATP64_MODE);
> +    }
> +    virt_mem_en = ((satp_mode != VM_1_10_MBARE) && (priv_mode != PRV_M));
> +#endif
> +    return virt_mem_en;
> +}
> +
> +int riscv_pm_get_pmlen(RISCVPmPmm pmm)
> +{
> +    switch (pmm) {
> +    case PMM_FIELD_DISABLED:
> +        return 0;
> +    case PMM_FIELD_PMLEN7:
> +        return 7;
> +    case PMM_FIELD_PMLEN16:
> +        return 16;
> +    default:
> +        g_assert_not_reached();
> +    }
> +    return -1;
> +}
> +
>   #ifndef CONFIG_USER_ONLY
>   
>   /*

Re: [PATCH v9 3/6] target/riscv: Add helper functions to calculate current number of masked bits for pointer masking

[LIU Zhiwei]

[-- Attachment #1: Type: text/plain, Size: 3196 bytes --]


On 2024/5/11 18:10, Alexey Baturo wrote:
> From: Alexey Baturo<baturo.alexey@gmail.com>
>
> Signed-off-by: Alexey Baturo<baturo.alexey@gmail.com>
>
> Reviewed-by: Alistair Francis<alistair.francis@wdc.com>
> ---
>   target/riscv/cpu.h        |  5 ++++
>   target/riscv/cpu_helper.c | 58 +++++++++++++++++++++++++++++++++++++++
>   2 files changed, 63 insertions(+)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 52b6ba73c8..9cac723b19 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -698,8 +698,13 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
>   
>   bool riscv_cpu_is_32bit(RISCVCPU *cpu);
>   
> +bool riscv_cpu_virt_mem_enabled(CPURISCVState *env);
> +RISCVPmPmm riscv_pm_get_pmm(CPURISCVState *env);
> +int riscv_pm_get_pmlen(RISCVPmPmm pmm);
> +
>   RISCVException riscv_csrr(CPURISCVState *env, int csrno,
>                             target_ulong *ret_value);
> +
>   RISCVException riscv_csrrw(CPURISCVState *env, int csrno,
>                              target_ulong *ret_value,
>                              target_ulong new_value, target_ulong write_mask);
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index bf58350669..e4a127ca84 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -142,6 +142,64 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
>       *pflags = flags;
>   }
>   
> +RISCVPmPmm riscv_pm_get_pmm(CPURISCVState *env)

I think we should process XLEN here.

Pack *XL32 into PMM_FIELD_DISABLED

Zhiwei
> +{
> +    int pmm = 0;
> +#ifndef CONFIG_USER_ONLY
> +    int priv_mode = cpu_address_mode(env);
> +    /* Get current PMM field */
> +    switch (priv_mode) {
> +    case PRV_M:
> +        pmm = riscv_cpu_cfg(env)->ext_smmpm ?
> +                  get_field(env->mseccfg, MSECCFG_PMM) : PMM_FIELD_DISABLED;
> +        break;
> +    case PRV_S:
> +        pmm = riscv_cpu_cfg(env)->ext_smnpm ?
> +                  get_field(env->menvcfg, MENVCFG_PMM) : PMM_FIELD_DISABLED;
> +        break;
> +    case PRV_U:
> +        pmm = riscv_cpu_cfg(env)->ext_ssnpm ?
> +                  get_field(env->senvcfg, SENVCFG_PMM) : PMM_FIELD_DISABLED;
> +        break;
> +    default:
> +        g_assert_not_reached();
> +    }
> +#endif
> +    return pmm;
> +}
> +
> +bool riscv_cpu_virt_mem_enabled(CPURISCVState *env)
> +{
> +    bool virt_mem_en = false;
> +#ifndef CONFIG_USER_ONLY
> +    int satp_mode = 0;
> +    int priv_mode = cpu_address_mode(env);
> +    /* Get current PMM field */
> +    if (riscv_cpu_mxl(env) == MXL_RV32) {
> +        satp_mode = get_field(env->satp, SATP32_MODE);
> +    } else {
> +        satp_mode = get_field(env->satp, SATP64_MODE);
> +    }
> +    virt_mem_en = ((satp_mode != VM_1_10_MBARE) && (priv_mode != PRV_M));
> +#endif
> +    return virt_mem_en;
> +}
> +
> +int riscv_pm_get_pmlen(RISCVPmPmm pmm)
> +{
> +    switch (pmm) {
> +    case PMM_FIELD_DISABLED:
> +        return 0;
> +    case PMM_FIELD_PMLEN7:
> +        return 7;
> +    case PMM_FIELD_PMLEN16:
> +        return 16;
> +    default:
> +        g_assert_not_reached();
> +    }
> +    return -1;
> +}
> +
>   #ifndef CONFIG_USER_ONLY
>   
>   /*

[-- Attachment #2: Type: text/html, Size: 3875 bytes --]

Re: [PATCH v9 3/6] target/riscv: Add helper functions to calculate current number of masked bits for pointer masking

[LIU Zhiwei]

[-- Attachment #1: Type: text/plain, Size: 3423 bytes --]


On 2024/5/13 20:35, LIU Zhiwei wrote:
>
>
> On 2024/5/11 18:10, Alexey Baturo wrote:
>> From: Alexey Baturo<baturo.alexey@gmail.com>
>>
>> Signed-off-by: Alexey Baturo<baturo.alexey@gmail.com>
>>
>> Reviewed-by: Alistair Francis<alistair.francis@wdc.com>
>> ---
>>   target/riscv/cpu.h        |  5 ++++
>>   target/riscv/cpu_helper.c | 58 +++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 63 insertions(+)
>>
>> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>> index 52b6ba73c8..9cac723b19 100644
>> --- a/target/riscv/cpu.h
>> +++ b/target/riscv/cpu.h
>> @@ -698,8 +698,13 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
>>   
>>   bool riscv_cpu_is_32bit(RISCVCPU *cpu);
>>   
>> +bool riscv_cpu_virt_mem_enabled(CPURISCVState *env);
>> +RISCVPmPmm riscv_pm_get_pmm(CPURISCVState *env);
>> +int riscv_pm_get_pmlen(RISCVPmPmm pmm);
>> +
>>   RISCVException riscv_csrr(CPURISCVState *env, int csrno,
>>                             target_ulong *ret_value);
>> +
>>   RISCVException riscv_csrrw(CPURISCVState *env, int csrno,
>>                              target_ulong *ret_value,
>>                              target_ulong new_value, target_ulong write_mask);
>> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
>> index bf58350669..e4a127ca84 100644
>> --- a/target/riscv/cpu_helper.c
>> +++ b/target/riscv/cpu_helper.c
>> @@ -142,6 +142,64 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
>>       *pflags = flags;
>>   }
>>   
>> +RISCVPmPmm riscv_pm_get_pmm(CPURISCVState *env)
>
> I think we should process XLEN here.
>
> Pack *XL32 into PMM_FIELD_DISABLED
>
Please ignore this comment. I see you have considered the XLEN in patch 5/6.
> Zhiwei
>> +{
>> +    int pmm = 0;
>> +#ifndef CONFIG_USER_ONLY
>> +    int priv_mode = cpu_address_mode(env);
>> +    /* Get current PMM field */
>> +    switch (priv_mode) {
>> +    case PRV_M:
>> +        pmm = riscv_cpu_cfg(env)->ext_smmpm ?
>> +                  get_field(env->mseccfg, MSECCFG_PMM) : PMM_FIELD_DISABLED;
>> +        break;
>> +    case PRV_S:
>> +        pmm = riscv_cpu_cfg(env)->ext_smnpm ?
>> +                  get_field(env->menvcfg, MENVCFG_PMM) : PMM_FIELD_DISABLED;
>> +        break;
>> +    case PRV_U:
>> +        pmm = riscv_cpu_cfg(env)->ext_ssnpm ?
>> +                  get_field(env->senvcfg, SENVCFG_PMM) : PMM_FIELD_DISABLED;
>> +        break;
>> +    default:
>> +        g_assert_not_reached();
>> +    }
>> +#endif
>> +    return pmm;
>> +}
>> +
>> +bool riscv_cpu_virt_mem_enabled(CPURISCVState *env)
>> +{
>> +    bool virt_mem_en = false;
>> +#ifndef CONFIG_USER_ONLY
>> +    int satp_mode = 0;
>> +    int priv_mode = cpu_address_mode(env);
>> +    /* Get current PMM field */
>> +    if (riscv_cpu_mxl(env) == MXL_RV32) {
>> +        satp_mode = get_field(env->satp, SATP32_MODE);
>> +    } else {
>> +        satp_mode = get_field(env->satp, SATP64_MODE);
>> +    }
>> +    virt_mem_en = ((satp_mode != VM_1_10_MBARE) && (priv_mode != PRV_M));
>> +#endif
>> +    return virt_mem_en;
>> +}
>> +
>> +int riscv_pm_get_pmlen(RISCVPmPmm pmm)
>> +{
>> +    switch (pmm) {
>> +    case PMM_FIELD_DISABLED:
>> +        return 0;
>> +    case PMM_FIELD_PMLEN7:
>> +        return 7;
>> +    case PMM_FIELD_PMLEN16:
>> +        return 16;
>> +    default:
>> +        g_assert_not_reached();
>> +    }
>> +    return -1;
>> +}
>> +
>>   #ifndef CONFIG_USER_ONLY
>>   
>>   /*

[-- Attachment #2: Type: text/html, Size: 4515 bytes --]

[PATCH v9 4/6] target/riscv: Add pointer masking tb flags

[Alexey Baturo]

From: Alexey Baturo <baturo.alexey@gmail.com>

Signed-off-by: Alexey Baturo <baturo.alexey@gmail.com>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/cpu.h        | 3 +++
 target/riscv/cpu_helper.c | 3 +++
 target/riscv/translate.c  | 5 +++++
 3 files changed, 11 insertions(+)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 9cac723b19..bbf3a0f64e 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -564,6 +564,9 @@ FIELD(TB_FLAGS, ITRIGGER, 20, 1)
 FIELD(TB_FLAGS, VIRT_ENABLED, 21, 1)
 FIELD(TB_FLAGS, PRIV, 22, 2)
 FIELD(TB_FLAGS, AXL, 24, 2)
+/* If pointer masking should be applied and address sign extended */
+FIELD(TB_FLAGS, PM_PMM, 26, 2)
+FIELD(TB_FLAGS, PM_SIGNEXTEND, 28, 1)
 
 #ifdef TARGET_RISCV32
 #define riscv_cpu_mxl(env)  ((void)(env), MXL_RV32)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index e4a127ca84..3f2473bd73 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -68,6 +68,7 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
     RISCVCPU *cpu = env_archcpu(env);
     RISCVExtStatus fs, vs;
     uint32_t flags = 0;
+    bool pm_signext = riscv_cpu_virt_mem_enabled(env);
 
     *pc = env->xl == MXL_RV32 ? env->pc & UINT32_MAX : env->pc;
     *cs_base = 0;
@@ -138,6 +139,8 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
     flags = FIELD_DP32(flags, TB_FLAGS, VS, vs);
     flags = FIELD_DP32(flags, TB_FLAGS, XL, env->xl);
     flags = FIELD_DP32(flags, TB_FLAGS, AXL, cpu_address_xl(env));
+    flags = FIELD_DP32(flags, TB_FLAGS, PM_PMM, riscv_pm_get_pmm(env));
+    flags = FIELD_DP32(flags, TB_FLAGS, PM_SIGNEXTEND, pm_signext);
 
     *pflags = flags;
 }
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index e5b339b1fa..3f578d6dd8 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -103,6 +103,9 @@ typedef struct DisasContext {
     bool vl_eq_vlmax;
     CPUState *cs;
     TCGv zero;
+    /* actual address width */
+    uint8_t addr_width;
+    bool addr_signed;
     /* Ztso */
     bool ztso;
     /* Use icount trigger for native debug */
@@ -1185,6 +1188,8 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
     ctx->xl = FIELD_EX32(tb_flags, TB_FLAGS, XL);
     ctx->address_xl = FIELD_EX32(tb_flags, TB_FLAGS, AXL);
     ctx->cs = cs;
+    ctx->addr_width = 0;
+    ctx->addr_signed = false;
     ctx->ztso = cpu->cfg.ext_ztso;
     ctx->itrigger = FIELD_EX32(tb_flags, TB_FLAGS, ITRIGGER);
     ctx->zero = tcg_constant_tl(0);
-- 
2.34.1

Re: [PATCH v9 4/6] target/riscv: Add pointer masking tb flags

[LIU Zhiwei]

On 2024/5/11 18:10, Alexey Baturo wrote:
> From: Alexey Baturo <baturo.alexey@gmail.com>
>
> Signed-off-by: Alexey Baturo <baturo.alexey@gmail.com>
>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>   target/riscv/cpu.h        | 3 +++
>   target/riscv/cpu_helper.c | 3 +++
>   target/riscv/translate.c  | 5 +++++
>   3 files changed, 11 insertions(+)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 9cac723b19..bbf3a0f64e 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -564,6 +564,9 @@ FIELD(TB_FLAGS, ITRIGGER, 20, 1)
>   FIELD(TB_FLAGS, VIRT_ENABLED, 21, 1)
>   FIELD(TB_FLAGS, PRIV, 22, 2)
>   FIELD(TB_FLAGS, AXL, 24, 2)
> +/* If pointer masking should be applied and address sign extended */
> +FIELD(TB_FLAGS, PM_PMM, 26, 2)
> +FIELD(TB_FLAGS, PM_SIGNEXTEND, 28, 1)
>   
>   #ifdef TARGET_RISCV32
>   #define riscv_cpu_mxl(env)  ((void)(env), MXL_RV32)
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index e4a127ca84..3f2473bd73 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -68,6 +68,7 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
>       RISCVCPU *cpu = env_archcpu(env);
>       RISCVExtStatus fs, vs;
>       uint32_t flags = 0;
> +    bool pm_signext = riscv_cpu_virt_mem_enabled(env);
>   
>       *pc = env->xl == MXL_RV32 ? env->pc & UINT32_MAX : env->pc;
>       *cs_base = 0;
> @@ -138,6 +139,8 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
>       flags = FIELD_DP32(flags, TB_FLAGS, VS, vs);
>       flags = FIELD_DP32(flags, TB_FLAGS, XL, env->xl);
>       flags = FIELD_DP32(flags, TB_FLAGS, AXL, cpu_address_xl(env));
> +    flags = FIELD_DP32(flags, TB_FLAGS, PM_PMM, riscv_pm_get_pmm(env));
> +    flags = FIELD_DP32(flags, TB_FLAGS, PM_SIGNEXTEND, pm_signext);
>   
>       *pflags = flags;
>   }
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index e5b339b1fa..3f578d6dd8 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -103,6 +103,9 @@ typedef struct DisasContext {
>       bool vl_eq_vlmax;
>       CPUState *cs;
>       TCGv zero;
> +    /* actual address width */
> +    uint8_t addr_width;
> +    bool addr_signed;
>       /* Ztso */
>       bool ztso;
>       /* Use icount trigger for native debug */
> @@ -1185,6 +1188,8 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
>       ctx->xl = FIELD_EX32(tb_flags, TB_FLAGS, XL);
>       ctx->address_xl = FIELD_EX32(tb_flags, TB_FLAGS, AXL);
>       ctx->cs = cs;
> +    ctx->addr_width = 0;
> +    ctx->addr_signed = false;

Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>

Zhiwei

>       ctx->ztso = cpu->cfg.ext_ztso;
>       ctx->itrigger = FIELD_EX32(tb_flags, TB_FLAGS, ITRIGGER);
>       ctx->zero = tcg_constant_tl(0);

[PATCH v9 5/6] target/riscv: Update address modify functions to take into account pointer masking

[Alexey Baturo]

From: Alexey Baturo <baturo.alexey@gmail.com>

Signed-off-by: Alexey Baturo <baturo.alexey@gmail.com>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/translate.c     | 22 ++++++++++++++++------
 target/riscv/vector_helper.c | 13 +++++++++++++
 2 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 3f578d6dd8..da46e636f8 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -580,8 +580,10 @@ static TCGv get_address(DisasContext *ctx, int rs1, int imm)
     TCGv src1 = get_gpr(ctx, rs1, EXT_NONE);
 
     tcg_gen_addi_tl(addr, src1, imm);
-    if (get_address_xl(ctx) == MXL_RV32) {
-        tcg_gen_ext32u_tl(addr, addr);
+    if (ctx->addr_signed) {
+        tcg_gen_sextract_tl(addr, addr, 0, ctx->addr_width);
+    } else {
+        tcg_gen_extract_tl(addr, addr, 0, ctx->addr_width);
     }
 
     return addr;
@@ -594,8 +596,10 @@ static TCGv get_address_indexed(DisasContext *ctx, int rs1, TCGv offs)
     TCGv src1 = get_gpr(ctx, rs1, EXT_NONE);
 
     tcg_gen_add_tl(addr, src1, offs);
-    if (get_xl(ctx) == MXL_RV32) {
-        tcg_gen_ext32u_tl(addr, addr);
+    if (ctx->addr_signed) {
+        tcg_gen_sextract_tl(addr, addr, 0, ctx->addr_width);
+    } else {
+        tcg_gen_extract_tl(addr, addr, 0, ctx->addr_width);
     }
     return addr;
 }
@@ -1188,8 +1192,14 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
     ctx->xl = FIELD_EX32(tb_flags, TB_FLAGS, XL);
     ctx->address_xl = FIELD_EX32(tb_flags, TB_FLAGS, AXL);
     ctx->cs = cs;
-    ctx->addr_width = 0;
-    ctx->addr_signed = false;
+    if (get_xl(ctx) == MXL_RV32) {
+        ctx->addr_width = 32;
+        ctx->addr_signed = false;
+    } else {
+        int pm_pmm = FIELD_EX32(tb_flags, TB_FLAGS, PM_PMM);
+        ctx->addr_width = 64 - riscv_pm_get_pmlen(pm_pmm);
+        ctx->addr_signed = FIELD_EX32(tb_flags, TB_FLAGS, PM_SIGNEXTEND);
+    }
     ctx->ztso = cpu->cfg.ext_ztso;
     ctx->itrigger = FIELD_EX32(tb_flags, TB_FLAGS, ITRIGGER);
     ctx->zero = tcg_constant_tl(0);
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index 39ba2a09dd..28861cc509 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -104,6 +104,19 @@ static inline uint32_t vext_max_elems(uint32_t desc, uint32_t log2_esz)
 
 static inline target_ulong adjust_addr(CPURISCVState *env, target_ulong 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;
 }
 
-- 
2.34.1

Re: [PATCH v9 5/6] target/riscv: Update address modify functions to take into account pointer masking

[LIU Zhiwei]

On 2024/5/11 18:10, Alexey Baturo wrote:
> From: Alexey Baturo <baturo.alexey@gmail.com>
>
> Signed-off-by: Alexey Baturo <baturo.alexey@gmail.com>
>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>   target/riscv/translate.c     | 22 ++++++++++++++++------
>   target/riscv/vector_helper.c | 13 +++++++++++++
>   2 files changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index 3f578d6dd8..da46e636f8 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -580,8 +580,10 @@ static TCGv get_address(DisasContext *ctx, int rs1, int imm)
>       TCGv src1 = get_gpr(ctx, rs1, EXT_NONE);
>   
>       tcg_gen_addi_tl(addr, src1, imm);
> -    if (get_address_xl(ctx) == MXL_RV32) {
> -        tcg_gen_ext32u_tl(addr, addr);
> +    if (ctx->addr_signed) {
> +        tcg_gen_sextract_tl(addr, addr, 0, ctx->addr_width);
> +    } else {
> +        tcg_gen_extract_tl(addr, addr, 0, ctx->addr_width);
>       }
>   
>       return addr;
> @@ -594,8 +596,10 @@ static TCGv get_address_indexed(DisasContext *ctx, int rs1, TCGv offs)
>       TCGv src1 = get_gpr(ctx, rs1, EXT_NONE);
>   
>       tcg_gen_add_tl(addr, src1, offs);
> -    if (get_xl(ctx) == MXL_RV32) {
> -        tcg_gen_ext32u_tl(addr, addr);
> +    if (ctx->addr_signed) {
> +        tcg_gen_sextract_tl(addr, addr, 0, ctx->addr_width);
> +    } else {
> +        tcg_gen_extract_tl(addr, addr, 0, ctx->addr_width);
>       }
>       return addr;
>   }
> @@ -1188,8 +1192,14 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
>       ctx->xl = FIELD_EX32(tb_flags, TB_FLAGS, XL);
>       ctx->address_xl = FIELD_EX32(tb_flags, TB_FLAGS, AXL);
>       ctx->cs = cs;
> -    ctx->addr_width = 0;
> -    ctx->addr_signed = false;
> +    if (get_xl(ctx) == MXL_RV32) {
Maybe ctx->address_xl?
> +        ctx->addr_width = 32;
> +        ctx->addr_signed = false;
In tcg inline code, we have considered the XL. But
> +    } else {
> +        int pm_pmm = FIELD_EX32(tb_flags, TB_FLAGS, PM_PMM);
> +        ctx->addr_width = 64 - riscv_pm_get_pmlen(pm_pmm);
> +        ctx->addr_signed = FIELD_EX32(tb_flags, TB_FLAGS, PM_SIGNEXTEND);
> +    }
>       ctx->ztso = cpu->cfg.ext_ztso;
>       ctx->itrigger = FIELD_EX32(tb_flags, TB_FLAGS, ITRIGGER);
>       ctx->zero = tcg_constant_tl(0);
> diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
> index 39ba2a09dd..28861cc509 100644
> --- a/target/riscv/vector_helper.c
> +++ b/target/riscv/vector_helper.c
> @@ -104,6 +104,19 @@ static inline uint32_t vext_max_elems(uint32_t desc, uint32_t log2_esz)
>   
>   static inline target_ulong adjust_addr(CPURISCVState *env, target_ulong addr)
>   {

here, we don't process the MXL_RV32. We should process it explicitly here.

Zhiwei

> +    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;
>   }
>   

[PATCH v9 6/6] target/riscv: Enable updates for pointer masking variables and thus enable pointer masking extension

[Alexey Baturo]

From: Alexey Baturo <baturo.alexey@gmail.com>

Signed-off-by: Alexey Baturo <baturo.alexey@gmail.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/cpu.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 1e350e9bd8..b3b3a6275f 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -192,6 +192,9 @@ const RISCVIsaExtData isa_edata_arr[] = {
     ISA_EXT_DATA_ENTRY(svinval, PRIV_VERSION_1_12_0, ext_svinval),
     ISA_EXT_DATA_ENTRY(svnapot, PRIV_VERSION_1_12_0, ext_svnapot),
     ISA_EXT_DATA_ENTRY(svpbmt, PRIV_VERSION_1_12_0, ext_svpbmt),
+    ISA_EXT_DATA_ENTRY(ssnpm, PRIV_VERSION_1_12_0, ext_ssnpm),
+    ISA_EXT_DATA_ENTRY(smnpm, PRIV_VERSION_1_12_0, ext_smnpm),
+    ISA_EXT_DATA_ENTRY(smmpm, PRIV_VERSION_1_12_0, ext_smmpm),
     ISA_EXT_DATA_ENTRY(xtheadba, PRIV_VERSION_1_11_0, ext_xtheadba),
     ISA_EXT_DATA_ENTRY(xtheadbb, PRIV_VERSION_1_11_0, ext_xtheadbb),
     ISA_EXT_DATA_ENTRY(xtheadbs, PRIV_VERSION_1_11_0, ext_xtheadbs),
@@ -1565,6 +1568,11 @@ const RISCVCPUMultiExtConfig riscv_cpu_vendor_exts[] = {
 
 /* These are experimental so mark with 'x-' */
 const RISCVCPUMultiExtConfig riscv_cpu_experimental_exts[] = {
+    /* Zjpm v0.8 extensions */
+    MULTI_EXT_CFG_BOOL("x-ssnpm", ext_ssnpm, false),
+    MULTI_EXT_CFG_BOOL("x-smnpm", ext_smnpm, false),
+    MULTI_EXT_CFG_BOOL("x-smmpm", ext_smmpm, false),
+
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.34.1

Re: [PATCH v9 6/6] target/riscv: Enable updates for pointer masking variables and thus enable pointer masking extension

[LIU Zhiwei]

On 2024/5/11 18:10, Alexey Baturo wrote:
> From: Alexey Baturo <baturo.alexey@gmail.com>
>
> Signed-off-by: Alexey Baturo <baturo.alexey@gmail.com>
>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>   target/riscv/cpu.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 1e350e9bd8..b3b3a6275f 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -192,6 +192,9 @@ const RISCVIsaExtData isa_edata_arr[] = {
>       ISA_EXT_DATA_ENTRY(svinval, PRIV_VERSION_1_12_0, ext_svinval),
>       ISA_EXT_DATA_ENTRY(svnapot, PRIV_VERSION_1_12_0, ext_svnapot),
>       ISA_EXT_DATA_ENTRY(svpbmt, PRIV_VERSION_1_12_0, ext_svpbmt),
> +    ISA_EXT_DATA_ENTRY(ssnpm, PRIV_VERSION_1_12_0, ext_ssnpm),

PRIV_VERSION_1_13_0

> +    ISA_EXT_DATA_ENTRY(smnpm, PRIV_VERSION_1_12_0, ext_smnpm),
> +    ISA_EXT_DATA_ENTRY(smmpm, PRIV_VERSION_1_12_0, ext_smmpm),
>       ISA_EXT_DATA_ENTRY(xtheadba, PRIV_VERSION_1_11_0, ext_xtheadba),
>       ISA_EXT_DATA_ENTRY(xtheadbb, PRIV_VERSION_1_11_0, ext_xtheadbb),
>       ISA_EXT_DATA_ENTRY(xtheadbs, PRIV_VERSION_1_11_0, ext_xtheadbs),
> @@ -1565,6 +1568,11 @@ const RISCVCPUMultiExtConfig riscv_cpu_vendor_exts[] = {
>   
>   /* These are experimental so mark with 'x-' */
>   const RISCVCPUMultiExtConfig riscv_cpu_experimental_exts[] = {
> +    /* Zjpm v0.8 extensions */
Zjpm v1.0
> +    MULTI_EXT_CFG_BOOL("x-ssnpm", ext_ssnpm, false),
> +    MULTI_EXT_CFG_BOOL("x-smnpm", ext_smnpm, false),
> +    MULTI_EXT_CFG_BOOL("x-smmpm", ext_smmpm, false),

IMO, we can remove x-

Zhiwei

> +
>       DEFINE_PROP_END_OF_LIST(),
>   };
>   

Re: [PATCH v9 0/6] Pointer Masking update for Zjpm v1.0

[liwei]

On 2024/5/11 18:10, Alexey Baturo wrote:
> From: Alexey Baturo <baturo.alexey@gmail.com>
>
> Hi,
>
> It looks like Pointer Masking spec has reached v1.0 and been frozen,
> rebasing on riscv-to-apply.next branch and resubmitting patches.

Hi, any change from v0.8 to v1.0?

Regards,

Weiwei Li

>
> Thanks.
>
> [v8]:
> Rebasing patches on current qemu branch and resubmitting them.
>
>
> [v7]:
> I'm terribly sorry, but previous rebase went wrong and somehow I missed it.
> This time I double-checked rebased version.
> This patch series is properly rebased on https://github.com/alistair23/qemu/tree/riscv-to-apply.next
>
> [v6]:
> This patch series is rebased on https://github.com/alistair23/qemu/tree/riscv-to-apply.next
>
> [v5]:
> This patch series targets Zjpm v0.8 extension.
> The spec itself could be found here: https://github.com/riscv/riscv-j-extension/blob/8088461d8d66a7676872b61c908cbeb7cf5c5d1d/zjpm-spec.pdf
> This patch series is updated after the suggested comments:
> - add "x-" to the extension names to indicate experimental
>
> [v4]:
> Patch series updated after the suggested comments:
> - removed J-letter extension as it's unused
> - renamed and fixed function to detect if address should be sign-extended
> - zeroed unused context variables and moved computation logic to another patch
> - bumped pointer masking version_id and minimum_version_id by 1
>
> [v3]:
> There patches are updated after Richard's comments:
> - moved new tb flags to the end
> - used tcg_gen_(s)extract to get the final address
> - properly handle CONFIG_USER_ONLY
>
> [v2]:
> As per Richard's suggestion I made pmm field part of tb_flags.
> It allowed to get rid of global variable to store pmlen.
> Also it allowed to simplify all the machinery around it.
>
> [v1]:
> It looks like Zjpm v0.8 is almost frozen and we don't expect it change drastically anymore.
> Compared to the original implementation with explicit base and mask CSRs, we now only have
> several fixed options for number of masked bits which are set using existing CSRs.
> The changes have been tested with handwritten assembly tests and LLVM HWASAN
> test suite.
>
> Alexey Baturo (6):
>    target/riscv: Remove obsolete pointer masking  extension code.
>    target/riscv: Add new CSR fields for S{sn,mn,m}pm extensions as part
>      of Zjpm v0.8
>    target/riscv: Add helper functions to calculate current number of
>      masked bits for pointer masking
>    target/riscv: Add pointer masking tb flags
>    target/riscv: Update address modify functions to take into account
>      pointer masking
>    target/riscv: Enable updates for pointer masking variables and thus
>      enable pointer masking extension
>
>   target/riscv/cpu.c           |  21 +--
>   target/riscv/cpu.h           |  46 +++--
>   target/riscv/cpu_bits.h      |  90 +---------
>   target/riscv/cpu_cfg.h       |   3 +
>   target/riscv/cpu_helper.c    |  97 +++++-----
>   target/riscv/csr.c           | 337 ++---------------------------------
>   target/riscv/machine.c       |  20 +--
>   target/riscv/pmp.c           |  13 +-
>   target/riscv/pmp.h           |  11 +-
>   target/riscv/tcg/tcg-cpu.c   |   5 +-
>   target/riscv/translate.c     |  46 ++---
>   target/riscv/vector_helper.c |  15 +-
>   12 files changed, 158 insertions(+), 546 deletions(-)
>

Re: [PATCH v9 0/6] Pointer Masking update for Zjpm v1.0

[Alistair Francis]

On Sun, May 12, 2024 at 12:44 AM liwei <liwei1518@gmail.com> wrote:
>
>
> On 2024/5/11 18:10, Alexey Baturo wrote:
> > From: Alexey Baturo <baturo.alexey@gmail.com>
> >
> > Hi,
> >
> > It looks like Pointer Masking spec has reached v1.0 and been frozen,
> > rebasing on riscv-to-apply.next branch and resubmitting patches.
>
> Hi, any change from v0.8 to v1.0?

Good question.

Also, this needs another rebase. Sorry, it seems to always have
conflicts. If you re-send I'll apply it right away

Alistair

>
> Regards,
>
> Weiwei Li
>
> >
> > Thanks.
> >
> > [v8]:
> > Rebasing patches on current qemu branch and resubmitting them.
> >
> >
> > [v7]:
> > I'm terribly sorry, but previous rebase went wrong and somehow I missed it.
> > This time I double-checked rebased version.
> > This patch series is properly rebased on https://github.com/alistair23/qemu/tree/riscv-to-apply.next
> >
> > [v6]:
> > This patch series is rebased on https://github.com/alistair23/qemu/tree/riscv-to-apply.next
> >
> > [v5]:
> > This patch series targets Zjpm v0.8 extension.
> > The spec itself could be found here: https://github.com/riscv/riscv-j-extension/blob/8088461d8d66a7676872b61c908cbeb7cf5c5d1d/zjpm-spec.pdf
> > This patch series is updated after the suggested comments:
> > - add "x-" to the extension names to indicate experimental
> >
> > [v4]:
> > Patch series updated after the suggested comments:
> > - removed J-letter extension as it's unused
> > - renamed and fixed function to detect if address should be sign-extended
> > - zeroed unused context variables and moved computation logic to another patch
> > - bumped pointer masking version_id and minimum_version_id by 1
> >
> > [v3]:
> > There patches are updated after Richard's comments:
> > - moved new tb flags to the end
> > - used tcg_gen_(s)extract to get the final address
> > - properly handle CONFIG_USER_ONLY
> >
> > [v2]:
> > As per Richard's suggestion I made pmm field part of tb_flags.
> > It allowed to get rid of global variable to store pmlen.
> > Also it allowed to simplify all the machinery around it.
> >
> > [v1]:
> > It looks like Zjpm v0.8 is almost frozen and we don't expect it change drastically anymore.
> > Compared to the original implementation with explicit base and mask CSRs, we now only have
> > several fixed options for number of masked bits which are set using existing CSRs.
> > The changes have been tested with handwritten assembly tests and LLVM HWASAN
> > test suite.
> >
> > Alexey Baturo (6):
> >    target/riscv: Remove obsolete pointer masking  extension code.
> >    target/riscv: Add new CSR fields for S{sn,mn,m}pm extensions as part
> >      of Zjpm v0.8
> >    target/riscv: Add helper functions to calculate current number of
> >      masked bits for pointer masking
> >    target/riscv: Add pointer masking tb flags
> >    target/riscv: Update address modify functions to take into account
> >      pointer masking
> >    target/riscv: Enable updates for pointer masking variables and thus
> >      enable pointer masking extension
> >
> >   target/riscv/cpu.c           |  21 +--
> >   target/riscv/cpu.h           |  46 +++--
> >   target/riscv/cpu_bits.h      |  90 +---------
> >   target/riscv/cpu_cfg.h       |   3 +
> >   target/riscv/cpu_helper.c    |  97 +++++-----
> >   target/riscv/csr.c           | 337 ++---------------------------------
> >   target/riscv/machine.c       |  20 +--
> >   target/riscv/pmp.c           |  13 +-
> >   target/riscv/pmp.h           |  11 +-
> >   target/riscv/tcg/tcg-cpu.c   |   5 +-
> >   target/riscv/translate.c     |  46 ++---
> >   target/riscv/vector_helper.c |  15 +-
> >   12 files changed, 158 insertions(+), 546 deletions(-)
> >
>

Re: [PATCH v9 0/6] Pointer Masking update for Zjpm v1.0

[Alexey Baturo]

[-- Attachment #1: Type: text/plain, Size: 4637 bytes --]

Hi,

> Hi, any change from v0.8 to v1.0?
Not in the patches that were sent. I'd still suggest applying a
step-by-step approach with cleaning up old code and establishing the new
mechanisms first and then updating the code to match the spec 100%. Also I
heard Martin has some arch compliance tests for J-ext somewhere.
@Alistair Francis <alistair23@gmail.com> @liwei does this approach sound
reasonable to you?

>Also, this needs another rebase
Sure, no problem at all. I'll rebase and re-send them later today.

Thanks

пн, 13 мая 2024 г. в 13:25, Alistair Francis <alistair23@gmail.com>:

> On Sun, May 12, 2024 at 12:44 AM liwei <liwei1518@gmail.com> wrote:
> >
> >
> > On 2024/5/11 18:10, Alexey Baturo wrote:
> > > From: Alexey Baturo <baturo.alexey@gmail.com>
> > >
> > > Hi,
> > >
> > > It looks like Pointer Masking spec has reached v1.0 and been frozen,
> > > rebasing on riscv-to-apply.next branch and resubmitting patches.
> >
> > Hi, any change from v0.8 to v1.0?
>
> Good question.
>
> Also, this needs another rebase. Sorry, it seems to always have
> conflicts. If you re-send I'll apply it right away
>
> Alistair
>
> >
> > Regards,
> >
> > Weiwei Li
> >
> > >
> > > Thanks.
> > >
> > > [v8]:
> > > Rebasing patches on current qemu branch and resubmitting them.
> > >
> > >
> > > [v7]:
> > > I'm terribly sorry, but previous rebase went wrong and somehow I
> missed it.
> > > This time I double-checked rebased version.
> > > This patch series is properly rebased on
> https://github.com/alistair23/qemu/tree/riscv-to-apply.next
> > >
> > > [v6]:
> > > This patch series is rebased on
> https://github.com/alistair23/qemu/tree/riscv-to-apply.next
> > >
> > > [v5]:
> > > This patch series targets Zjpm v0.8 extension.
> > > The spec itself could be found here:
> https://github.com/riscv/riscv-j-extension/blob/8088461d8d66a7676872b61c908cbeb7cf5c5d1d/zjpm-spec.pdf
> > > This patch series is updated after the suggested comments:
> > > - add "x-" to the extension names to indicate experimental
> > >
> > > [v4]:
> > > Patch series updated after the suggested comments:
> > > - removed J-letter extension as it's unused
> > > - renamed and fixed function to detect if address should be
> sign-extended
> > > - zeroed unused context variables and moved computation logic to
> another patch
> > > - bumped pointer masking version_id and minimum_version_id by 1
> > >
> > > [v3]:
> > > There patches are updated after Richard's comments:
> > > - moved new tb flags to the end
> > > - used tcg_gen_(s)extract to get the final address
> > > - properly handle CONFIG_USER_ONLY
> > >
> > > [v2]:
> > > As per Richard's suggestion I made pmm field part of tb_flags.
> > > It allowed to get rid of global variable to store pmlen.
> > > Also it allowed to simplify all the machinery around it.
> > >
> > > [v1]:
> > > It looks like Zjpm v0.8 is almost frozen and we don't expect it change
> drastically anymore.
> > > Compared to the original implementation with explicit base and mask
> CSRs, we now only have
> > > several fixed options for number of masked bits which are set using
> existing CSRs.
> > > The changes have been tested with handwritten assembly tests and LLVM
> HWASAN
> > > test suite.
> > >
> > > Alexey Baturo (6):
> > >    target/riscv: Remove obsolete pointer masking  extension code.
> > >    target/riscv: Add new CSR fields for S{sn,mn,m}pm extensions as part
> > >      of Zjpm v0.8
> > >    target/riscv: Add helper functions to calculate current number of
> > >      masked bits for pointer masking
> > >    target/riscv: Add pointer masking tb flags
> > >    target/riscv: Update address modify functions to take into account
> > >      pointer masking
> > >    target/riscv: Enable updates for pointer masking variables and thus
> > >      enable pointer masking extension
> > >
> > >   target/riscv/cpu.c           |  21 +--
> > >   target/riscv/cpu.h           |  46 +++--
> > >   target/riscv/cpu_bits.h      |  90 +---------
> > >   target/riscv/cpu_cfg.h       |   3 +
> > >   target/riscv/cpu_helper.c    |  97 +++++-----
> > >   target/riscv/csr.c           | 337
> ++---------------------------------
> > >   target/riscv/machine.c       |  20 +--
> > >   target/riscv/pmp.c           |  13 +-
> > >   target/riscv/pmp.h           |  11 +-
> > >   target/riscv/tcg/tcg-cpu.c   |   5 +-
> > >   target/riscv/translate.c     |  46 ++---
> > >   target/riscv/vector_helper.c |  15 +-
> > >   12 files changed, 158 insertions(+), 546 deletions(-)
> > >
> >
>

[-- Attachment #2: Type: text/html, Size: 6393 bytes --]

Re: [PATCH v9 0/6] Pointer Masking update for Zjpm v1.0

[Alistair Francis]

On Mon, May 13, 2024 at 9:05 PM Alexey Baturo <baturo.alexey@gmail.com> wrote:
>
> Hi,
>
> > Hi, any change from v0.8 to v1.0?
> Not in the patches that were sent. I'd still suggest applying a step-by-step approach with cleaning up old code and establishing the new mechanisms first and then updating the code to match the spec 100%. Also I heard Martin has some arch compliance tests for J-ext somewhere.

The cover letter says that this implements version 1.0 of the spec,
this sounds like it doesn't.

Also, it's better to make the changes on top of the current code.
Instead of constantly removing and re-adding the code. Which is then
hard to review. Especially as I'm guessing there isn't a huge
difference between v0.8 and v1.0.

> @Alistair Francis @liwei does this approach sound reasonable to you?
>
> >Also, this needs another rebase
> Sure, no problem at all. I'll rebase and re-send them later today.

Thanks. Can you be very clear which version of the spec you have
developed and tested against as well.

Alistair

>
> Thanks
>
> пн, 13 мая 2024 г. в 13:25, Alistair Francis <alistair23@gmail.com>:
>>
>> On Sun, May 12, 2024 at 12:44 AM liwei <liwei1518@gmail.com> wrote:
>> >
>> >
>> > On 2024/5/11 18:10, Alexey Baturo wrote:
>> > > From: Alexey Baturo <baturo.alexey@gmail.com>
>> > >
>> > > Hi,
>> > >
>> > > It looks like Pointer Masking spec has reached v1.0 and been frozen,
>> > > rebasing on riscv-to-apply.next branch and resubmitting patches.
>> >
>> > Hi, any change from v0.8 to v1.0?
>>
>> Good question.
>>
>> Also, this needs another rebase. Sorry, it seems to always have
>> conflicts. If you re-send I'll apply it right away
>>
>> Alistair
>>
>> >
>> > Regards,
>> >
>> > Weiwei Li
>> >
>> > >
>> > > Thanks.
>> > >
>> > > [v8]:
>> > > Rebasing patches on current qemu branch and resubmitting them.
>> > >
>> > >
>> > > [v7]:
>> > > I'm terribly sorry, but previous rebase went wrong and somehow I missed it.
>> > > This time I double-checked rebased version.
>> > > This patch series is properly rebased on https://github.com/alistair23/qemu/tree/riscv-to-apply.next
>> > >
>> > > [v6]:
>> > > This patch series is rebased on https://github.com/alistair23/qemu/tree/riscv-to-apply.next
>> > >
>> > > [v5]:
>> > > This patch series targets Zjpm v0.8 extension.
>> > > The spec itself could be found here: https://github.com/riscv/riscv-j-extension/blob/8088461d8d66a7676872b61c908cbeb7cf5c5d1d/zjpm-spec.pdf
>> > > This patch series is updated after the suggested comments:
>> > > - add "x-" to the extension names to indicate experimental
>> > >
>> > > [v4]:
>> > > Patch series updated after the suggested comments:
>> > > - removed J-letter extension as it's unused
>> > > - renamed and fixed function to detect if address should be sign-extended
>> > > - zeroed unused context variables and moved computation logic to another patch
>> > > - bumped pointer masking version_id and minimum_version_id by 1
>> > >
>> > > [v3]:
>> > > There patches are updated after Richard's comments:
>> > > - moved new tb flags to the end
>> > > - used tcg_gen_(s)extract to get the final address
>> > > - properly handle CONFIG_USER_ONLY
>> > >
>> > > [v2]:
>> > > As per Richard's suggestion I made pmm field part of tb_flags.
>> > > It allowed to get rid of global variable to store pmlen.
>> > > Also it allowed to simplify all the machinery around it.
>> > >
>> > > [v1]:
>> > > It looks like Zjpm v0.8 is almost frozen and we don't expect it change drastically anymore.
>> > > Compared to the original implementation with explicit base and mask CSRs, we now only have
>> > > several fixed options for number of masked bits which are set using existing CSRs.
>> > > The changes have been tested with handwritten assembly tests and LLVM HWASAN
>> > > test suite.
>> > >
>> > > Alexey Baturo (6):
>> > >    target/riscv: Remove obsolete pointer masking  extension code.
>> > >    target/riscv: Add new CSR fields for S{sn,mn,m}pm extensions as part
>> > >      of Zjpm v0.8
>> > >    target/riscv: Add helper functions to calculate current number of
>> > >      masked bits for pointer masking
>> > >    target/riscv: Add pointer masking tb flags
>> > >    target/riscv: Update address modify functions to take into account
>> > >      pointer masking
>> > >    target/riscv: Enable updates for pointer masking variables and thus
>> > >      enable pointer masking extension
>> > >
>> > >   target/riscv/cpu.c           |  21 +--
>> > >   target/riscv/cpu.h           |  46 +++--
>> > >   target/riscv/cpu_bits.h      |  90 +---------
>> > >   target/riscv/cpu_cfg.h       |   3 +
>> > >   target/riscv/cpu_helper.c    |  97 +++++-----
>> > >   target/riscv/csr.c           | 337 ++---------------------------------
>> > >   target/riscv/machine.c       |  20 +--
>> > >   target/riscv/pmp.c           |  13 +-
>> > >   target/riscv/pmp.h           |  11 +-
>> > >   target/riscv/tcg/tcg-cpu.c   |   5 +-
>> > >   target/riscv/translate.c     |  46 ++---
>> > >   target/riscv/vector_helper.c |  15 +-
>> > >   12 files changed, 158 insertions(+), 546 deletions(-)
>> > >
>> >

Re: [PATCH v9 0/6] Pointer Masking update for Zjpm v1.0

[Alistair Francis]

On Mon, May 13, 2024 at 9:14 PM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Mon, May 13, 2024 at 9:05 PM Alexey Baturo <baturo.alexey@gmail.com> wrote:
> >
> > Hi,
> >
> > > Hi, any change from v0.8 to v1.0?
> > Not in the patches that were sent. I'd still suggest applying a step-by-step approach with cleaning up old code and establishing the new mechanisms first and then updating the code to match the spec 100%. Also I heard Martin has some arch compliance tests for J-ext somewhere.
>
> The cover letter says that this implements version 1.0 of the spec,
> this sounds like it doesn't.
>
> Also, it's better to make the changes on top of the current code.
> Instead of constantly removing and re-adding the code. Which is then
> hard to review. Especially as I'm guessing there isn't a huge
> difference between v0.8 and v1.0.
>
> > @Alistair Francis @liwei does this approach sound reasonable to you?
> >
> > >Also, this needs another rebase
> > Sure, no problem at all. I'll rebase and re-send them later today.

Sorry, it did apply correctly! That was my mistake.

But this series generates a warning. Do you mind fixing that and
addressing the other comments/concerns

Alistair

>
> Thanks. Can you be very clear which version of the spec you have
> developed and tested against as well.
>
> Alistair

Re: [PATCH v9 0/6] Pointer Masking update for Zjpm v1.0

[Alexey Baturo]

[-- Attachment #1: Type: text/plain, Size: 2043 bytes --]

>The cover letter says that this implements version 1.0 of the spec, this
sounds like it doesn't.
Yeah, sorry about the confusion. Yes, the patch is actually for v0.8 but as
you've correctly mentioned v0.8 has not so much differences to v1.0.

> Instead of constantly removing and re-adding the code
I was talking about only one removing the existing code and replacing it
with current patches and then making updates on top of them.

>Do you mind fixing that and addressing the other comments/concerns
Sure

пн, 13 мая 2024 г. в 14:32, Alistair Francis <alistair23@gmail.com>:

> On Mon, May 13, 2024 at 9:14 PM Alistair Francis <alistair23@gmail.com>
> wrote:
> >
> > On Mon, May 13, 2024 at 9:05 PM Alexey Baturo <baturo.alexey@gmail.com>
> wrote:
> > >
> > > Hi,
> > >
> > > > Hi, any change from v0.8 to v1.0?
> > > Not in the patches that were sent. I'd still suggest applying a
> step-by-step approach with cleaning up old code and establishing the new
> mechanisms first and then updating the code to match the spec 100%. Also I
> heard Martin has some arch compliance tests for J-ext somewhere.
> >
> > The cover letter says that this implements version 1.0 of the spec,
> > this sounds like it doesn't.
> >
> > Also, it's better to make the changes on top of the current code.
> > Instead of constantly removing and re-adding the code. Which is then
> > hard to review. Especially as I'm guessing there isn't a huge
> > difference between v0.8 and v1.0.
> >
> > > @Alistair Francis @liwei does this approach sound reasonable to you?
> > >
> > > >Also, this needs another rebase
> > > Sure, no problem at all. I'll rebase and re-send them later today.
>
> Sorry, it did apply correctly! That was my mistake.
>
> But this series generates a warning. Do you mind fixing that and
> addressing the other comments/concerns
>
> Alistair
>
> >
> > Thanks. Can you be very clear which version of the spec you have
> > developed and tested against as well.
> >
> > Alistair
>

[-- Attachment #2: Type: text/html, Size: 2720 bytes --]

Re: [PATCH v9 0/6] Pointer Masking update for Zjpm v1.0

[LIU Zhiwei]

On 2024/5/11 18:10, Alexey Baturo wrote:
> From: Alexey Baturo <baturo.alexey@gmail.com>
>
> Hi,
>
> It looks like Pointer Masking spec has reached v1.0 and been frozen,

I think one big feature that this patch set missing is lack of pointer 
masking about the hypervisor load/store instructions.

Zhiwei

> rebasing on riscv-to-apply.next branch and resubmitting patches.
>
> Thanks.
>
> [v8]:
> Rebasing patches on current qemu branch and resubmitting them.
>
>
> [v7]:
> I'm terribly sorry, but previous rebase went wrong and somehow I missed it.
> This time I double-checked rebased version.
> This patch series is properly rebased on https://github.com/alistair23/qemu/tree/riscv-to-apply.next
>
> [v6]:
> This patch series is rebased on https://github.com/alistair23/qemu/tree/riscv-to-apply.next
>
> [v5]:
> This patch series targets Zjpm v0.8 extension.
> The spec itself could be found here: https://github.com/riscv/riscv-j-extension/blob/8088461d8d66a7676872b61c908cbeb7cf5c5d1d/zjpm-spec.pdf
> This patch series is updated after the suggested comments:
> - add "x-" to the extension names to indicate experimental
>
> [v4]:
> Patch series updated after the suggested comments:
> - removed J-letter extension as it's unused
> - renamed and fixed function to detect if address should be sign-extended
> - zeroed unused context variables and moved computation logic to another patch
> - bumped pointer masking version_id and minimum_version_id by 1
>
> [v3]:
> There patches are updated after Richard's comments:
> - moved new tb flags to the end
> - used tcg_gen_(s)extract to get the final address
> - properly handle CONFIG_USER_ONLY
>
> [v2]:
> As per Richard's suggestion I made pmm field part of tb_flags.
> It allowed to get rid of global variable to store pmlen.
> Also it allowed to simplify all the machinery around it.
>
> [v1]:
> It looks like Zjpm v0.8 is almost frozen and we don't expect it change drastically anymore.
> Compared to the original implementation with explicit base and mask CSRs, we now only have
> several fixed options for number of masked bits which are set using existing CSRs.
> The changes have been tested with handwritten assembly tests and LLVM HWASAN
> test suite.
>
> Alexey Baturo (6):
>    target/riscv: Remove obsolete pointer masking  extension code.
>    target/riscv: Add new CSR fields for S{sn,mn,m}pm extensions as part
>      of Zjpm v0.8
>    target/riscv: Add helper functions to calculate current number of
>      masked bits for pointer masking
>    target/riscv: Add pointer masking tb flags
>    target/riscv: Update address modify functions to take into account
>      pointer masking
>    target/riscv: Enable updates for pointer masking variables and thus
>      enable pointer masking extension
>
>   target/riscv/cpu.c           |  21 +--
>   target/riscv/cpu.h           |  46 +++--
>   target/riscv/cpu_bits.h      |  90 +---------
>   target/riscv/cpu_cfg.h       |   3 +
>   target/riscv/cpu_helper.c    |  97 +++++-----
>   target/riscv/csr.c           | 337 ++---------------------------------
>   target/riscv/machine.c       |  20 +--
>   target/riscv/pmp.c           |  13 +-
>   target/riscv/pmp.h           |  11 +-
>   target/riscv/tcg/tcg-cpu.c   |   5 +-
>   target/riscv/translate.c     |  46 ++---
>   target/riscv/vector_helper.c |  15 +-
>   12 files changed, 158 insertions(+), 546 deletions(-)
>

Re: [PATCH v9 0/6] Pointer Masking update for Zjpm v1.0

[Daniel Henrique Barboza]

Hi Alexey,


Do you have plans to post a new version of this series? Aside from a few comments
it seems like this was almost there.

We might not be able to get it merged in time for this current release (code freeze
is Nov 5th) but we can get it in Alistair's tree for the next release.


Thanks,

Daniel

On 5/11/24 7:10 AM, Alexey Baturo wrote:
> From: Alexey Baturo <baturo.alexey@gmail.com>
> 
> Hi,
> 
> It looks like Pointer Masking spec has reached v1.0 and been frozen,
> rebasing on riscv-to-apply.next branch and resubmitting patches.
> 
> Thanks.
> 
> [v8]:
> Rebasing patches on current qemu branch and resubmitting them.
> 
> 
> [v7]:
> I'm terribly sorry, but previous rebase went wrong and somehow I missed it.
> This time I double-checked rebased version.
> This patch series is properly rebased on https://github.com/alistair23/qemu/tree/riscv-to-apply.next
> 
> [v6]:
> This patch series is rebased on https://github.com/alistair23/qemu/tree/riscv-to-apply.next
> 
> [v5]:
> This patch series targets Zjpm v0.8 extension.
> The spec itself could be found here: https://github.com/riscv/riscv-j-extension/blob/8088461d8d66a7676872b61c908cbeb7cf5c5d1d/zjpm-spec.pdf
> This patch series is updated after the suggested comments:
> - add "x-" to the extension names to indicate experimental
> 
> [v4]:
> Patch series updated after the suggested comments:
> - removed J-letter extension as it's unused
> - renamed and fixed function to detect if address should be sign-extended
> - zeroed unused context variables and moved computation logic to another patch
> - bumped pointer masking version_id and minimum_version_id by 1
> 
> [v3]:
> There patches are updated after Richard's comments:
> - moved new tb flags to the end
> - used tcg_gen_(s)extract to get the final address
> - properly handle CONFIG_USER_ONLY
> 
> [v2]:
> As per Richard's suggestion I made pmm field part of tb_flags.
> It allowed to get rid of global variable to store pmlen.
> Also it allowed to simplify all the machinery around it.
> 
> [v1]:
> It looks like Zjpm v0.8 is almost frozen and we don't expect it change drastically anymore.
> Compared to the original implementation with explicit base and mask CSRs, we now only have
> several fixed options for number of masked bits which are set using existing CSRs.
> The changes have been tested with handwritten assembly tests and LLVM HWASAN
> test suite.
> 
> Alexey Baturo (6):
>    target/riscv: Remove obsolete pointer masking  extension code.
>    target/riscv: Add new CSR fields for S{sn,mn,m}pm extensions as part
>      of Zjpm v0.8
>    target/riscv: Add helper functions to calculate current number of
>      masked bits for pointer masking
>    target/riscv: Add pointer masking tb flags
>    target/riscv: Update address modify functions to take into account
>      pointer masking
>    target/riscv: Enable updates for pointer masking variables and thus
>      enable pointer masking extension
> 
>   target/riscv/cpu.c           |  21 +--
>   target/riscv/cpu.h           |  46 +++--
>   target/riscv/cpu_bits.h      |  90 +---------
>   target/riscv/cpu_cfg.h       |   3 +
>   target/riscv/cpu_helper.c    |  97 +++++-----
>   target/riscv/csr.c           | 337 ++---------------------------------
>   target/riscv/machine.c       |  20 +--
>   target/riscv/pmp.c           |  13 +-
>   target/riscv/pmp.h           |  11 +-
>   target/riscv/tcg/tcg-cpu.c   |   5 +-
>   target/riscv/translate.c     |  46 ++---
>   target/riscv/vector_helper.c |  15 +-
>   12 files changed, 158 insertions(+), 546 deletions(-)
> 

Re: [PATCH v9 0/6] Pointer Masking update for Zjpm v1.0

[Alexey Baturo]

[-- Attachment #1: Type: text/plain, Size: 4115 bytes --]

Hi Daniel,

Let's try to push it before EOY.
I'm planning to start working on it in the first half of November.
Does that work for you?

Thanks

вт, 29 окт. 2024 г. в 20:40, Daniel Henrique Barboza <
dbarboza@ventanamicro.com>:

> Hi Alexey,
>
>
> Do you have plans to post a new version of this series? Aside from a few
> comments
> it seems like this was almost there.
>
> We might not be able to get it merged in time for this current release
> (code freeze
> is Nov 5th) but we can get it in Alistair's tree for the next release.
>
>
> Thanks,
>
> Daniel
>
> On 5/11/24 7:10 AM, Alexey Baturo wrote:
> > From: Alexey Baturo <baturo.alexey@gmail.com>
> >
> > Hi,
> >
> > It looks like Pointer Masking spec has reached v1.0 and been frozen,
> > rebasing on riscv-to-apply.next branch and resubmitting patches.
> >
> > Thanks.
> >
> > [v8]:
> > Rebasing patches on current qemu branch and resubmitting them.
> >
> >
> > [v7]:
> > I'm terribly sorry, but previous rebase went wrong and somehow I missed
> it.
> > This time I double-checked rebased version.
> > This patch series is properly rebased on
> https://github.com/alistair23/qemu/tree/riscv-to-apply.next
> >
> > [v6]:
> > This patch series is rebased on
> https://github.com/alistair23/qemu/tree/riscv-to-apply.next
> >
> > [v5]:
> > This patch series targets Zjpm v0.8 extension.
> > The spec itself could be found here:
> https://github.com/riscv/riscv-j-extension/blob/8088461d8d66a7676872b61c908cbeb7cf5c5d1d/zjpm-spec.pdf
> > This patch series is updated after the suggested comments:
> > - add "x-" to the extension names to indicate experimental
> >
> > [v4]:
> > Patch series updated after the suggested comments:
> > - removed J-letter extension as it's unused
> > - renamed and fixed function to detect if address should be sign-extended
> > - zeroed unused context variables and moved computation logic to another
> patch
> > - bumped pointer masking version_id and minimum_version_id by 1
> >
> > [v3]:
> > There patches are updated after Richard's comments:
> > - moved new tb flags to the end
> > - used tcg_gen_(s)extract to get the final address
> > - properly handle CONFIG_USER_ONLY
> >
> > [v2]:
> > As per Richard's suggestion I made pmm field part of tb_flags.
> > It allowed to get rid of global variable to store pmlen.
> > Also it allowed to simplify all the machinery around it.
> >
> > [v1]:
> > It looks like Zjpm v0.8 is almost frozen and we don't expect it change
> drastically anymore.
> > Compared to the original implementation with explicit base and mask
> CSRs, we now only have
> > several fixed options for number of masked bits which are set using
> existing CSRs.
> > The changes have been tested with handwritten assembly tests and LLVM
> HWASAN
> > test suite.
> >
> > Alexey Baturo (6):
> >    target/riscv: Remove obsolete pointer masking  extension code.
> >    target/riscv: Add new CSR fields for S{sn,mn,m}pm extensions as part
> >      of Zjpm v0.8
> >    target/riscv: Add helper functions to calculate current number of
> >      masked bits for pointer masking
> >    target/riscv: Add pointer masking tb flags
> >    target/riscv: Update address modify functions to take into account
> >      pointer masking
> >    target/riscv: Enable updates for pointer masking variables and thus
> >      enable pointer masking extension
> >
> >   target/riscv/cpu.c           |  21 +--
> >   target/riscv/cpu.h           |  46 +++--
> >   target/riscv/cpu_bits.h      |  90 +---------
> >   target/riscv/cpu_cfg.h       |   3 +
> >   target/riscv/cpu_helper.c    |  97 +++++-----
> >   target/riscv/csr.c           | 337 ++---------------------------------
> >   target/riscv/machine.c       |  20 +--
> >   target/riscv/pmp.c           |  13 +-
> >   target/riscv/pmp.h           |  11 +-
> >   target/riscv/tcg/tcg-cpu.c   |   5 +-
> >   target/riscv/translate.c     |  46 ++---
> >   target/riscv/vector_helper.c |  15 +-
> >   12 files changed, 158 insertions(+), 546 deletions(-)
> >
>

[-- Attachment #2: Type: text/html, Size: 5447 bytes --]

Re: [PATCH v9 0/6] Pointer Masking update for Zjpm v1.0

[Daniel Henrique Barboza]

On 10/30/24 11:17 AM, Alexey Baturo wrote:
> Hi Daniel,
> 
> Let's try to push it before EOY.
> I'm planning to start working on it in the first half of November.
> Does that work for you?

Fine by me. We can add it to Alistair's tree for 10.0 to be upstreamed in January 2025.

Thanks,

Daniel

> 
> Thanks
> 
> вт, 29 окт. 2024 г. в 20:40, Daniel Henrique Barboza <dbarboza@ventanamicro.com <mailto:dbarboza@ventanamicro.com>>:
> 
>     Hi Alexey,
> 
> 
>     Do you have plans to post a new version of this series? Aside from a few comments
>     it seems like this was almost there.
> 
>     We might not be able to get it merged in time for this current release (code freeze
>     is Nov 5th) but we can get it in Alistair's tree for the next release.
> 
> 
>     Thanks,
> 
>     Daniel
> 
>     On 5/11/24 7:10 AM, Alexey Baturo wrote:
>      > From: Alexey Baturo <baturo.alexey@gmail.com <mailto:baturo.alexey@gmail.com>>
>      >
>      > Hi,
>      >
>      > It looks like Pointer Masking spec has reached v1.0 and been frozen,
>      > rebasing on riscv-to-apply.next branch and resubmitting patches.
>      >
>      > Thanks.
>      >
>      > [v8]:
>      > Rebasing patches on current qemu branch and resubmitting them.
>      >
>      >
>      > [v7]:
>      > I'm terribly sorry, but previous rebase went wrong and somehow I missed it.
>      > This time I double-checked rebased version.
>      > This patch series is properly rebased on https://github.com/alistair23/qemu/tree/riscv-to-apply.next <https://github.com/alistair23/qemu/tree/riscv-to-apply.next>
>      >
>      > [v6]:
>      > This patch series is rebased on https://github.com/alistair23/qemu/tree/riscv-to-apply.next <https://github.com/alistair23/qemu/tree/riscv-to-apply.next>
>      >
>      > [v5]:
>      > This patch series targets Zjpm v0.8 extension.
>      > The spec itself could be found here: https://github.com/riscv/riscv-j-extension/blob/8088461d8d66a7676872b61c908cbeb7cf5c5d1d/zjpm-spec.pdf <https://github.com/riscv/riscv-j-extension/blob/8088461d8d66a7676872b61c908cbeb7cf5c5d1d/zjpm-spec.pdf>
>      > This patch series is updated after the suggested comments:
>      > - add "x-" to the extension names to indicate experimental
>      >
>      > [v4]:
>      > Patch series updated after the suggested comments:
>      > - removed J-letter extension as it's unused
>      > - renamed and fixed function to detect if address should be sign-extended
>      > - zeroed unused context variables and moved computation logic to another patch
>      > - bumped pointer masking version_id and minimum_version_id by 1
>      >
>      > [v3]:
>      > There patches are updated after Richard's comments:
>      > - moved new tb flags to the end
>      > - used tcg_gen_(s)extract to get the final address
>      > - properly handle CONFIG_USER_ONLY
>      >
>      > [v2]:
>      > As per Richard's suggestion I made pmm field part of tb_flags.
>      > It allowed to get rid of global variable to store pmlen.
>      > Also it allowed to simplify all the machinery around it.
>      >
>      > [v1]:
>      > It looks like Zjpm v0.8 is almost frozen and we don't expect it change drastically anymore.
>      > Compared to the original implementation with explicit base and mask CSRs, we now only have
>      > several fixed options for number of masked bits which are set using existing CSRs.
>      > The changes have been tested with handwritten assembly tests and LLVM HWASAN
>      > test suite.
>      >
>      > Alexey Baturo (6):
>      >    target/riscv: Remove obsolete pointer masking  extension code.
>      >    target/riscv: Add new CSR fields for S{sn,mn,m}pm extensions as part
>      >      of Zjpm v0.8
>      >    target/riscv: Add helper functions to calculate current number of
>      >      masked bits for pointer masking
>      >    target/riscv: Add pointer masking tb flags
>      >    target/riscv: Update address modify functions to take into account
>      >      pointer masking
>      >    target/riscv: Enable updates for pointer masking variables and thus
>      >      enable pointer masking extension
>      >
>      >   target/riscv/cpu.c           |  21 +--
>      >   target/riscv/cpu.h           |  46 +++--
>      >   target/riscv/cpu_bits.h      |  90 +---------
>      >   target/riscv/cpu_cfg.h       |   3 +
>      >   target/riscv/cpu_helper.c    |  97 +++++-----
>      >   target/riscv/csr.c           | 337 ++---------------------------------
>      >   target/riscv/machine.c       |  20 +--
>      >   target/riscv/pmp.c           |  13 +-
>      >   target/riscv/pmp.h           |  11 +-
>      >   target/riscv/tcg/tcg-cpu.c   |   5 +-
>      >   target/riscv/translate.c     |  46 ++---
>      >   target/riscv/vector_helper.c |  15 +-
>      >   12 files changed, 158 insertions(+), 546 deletions(-)
>      >
>