All of lore.kernel.org
 help / color / mirror / Atom feed
* [PULL 00/19] riscv-to-apply queue
@ 2022-07-03  0:09 Alistair Francis
  2022-07-03  0:09 ` [PULL 01/19] target/riscv: Remove condition guarding register zero for auipc and lui Alistair Francis
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Alistair Francis @ 2022-07-03  0:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: alistair23, Alistair Francis

From: Alistair Francis <alistair@alistair23.me>

The following changes since commit d495e432c04a6394126c35cf96517749708b410f:

  Merge tag 'pull-aspeed-20220630' of https://github.com/legoater/qemu into staging (2022-06-30 22:04:12 +0530)

are available in the Git repository at:

  git@github.com:alistair23/qemu.git tags/pull-riscv-to-apply-20220703

for you to fetch changes up to 435774992e82d2d16f025afbb20b4f7be9b242b0:

  target/riscv: Update default priority table for local interrupts (2022-07-03 10:03:20 +1000)

----------------------------------------------------------------
Fifth RISC-V PR for QEMU 7.1

* Fix register zero guarding for auipc and lui
* Ensure bins (mtval) is set correctly
* Minimize the calls to decode_save_opc
* Guard against PMP ranges with a negative size
* Implement mcountinhibit CSR
* Add support for hpmcounters/hpmevents
* Improve PMU implenentation
* Support mcycle/minstret write operation
* Fixup MSECCFG minimum priv check
* Ibex (OpenTitan) fixup priv version
* Fix bug resulting in always using latest priv spec
* Reduce FDT address alignment constraints
* Set minumum priv spec version for mcountinhibit
* AIA update to v0.3 of the spec

----------------------------------------------------------------
Alistair Francis (3):
      target/riscv: Fixup MSECCFG minimum priv check
      target/riscv: Ibex: Support priv version 1.11
      hw/riscv: boot: Reduce FDT address alignment constraints

Anup Patel (4):
      target/riscv: Don't force update priv spec version to latest
      target/riscv: Set minumum priv spec version for mcountinhibit
      target/riscv: Remove CSRs that set/clear an IMSIC interrupt file bits
      target/riscv: Update default priority table for local interrupts

Atish Patra (7):
      target/riscv: Fix PMU CSR predicate function
      target/riscv: Implement PMU CSR predicate function for S-mode
      target/riscv: pmu: Rename the counters extension to pmu
      target/riscv: pmu: Make number of counters configurable
      target/riscv: Implement mcountinhibit CSR
      target/riscv: Add support for hpmcounters/hpmevents
      target/riscv: Support mcycle/minstret write operation

Nicolas Pitre (1):
      target/riscv/pmp: guard against PMP ranges with a negative size

Richard Henderson (3):
      target/riscv: Set env->bins in gen_exception_illegal
      target/riscv: Remove generate_exception_mtval
      target/riscv: Minimize the calls to decode_save_opc

Víctor Colombo (1):
      target/riscv: Remove condition guarding register zero for auipc and lui

 target/riscv/cpu.h                             |  24 +-
 target/riscv/cpu_bits.h                        |  30 +-
 target/riscv/pmu.h                             |  28 +
 hw/riscv/boot.c                                |   4 +-
 target/riscv/cpu.c                             |  17 +-
 target/riscv/cpu_helper.c                      | 134 ++--
 target/riscv/csr.c                             | 857 +++++++++++++++----------
 target/riscv/machine.c                         |  25 +
 target/riscv/pmp.c                             |   3 +
 target/riscv/pmu.c                             |  32 +
 target/riscv/translate.c                       |  31 +-
 target/riscv/insn_trans/trans_privileged.c.inc |   4 +
 target/riscv/insn_trans/trans_rvh.c.inc        |   2 +
 target/riscv/insn_trans/trans_rvi.c.inc        |  10 +-
 target/riscv/meson.build                       |   3 +-
 tests/tcg/riscv64/Makefile.softmmu-target      |  21 +
 tests/tcg/riscv64/issue1060.S                  |  53 ++
 tests/tcg/riscv64/semihost.ld                  |  21 +
 18 files changed, 843 insertions(+), 456 deletions(-)
 create mode 100644 target/riscv/pmu.h
 create mode 100644 target/riscv/pmu.c
 create mode 100644 tests/tcg/riscv64/Makefile.softmmu-target
 create mode 100644 tests/tcg/riscv64/issue1060.S
 create mode 100644 tests/tcg/riscv64/semihost.ld


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PULL 01/19] target/riscv: Remove condition guarding register zero for auipc and lui
  2022-07-03  0:09 [PULL 00/19] riscv-to-apply queue Alistair Francis
@ 2022-07-03  0:09 ` Alistair Francis
  2022-07-03  0:09 ` [PULL 02/19] target/riscv: Set env->bins in gen_exception_illegal Alistair Francis
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Alistair Francis @ 2022-07-03  0:09 UTC (permalink / raw)
  To: qemu-devel
  Cc: alistair23, Víctor Colombo, Richard Henderson, Alistair Francis

From: Víctor Colombo <victor.colombo@eldorado.org.br>

Commit 57c108b8646 introduced gen_set_gpri(), which already contains
a check for if the destination register is 'zero'. The check in auipc
and lui are then redundant. This patch removes those checks.

Signed-off-by: Víctor Colombo <victor.colombo@eldorado.org.br>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-Id: <20220610165517.47517-1-victor.colombo@eldorado.org.br>
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/insn_trans/trans_rvi.c.inc | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc
index f1342f30f8..c190a59f22 100644
--- a/target/riscv/insn_trans/trans_rvi.c.inc
+++ b/target/riscv/insn_trans/trans_rvi.c.inc
@@ -32,17 +32,13 @@ static bool trans_c64_illegal(DisasContext *ctx, arg_empty *a)
 
 static bool trans_lui(DisasContext *ctx, arg_lui *a)
 {
-    if (a->rd != 0) {
-        gen_set_gpri(ctx, a->rd, a->imm);
-    }
+    gen_set_gpri(ctx, a->rd, a->imm);
     return true;
 }
 
 static bool trans_auipc(DisasContext *ctx, arg_auipc *a)
 {
-    if (a->rd != 0) {
-        gen_set_gpri(ctx, a->rd, a->imm + ctx->base.pc_next);
-    }
+    gen_set_gpri(ctx, a->rd, a->imm + ctx->base.pc_next);
     return true;
 }
 
-- 
2.36.1



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PULL 02/19] target/riscv: Set env->bins in gen_exception_illegal
  2022-07-03  0:09 [PULL 00/19] riscv-to-apply queue Alistair Francis
  2022-07-03  0:09 ` [PULL 01/19] target/riscv: Remove condition guarding register zero for auipc and lui Alistair Francis
