[PATCH] target/riscv/csr.c: fix read of pmpaddr(0-63) CSRs

Daniel Henrique Barboza posted 1 patch 2 weeks, 2 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260514123342.2139464-1-daniel.barboza@oss.qualcomm.com
Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, Chao Liu <chao.liu.zevorn@gmail.com>
target/riscv/csr.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
[PATCH] target/riscv/csr.c: fix read of pmpaddr(0-63) CSRs
Posted by Daniel Henrique Barboza 2 weeks, 2 days ago
The priv spec defines, for RV64, that the upper 10 bits of
pmpaddr0-pmpaddr63 are WARL and are supposed to be cleared.

After this patch, using the bug reproducer in [1], writing
ffffffffffffffff in pmpaddr0 and reading it back now results in
003fffffffffffff.  Here's the 'diff -cp' dump before and after
this change:

*************** IN:
*** 5272,5278 ****
   pmpcfg10 0000000000000000
   pmpcfg12 0000000000000000
   pmpcfg14 0000000000000000
!  pmpaddr0 ffffffffffffffff
   pmpaddr1 0000000000000000
   pmpaddr2 0000000000000000
   pmpaddr3 0000000000000000
--- 5545,5551 ----
   pmpcfg10 0000000000000000
   pmpcfg12 0000000000000000
   pmpcfg14 0000000000000000
!  pmpaddr0 003fffffffffffff
   pmpaddr1 0000000000000000
   pmpaddr2 0000000000000000
   pmpaddr3 0000000000000000

[1] https://gitlab.com/qemu-project/qemu/-/work_items/3362

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3362
Signed-off-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>
---
 target/riscv/csr.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index e1cd4a299c..476a620035 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -5362,6 +5362,23 @@ static RISCVException read_pmpaddr(CPURISCVState *env, int csrno,
                                    target_ulong *val)
 {
     *val = pmpaddr_csr_read(env, csrno - CSR_PMPADDR0);
+
+    /*
+     * For RV64, bits 54-63 of the address registers
+     * PMPAADDR(0-63) is a WARL zero field (priv spec,
+     * section "Physical Memory Protection CSRs").
+     *
+     * We'll have to add an annoying TARGET_RISCV64 gate
+     * here to avoid complaints about masking bits 0-53
+     * of a potential 32 bit target_ulong '*var'.
+     */
+#ifdef TARGET_RISCV64
+    if (env->misa_mxl == MXL_RV64
+        && csrno >= CSR_PMPADDR0 && csrno <= CSR_PMPADDR63) {
+        target_ulong read_mask = MAKE_64BIT_MASK(0, 54);
+        *val &= read_mask;
+    }
+#endif
     return RISCV_EXCP_NONE;
 }
 
-- 
2.43.0
Re: [PATCH] target/riscv/csr.c: fix read of pmpaddr(0-63) CSRs
Posted by Philippe Mathieu-Daudé 1 week, 4 days ago
On 14/5/26 14:33, Daniel Henrique Barboza wrote:
> The priv spec defines, for RV64, that the upper 10 bits of
> pmpaddr0-pmpaddr63 are WARL and are supposed to be cleared.
> 
> After this patch, using the bug reproducer in [1], writing
> ffffffffffffffff in pmpaddr0 and reading it back now results in
> 003fffffffffffff.  Here's the 'diff -cp' dump before and after
> this change:
> 
> *************** IN:
> *** 5272,5278 ****
>     pmpcfg10 0000000000000000
>     pmpcfg12 0000000000000000
>     pmpcfg14 0000000000000000
> !  pmpaddr0 ffffffffffffffff
>     pmpaddr1 0000000000000000
>     pmpaddr2 0000000000000000
>     pmpaddr3 0000000000000000
> --- 5545,5551 ----
>     pmpcfg10 0000000000000000
>     pmpcfg12 0000000000000000
>     pmpcfg14 0000000000000000
> !  pmpaddr0 003fffffffffffff
>     pmpaddr1 0000000000000000
>     pmpaddr2 0000000000000000
>     pmpaddr3 0000000000000000
> 
> [1] https://gitlab.com/qemu-project/qemu/-/work_items/3362
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3362
> Signed-off-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>
> ---
>   target/riscv/csr.c | 17 +++++++++++++++++
>   1 file changed, 17 insertions(+)
> 
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index e1cd4a299c..476a620035 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -5362,6 +5362,23 @@ static RISCVException read_pmpaddr(CPURISCVState *env, int csrno,
>                                      target_ulong *val)
>   {
>       *val = pmpaddr_csr_read(env, csrno - CSR_PMPADDR0);
> +
> +    /*
> +     * For RV64, bits 54-63 of the address registers
> +     * PMPAADDR(0-63) is a WARL zero field (priv spec,
> +     * section "Physical Memory Protection CSRs").
> +     *
> +     * We'll have to add an annoying TARGET_RISCV64 gate
> +     * here to avoid complaints about masking bits 0-53
> +     * of a potential 32 bit target_ulong '*var'.

deposit64() shouldn't generate complains.

       *val = deposit64(*val, 54, 10, 0x000);

Anyhow,

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> +     */
> +#ifdef TARGET_RISCV64
> +    if (env->misa_mxl == MXL_RV64
> +        && csrno >= CSR_PMPADDR0 && csrno <= CSR_PMPADDR63) {
> +        target_ulong read_mask = MAKE_64BIT_MASK(0, 54);
> +        *val &= read_mask;
> +    }
> +#endif
>       return RISCV_EXCP_NONE;
>   }
>   


