[PATCH v4 1/5] target/riscv: enforce XTheadCmo U-mode privilege checks

[PATCH v4 1/5] target/riscv: enforce XTheadCmo U-mode privilege checks

[wangyang]

The XTheadCmo specification restricts th.dcache.cva and th.dcache.iva
to privilege modes above U, while th.dcache.civa remains available in
U-mode. Both restricted instructions currently use the empty
REQUIRE_PRIV_MSU macro, so they retire in U-mode. Use REQUIRE_PRIV_MS
for the two restricted instructions.

Tested: RV32/RV64 Linux-user test cases with xtheadcmo enabled and
disabled.
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4412
Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>
Signed-off-by: wangyang <wangyang25@otcaix.iscas.ac.cn>
---
 target/riscv/tcg/insn_trans/trans_xthead.c.inc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/riscv/tcg/insn_trans/trans_xthead.c.inc b/target/riscv/tcg/insn_trans/trans_xthead.c.inc
index f4e3051000..681f70e5bc 100644
--- a/target/riscv/tcg/insn_trans/trans_xthead.c.inc
+++ b/target/riscv/tcg/insn_trans/trans_xthead.c.inc
@@ -289,9 +289,9 @@ NOP_PRIVCHECK(th_dcache_iall, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MS)
 NOP_PRIVCHECK(th_dcache_cpa, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MS)
 NOP_PRIVCHECK(th_dcache_cipa, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MS)
 NOP_PRIVCHECK(th_dcache_ipa, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MS)
-NOP_PRIVCHECK(th_dcache_cva, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MSU)
+NOP_PRIVCHECK(th_dcache_cva, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MS)
 NOP_PRIVCHECK(th_dcache_civa, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MSU)
-NOP_PRIVCHECK(th_dcache_iva, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MSU)
+NOP_PRIVCHECK(th_dcache_iva, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MS)
 NOP_PRIVCHECK(th_dcache_csw, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MS)
 NOP_PRIVCHECK(th_dcache_cisw, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MS)
 NOP_PRIVCHECK(th_dcache_isw, REQUIRE_XTHEADCMO, REQUIRE_PRIV_MS)
-- 
2.55.0.windows.2

[PATCH v4 3/5] target/riscv: require read permission for HLVX accesses

[wangyang]

HLVX address translation uses execute permission, but the final PMP
check on the supervisor physical address must also require read
permission. Carry the HLVX operation through the MMU index and include
PMP_READ in the final check so execute-only pages fault.

Tested: RV32 system-mode RWX, HLV, HLVX.HU, and HLVX.WU PMP
test cases.
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4414
Signed-off-by: wangyang <wangyang25@otcaix.iscas.ac.cn>
---
 target/riscv/internals.h      |  6 ++++++
 target/riscv/tcg/cpu_helper.c | 24 ++++++++++++++++--------
 target/riscv/tcg/op_helper.c  | 11 +++++------
 3 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/target/riscv/internals.h b/target/riscv/internals.h
index 5d84e4de96..832c6406bc 100644
--- a/target/riscv/internals.h
+++ b/target/riscv/internals.h
@@ -42,6 +42,7 @@
 #define MMUIdx_M            3
 #define MMU_2STAGE_BIT      (1 << 2)
 #define MMU_IDX_SS_WRITE    (1 << 3)
+#define MMU_IDX_HLVX        (1 << 4)
 
 static inline privilege_mode_t mmuidx_priv(int mmu_idx)
 {
@@ -62,6 +63,11 @@ static inline bool mmuidx_2stage(int mmu_idx)
     return mmu_idx & MMU_2STAGE_BIT;
 }
 
