:p
atchew
Login
This patch ensures that the read-only zero bit (i.e. the 11th bit) of medeleg remains zero, adhering the RISC-V specification. I'm a first time contributor. I was a bit confused when it came to writing tests for this. However, I did debug it via gdb myself and confirmed the patch works. I would like to seek guidance on how can I add a test for CSR issues like this -- or if it's even required in this specific case. Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3438 Abhigyan Kumar (1): target/riscv: Fix medeleg[11] read-only zero bit for M-mode ECALL target/riscv/csr.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) -- 2.54.0
RISC-V Privileged Specification 3.1.8 (Machine Trap Delegation Registers (medeleg and mideleg)) mentions: "For exceptions that cannot occur in less privileged modes, the corresponding medeleg bits should be read-only zero. In particular, medeleg[11] is read-only zero." QEMU incorrectly included RISCV_EXCP_M_ECALL in DELEGABLE_EXCPS. It allowed the 11th bit to be written and read as set. Fixed by removing it from the DELEGABLE_EXCPS mask, adhering to the specification. Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3438 Signed-off-by: Abhigyan Kumar <314abh@gmail.com> --- target/riscv/csr.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/target/riscv/csr.c b/target/riscv/csr.c index XXXXXXX..XXXXXXX 100644 --- a/target/riscv/csr.c +++ b/target/riscv/csr.c @@ -XXX,XX +XXX,XX @@ static const uint64_t vs_delegable_ints = (VS_MODE_INTERRUPTS | LOCAL_INTERRUPTS) & ~MIP_LCOFIP; static const uint64_t all_ints = M_MODE_INTERRUPTS | S_MODE_INTERRUPTS | HS_MODE_INTERRUPTS | LOCAL_INTERRUPTS; +/* + * As per RSIC-V Privileged Spec Section 3.1.8, M-mode ecall should be a + * read-only zero. Therefore, medeleg[11] is set to zero below. + */ #define DELEGABLE_EXCPS ((1ULL << (RISCV_EXCP_INST_ADDR_MIS)) | \ (1ULL << (RISCV_EXCP_INST_ACCESS_FAULT)) | \ (1ULL << (RISCV_EXCP_ILLEGAL_INST)) | \ @@ -XXX,XX +XXX,XX @@ static const uint64_t all_ints = M_MODE_INTERRUPTS | S_MODE_INTERRUPTS | (1ULL << (RISCV_EXCP_U_ECALL)) | \ (1ULL << (RISCV_EXCP_S_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)) | \ -- 2.54.0
This patch ensures that the read-only zero bit (i.e. the 11th bit) of medeleg remains zero, adhering the RISC-V specification. I'm a first time contributor. I was a bit confused when it came to writing tests for this. However, I did debug it via gdb myself and confirmed the patch works. I would like to seek guidance on how can I add a test for CSR issues like this -- or if it's even required in this specific case. Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3438 Abhigyan Kumar (1): target/riscv: Fix medeleg[11] read-only zero bit for M-mode ECALL target/riscv/csr.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) -- 2.54.0
RISC-V Privileged Specification 3.1.8 (Machine Trap Delegation Registers (medeleg and mideleg)) mentions: "For exceptions that cannot occur in less privileged modes, the corresponding medeleg bits should be read-only zero. In particular, medeleg[11] is read-only zero." QEMU incorrectly included RISCV_EXCP_M_ECALL in DELEGABLE_EXCPS. It allowed the 11th bit to be written and read as set. Fixed by removing it from the DELEGABLE_EXCPS mask, adhering to the specification. Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3438 Signed-off-by: Abhigyan Kumar <314abh@gmail.com> --- target/riscv/csr.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/target/riscv/csr.c b/target/riscv/csr.c index XXXXXXX..XXXXXXX 100644 --- a/target/riscv/csr.c +++ b/target/riscv/csr.c @@ -XXX,XX +XXX,XX @@ static const uint64_t vs_delegable_ints = (VS_MODE_INTERRUPTS | LOCAL_INTERRUPTS) & ~MIP_LCOFIP; static const uint64_t all_ints = M_MODE_INTERRUPTS | S_MODE_INTERRUPTS | HS_MODE_INTERRUPTS | LOCAL_INTERRUPTS; +/* + * As per RSIC-V Privileged Spec Section 3.1.8, M-mode ecall should be a + * read-only zero. Therefore, medeleg[11] is set to zero below. + */ #define DELEGABLE_EXCPS ((1ULL << (RISCV_EXCP_INST_ADDR_MIS)) | \ (1ULL << (RISCV_EXCP_INST_ACCESS_FAULT)) | \ (1ULL << (RISCV_EXCP_ILLEGAL_INST)) | \ @@ -XXX,XX +XXX,XX @@ static const uint64_t all_ints = M_MODE_INTERRUPTS | S_MODE_INTERRUPTS | (1ULL << (RISCV_EXCP_U_ECALL)) | \ (1ULL << (RISCV_EXCP_S_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)) | \ -- 2.54.0