[PATCH] target/riscv: Add support for machine specific pmu's events
Was added call backs for machine specific pmu events.
Simplify monitor functions by adding new hash table, which going to map
counter number and event index.
Was added read/write callbacks which going to simplify support for events,
which expected to have different behavior.
Signed-off-by: Alexei Filippov
---
target/riscv/cpu.h | 9 +++
target/riscv/csr.c | 43 +-
target/riscv/pmu.c | 139 ++---
target/riscv/pmu.h | 11 ++--
4 files changed, 115 insertions(+), 87 deletions(-)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index a137b0f5a1..12542f413b 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -368,6 +368,13 @@ struct CPUArchState {
uint64_t (*rdtime_fn)(void *);
void *rdtime_fn_arg;
+/*machine specific pmu callback */
+void (*pmu_ctr_write)(PMUCTRState *counter, uint32_t event_idx,
+ target_ulong val, bool high_half);
+target_ulong (*pmu_ctr_read)(PMUCTRState *counter, uint32_t event_idx,
+ bool high_half);
+bool (*pmu_vendor_support)(uint32_t event_idx);
+
/* machine specific AIA ireg read-modify-write callback */
#define AIA_MAKE_IREG(__isel, __priv, __virt, __vgein, __xlen) \
__xlen) & 0xff) << 24) | \
@@ -454,6 +461,8 @@ struct ArchCPU {
uint32_t pmu_avail_ctrs;
/* Mapping of events to counters */
GHashTable *pmu_event_ctr_map;
+/* Mapping of counters to events */
+GHashTable *pmu_ctr_event_map;
};
/**
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index c47056ec33..21bb5bfdc4 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -862,20 +862,25 @@ static int write_mhpmcounter(CPURISCVState *env, int
csrno, target_ulong val)
int ctr_idx = csrno - CSR_MCYCLE;
PMUCTRState *counter = >pmu_ctrs[ctr_idx];
uint64_t mhpmctr_val = val;
+int event_idx;
counter->mhpmcounter_val = val;
-if (riscv_pmu_ctr_monitor_cycles(env, ctr_idx) ||
-riscv_pmu_ctr_monitor_instructions(env, ctr_idx)) {
-counter->mhpmcounter_prev = get_ticks(false);
-if (ctr_idx > 2) {
+event_idx = riscv_pmu_get_event_by_ctr(env, ctr_idx);
+
+if (event_idx != RISCV_PMU_EVENT_NOT_PRESENTED) {
+if (RISCV_PMU_CTR_IS_HPM(ctr_idx) && env->pmu_ctr_write) {
+env->pmu_ctr_write(counter, event_idx, val, false);
+} else {
+counter->mhpmcounter_prev = get_ticks(false);
+}
+if (RISCV_PMU_CTR_IS_HPM(ctr_idx)) {
if (riscv_cpu_mxl(env) == MXL_RV32) {
mhpmctr_val = mhpmctr_val |
((uint64_t)counter->mhpmcounterh_val << 32);
}
riscv_pmu_setup_timer(env, mhpmctr_val, ctr_idx);
}
- } else {
-/* Other counters can keep incrementing from the given value */
+} else {
counter->mhpmcounter_prev = val;
}
@@ -888,13 +893,19 @@ static int write_mhpmcounterh(CPURISCVState *env, int
csrno, target_ulong val)
PMUCTRState *counter = >pmu_ctrs[ctr_idx];
uint64_t mhpmctr_val = counter->mhpmcounter_val;
uint64_t mhpmctrh_val = val;
+int event_idx;
counter->mhpmcounterh_val = val;
mhpmctr_val = mhpmctr_val | (mhpmctrh_val << 32);
-if (riscv_pmu_ctr_monitor_cycles(env, ctr_idx) ||
-riscv_pmu_ctr_monitor_instructions(env, ctr_idx)) {
-counter->mhpmcounterh_prev = get_ticks(true);
-if (ctr_idx > 2) {
+event_idx = riscv_pmu_get_event_by_ctr(env, ctr_idx);
+
+if (event_idx != RISCV_PMU_EVENT_NOT_PRESENTED) {
+if (RISCV_PMU_CTR_IS_HPM(ctr_idx) && env->pmu_ctr_write) {
+env->pmu_ctr_write(counter, event_idx, val, true);
+} else {
+counter->mhpmcounterh_prev = get_ticks(true);
+}
+if (RISCV_PMU_CTR_IS_HPM(ctr_idx)) {
riscv_pmu_setup_timer(env, mhpmctr_val, ctr_idx);
}
} else {
@@ -912,6 +923,7 @@ static RISCVException riscv_pmu_read_ctr(CPURISCVState
*env, target_ulong *val,
counter->mhpmcounter_prev;
target_ulong ctr_val = upper_half ? counter->mhpmcounterh_val :
counter->mhpmcounter_val;
+int event_idx;
if (get_field(env->mcountinhibit, BIT(ctr_idx))) {
/*
@@ -932,9 +944,14 @@ static RISCVException riscv_pmu_read_ctr(CPURISCVState
*env, target_ulong *val,
* The kernel computes the perf delta by subtracting the current value from
* the value it initialized previously (ctr_val).
*/
-if (riscv_pmu_ctr_monitor_cycles(env, ctr_idx) ||
-riscv_pmu_ctr_monitor_instructions(env, ctr_idx)) {
-*val = get_ticks(upper_half) - ctr_prev + ctr_val;
+event_idx = riscv_pmu_get_event_by_ctr(env, ctr_idx);
+if (event_idx != RISCV_PMU_EVENT_NOT_PR
[PATCH v8] target/riscv/kvm/kvm-cpu.c: kvm_riscv_handle_sbi() fail with vendor-specific SBI
kvm_riscv_handle_sbi() may return not supported return code to not
trigger qemu abort with vendor-specific sbi.
Add new error path to provide proper error in case of
qemu_chr_fe_read_all() may not return sizeof(ch), because exactly zero
just means we failed to read input, which can happen, so
telling the SBI caller we failed to read, but telling the caller of this
function that we successfully emulated the SBI call, is correct. However,
anything else, other than sizeof(ch), means something unexpected happened,
so we should return an error.
Added SBI related return code's defines.
Signed-off-by: Alexei Filippov
Fixes: 4eb47125 ("target/riscv: Handle KVM_EXIT_RISCV_SBI exit")
---
Changes since v7:
- Fix error handling according to Andrew Jones suggestion.
target/riscv/kvm/kvm-cpu.c | 9 +
target/riscv/sbi_ecall_interface.h | 12
2 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index 235e2cdaca..1afbabe19f 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -1515,19 +1515,20 @@ static int kvm_riscv_handle_sbi(CPUState *cs, struct
kvm_run *run)
ret = qemu_chr_fe_read_all(serial_hd(0)->be, , sizeof(ch));
if (ret == sizeof(ch)) {
run->riscv_sbi.ret[0] = ch;
+} else if (ret == 0) {
+run->riscv_sbi.ret[0] = SBI_ERR_FAILURE;
} else {
-run->riscv_sbi.ret[0] = -1;
+ret = -1;
}
-ret = 0;
break;
case SBI_EXT_DBCN:
kvm_riscv_handle_sbi_dbcn(cs, run);
break;
default:
qemu_log_mask(LOG_UNIMP,
- "%s: un-handled SBI EXIT, specific reasons is %lu\n",
+ "%s: Unhandled SBI exit with extension-id %lu\n",
__func__, run->riscv_sbi.extension_id);
-ret = -1;
+run->riscv_sbi.ret[0] = SBI_ERR_NOT_SUPPORTED;
break;
}
return ret;
diff --git a/target/riscv/sbi_ecall_interface.h
b/target/riscv/sbi_ecall_interface.h
index 7dfe5f72c6..4df0accd78 100644
--- a/target/riscv/sbi_ecall_interface.h
+++ b/target/riscv/sbi_ecall_interface.h
@@ -86,4 +86,16 @@
#define SBI_EXT_VENDOR_END 0x09FF
/* clang-format on */
+/* SBI return error codes */
+#define SBI_SUCCESS 0
+#define SBI_ERR_FAILURE -1
+#define SBI_ERR_NOT_SUPPORTED -2
+#define SBI_ERR_INVALID_PARAM -3
+#define SBI_ERR_DENIED -4
+#define SBI_ERR_INVALID_ADDRESS -5
+#define SBI_ERR_ALREADY_AVAILABLE -6
+#define SBI_ERR_ALREADY_STARTED -7
+#define SBI_ERR_ALREADY_STOPPED -8
+#define SBI_ERR_NO_SHMEM-9
+
#endif
--
2.34.1
[PATCH v2] target/riscv: Add support for machine specific pmu's events
Was added call backs for machine specific pmu events.
Simplify monitor functions by adding new hash table, which going to map
counter number and event index.
Was added read/write callbacks which going to simplify support for events,
which expected to have different behavior.
Signed-off-by: Alexei Filippov
---
Changes since v2:
-rebased to latest master
target/riscv/cpu.h | 9 +++
target/riscv/csr.c | 43 +-
target/riscv/pmu.c | 139 ++---
target/riscv/pmu.h | 11 ++--
4 files changed, 115 insertions(+), 87 deletions(-)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 6fe0d712b4..fbf82b050b 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -374,6 +374,13 @@ struct CPUArchState {
uint64_t (*rdtime_fn)(void *);
void *rdtime_fn_arg;
+/*machine specific pmu callback */
+void (*pmu_ctr_write)(PMUCTRState *counter, uint32_t event_idx,
+ target_ulong val, bool high_half);
+target_ulong (*pmu_ctr_read)(PMUCTRState *counter, uint32_t event_idx,
+ bool high_half);
+bool (*pmu_vendor_support)(uint32_t event_idx);
+
/* machine specific AIA ireg read-modify-write callback */
#define AIA_MAKE_IREG(__isel, __priv, __virt, __vgein, __xlen) \
__xlen) & 0xff) << 24) | \
@@ -455,6 +462,8 @@ struct ArchCPU {
uint32_t pmu_avail_ctrs;
/* Mapping of events to counters */
GHashTable *pmu_event_ctr_map;
+/* Mapping of counters to events */
+GHashTable *pmu_ctr_event_map;
const GPtrArray *decoders;
};
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 58ef7079dc..b541852c84 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -875,20 +875,25 @@ static RISCVException write_mhpmcounter(CPURISCVState
*env, int csrno,
int ctr_idx = csrno - CSR_MCYCLE;
PMUCTRState *counter = >pmu_ctrs[ctr_idx];
uint64_t mhpmctr_val = val;
+int event_idx;
counter->mhpmcounter_val = val;
-if (riscv_pmu_ctr_monitor_cycles(env, ctr_idx) ||
-riscv_pmu_ctr_monitor_instructions(env, ctr_idx)) {
-counter->mhpmcounter_prev = get_ticks(false);
-if (ctr_idx > 2) {
+event_idx = riscv_pmu_get_event_by_ctr(env, ctr_idx);
+
+if (event_idx != RISCV_PMU_EVENT_NOT_PRESENTED) {
+if (RISCV_PMU_CTR_IS_HPM(ctr_idx) && env->pmu_ctr_write) {
+env->pmu_ctr_write(counter, event_idx, val, false);
+} else {
+counter->mhpmcounter_prev = get_ticks(false);
+}
+if (RISCV_PMU_CTR_IS_HPM(ctr_idx)) {
if (riscv_cpu_mxl(env) == MXL_RV32) {
mhpmctr_val = mhpmctr_val |
((uint64_t)counter->mhpmcounterh_val << 32);
}
riscv_pmu_setup_timer(env, mhpmctr_val, ctr_idx);
}
- } else {
-/* Other counters can keep incrementing from the given value */
+} else {
counter->mhpmcounter_prev = val;
}
@@ -902,13 +907,19 @@ static RISCVException write_mhpmcounterh(CPURISCVState
*env, int csrno,
PMUCTRState *counter = >pmu_ctrs[ctr_idx];
uint64_t mhpmctr_val = counter->mhpmcounter_val;
uint64_t mhpmctrh_val = val;
+int event_idx;
counter->mhpmcounterh_val = val;
mhpmctr_val = mhpmctr_val | (mhpmctrh_val << 32);
-if (riscv_pmu_ctr_monitor_cycles(env, ctr_idx) ||
-riscv_pmu_ctr_monitor_instructions(env, ctr_idx)) {
-counter->mhpmcounterh_prev = get_ticks(true);
-if (ctr_idx > 2) {
+event_idx = riscv_pmu_get_event_by_ctr(env, ctr_idx);
+
+if (event_idx != RISCV_PMU_EVENT_NOT_PRESENTED) {
+if (RISCV_PMU_CTR_IS_HPM(ctr_idx) && env->pmu_ctr_write) {
+env->pmu_ctr_write(counter, event_idx, val, true);
+} else {
+counter->mhpmcounterh_prev = get_ticks(true);
+}
+if (RISCV_PMU_CTR_IS_HPM(ctr_idx)) {
riscv_pmu_setup_timer(env, mhpmctr_val, ctr_idx);
}
} else {
@@ -926,6 +937,7 @@ static RISCVException riscv_pmu_read_ctr(CPURISCVState
*env, target_ulong *val,
counter->mhpmcounter_prev;
target_ulong ctr_val = upper_half ? counter->mhpmcounterh_val :
counter->mhpmcounter_val;
+int event_idx;
if (get_field(env->mcountinhibit, BIT(ctr_idx))) {
/*
@@ -946,9 +958,14 @@ static RISCVException riscv_pmu_read_ctr(CPURISCVState
*env, target_ulong *val,
* The kernel computes the perf delta by subtracting the current value from
* the value it initialized previously (ctr_val).
*/
-if (riscv_pmu_ctr_monitor_cycles(env, ctr_idx) ||
-riscv_pmu_ctr_monitor_instructions(env, ctr_idx)) {
-*val = get_ticks(upper_half) - ctr_prev + ctr_val;
+even
[PATCH v7] target/riscv/kvm/kvm-cpu.c: kvm_riscv_handle_sbi() fail with vendor-specific SBI
kvm_riscv_handle_sbi() may return not supported return code to not
trigger qemu abort with vendor-specific sbi.
Add new error path to provide proper error in case of
qemu_chr_fe_read_all() may not return sizeof(ch), because exactly zero
just means we failed to read input, which can happen, so
telling the SBI caller we failed to read, but telling the caller of this
function that we successfully emulated the SBI call, is correct. However,
anything else, other than sizeof(ch), means something unexpected happened,
so we should return an error.
Added SBI related return code's defines.
Signed-off-by: Alexei Filippov
Fixes: 4eb47125 ("target/riscv: Handle KVM_EXIT_RISCV_SBI exit")
---
Changes since v6:
- Add appropriate commit message.
- Fix error handling according to Andrew Jones suggestion.
target/riscv/kvm/kvm-cpu.c | 11 +++
target/riscv/sbi_ecall_interface.h | 12
2 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index f9dbc18a76..a84bcda9d9 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -1173,16 +1173,19 @@ static int kvm_riscv_handle_sbi(CPUState *cs, struct
kvm_run *run)
ret = qemu_chr_fe_read_all(serial_hd(0)->be, , sizeof(ch));
if (ret == sizeof(ch)) {
run->riscv_sbi.ret[0] = ch;
+ret = 0;
} else {
-run->riscv_sbi.ret[0] = -1;
+if (ret == 0) {
+run->riscv_sbi.ret[0] = SBI_ERR_FAILURE;
+}
+ret = -1;
}
-ret = 0;
break;
default:
qemu_log_mask(LOG_UNIMP,
- "%s: un-handled SBI EXIT, specific reasons is %lu\n",
+ "%s: Unhandled SBI exit with extension-id %lu\n",
__func__, run->riscv_sbi.extension_id);
-ret = -1;
+run->riscv_sbi.ret[0] = SBI_ERR_NOT_SUPPORTED;
break;
}
return ret;
diff --git a/target/riscv/sbi_ecall_interface.h
b/target/riscv/sbi_ecall_interface.h
index 43899d08f6..a2e21d9b8c 100644
--- a/target/riscv/sbi_ecall_interface.h
+++ b/target/riscv/sbi_ecall_interface.h
@@ -69,4 +69,16 @@
#define SBI_EXT_VENDOR_END 0x09FF
/* clang-format on */
+/* SBI return error codes */
+#define SBI_SUCCESS 0
+#define SBI_ERR_FAILURE -1
+#define SBI_ERR_NOT_SUPPORTED -2
+#define SBI_ERR_INVALID_PARAM -3
+#define SBI_ERR_DENIED -4
+#define SBI_ERR_INVALID_ADDRESS -5
+#define SBI_ERR_ALREADY_AVAILABLE -6
+#define SBI_ERR_ALREADY_STARTED -7
+#define SBI_ERR_ALREADY_STOPPED -8
+#define SBI_ERR_NO_SHMEM-9
+
#endif
--
2.34.1
[PATCH v2 1/2] target/riscv: prioritize pmp errors in raise_mmu_exception()
From: Daniel Henrique Barboza
raise_mmu_exception(), as is today, is prioritizing guest page faults by
checking first if virt_enabled && !first_stage, and then considering the
regular inst/load/store faults.
There's no mention in the spec about guest page fault being a higher
priority that PMP faults. In fact, privileged spec section 3.7.1 says:
"Attempting to fetch an instruction from a PMP region that does not have
execute permissions raises an instruction access-fault exception.
Attempting to execute a load or load-reserved instruction which accesses
a physical address within a PMP region without read permissions raises a
load access-fault exception. Attempting to execute a store,
store-conditional, or AMO instruction which accesses a physical address
within a PMP region without write permissions raises a store
access-fault exception."
So, in fact, we're doing it wrong - PMP faults should always be thrown,
regardless of also being a first or second stage fault.
The way riscv_cpu_tlb_fill() and get_physical_address() work is
adequate: a TRANSLATE_PMP_FAIL error is immediately reported and
reflected in the 'pmp_violation' flag. What we need is to change
raise_mmu_exception() to prioritize it.
Reported-by: Joseph Chan
Fixes: 82d53adfbb ("target/riscv/cpu_helper.c: Invalid exception on MMU
translation stage")
Reviewed-by: Alistair Francis
Signed-off-by: Daniel Henrique Barboza
---
target/riscv/cpu_helper.c | 22 --
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index fc090d729a..e3a7797d00 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -1176,28 +1176,30 @@ static void raise_mmu_exception(CPURISCVState *env,
target_ulong address,
switch (access_type) {
case MMU_INST_FETCH:
-if (env->virt_enabled && !first_stage) {
+if (pmp_violation) {
+cs->exception_index = RISCV_EXCP_INST_ACCESS_FAULT;
+} else if (env->virt_enabled && !first_stage) {
cs->exception_index = RISCV_EXCP_INST_GUEST_PAGE_FAULT;
} else {
-cs->exception_index = pmp_violation ?
-RISCV_EXCP_INST_ACCESS_FAULT : RISCV_EXCP_INST_PAGE_FAULT;
+cs->exception_index = RISCV_EXCP_INST_PAGE_FAULT;
}
break;
case MMU_DATA_LOAD:
-if (two_stage && !first_stage) {
+if (pmp_violation) {
+cs->exception_index = RISCV_EXCP_LOAD_ACCESS_FAULT;
+} else if (two_stage && !first_stage) {
cs->exception_index = RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT;
} else {
-cs->exception_index = pmp_violation ?
-RISCV_EXCP_LOAD_ACCESS_FAULT : RISCV_EXCP_LOAD_PAGE_FAULT;
+cs->exception_index = RISCV_EXCP_LOAD_PAGE_FAULT;
}
break;
case MMU_DATA_STORE:
-if (two_stage && !first_stage) {
+if (pmp_violation) {
+cs->exception_index = RISCV_EXCP_STORE_AMO_ACCESS_FAULT;
+} else if (two_stage && !first_stage) {
cs->exception_index = RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT;
} else {
-cs->exception_index = pmp_violation ?
-RISCV_EXCP_STORE_AMO_ACCESS_FAULT :
-RISCV_EXCP_STORE_PAGE_FAULT;
+cs->exception_index = RISCV_EXCP_STORE_PAGE_FAULT;
}
break;
default:
--
2.34.1
[PATCH v2 2/2] target/riscv: do not set mtval2 for non guest-page faults
Previous patch fixed the PMP priority in raise_mmu_exception() but we're still
setting mtval2 incorrectly. In riscv_cpu_tlb_fill(), after pmp check in 2 stage
translation part, mtval2 will be set in case of successes 2 stage translation
but
failed pmp check.
In this case we gonna set mtval2 via env->guest_phys_fault_addr in context of
riscv_cpu_tlb_fill(), as this was a guest-page-fault, but it didn't and mtval2
should be zero, according to RISCV privileged spec sect. 9.4.4: When a guest
page-fault is taken into M-mode, mtval2 is written with either zero or guest
physical address that faulted, shifted by 2 bits. *For other traps, mtval2
is set to zero...*
Signed-off-by: Alexei Filippov
Reviewed-by: Daniel Henrique Barboza
---
Changes since v1:
-Added Reviewed-by tag.
target/riscv/cpu_helper.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index e3a7797d00..484edad900 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -1375,17 +1375,17 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address,
int size,
__func__, pa, ret, prot_pmp, tlb_size);
prot &= prot_pmp;
-}
-
-if (ret != TRANSLATE_SUCCESS) {
+} else {
/*
* Guest physical address translation failed, this is a HS
* level exception
*/
first_stage_error = false;
-env->guest_phys_fault_addr = (im_address |
- (address &
- (TARGET_PAGE_SIZE - 1))) >> 2;
+if (ret != TRANSLATE_PMP_FAIL) {
+env->guest_phys_fault_addr = (im_address |
+ (address &
+ (TARGET_PAGE_SIZE - 1))) >>
2;
+}
}
}
} else {
--
2.34.1
[PATCH v6] target/riscv/kvm/kvm-cpu.c: kvm_riscv_handle_sbi() fail with vendor-specific SBI
kvm_riscv_handle_sbi() may return not supported return code to not
trigger qemu abort with vendor-specific sbi.
Add new error path to provide proper error in case of
qemu_chr_fe_read_all() may not return sizeof(ch).
Added SBI related return code's defines.
Signed-off-by: Alexei Filippov
---
Changes since v4-5:
-Added new error path in case of qemu_chr_fe_read_all() may not
return sizeof(ch).
-Added more comments in commit message.
target/riscv/kvm/kvm-cpu.c | 10 ++
target/riscv/sbi_ecall_interface.h | 12
2 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index f9dbc18a76..5bb7b74d03 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -1173,16 +1173,18 @@ static int kvm_riscv_handle_sbi(CPUState *cs, struct
kvm_run *run)
ret = qemu_chr_fe_read_all(serial_hd(0)->be, , sizeof(ch));
if (ret == sizeof(ch)) {
run->riscv_sbi.ret[0] = ch;
+ret = 0;
+} else if (ret == 0) {
+run->riscv_sbi.ret[0] = SBI_ERR_FAILURE;
} else {
-run->riscv_sbi.ret[0] = -1;
+ret = -1;
}
-ret = 0;
break;
default:
qemu_log_mask(LOG_UNIMP,
- "%s: un-handled SBI EXIT, specific reasons is %lu\n",
+ "%s: Unhandled SBI exit with extension-id %lu\n"
__func__, run->riscv_sbi.extension_id);
-ret = -1;
+run->riscv_sbi.ret[0] = SBI_ERR_NOT_SUPPORTED;
break;
}
return ret;
diff --git a/target/riscv/sbi_ecall_interface.h
b/target/riscv/sbi_ecall_interface.h
index 43899d08f6..a2e21d9b8c 100644
--- a/target/riscv/sbi_ecall_interface.h
+++ b/target/riscv/sbi_ecall_interface.h
@@ -69,4 +69,16 @@
#define SBI_EXT_VENDOR_END 0x09FF
/* clang-format on */
+/* SBI return error codes */
+#define SBI_SUCCESS 0
+#define SBI_ERR_FAILURE -1
+#define SBI_ERR_NOT_SUPPORTED -2
+#define SBI_ERR_INVALID_PARAM -3
+#define SBI_ERR_DENIED -4
+#define SBI_ERR_INVALID_ADDRESS -5
+#define SBI_ERR_ALREADY_AVAILABLE -6
+#define SBI_ERR_ALREADY_STARTED -7
+#define SBI_ERR_ALREADY_STOPPED -8
+#define SBI_ERR_NO_SHMEM-9
+
#endif
--
2.34.1
[PATCH v5] target/riscv/kvm/kvm-cpu.c: kvm_riscv_handle_sbi() fail with vendor-specific sbi.
kvm_riscv_handle_sbi() may return not supported return code to not
trigger qemu abort with vendor-specific sbi.
Add new error path to provide proper error in case of
qemu_chr_fe_read_all() may not return sizeof(ch).
Added SBI related return code's defines.
Signed-off-by: Alexei Filippov
---
target/riscv/kvm/kvm-cpu.c | 9 +
target/riscv/sbi_ecall_interface.h | 1 +
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index aeca1e3e83..5bb7b74d03 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -1173,17 +1173,18 @@ static int kvm_riscv_handle_sbi(CPUState *cs, struct
kvm_run *run)
ret = qemu_chr_fe_read_all(serial_hd(0)->be, , sizeof(ch));
if (ret == sizeof(ch)) {
run->riscv_sbi.ret[0] = ch;
-} else {
+ret = 0;
+} else if (ret == 0) {
run->riscv_sbi.ret[0] = SBI_ERR_FAILURE;
+} else {
+ret = -1;
}
-ret = 0;
break;
default:
qemu_log_mask(LOG_UNIMP,
- "%s: un-handled SBI EXIT, specific reasons is %lu\n",
+ "%s: Unhandled SBI exit with extension-id %lu\n"
__func__, run->riscv_sbi.extension_id);
run->riscv_sbi.ret[0] = SBI_ERR_NOT_SUPPORTED;
-ret = 0;
break;
}
return ret;
diff --git a/target/riscv/sbi_ecall_interface.h
b/target/riscv/sbi_ecall_interface.h
index 0279e92a36..a2e21d9b8c 100644
--- a/target/riscv/sbi_ecall_interface.h
+++ b/target/riscv/sbi_ecall_interface.h
@@ -79,5 +79,6 @@
#define SBI_ERR_ALREADY_AVAILABLE -6
#define SBI_ERR_ALREADY_STARTED -7
#define SBI_ERR_ALREADY_STOPPED -8
+#define SBI_ERR_NO_SHMEM-9
#endif
--
2.34.1
[PATCH v4] target/riscv/kvm/kvm-cpu.c: kvm_riscv_handle_sbi() fail with vendor-specific SBI
kvm_riscv_handle_sbi() may return not supported return code to not trigger
qemu abort with vendor-specific sbi.
Added SBI related return code's defines.
Signed-off-by: Alexei Filippov
Fixes: 4eb47125 ("target/riscv: Handle KVM_EXIT_RISCV_SBI exit")
---
Changes since v3:
-Clear Reviewed-by tags
target/riscv/kvm/kvm-cpu.c | 13 +
target/riscv/sbi_ecall_interface.h | 12
2 files changed, 17 insertions(+), 8 deletions(-)
diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index 6a6c6cae80..844942d9ba 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -1392,7 +1392,6 @@ bool kvm_arch_stop_on_emulation_error(CPUState *cs)
static int kvm_riscv_handle_sbi(CPUState *cs, struct kvm_run *run)
{
-int ret = 0;
unsigned char ch;
switch (run->riscv_sbi.extension_id) {
case SBI_EXT_0_1_CONSOLE_PUTCHAR:
@@ -1400,22 +1399,20 @@ static int kvm_riscv_handle_sbi(CPUState *cs, struct
kvm_run *run)
qemu_chr_fe_write(serial_hd(0)->be, , sizeof(ch));
break;
case SBI_EXT_0_1_CONSOLE_GETCHAR:
-ret = qemu_chr_fe_read_all(serial_hd(0)->be, , sizeof(ch));
-if (ret == sizeof(ch)) {
+if (qemu_chr_fe_read_all(serial_hd(0)->be, , sizeof(ch)) ==
sizeof(ch)) {
run->riscv_sbi.ret[0] = ch;
} else {
-run->riscv_sbi.ret[0] = -1;
+run->riscv_sbi.ret[0] = SBI_ERR_FAILURE;
}
-ret = 0;
break;
default:
qemu_log_mask(LOG_UNIMP,
- "%s: un-handled SBI EXIT, specific reasons is %lu\n",
+ "%s: Unhandled SBI exit with extension-id %lu\n",
__func__, run->riscv_sbi.extension_id);
-ret = -1;
+run->riscv_sbi.ret[0] = SBI_ERR_NOT_SUPPORTED;
break;
}
-return ret;
+return 0;
}
int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
diff --git a/target/riscv/sbi_ecall_interface.h
b/target/riscv/sbi_ecall_interface.h
index 43899d08f6..a2e21d9b8c 100644
--- a/target/riscv/sbi_ecall_interface.h
+++ b/target/riscv/sbi_ecall_interface.h
@@ -69,4 +69,16 @@
#define SBI_EXT_VENDOR_END 0x09FF
/* clang-format on */
+/* SBI return error codes */
+#define SBI_SUCCESS 0
+#define SBI_ERR_FAILURE -1
+#define SBI_ERR_NOT_SUPPORTED -2
+#define SBI_ERR_INVALID_PARAM -3
+#define SBI_ERR_DENIED -4
+#define SBI_ERR_INVALID_ADDRESS -5
+#define SBI_ERR_ALREADY_AVAILABLE -6
+#define SBI_ERR_ALREADY_STARTED -7
+#define SBI_ERR_ALREADY_STOPPED -8
+#define SBI_ERR_NO_SHMEM-9
+
#endif
--
2.34.1
[PATCH 2/2] target/riscv: do not set mtval2 for non guest-page faults
Previous patch fixed the PMP priority in raise_mmu_exception() but we're still
setting mtval2 incorrectly. In riscv_cpu_tlb_fill(), after pmp check in 2 stage
translation part, mtval2 will be set in case of successes 2 stage translation
but
failed pmp check.
In this case we gonna set mtval2 via env->guest_phys_fault_addr in context of
riscv_cpu_tlb_fill(), as this was a guest-page-fault, but it didn't and mtval2
should be zero, according to RISCV privileged spec sect. 9.4.4: When a guest
page-fault is taken into M-mode, mtval2 is written with either zero or guest
physical address that faulted, shifted by 2 bits. *For other traps, mtval2
is set to zero...*
Signed-off-by: Alexei Filippov
---
target/riscv/cpu_helper.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 196166f8dd..89e659fe3a 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -1410,17 +1410,17 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address,
int size,
__func__, pa, ret, prot_pmp, tlb_size);
prot &= prot_pmp;
-}
-
-if (ret != TRANSLATE_SUCCESS) {
+} else {
/*
* Guest physical address translation failed, this is a HS
* level exception
*/
first_stage_error = false;
-env->guest_phys_fault_addr = (im_address |
- (address &
- (TARGET_PAGE_SIZE - 1))) >> 2;
+if (ret != TRANSLATE_PMP_FAIL) {
+env->guest_phys_fault_addr = (im_address |
+ (address &
+ (TARGET_PAGE_SIZE - 1))) >>
2;
+}
}
}
} else {
--
2.34.1
[PATCH 1/2] target/riscv: prioritize pmp errors in raise_mmu_exception()
From: Daniel Henrique Barboza
raise_mmu_exception(), as is today, is prioritizing guest page faults by
checking first if virt_enabled && !first_stage, and then considering the
regular inst/load/store faults.
There's no mention in the spec about guest page fault being a higher
priority that PMP faults. In fact, privileged spec section 3.7.1 says:
"Attempting to fetch an instruction from a PMP region that does not have
execute permissions raises an instruction access-fault exception.
Attempting to execute a load or load-reserved instruction which accesses
a physical address within a PMP region without read permissions raises a
load access-fault exception. Attempting to execute a store,
store-conditional, or AMO instruction which accesses a physical address
within a PMP region without write permissions raises a store
access-fault exception."
So, in fact, we're doing it wrong - PMP faults should always be thrown,
regardless of also being a first or second stage fault.
The way riscv_cpu_tlb_fill() and get_physical_address() work is
adequate: a TRANSLATE_PMP_FAIL error is immediately reported and
reflected in the 'pmp_violation' flag. What we need is to change
raise_mmu_exception() to prioritize it.
Reported-by: Joseph Chan
Fixes: 82d53adfbb ("target/riscv/cpu_helper.c: Invalid exception on MMU
translation stage")
Signed-off-by: Daniel Henrique Barboza
---
target/riscv/cpu_helper.c | 22 --
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index bc70ab5abc..196166f8dd 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -1203,28 +1203,30 @@ static void raise_mmu_exception(CPURISCVState *env,
target_ulong address,
switch (access_type) {
case MMU_INST_FETCH:
-if (env->virt_enabled && !first_stage) {
+if (pmp_violation) {
+cs->exception_index = RISCV_EXCP_INST_ACCESS_FAULT;
+} else if (env->virt_enabled && !first_stage) {
cs->exception_index = RISCV_EXCP_INST_GUEST_PAGE_FAULT;
} else {
-cs->exception_index = pmp_violation ?
-RISCV_EXCP_INST_ACCESS_FAULT : RISCV_EXCP_INST_PAGE_FAULT;
+cs->exception_index = RISCV_EXCP_INST_PAGE_FAULT;
}
break;
case MMU_DATA_LOAD:
-if (two_stage && !first_stage) {
+if (pmp_violation) {
+cs->exception_index = RISCV_EXCP_LOAD_ACCESS_FAULT;
+} else if (two_stage && !first_stage) {
cs->exception_index = RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT;
} else {
-cs->exception_index = pmp_violation ?
-RISCV_EXCP_LOAD_ACCESS_FAULT : RISCV_EXCP_LOAD_PAGE_FAULT;
+cs->exception_index = RISCV_EXCP_LOAD_PAGE_FAULT;
}
break;
case MMU_DATA_STORE:
-if (two_stage && !first_stage) {
+if (pmp_violation) {
+cs->exception_index = RISCV_EXCP_STORE_AMO_ACCESS_FAULT;
+} else if (two_stage && !first_stage) {
cs->exception_index = RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT;
} else {
-cs->exception_index = pmp_violation ?
-RISCV_EXCP_STORE_AMO_ACCESS_FAULT :
-RISCV_EXCP_STORE_PAGE_FAULT;
+cs->exception_index = RISCV_EXCP_STORE_PAGE_FAULT;
}
break;
default:
--
2.34.1
[PATCH] target/riscv/cpu_helper.c: fix wrong exception raise
Successed two stage translation, but failed pmp check can cause guest
page fault instead of regular page fault.
In case of execution ld instuction in VS mode we can
face situation when two stages of translation was passed successfully,
and if PMP check was failed first_stage_error variable going to be
setted to false and raise_mmu_exception will raise
RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT(scause == 21) instead of
RISCV_EXCP_LOAD_ACCESS_FAULT(scause == 5) and this is wrong, according
to RISCV privileged spec sect. 3.7: Attempting to execute a load or
load-reserved instruction which accesses a physical address within a
PMP region without read permissions raises a load access-fault
exception.
Signed-off-by: Alexei Filippov
---
target/riscv/cpu_helper.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index bc70ab5abc..eaef1c2c62 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -1408,9 +1408,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int
size,
__func__, pa, ret, prot_pmp, tlb_size);
prot &= prot_pmp;
-}
-
-if (ret != TRANSLATE_SUCCESS) {
+} else {
/*
* Guest physical address translation failed, this is a HS
* level exception
--
2.34.1
[PATCH v3] target/riscv/kvm/kvm-cpu.c: kvm_riscv_handle_sbi() fail with vendor-specific SBI
kvm_riscv_handle_sbi() may return not supported return code to not trigger
qemu abort with vendor-specific sbi.
Added SBI related return code's defines.
Signed-off-by: Alexei Filippov
Fixes: 4eb47125 ("target/riscv: Handle KVM_EXIT_RISCV_SBI exit")
Reviewed-by: Daniel Henrique Barboza
Reviewed-by: Alistair Francis
---
Changes since v2:
-Clear kvm_riscv_handle_sbi()
-Add SBI_ERR_NO_SHMEM return code's define
target/riscv/kvm/kvm-cpu.c | 13 +
target/riscv/sbi_ecall_interface.h | 12
2 files changed, 17 insertions(+), 8 deletions(-)
diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index 6a6c6cae80..844942d9ba 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -1392,7 +1392,6 @@ bool kvm_arch_stop_on_emulation_error(CPUState *cs)
static int kvm_riscv_handle_sbi(CPUState *cs, struct kvm_run *run)
{
-int ret = 0;
unsigned char ch;
switch (run->riscv_sbi.extension_id) {
case SBI_EXT_0_1_CONSOLE_PUTCHAR:
@@ -1400,22 +1399,20 @@ static int kvm_riscv_handle_sbi(CPUState *cs, struct
kvm_run *run)
qemu_chr_fe_write(serial_hd(0)->be, , sizeof(ch));
break;
case SBI_EXT_0_1_CONSOLE_GETCHAR:
-ret = qemu_chr_fe_read_all(serial_hd(0)->be, , sizeof(ch));
-if (ret == sizeof(ch)) {
+if (qemu_chr_fe_read_all(serial_hd(0)->be, , sizeof(ch)) ==
sizeof(ch)) {
run->riscv_sbi.ret[0] = ch;
} else {
-run->riscv_sbi.ret[0] = -1;
+run->riscv_sbi.ret[0] = SBI_ERR_FAILURE;
}
-ret = 0;
break;
default:
qemu_log_mask(LOG_UNIMP,
- "%s: un-handled SBI EXIT, specific reasons is %lu\n",
+ "%s: Unhandled SBI exit with extension-id %lu\n",
__func__, run->riscv_sbi.extension_id);
-ret = -1;
+run->riscv_sbi.ret[0] = SBI_ERR_NOT_SUPPORTED;
break;
}
-return ret;
+return 0;
}
int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
diff --git a/target/riscv/sbi_ecall_interface.h
b/target/riscv/sbi_ecall_interface.h
index 43899d08f6..a2e21d9b8c 100644
--- a/target/riscv/sbi_ecall_interface.h
+++ b/target/riscv/sbi_ecall_interface.h
@@ -69,4 +69,16 @@
#define SBI_EXT_VENDOR_END 0x09FF
/* clang-format on */
+/* SBI return error codes */
+#define SBI_SUCCESS 0
+#define SBI_ERR_FAILURE -1
+#define SBI_ERR_NOT_SUPPORTED -2
+#define SBI_ERR_INVALID_PARAM -3
+#define SBI_ERR_DENIED -4
+#define SBI_ERR_INVALID_ADDRESS -5
+#define SBI_ERR_ALREADY_AVAILABLE -6
+#define SBI_ERR_ALREADY_STARTED -7
+#define SBI_ERR_ALREADY_STOPPED -8
+#define SBI_ERR_NO_SHMEM-9
+
#endif
--
2.34.1
[PATCH] target/riscv/kvm/kvm-cpu.c: kvm_riscv_handle_sbi() fail with vendor-specific SBI
kvm_riscv_handle_sbi() may return not supported return code to not trigger
qemu abort with vendor-specific sbi.
Added SBI related return code's defines.
Signed-off-by: Alexei Filippov
---
target/riscv/kvm/kvm-cpu.c | 5 +++--
target/riscv/sbi_ecall_interface.h | 11 +++
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index 6a6c6cae80..a4f84ad950 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -1404,7 +1404,7 @@ static int kvm_riscv_handle_sbi(CPUState *cs, struct
kvm_run *run)
if (ret == sizeof(ch)) {
run->riscv_sbi.ret[0] = ch;
} else {
-run->riscv_sbi.ret[0] = -1;
+run->riscv_sbi.ret[0] = SBI_ERR_FAILURE;
}
ret = 0;
break;
@@ -1412,7 +1412,8 @@ static int kvm_riscv_handle_sbi(CPUState *cs, struct
kvm_run *run)
qemu_log_mask(LOG_UNIMP,
"%s: un-handled SBI EXIT, specific reasons is %lu\n",
__func__, run->riscv_sbi.extension_id);
-ret = -1;
+run->riscv_sbi.ret[0] = SBI_ERR_NOT_SUPPORTED;
+ret = 0;
break;
}
return ret;
diff --git a/target/riscv/sbi_ecall_interface.h
b/target/riscv/sbi_ecall_interface.h
index 43899d08f6..0279e92a36 100644
--- a/target/riscv/sbi_ecall_interface.h
+++ b/target/riscv/sbi_ecall_interface.h
@@ -69,4 +69,15 @@
#define SBI_EXT_VENDOR_END 0x09FF
/* clang-format on */
+/* SBI return error codes */
+#define SBI_SUCCESS 0
+#define SBI_ERR_FAILURE -1
+#define SBI_ERR_NOT_SUPPORTED -2
+#define SBI_ERR_INVALID_PARAM -3
+#define SBI_ERR_DENIED -4
+#define SBI_ERR_INVALID_ADDRESS -5
+#define SBI_ERR_ALREADY_AVAILABLE -6
+#define SBI_ERR_ALREADY_STARTED -7
+#define SBI_ERR_ALREADY_STOPPED -8
+
#endif
--
2.34.1
[PATCH v2] target/riscv/kvm/kvm-cpu.c: kvm_riscv_handle_sbi() fail with vendor-specific SBI
kvm_riscv_handle_sbi() may return not supported return code to not trigger
qemu abort with vendor-specific sbi.
Added SBI related return code's defines.
Signed-off-by: Alexei Filippov
Fixes: 4eb47125 ("target/riscv: Handle KVM_EXIT_RISCV_SBI exit")
Reviewed-by: Daniel Henrique Barboza
---
Changes since v1:
-Add Fixes and Revied-by lines.
target/riscv/kvm/kvm-cpu.c | 5 +++--
target/riscv/sbi_ecall_interface.h | 11 +++
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index 6a6c6cae80..a4f84ad950 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -1404,7 +1404,7 @@ static int kvm_riscv_handle_sbi(CPUState *cs, struct
kvm_run *run)
if (ret == sizeof(ch)) {
run->riscv_sbi.ret[0] = ch;
} else {
-run->riscv_sbi.ret[0] = -1;
+run->riscv_sbi.ret[0] = SBI_ERR_FAILURE;
}
ret = 0;
break;
@@ -1412,7 +1412,8 @@ static int kvm_riscv_handle_sbi(CPUState *cs, struct
kvm_run *run)
qemu_log_mask(LOG_UNIMP,
"%s: un-handled SBI EXIT, specific reasons is %lu\n",
__func__, run->riscv_sbi.extension_id);
-ret = -1;
+run->riscv_sbi.ret[0] = SBI_ERR_NOT_SUPPORTED;
+ret = 0;
break;
}
return ret;
diff --git a/target/riscv/sbi_ecall_interface.h
b/target/riscv/sbi_ecall_interface.h
index 43899d08f6..0279e92a36 100644
--- a/target/riscv/sbi_ecall_interface.h
+++ b/target/riscv/sbi_ecall_interface.h
@@ -69,4 +69,15 @@
#define SBI_EXT_VENDOR_END 0x09FF
/* clang-format on */
+/* SBI return error codes */
+#define SBI_SUCCESS 0
+#define SBI_ERR_FAILURE -1
+#define SBI_ERR_NOT_SUPPORTED -2
+#define SBI_ERR_INVALID_PARAM -3
+#define SBI_ERR_DENIED -4
+#define SBI_ERR_INVALID_ADDRESS -5
+#define SBI_ERR_ALREADY_AVAILABLE -6
+#define SBI_ERR_ALREADY_STARTED -7
+#define SBI_ERR_ALREADY_STOPPED -8
+
#endif
--
2.34.1