+static inline bool mmuidx_hlvx(int mmu_idx)
+{
+    return mmu_idx & MMU_IDX_HLVX;
+}
+
 /*
  * Return the endianness for the current privilege
  * level, based on the MSTATUS MBE/SBE/UBE bits.
diff --git a/target/riscv/tcg/cpu_helper.c b/target/riscv/tcg/cpu_helper.c
index 07d9222652..ad41de9d06 100644
--- a/target/riscv/tcg/cpu_helper.c
+++ b/target/riscv/tcg/cpu_helper.c
@@ -905,6 +905,7 @@ void riscv_cpu_set_mode(CPURISCVState *env, privilege_mode_t newpriv,
  */
 static int get_physical_address_pmp(CPURISCVState *env, int *prot, hwaddr addr,
                                     int size, MMUAccessType access_type,
+                                    pmp_priv_t extra_privs,
                                     privilege_mode_t mode)
 {
     pmp_priv_t pmp_priv;
@@ -915,7 +916,8 @@ static int get_physical_address_pmp(CPURISCVState *env, int *prot, hwaddr addr,
         return TRANSLATE_SUCCESS;
     }
 
-    pmp_has_privs = pmp_hart_has_privs(env, addr, size, 1 << access_type,
+    pmp_has_privs = pmp_hart_has_privs(env, addr, size,
+                                       (1 << access_type) | extra_privs,
                                        &pmp_priv, mode);
     if (!pmp_has_privs) {
         *prot = 0;
@@ -1177,7 +1179,7 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical,
         int pmp_prot;
         int pmp_ret = get_physical_address_pmp(env, &pmp_prot, pte_addr,
                                                sxlen_bytes,
-                                               MMU_DATA_LOAD, PRV_S);
+                                               MMU_DATA_LOAD, 0, PRV_S);
         if (pmp_ret != TRANSLATE_SUCCESS) {
             return TRANSLATE_PMP_FAIL;
         }
@@ -1425,7 +1427,8 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical,
         }
 
         pmp_ret = get_physical_address_pmp(env, &pmp_prot, pte_addr,
-                                           sxlen_bytes, MMU_DATA_STORE, PRV_S);
+                                           sxlen_bytes, MMU_DATA_STORE, 0,
+                                           PRV_S);
         if (pmp_ret != TRANSLATE_SUCCESS) {
             return TRANSLATE_PMP_FAIL;
         }
@@ -1711,7 +1714,9 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
 
             if (ret == TRANSLATE_SUCCESS) {
                 ret = get_physical_address_pmp(env, &prot_pmp, pa,
-                                               size, access_type, mode);
+                                               size, access_type,
+                                               mmuidx_hlvx(mmu_idx) ?
+                                               PMP_READ : 0, mode);
                 tlb_size = pmp_get_tlb_size(env, pa);
 
                 qemu_log_mask(CPU_LOG_MMU,
@@ -1746,7 +1751,9 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
 
         if (ret == TRANSLATE_SUCCESS) {
             ret = get_physical_address_pmp(env, &prot_pmp, pa,
-                                           size, access_type, mode);
+                                           size, access_type,
+                                           mmuidx_hlvx(mmu_idx) ?
+                                           PMP_READ : 0, mode);
             tlb_size = pmp_get_tlb_size(env, pa);
 
             qemu_log_mask(CPU_LOG_MMU,
@@ -1786,9 +1793,10 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
         cpu_check_watchpoint(cs, address, size, MEMTXATTRS_UNSPECIFIED,
                              wp_access, retaddr);
 
-        raise_mmu_exception(env, address, access_type, pmp_pma_violation,
-                            first_stage_error, two_stage_lookup,
-                            two_stage_indirect_error);
+        raise_mmu_exception(env, address,
+                            mmuidx_hlvx(mmu_idx) ? MMU_DATA_LOAD : access_type,
+                            pmp_pma_violation, first_stage_error,
+                            two_stage_lookup, two_stage_indirect_error);
         cpu_loop_exit_restore(cs, retaddr);
     }
 
diff --git a/target/riscv/tcg/op_helper.c b/target/riscv/tcg/op_helper.c
index 3e94005d2b..060d97ee9c 100644
--- a/target/riscv/tcg/op_helper.c
+++ b/target/riscv/tcg/op_helper.c
@@ -650,7 +650,7 @@ static int check_access_hlsv(CPURISCVState *env, bool x, uintptr_t ra)
     if (!x && mode == PRV_S && get_field(env->vsstatus, MSTATUS_SUM)) {
         mode = MMUIdx_S_SUM;
     }
-    return mode | MMU_2STAGE_BIT;
+    return mode | MMU_2STAGE_BIT | (x ? MMU_IDX_HLVX : 0);
 }
 
 target_ulong helper_hyp_hlv_bu(CPURISCVState *env, target_ulong addr)