Re: [PATCH] target/riscv/csr.c: fix read of pmpaddr(0-63) CSRs
Posted by Alistair Francis 1 week, 4 days ago
On Thu, May 14, 2026 at 10:35 PM Daniel Henrique Barboza
<daniel.barboza@oss.qualcomm.com> wrote:
>
> The priv spec defines, for RV64, that the upper 10 bits of
> pmpaddr0-pmpaddr63 are WARL and are supposed to be cleared.
>
> After this patch, using the bug reproducer in [1], writing
> ffffffffffffffff in pmpaddr0 and reading it back now results in
> 003fffffffffffff.  Here's the 'diff -cp' dump before and after
> this change:
>
> *************** IN:
> *** 5272,5278 ****
>    pmpcfg10 0000000000000000
>    pmpcfg12 0000000000000000
>    pmpcfg14 0000000000000000
> !  pmpaddr0 ffffffffffffffff
>    pmpaddr1 0000000000000000
>    pmpaddr2 0000000000000000
>    pmpaddr3 0000000000000000
> --- 5545,5551 ----
>    pmpcfg10 0000000000000000
>    pmpcfg12 0000000000000000
>    pmpcfg14 0000000000000000
> !  pmpaddr0 003fffffffffffff
>    pmpaddr1 0000000000000000
>    pmpaddr2 0000000000000000
>    pmpaddr3 0000000000000000
>
> [1] https://gitlab.com/qemu-project/qemu/-/work_items/3362
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3362
> Signed-off-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
>  target/riscv/csr.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index e1cd4a299c..476a620035 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -5362,6 +5362,23 @@ static RISCVException read_pmpaddr(CPURISCVState *env, int csrno,
>                                     target_ulong *val)
>  {
>      *val = pmpaddr_csr_read(env, csrno - CSR_PMPADDR0);
> +
> +    /*
> +     * For RV64, bits 54-63 of the address registers
> +     * PMPAADDR(0-63) is a WARL zero field (priv spec,
> +     * section "Physical Memory Protection CSRs").
> +     *
> +     * We'll have to add an annoying TARGET_RISCV64 gate
> +     * here to avoid complaints about masking bits 0-53
> +     * of a potential 32 bit target_ulong '*var'.
> +     */
> +#ifdef TARGET_RISCV64
> +    if (env->misa_mxl == MXL_RV64
> +        && csrno >= CSR_PMPADDR0 && csrno <= CSR_PMPADDR63) {
> +        target_ulong read_mask = MAKE_64BIT_MASK(0, 54);
> +        *val &= read_mask;
> +    }
> +#endif
>      return RISCV_EXCP_NONE;
>  }
>
> --
> 2.43.0
>
>
Re: [PATCH] target/riscv/csr.c: fix read of pmpaddr(0-63) CSRs
Posted by Alistair Francis 1 week, 4 days ago
On Thu, May 14, 2026 at 10:35 PM Daniel Henrique Barboza
<daniel.barboza@oss.qualcomm.com> wrote:
>
> The priv spec defines, for RV64, that the upper 10 bits of
> pmpaddr0-pmpaddr63 are WARL and are supposed to be cleared.
>
> After this patch, using the bug reproducer in [1], writing
> ffffffffffffffff in pmpaddr0 and reading it back now results in
> 003fffffffffffff.  Here's the 'diff -cp' dump before and after
> this change:
>
> *************** IN:
> *** 5272,5278 ****
>    pmpcfg10 0000000000000000
>    pmpcfg12 0000000000000000
>    pmpcfg14 0000000000000000
> !  pmpaddr0 ffffffffffffffff
>    pmpaddr1 0000000000000000
>    pmpaddr2 0000000000000000
>    pmpaddr3 0000000000000000
> --- 5545,5551 ----
>    pmpcfg10 0000000000000000
>    pmpcfg12 0000000000000000
>    pmpcfg14 0000000000000000
> !  pmpaddr0 003fffffffffffff
>    pmpaddr1 0000000000000000
>    pmpaddr2 0000000000000000
>    pmpaddr3 0000000000000000
>
> [1] https://gitlab.com/qemu-project/qemu/-/work_items/3362
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3362
> Signed-off-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/csr.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index e1cd4a299c..476a620035 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -5362,6 +5362,23 @@ static RISCVException read_pmpaddr(CPURISCVState *env, int csrno,
>                                     target_ulong *val)
>  {
>      *val = pmpaddr_csr_read(env, csrno - CSR_PMPADDR0);
> +
> +    /*
> +     * For RV64, bits 54-63 of the address registers
> +     * PMPAADDR(0-63) is a WARL zero field (priv spec,
> +     * section "Physical Memory Protection CSRs").
> +     *
> +     * We'll have to add an annoying TARGET_RISCV64 gate
> +     * here to avoid complaints about masking bits 0-53
> +     * of a potential 32 bit target_ulong '*var'.
> +     */
> +#ifdef TARGET_RISCV64
> +    if (env->misa_mxl == MXL_RV64
> +        && csrno >= CSR_PMPADDR0 && csrno <= CSR_PMPADDR63) {
> +        target_ulong read_mask = MAKE_64BIT_MASK(0, 54);
> +        *val &= read_mask;
> +    }
> +#endif
>      return RISCV_EXCP_NONE;
>  }
>
> --
> 2.43.0
>
>