@ 2022-07-03  0:09 ` Alistair Francis
  2022-07-03  0:09 ` [PULL 03/19] target/riscv: Remove generate_exception_mtval Alistair Francis
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Alistair Francis @ 2022-07-03  0:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: alistair23, Richard Henderson, Alistair Francis

From: Richard Henderson <richard.henderson@linaro.org>

While we set env->bins when unwinding for ILLEGAL_INST,
from e.g. csrrw, we weren't setting it for immediately
illegal instructions.

Add a testcase for mtval via both exception paths.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1060
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-Id: <20220604231004.49990-2-richard.henderson@linaro.org>
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/translate.c                  |  2 +
 tests/tcg/riscv64/Makefile.softmmu-target | 21 +++++++++
 tests/tcg/riscv64/issue1060.S             | 53 +++++++++++++++++++++++
 tests/tcg/riscv64/semihost.ld             | 21 +++++++++
 4 files changed, 97 insertions(+)
 create mode 100644 tests/tcg/riscv64/Makefile.softmmu-target
 create mode 100644 tests/tcg/riscv64/issue1060.S
 create mode 100644 tests/tcg/riscv64/semihost.ld

diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index b151c20674..a10f3f939c 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -240,6 +240,8 @@ static void generate_exception_mtval(DisasContext *ctx, int excp)
 
 static void gen_exception_illegal(DisasContext *ctx)
 {
+    tcg_gen_st_i32(tcg_constant_i32(ctx->opcode), cpu_env,
+                   offsetof(CPURISCVState, bins));
     generate_exception(ctx, RISCV_EXCP_ILLEGAL_INST);
 }
 
