| Message ID | 20200303004848.136788-5-palmerdabbelt@google.com |
|---|---|
| State | New |
| Headers | show |
| Series | [PULL,01/38] target/riscv: Convert MIP CSR to target_ulong | expand |
On Tue, 3 Mar 2020 at 00:49, Palmer Dabbelt <palmerdabbelt@google.com> wrote: > > From: Alistair Francis <alistair.francis@wdc.com> > > The v0.5 Hypervisor spec add new execption numbers, let's add support > for those. > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > Reviewed-by: Palmer Dabbelt <palmerdabbelt@google.com> > Signed-off-by: Palmer Dabbelt <palmerdabbelt@google.com> > --- > target/riscv/cpu.c | 8 ++++++++ > target/riscv/cpu_bits.h | 35 +++++++++++++++++++---------------- > target/riscv/cpu_helper.c | 7 +++++-- > target/riscv/csr.c | 7 +++++-- > 4 files changed, 37 insertions(+), 20 deletions(-) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index efbd676edb..2f62f5ea19 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -67,6 +67,14 @@ const char * const riscv_excp_names[] = { > "load_page_fault", > "reserved", > "store_page_fault" > + "reserved", Hi; Coverity (CID 1420223) notice that there's no comma after "store_page_fault", which means that there's been a concatenation of that string and the following "reserved". Could one of you send a patch which adds the missing comma? > + "reserved", > + "reserved", > + "reserved", > + "guest_exec_page_fault", > + "guest_load_page_fault", > + "reserved", > + "guest_store_page_fault" You might also like to add a trailing comma here to avoid the bug happening again in future. > }; > thanks -- PMM
On Thu, Mar 5, 2020 at 8:52 AM Peter Maydell <peter.maydell@linaro.org> wrote: > > On Tue, 3 Mar 2020 at 00:49, Palmer Dabbelt <palmerdabbelt@google.com> wrote: > > > > From: Alistair Francis <alistair.francis@wdc.com> > > > > The v0.5 Hypervisor spec add new execption numbers, let's add support > > for those. > > > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > > Reviewed-by: Palmer Dabbelt <palmerdabbelt@google.com> > > Signed-off-by: Palmer Dabbelt <palmerdabbelt@google.com> > > --- > > target/riscv/cpu.c | 8 ++++++++ > > target/riscv/cpu_bits.h | 35 +++++++++++++++++++---------------- > > target/riscv/cpu_helper.c | 7 +++++-- > > target/riscv/csr.c | 7 +++++-- > > 4 files changed, 37 insertions(+), 20 deletions(-) > > > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > > index efbd676edb..2f62f5ea19 100644 > > --- a/target/riscv/cpu.c > > +++ b/target/riscv/cpu.c > > @@ -67,6 +67,14 @@ const char * const riscv_excp_names[] = { > > "load_page_fault", > > "reserved", > > "store_page_fault" > > + "reserved", > > Hi; Coverity (CID 1420223) notice that there's no comma > after "store_page_fault", which means that there's been > a concatenation of that string and the following "reserved". > Could one of you send a patch which adds the missing comma? > > > + "reserved", > > + "reserved", > > + "reserved", > > + "guest_exec_page_fault", > > + "guest_load_page_fault", > > + "reserved", > > + "guest_store_page_fault" > > You might also like to add a trailing comma here to avoid > the bug happening again in future. Thanks for the report Peter, I'll send a patch. Alistair > > > }; > > > > thanks > -- PMM >
On Thu, 05 Mar 2020 08:44:20 PST (-0800), Peter Maydell wrote: > On Tue, 3 Mar 2020 at 00:49, Palmer Dabbelt <palmerdabbelt@google.com> wrote: >> >> From: Alistair Francis <alistair.francis@wdc.com> >> >> The v0.5 Hypervisor spec add new execption numbers, let's add support >> for those. >> >> Signed-off-by: Alistair Francis <alistair.francis@wdc.com> >> Reviewed-by: Palmer Dabbelt <palmerdabbelt@google.com> >> Signed-off-by: Palmer Dabbelt <palmerdabbelt@google.com> >> --- >> target/riscv/cpu.c | 8 ++++++++ >> target/riscv/cpu_bits.h | 35 +++++++++++++++++++---------------- >> target/riscv/cpu_helper.c | 7 +++++-- >> target/riscv/csr.c | 7 +++++-- >> 4 files changed, 37 insertions(+), 20 deletions(-) >> >> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c >> index efbd676edb..2f62f5ea19 100644 >> --- a/target/riscv/cpu.c >> +++ b/target/riscv/cpu.c >> @@ -67,6 +67,14 @@ const char * const riscv_excp_names[] = { >> "load_page_fault", >> "reserved", >> "store_page_fault" >> + "reserved", > > Hi; Coverity (CID 1420223) notice that there's no comma > after "store_page_fault", which means that there's been > a concatenation of that string and the following "reserved". > Could one of you send a patch which adds the missing comma? Sorry about that. I just sent the patch, LMK if you want me to PR it or if you want to just pick it up. I do have a few other things in my queue right now, but I'm not quite ready to send those (testing and such). > >> + "reserved", >> + "reserved", >> + "reserved", >> + "guest_exec_page_fault", >> + "guest_load_page_fault", >> + "reserved", >> + "guest_store_page_fault" > > You might also like to add a trailing comma here to avoid > the bug happening again in future. > >> }; >> > > thanks > -- PMM
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index efbd676edb..2f62f5ea19 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -67,6 +67,14 @@ const char * const riscv_excp_names[] = { "load_page_fault", "reserved", "store_page_fault" + "reserved", + "reserved", + "reserved", + "reserved", + "guest_exec_page_fault", + "guest_load_page_fault", + "reserved", + "guest_store_page_fault" }; const char * const riscv_intr_names[] = { diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h index 25c0fb258d..9ce73c36de 100644 --- a/target/riscv/cpu_bits.h +++ b/target/riscv/cpu_bits.h @@ -488,22 +488,25 @@ #define DEFAULT_RSTVEC 0x1000 /* Exception causes */ -#define EXCP_NONE -1 /* sentinel value */ -#define RISCV_EXCP_INST_ADDR_MIS 0x0 -#define RISCV_EXCP_INST_ACCESS_FAULT 0x1 -#define RISCV_EXCP_ILLEGAL_INST 0x2 -#define RISCV_EXCP_BREAKPOINT 0x3 -#define RISCV_EXCP_LOAD_ADDR_MIS 0x4 -#define RISCV_EXCP_LOAD_ACCESS_FAULT 0x5 -#define RISCV_EXCP_STORE_AMO_ADDR_MIS 0x6 -#define RISCV_EXCP_STORE_AMO_ACCESS_FAULT 0x7 -#define RISCV_EXCP_U_ECALL 0x8 -#define RISCV_EXCP_S_ECALL 0x9 -#define RISCV_EXCP_H_ECALL 0xa -#define RISCV_EXCP_M_ECALL 0xb -#define RISCV_EXCP_INST_PAGE_FAULT 0xc /* since: priv-1.10.0 */ -#define RISCV_EXCP_LOAD_PAGE_FAULT 0xd /* since: priv-1.10.0 */ -#define RISCV_EXCP_STORE_PAGE_FAULT 0xf /* since: priv-1.10.0 */ +#define EXCP_NONE -1 /* sentinel value */ +#define RISCV_EXCP_INST_ADDR_MIS 0x0 +#define RISCV_EXCP_INST_ACCESS_FAULT 0x1 +#define RISCV_EXCP_ILLEGAL_INST 0x2 +#define RISCV_EXCP_BREAKPOINT 0x3 +#define RISCV_EXCP_LOAD_ADDR_MIS 0x4 +#define RISCV_EXCP_LOAD_ACCESS_FAULT 0x5 +#define RISCV_EXCP_STORE_AMO_ADDR_MIS 0x6 +#define RISCV_EXCP_STORE_AMO_ACCESS_FAULT 0x7 +#define RISCV_EXCP_U_ECALL 0x8 +#define RISCV_EXCP_S_ECALL 0x9 +#define RISCV_EXCP_VS_ECALL 0xa +#define RISCV_EXCP_M_ECALL 0xb +#define RISCV_EXCP_INST_PAGE_FAULT 0xc /* since: priv-1.10.0 */ +#define RISCV_EXCP_LOAD_PAGE_FAULT 0xd /* since: priv-1.10.0 */ +#define RISCV_EXCP_STORE_PAGE_FAULT 0xf /* since: priv-1.10.0 */ +#define RISCV_EXCP_INST_GUEST_PAGE_FAULT 0x14 +#define RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT 0x15 +#define RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT 0x17 #define RISCV_EXCP_INT_FLAG 0x80000000 #define RISCV_EXCP_INT_MASK 0x7fffffff diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c index 85403da9c8..a10582b310 100644 --- a/target/riscv/cpu_helper.c +++ b/target/riscv/cpu_helper.c @@ -528,13 +528,16 @@ void riscv_cpu_do_interrupt(CPUState *cs) static const int ecall_cause_map[] = { [PRV_U] = RISCV_EXCP_U_ECALL, [PRV_S] = RISCV_EXCP_S_ECALL, - [PRV_H] = RISCV_EXCP_H_ECALL, + [PRV_H] = RISCV_EXCP_VS_ECALL, [PRV_M] = RISCV_EXCP_M_ECALL }; if (!async) { /* set tval to badaddr for traps with address information */ switch (cause) { + case RISCV_EXCP_INST_GUEST_PAGE_FAULT: + case RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT: + case RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT: case RISCV_EXCP_INST_ADDR_MIS: case RISCV_EXCP_INST_ACCESS_FAULT: case RISCV_EXCP_LOAD_ADDR_MIS: @@ -556,7 +559,7 @@ void riscv_cpu_do_interrupt(CPUState *cs) } } - trace_riscv_trap(env->mhartid, async, cause, env->pc, tval, cause < 16 ? + trace_riscv_trap(env->mhartid, async, cause, env->pc, tval, cause < 23 ? (async ? riscv_intr_names : riscv_excp_names)[cause] : "(unknown)"); if (env->priv <= PRV_S && diff --git a/target/riscv/csr.c b/target/riscv/csr.c index 0e34c292c5..ca27359c7e 100644 --- a/target/riscv/csr.c +++ b/target/riscv/csr.c @@ -242,11 +242,14 @@ static const target_ulong delegable_excps = (1ULL << (RISCV_EXCP_STORE_AMO_ACCESS_FAULT)) | (1ULL << (RISCV_EXCP_U_ECALL)) | (1ULL << (RISCV_EXCP_S_ECALL)) | - (1ULL << (RISCV_EXCP_H_ECALL)) | + (1ULL << (RISCV_EXCP_VS_ECALL)) | (1ULL << (RISCV_EXCP_M_ECALL)) | (1ULL << (RISCV_EXCP_INST_PAGE_FAULT)) | (1ULL << (RISCV_EXCP_LOAD_PAGE_FAULT)) | - (1ULL << (RISCV_EXCP_STORE_PAGE_FAULT)); + (1ULL << (RISCV_EXCP_STORE_PAGE_FAULT)) | + (1ULL << (RISCV_EXCP_INST_GUEST_PAGE_FAULT)) | + (1ULL << (RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT)) | + (1ULL << (RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT)); static const target_ulong sstatus_v1_9_mask = SSTATUS_SIE | SSTATUS_SPIE | SSTATUS_UIE | SSTATUS_UPIE | SSTATUS_SPP | SSTATUS_FS | SSTATUS_XS | SSTATUS_SUM | SSTATUS_SD;