| Message ID | 20240329134527.1570936-1-alexei.filippov@syntacore.com |
|---|---|
| State | New |
| Headers | show |
| Series | target/riscv/cpu_helper.c: fix wrong exception raise | expand |
On 3/29/24 10:45, Alexei Filippov wrote: > 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 <alexei.filippov@syntacore.com> > --- > 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 { This solves the issue but we're leaving the 'first_stage_error' flag in an inconsistent state - the call for get_physical_address_pmp() inside "if (ret == TRANSLATE_SUCCESS)" is both a PMP error and also a non-first stage error, but now we're leaving it to 'true'. Raise raise_mmu_exception() will give the expected PMP fault for the wrong reasons, since it thinks that it's a first-stage fault when it's not. This will work now, but any future change to raise_mmu_exception can break this logic. I have an internal (not yet sent to review) fix for the same problem. In my case I was supposed to have an INST_ACCESS_FAULT (1) and was getting an INST_GUEST_PAGE_FAULT (20). I'll see if I can send it ASAP so you can have a look and see if it's also good for your problem. Thanks, Daniel > /* > * Guest physical address translation failed, this is a HS > * level exception
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
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 <alexei.filippov@syntacore.com> --- target/riscv/cpu_helper.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)