diff --git a/tests/tcg/riscv64/Makefile.softmmu-target b/tests/tcg/riscv64/Makefile.softmmu-target
new file mode 100644
index 0000000000..e22cdb34c5
--- /dev/null
+++ b/tests/tcg/riscv64/Makefile.softmmu-target
@@ -0,0 +1,21 @@
+#
+# RISC-V system tests
+#
+
+TEST_SRC = $(SRC_PATH)/tests/tcg/riscv64
+VPATH += $(TEST_SRC)
+
+LINK_SCRIPT = $(TEST_SRC)/semihost.ld
+LDFLAGS = -T $(LINK_SCRIPT)
+CFLAGS += -g -Og
+
+%.o: %.S
+	$(CC) $(CFLAGS) $< -c -o $@
+%: %.o $(LINK_SCRIPT)
+	$(LD) $(LDFLAGS) $< -o $@
+
+QEMU_OPTS += -M virt -display none -semihosting -device loader,file=
+
+EXTRA_RUNS += run-issue1060
+run-issue1060: issue1060
+	$(call run-test, $<, $(QEMU) $(QEMU_OPTS)$<)
diff --git a/tests/tcg/riscv64/issue1060.S b/tests/tcg/riscv64/issue1060.S
new file mode 100644
index 0000000000..17b7fe1be2
--- /dev/null
+++ b/tests/tcg/riscv64/issue1060.S
@@ -0,0 +1,53 @@
+	.option	norvc
+
+	.text
+	.global _start
+_start:
+	lla	t0, trap
+	csrw	mtvec, t0
+
+	# These are all illegal instructions
+	csrw	time, x0
+	.insn	i CUSTOM_0, 0, x0, x0, 0x321
+	csrw	time, x0
+	.insn	i CUSTOM_0, 0, x0, x0, 0x123
+	csrw	cycle, x0
+
+	# Success!
+	li	a0, 0
+	j	_exit
+
+trap:
+	# When an instruction traps, compare it to the insn in memory.
+	csrr	t0, mepc
+	csrr	t1, mtval
+	lwu	t2, 0(t0)
+	bne	t1, t2, fail
+
+	# Skip the insn and continue.
+	addi	t0, t0, 4
+	csrw	mepc, t0
+	mret
+
+fail:
+	li	a0, 1
+
+# Exit code in a0
+_exit:
+	lla	a1, semiargs
+	li	t0, 0x20026	# ADP_Stopped_ApplicationExit
+	sd	t0, 0(a1)
+	sd	a0, 8(a1)
+	li	a0, 0x20	# TARGET_SYS_EXIT_EXTENDED
+
+	# Semihosting call sequence
+	.balign	16
+	slli	zero, zero, 0x1f
+	ebreak
+	srai	zero, zero, 0x7
+	j	.
+
+	.data
+	.balign	16
+semiargs:
+	.space	16
diff --git a/tests/tcg/riscv64/semihost.ld b/tests/tcg/riscv64/semihost.ld
new file mode 100644
index 0000000000..a59cc56b28
--- /dev/null
+++ b/tests/tcg/riscv64/semihost.ld
@@ -0,0 +1,21 @@
+ENTRY(_start)
+
+SECTIONS
+{
+    /* virt machine, RAM starts at 2gb */
+    . = 0x80000000;
+    .text : {
+        *(.text)
+    }
+    .rodata : {
+        *(.rodata)
+    }
+    /* align r/w section to next 2mb */
+    . = ALIGN(1 << 21);
+    .data : {
+        *(.data)
+    }
+    .bss : {
+        *(.bss)
+    }
+}
-- 
2.36.1



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PULL 03/19] target/riscv: Remove generate_exception_mtval
  2022-07-03  0:09 [PULL 00/19] riscv-to-apply queue Alistair Francis
  2022-07-03  0:09 ` [PULL 01/19] target/riscv: Remove condition guarding register zero for auipc and lui Alistair Francis
  2022-07-03  0:09 ` [PULL 02/19] target/riscv: Set env->bins in gen_exception_illegal Alistair Francis
@ 2022-07-03  0:09 ` Alistair Francis
  2022-07-03  0:09 ` [PULL 04/19] target/riscv: Minimize the calls to decode_save_opc Alistair Francis
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Alistair Francis @ 2022-07-03  0:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: alistair23, Richard Henderson, Alistair Francis

From: Richard Henderson <richard.henderson@linaro.org>

The function doesn't set mtval, it sets badaddr. Move the set
of badaddr directly into gen_exception_inst_addr_mis and use
generate_exception.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-Id: <20220604231004.49990-3-richard.henderson@linaro.org>
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/translate.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index a10f3f939c..7205a29603 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -230,14 +230,6 @@ static void generate_exception(DisasContext *ctx, int excp)
     ctx->base.is_jmp = DISAS_NORETURN;
 }
 