@@ -726,11 +726,10 @@ void helper_hyp_hsv_d(CPURISCVState *env, target_ulong addr, target_ulong val)
 }
 
 /*
- * TODO: These implementations are not quite correct.  They perform the
- * access using execute permission just fine, but the final PMP check
- * is supposed to have read permission as well.  Without replicating
- * a fair fraction of cputlb.c, fixing this requires adding new mmu_idx
- * which would imply that exact check in tlb_fill.
+ * HLVX accesses are translated with execute permission (first stage),
+ * but the final PMP check on the supervisor physical address must
+ * require read permission as well.  The MMU_IDX_HLVX mmu_idx bit set
+ * by check_access_hlsv() makes riscv_cpu_tlb_fill() enforce this.
  */
 target_ulong helper_hyp_hlvx_hu(CPURISCVState *env, target_ulong addr)
 {
-- 
2.55.0.windows.2

[PATCH v4 4/5] target/riscv: prioritize Zicfilp checks for misaligned JALR

[wangyang]

When Zicfilp tracking is active, a misaligned indirect JALR target also
violates the landing-pad requirement. Generate the landing-pad
software-check before the generic instruction-address-misaligned
exception for tracked targets, while retaining the existing exception
for untracked targets.

Tested: RV64 M-mode and S-mode Zicfilp tracked and untracked
misaligned targets with c=true and c=false.
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4415
Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>
Signed-off-by: wangyang <wangyang25@otcaix.iscas.ac.cn>
---
 target/riscv/tcg/insn_trans/trans_rvi.c.inc | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/target/riscv/tcg/insn_trans/trans_rvi.c.inc b/target/riscv/tcg/insn_trans/trans_rvi.c.inc
index cc1b5dbbad..aa79a5ef3c 100644
--- a/target/riscv/tcg/insn_trans/trans_rvi.c.inc
+++ b/target/riscv/tcg/insn_trans/trans_rvi.c.inc
@@ -187,7 +187,14 @@ static bool trans_jalr(DisasContext *ctx, arg_jalr *a)
 
     if (misaligned) {
         gen_set_label(misaligned);
-        gen_exception_inst_addr_mis(ctx, target_pc);
+        if (ctx->fcfi_enabled &&
+            a->rs1 != xRA && a->rs1 != xT0 && a->rs1 != xT2) {
+            tcg_gen_st8_i32(tcg_constant_i32(RISCV_EXCP_SW_CHECK_FCFI_TVAL),
+                            tcg_env, offsetof(CPURISCVState, sw_check_code));
+            generate_exception(ctx, RISCV_EXCP_SW_CHECK);
+        } else {
+            gen_exception_inst_addr_mis(ctx, target_pc);
+        }
     }
     ctx->base.is_jmp = DISAS_NORETURN;
 
-- 
2.55.0.windows.2

[PATCH v4 5/5] target/riscv: canonicalize reserved CBIE encoding

[wangyang]

CBIE encoding 10 is reserved when Zicbom is implemented, while
encoding 11 has defined behavior. Canonicalize only encoding 10 in the
menvcfg, senvcfg, and henvcfg write paths so the reserved value is not
retained and the defined encoding remains unchanged.

Tested: RV64 menvcfg, senvcfg, and henvcfg CBIE 10 and CBIE 11
write/readback test cases.
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4416
Signed-off-by: wangyang <wangyang25@otcaix.iscas.ac.cn>
---
 target/riscv/tcg/csr.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/target/riscv/tcg/csr.c b/target/riscv/tcg/csr.c
index bd4b6dc114..cf59f9ae54 100644
--- a/target/riscv/tcg/csr.c
+++ b/target/riscv/tcg/csr.c
@@ -3260,6 +3260,16 @@ static RISCVException write_menvcfg(CPURISCVState *env, int csrno,
             stce_changed = true;
         }
     }