-static void generate_exception_mtval(DisasContext *ctx, int excp)
-{
-    gen_set_pc_imm(ctx, ctx->base.pc_next);
-    tcg_gen_st_tl(cpu_pc, cpu_env, offsetof(CPURISCVState, badaddr));
-    gen_helper_raise_exception(cpu_env, tcg_constant_i32(excp));
-    ctx->base.is_jmp = DISAS_NORETURN;
-}
-
 static void gen_exception_illegal(DisasContext *ctx)
 {
     tcg_gen_st_i32(tcg_constant_i32(ctx->opcode), cpu_env,
@@ -247,7 +239,8 @@ static void gen_exception_illegal(DisasContext *ctx)
 
 static void gen_exception_inst_addr_mis(DisasContext *ctx)
 {
-    generate_exception_mtval(ctx, RISCV_EXCP_INST_ADDR_MIS);
+    tcg_gen_st_tl(cpu_pc, cpu_env, offsetof(CPURISCVState, badaddr));
+    generate_exception(ctx, RISCV_EXCP_INST_ADDR_MIS);
 }
 
 static void gen_goto_tb(DisasContext *ctx, int n, target_ulong dest)
-- 
2.36.1



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PULL 04/19] target/riscv: Minimize the calls to decode_save_opc
  2022-07-03  0:09 [PULL 00/19] riscv-to-apply queue Alistair Francis
                   ` (2 preceding siblings ...)
  2022-07-03  0:09 ` [PULL 03/19] target/riscv: Remove generate_exception_mtval Alistair Francis
@ 2022-07-03  0:09 ` Alistair Francis
  2022-07-03  0:09 ` [PULL 05/19] target/riscv/pmp: guard against PMP ranges with a negative size Alistair Francis
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Alistair Francis @ 2022-07-03  0:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: alistair23, Richard Henderson, Alistair Francis

From: Richard Henderson <richard.henderson@linaro.org>

The set of instructions that require decode_save_opc for
unwinding is really fairly small -- only insns that can
raise ILLEGAL_INSN at runtime.  This includes CSR, anything
that uses a *new* fp rounding mode, and many privileged insns.

Since unwind info is stored as the difference from the
previous insn, storing a 0 for most insns minimizes the
size of the unwind info.

Booting a debian kernel image to the missing rootfs panic yields

- gen code size       22226819/1026886656
+ gen code size       21601907/1026886656

on 41k TranslationBlocks, a savings of 610kB or a bit less than 3%.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-Id: <20220604231004.49990-4-richard.henderson@linaro.org>
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/translate.c                       | 18 +++++++++---------
 target/riscv/insn_trans/trans_privileged.c.inc |  4 ++++
 target/riscv/insn_trans/trans_rvh.c.inc        |  2 ++
 target/riscv/insn_trans/trans_rvi.c.inc        |  2 ++
 4 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 7205a29603..63b04e8a94 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -206,6 +206,13 @@ static void gen_check_nanbox_s(TCGv_i64 out, TCGv_i64 in)
     tcg_gen_movcond_i64(TCG_COND_GEU, out, in, t_max, in, t_nan);
 }
 
+static void decode_save_opc(DisasContext *ctx)
+{
+    assert(ctx->insn_start != NULL);
+    tcg_set_insn_start_param(ctx->insn_start, 1, ctx->opcode);
+    ctx->insn_start = NULL;
+}
+
 static void gen_set_pc_imm(DisasContext *ctx, target_ulong dest)
 {
     if (get_xl(ctx) == MXL_RV32) {
@@ -635,6 +642,8 @@ static void gen_set_rm(DisasContext *ctx, int rm)
         return;
     }
 
+    /* The helper may raise ILLEGAL_INSN -- record binv for unwind. */
+    decode_save_opc(ctx);
     gen_helper_set_rounding_mode(cpu_env, tcg_constant_i32(rm));
 }
 
@@ -1013,13 +1022,6 @@ static uint32_t opcode_at(DisasContextBase *dcbase, target_ulong pc)
 /* Include decoders for factored-out extensions */
 #include "decode-XVentanaCondOps.c.inc"
 
-static inline void decode_save_opc(DisasContext *ctx, target_ulong opc)
-{
-    assert(ctx->insn_start != NULL);
-    tcg_set_insn_start_param(ctx->insn_start, 1, opc);
-    ctx->insn_start = NULL;
-}
-
 static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
 {
     /*
@@ -1036,7 +1038,6 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
 
     /* Check for compressed insn */
     if (extract16(opcode, 0, 2) != 3) {
-        decode_save_opc(ctx, opcode);
         if (!has_ext(ctx, RVC)) {
             gen_exception_illegal(ctx);
         } else {
@@ -1051,7 +1052,6 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
         opcode32 = deposit32(opcode32, 16, 16,
                              translator_lduw(env, &ctx->base,
                                              ctx->base.pc_next + 2));
-        decode_save_opc(ctx, opcode32);
         ctx->opcode = opcode32;
         ctx->pc_succ_insn = ctx->base.pc_next + 4;
 
diff --git a/target/riscv/insn_trans/trans_privileged.c.inc b/target/riscv/insn_trans/trans_privileged.c.inc
index 53613682e8..46f96ad74d 100644
--- a/target/riscv/insn_trans/trans_privileged.c.inc
+++ b/target/riscv/insn_trans/trans_privileged.c.inc
@@ -75,6 +75,7 @@ static bool trans_sret(DisasContext *ctx, arg_sret *a)
 {
 #ifndef CONFIG_USER_ONLY
     if (has_ext(ctx, RVS)) {
+        decode_save_opc(ctx);
         gen_helper_sret(cpu_pc, cpu_env);
         tcg_gen_exit_tb(NULL, 0); /* no chaining */
         ctx->base.is_jmp = DISAS_NORETURN;
@@ -90,6 +91,7 @@ static bool trans_sret(DisasContext *ctx, arg_sret *a)
 static bool trans_mret(DisasContext *ctx, arg_mret *a)
 {
 #ifndef CONFIG_USER_ONLY
+    decode_save_opc(ctx);
     gen_helper_mret(cpu_pc, cpu_env);
     tcg_gen_exit_tb(NULL, 0); /* no chaining */
     ctx->base.is_jmp = DISAS_NORETURN;
@@ -102,6 +104,7 @@ static bool trans_mret(DisasContext *ctx, arg_mret *a)
 static bool trans_wfi(DisasContext *ctx, arg_wfi *a)
 {
 #ifndef CONFIG_USER_ONLY
+    decode_save_opc(ctx);
     gen_set_pc_imm(ctx, ctx->pc_succ_insn);
     gen_helper_wfi(cpu_env);
     return true;
@@ -113,6 +116,7 @@ static bool trans_wfi(DisasContext *ctx, arg_wfi *a)
 static bool trans_sfence_vma(DisasContext *ctx, arg_sfence_vma *a)
 {
 #ifndef CONFIG_USER_ONLY
+    decode_save_opc(ctx);
     gen_helper_tlb_flush(cpu_env);
     return true;
 #endif
diff --git a/target/riscv/insn_trans/trans_rvh.c.inc b/target/riscv/insn_trans/trans_rvh.c.inc
index cebcb3f8f6..4f8aecddc7 100644
--- a/target/riscv/insn_trans/trans_rvh.c.inc
+++ b/target/riscv/insn_trans/trans_rvh.c.inc
@@ -169,6 +169,7 @@ static bool trans_hfence_gvma(DisasContext *ctx, arg_sfence_vma *a)
 {
     REQUIRE_EXT(ctx, RVH);
 #ifndef CONFIG_USER_ONLY
+    decode_save_opc(ctx);
     gen_helper_hyp_gvma_tlb_flush(cpu_env);
     return true;
 #endif
@@ -179,6 +180,7 @@ static bool trans_hfence_vvma(DisasContext *ctx, arg_sfence_vma *a)
 {
     REQUIRE_EXT(ctx, RVH);
 #ifndef CONFIG_USER_ONLY
+    decode_save_opc(ctx);
     gen_helper_hyp_tlb_flush(cpu_env);
     return true;
 #endif
diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc
index c190a59f22..ca8e3d1ea1 100644
--- a/target/riscv/insn_trans/trans_rvi.c.inc
+++ b/target/riscv/insn_trans/trans_rvi.c.inc
@@ -818,6 +818,8 @@ static bool trans_fence_i(DisasContext *ctx, arg_fence_i *a)
 
 static bool do_csr_post(DisasContext *ctx)
 {
+    /* The helper may raise ILLEGAL_INSN -- record binv for unwind. */
+    decode_save_opc(ctx);
     /* We may have changed important cpu state -- exit to main loop. */
     gen_set_pc_imm(ctx, ctx->pc_succ_insn);
     tcg_gen_exit_tb(NULL, 0);
-- 
2.36.1



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PULL 05/19] target/riscv/pmp: guard against PMP ranges with a negative size
  2022-07-03  0:09 [PULL 00/19] riscv-to-apply queue Alistair Francis
                   ` (3 preceding siblings ...)
  2022-07-03  0:09 ` [PULL 04/19] target/riscv: Minimize the calls to decode_save_opc Alistair Francis
@ 2022-07-03  0:09 ` Alistair Francis
  2022-07-03  0:09 ` [PULL 06/19] target/riscv: Fix PMU CSR predicate function Alistair Francis
  2022-07-03  0:12 ` [PULL 00/19] riscv-to-apply queue Alistair Francis
  6 siblings, 0 replies; 8+ messages in thread
From: Alistair Francis @ 2022-07-03  0:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: alistair23, Nicolas Pitre, Alistair Francis

From: Nicolas Pitre <nico@fluxnic.net>

For a TOR entry to match, the stard address must be lower than the end
address. Normally this is always the case, but correct code might still
run into the following scenario:

Initial state:

	pmpaddr3 = 0x2000	pmp3cfg = OFF
	pmpaddr4 = 0x3000	pmp4cfg = TOR

Execution:

	1. write 0x40ff to pmpaddr3
	2. write 0x32ff to pmpaddr4
	3. set pmp3cfg to NAPOT with a read-modify-write on pmpcfg0
	4. set pmp4cfg to NAPOT with a read-modify-write on pmpcfg1

When (2) is emulated, a call to pmp_update_rule() creates a negative
range for pmp4 as pmp4cfg is still set to TOR. And when (3) is emulated,
a call to tlb_flush() is performed, causing pmp_get_tlb_size() to return
a very creatively large TLB size for pmp4. This, in turn, may result in
accesses to non-existent/unitialized memory regions and a fault, so that
(4) ends up never being executed.

This is in m-mode with MPRV unset, meaning that unlocked PMP entries
should have no effect. Therefore such a behavior based on PMP content
is very unexpected.

Make sure no negative PMP range can be created, whether explicitly by
the emulated code or implicitly like the above.

Signed-off-by: Nicolas Pitre <nico@fluxnic.net>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-Id: <3oq0sqs1-67o0-145-5n1s-453o118804q@syhkavp.arg>
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/pmp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
index 151da3fa08..ea2b67d947 100644
--- a/target/riscv/pmp.c
+++ b/target/riscv/pmp.c
@@ -167,6 +167,9 @@ void pmp_update_rule_addr(CPURISCVState *env, uint32_t pmp_index)
     case PMP_AMATCH_TOR:
         sa = prev_addr << 2; /* shift up from [xx:0] to [xx+2:2] */
         ea = (this_addr << 2) - 1u;
+        if (sa > ea) {
+            sa = ea = 0u;
+        }
         break;
 
     case PMP_AMATCH_NA4:
-- 
2.36.1



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PULL 06/19] target/riscv: Fix PMU CSR predicate function
  2022-07-03  0:09 [PULL 00/19] riscv-to-apply queue Alistair Francis
                   ` (4 preceding siblings ...)
  2022-07-03  0:09 ` [PULL 05/19] target/riscv/pmp: guard against PMP ranges with a negative size Alistair Francis
@ 2022-07-03  0:09 ` Alistair Francis
  2022-07-03  0:12 ` [PULL 00/19] riscv-to-apply queue Alistair Francis
  6 siblings, 0 replies; 8+ messages in thread
From: Alistair Francis @ 2022-07-03  0:09 UTC (permalink / raw)
  To: qemu-devel
  Cc: alistair23, Atish Patra, Alistair Francis, Bin Meng, Atish Patra

From: Atish Patra <atish.patra@wdc.com>

The predicate function calculates the counter index incorrectly for
hpmcounterx. Fix the counter index to reflect correct CSR number.

Fixes: e39a8320b088 ("target/riscv: Support the Virtual Instruction fault")
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Signed-off-by: Atish Patra <atish.patra@wdc.com>
Signed-off-by: Atish Patra <atishp@rivosinc.com>
Message-Id: <20220620231603.2547260-2-atishp@rivosinc.com>
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/csr.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 6dbe9b541f..46bd417cc1 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -72,6 +72,7 @@ static RISCVException ctr(CPURISCVState *env, int csrno)
 #if !defined(CONFIG_USER_ONLY)
     CPUState *cs = env_cpu(env);
     RISCVCPU *cpu = RISCV_CPU(cs);
+    int ctr_index;
 
     if (!cpu->cfg.ext_counters) {
         /* The Counters extensions is not enabled */
@@ -99,8 +100,9 @@ static RISCVException ctr(CPURISCVState *env, int csrno)
             }
             break;
         case CSR_HPMCOUNTER3...CSR_HPMCOUNTER31:
-            if (!get_field(env->hcounteren, 1 << (csrno - CSR_HPMCOUNTER3)) &&
-                get_field(env->mcounteren, 1 << (csrno - CSR_HPMCOUNTER3))) {
+            ctr_index = csrno - CSR_CYCLE;
+            if (!get_field(env->hcounteren, 1 << ctr_index) &&
+                 get_field(env->mcounteren, 1 << ctr_index)) {
                 return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
             }
             break;
@@ -126,8 +128,9 @@ static RISCVException ctr(CPURISCVState *env, int csrno)
                 }
                 break;
             case CSR_HPMCOUNTER3H...CSR_HPMCOUNTER31H:
-                if (!get_field(env->hcounteren, 1 << (csrno - CSR_HPMCOUNTER3H)) &&
-                    get_field(env->mcounteren, 1 << (csrno - CSR_HPMCOUNTER3H))) {
+                ctr_index = csrno - CSR_CYCLEH;
+                if (!get_field(env->hcounteren, 1 << ctr_index) &&
+                     get_field(env->mcounteren, 1 << ctr_index)) {
                     return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
                 }
                 break;
-- 
2.36.1



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PULL 00/19] riscv-to-apply queue
  2022-07-03  0:09 [PULL 00/19] riscv-to-apply queue Alistair Francis
                   ` (5 preceding siblings ...)
  2022-07-03  0:09 ` [PULL 06/19] target/riscv: Fix PMU CSR predicate function Alistair Francis
@ 2022-07-03  0:12 ` Alistair Francis
  6 siblings, 0 replies; 8+ messages in thread
From: Alistair Francis @ 2022-07-03  0:12 UTC (permalink / raw)
  To: Alistair Francis; +Cc: qemu-devel@nongnu.org Developers, Alistair Francis

On Sun, Jul 3, 2022 at 10:09 AM Alistair Francis
<alistair.francis@opensource.wdc.com> wrote:
>
> From: Alistair Francis <alistair@alistair23.me>
>
> The following changes since commit d495e432c04a6394126c35cf96517749708b410f:
>
>   Merge tag 'pull-aspeed-20220630' of https://github.com/legoater/qemu into staging (2022-06-30 22:04:12 +0530)
>
> are available in the Git repository at:
>
>   git@github.com:alistair23/qemu.git tags/pull-riscv-to-apply-20220703
>
> for you to fetch changes up to 435774992e82d2d16f025afbb20b4f7be9b242b0:
>
>   target/riscv: Update default priority table for local interrupts (2022-07-03 10:03:20 +1000)

Urgh, this is wrong. Sending a v2

Alistair


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-07-03  0:17 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-03  0:09 [PULL 00/19] riscv-to-apply queue Alistair Francis
2022-07-03  0:09 ` [PULL 01/19] target/riscv: Remove condition guarding register zero for auipc and lui Alistair Francis
2022-07-03  0:09 ` [PULL 02/19] target/riscv: Set env->bins in gen_exception_illegal Alistair Francis
2022-07-03  0:09 ` [PULL 03/19] target/riscv: Remove generate_exception_mtval Alistair Francis
2022-07-03  0:09 ` [PULL 04/19] target/riscv: Minimize the calls to decode_save_opc Alistair Francis
2022-07-03  0:09 ` [PULL 05/19] target/riscv/pmp: guard against PMP ranges with a negative size Alistair Francis
2022-07-03  0:09 ` [PULL 06/19] target/riscv: Fix PMU CSR predicate function Alistair Francis
2022-07-03  0:12 ` [PULL 00/19] riscv-to-apply queue Alistair Francis

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.