+
+    /*
+     * CBIE is a WARL field: encoding 10 is reserved.  A software write of
+     * this encoding must canonicalize to a supported value instead of being
+     * retained in the readback.
+     */
+    if (cfg->ext_zicbom && get_field(val, MENVCFG_CBIE) == 2) {
+        val &= ~MENVCFG_CBIE;
+    }
+
     env->menvcfg = (env->menvcfg & ~mask) | (val & mask);
 
     if (stce_changed) {
@@ -3354,6 +3364,16 @@ static RISCVException write_senvcfg(CPURISCVState *env, int csrno,
         mask |= SENVCFG_UKTE;
     }
 
+    /*
+     * CBIE is a WARL field: encoding 10 is reserved.  A software write of
+     * this encoding must canonicalize to a supported value instead of being
+     * retained in the readback.
+     */
+    if (env_archcpu(env)->cfg.ext_zicbom &&
+        get_field(val, SENVCFG_CBIE) == 2) {
+        val &= ~SENVCFG_CBIE;
+    }
+
     env->senvcfg = (env->senvcfg & ~mask) | (val & mask);
     return RISCV_EXCP_NONE;
 }
@@ -3422,6 +3442,15 @@ static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
         }
     }
 
+    /*
+     * CBIE is a WARL field: encoding 10 is reserved.  A software write of
+     * this encoding must canonicalize to a supported value instead of being
+     * retained in the readback.
+     */
+    if (cfg->ext_zicbom && get_field(val, HENVCFG_CBIE) == 2) {
+        val &= ~HENVCFG_CBIE;
+    }
+
     if (riscv_cpu_mxl(env) == MXL_RV32) {
         /*
          * RV32 stores STCE/ADUE/PBMTE/DTE in henvcfgh, so a low-half henvcfg
-- 
2.55.0.windows.2

[PATCH v4 2/5] target/riscv: decode RV32 XTheadBb th.srri shift amount

[wangyang]

The XTheadBb th.srri instruction uses the low log2(XLEN) bits of
imm6. The generic shift helper rejects values greater than or equal to
XLEN, which incorrectly rejects imm6 values 32 through 63 on RV32.
Decode the RV32 form with a five-bit shift field so the shared helper
receives the width-correct operand.

Tested: RV32 imm6 0, 31, 32, and 63 test cases with XTheadBb
enabled and disabled; RV64 imm6 63 with XTheadBb enabled and
disabled.
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4413
Signed-off-by: wangyang <wangyang25@otcaix.iscas.ac.cn>
---
 target/riscv/tcg/insn_trans/trans_xthead.c.inc | 8 ++++++++
 target/riscv/xthead.decode                     | 5 ++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/target/riscv/tcg/insn_trans/trans_xthead.c.inc b/target/riscv/tcg/insn_trans/trans_xthead.c.inc
index 681f70e5bc..d47b5a85c6 100644
--- a/target/riscv/tcg/insn_trans/trans_xthead.c.inc
+++ b/target/riscv/tcg/insn_trans/trans_xthead.c.inc
@@ -145,6 +145,14 @@ static bool trans_th_srri(DisasContext *ctx, arg_th_srri * a)
                                    tcg_gen_rotri_tl, gen_roriw, NULL);
 }
 
+static bool trans_th_srri_rv32(DisasContext *ctx, arg_th_srri_rv32 *a)
+{
+    REQUIRE_XTHEADBB(ctx);
+    REQUIRE_32BIT(ctx);
+    return gen_shift_imm_fn_per_ol(ctx, a, EXT_NONE,
+                                   tcg_gen_rotri_tl, gen_roriw, NULL);
+}
+
 /* th.srriw is an alternate encoding for roriw (from Zbb) */
 static bool trans_th_srriw(DisasContext *ctx, arg_th_srriw *a)
 {
diff --git a/target/riscv/xthead.decode b/target/riscv/xthead.decode
index d1d104bcf2..33995168c4 100644
--- a/target/riscv/xthead.decode
+++ b/target/riscv/xthead.decode
@@ -64,7 +64,10 @@ th_ext           ...... ...... ..... 010 ..... 0001011 @th_bfext
 th_extu          ...... ...... ..... 011 ..... 0001011 @th_bfext
 th_ff0           1000010 00000 ..... 001 ..... 0001011 @r2
 th_ff1           1000011 00000 ..... 001 ..... 0001011 @r2
-th_srri          000100 ...... ..... 001 ..... 0001011 @sh6
+{
+  th_srri_rv32  000100 -..... ..... 001 ..... 0001011 @sh5
+  th_srri       000100 ...... ..... 001 ..... 0001011 @sh6
+}
 th_srriw         0001010 ..... ..... 001 ..... 0001011 @sh5
 th_rev           1000001 00000 ..... 001 ..... 0001011 @r2
 th_revw          1001000 00000 ..... 001 ..... 0001011 @r2
-- 
2.55.0.windows.2

[PATCH v4 0/5] target/riscv: fix privilege, PMP, CFI, envcfg checks

[wangyang]

This series fixes five independent RISC-V emulation issues in the
XTheadCmo, XTheadBb, HLVX/PMP, Zicfilp, and environment-configuration
paths.

The first two patches correct XThead instruction handling. The third
adds the missing final PMP read check for HLVX. The fourth fixes
Zicfilp exception priority for tracked misaligned JALR targets. The
fifth canonicalizes reserved CBIE encoding while preserving the
defined CBIE=11 encoding.

This series is based on 209b2afaface001c7d4d981e38f186afe7b24a50,
the current upstream master.

Validation performed:

  - QEMU 11.1.1 baseline and patched test-case replays passed.
  - HLVX.HU and HLVX.WU execute-only PMP cases and tracked and
    untracked misaligned JALR controls were checked.
  - Every intermediate patch state built for qemu-system-riscv32,
    qemu-system-riscv64, qemu-riscv32, and qemu-riscv64.
  - RV32 XTheadBb imm6 0, 31, 32, and 63 cases pass with the
    extension enabled and remain illegal when it is disabled.
  - The v4 series passes checkpatch.pl, git diff --check, and
    independent apply checks.

Changes since v3:

  - Patch 2 now decodes the RV32 form with @sh5 and the RV64 form with
    @sh6, as suggested by Chao Liu, and adds a dedicated RV32
    translator.
  - The runtime shift-amount mask and the old Reviewed-by tag were
    removed from patch 2; unchanged reviewed patches retain their
    existing Reviewed-by tags.
  - Resend the complete series as a fresh v4 top-level thread.

wangyang (5):
  target/riscv: enforce XTheadCmo U-mode privilege checks
  target/riscv: decode RV32 XTheadBb th.srri shift amount
  target/riscv: require read permission for HLVX accesses
  target/riscv: prioritize Zicfilp checks for misaligned JALR
  target/riscv: canonicalize reserved CBIE encoding

 target/riscv/internals.h                      |  6 ++++
 target/riscv/tcg/cpu_helper.c                 | 24 ++++++++++-----
 target/riscv/tcg/csr.c                        | 29 +++++++++++++++++++
 target/riscv/tcg/insn_trans/trans_rvi.c.inc   |  9 +++++-
 .../riscv/tcg/insn_trans/trans_xthead.c.inc   | 12 ++++++--
 target/riscv/tcg/op_helper.c                  | 11 ++++---
 target/riscv/xthead.decode                    |  5 +++-
 7 files changed, 78 insertions(+), 18 deletions(-)

-- 
2.55.0.windows.2