[PATCH v3 34/34] target/riscv: Make pmp.h target_ulong agnostic

Anton Johansson via posted 34 patches 3 months, 4 weeks ago
Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, Laurent Vivier <laurent@vivier.eu>, Christoph Muellner <christoph.muellner@vrull.eu>
There is a newer version of this series
[PATCH v3 34/34] target/riscv: Make pmp.h target_ulong agnostic
Posted by Anton Johansson via 3 months, 4 weeks ago
The pmp.h header is exposed through cpu.h.  pmp_table_t is also used in
CPUArchState.  CSR declarations are only used in target/ and are moved to
csr.h.  In pmp.h, addr_reg is widened to 64 bits and the privilege mode
parameter is fixed to 8 bits, similar to previous commits.

Note, the cpu/pmp/entry and cpu/pmp VMSTATE versions are bumped, breaking
migration from older versions.

Signed-off-by: Anton Johansson <anjo@rev.ng>
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/riscv/csr.h     | 12 ++++++++++++
 target/riscv/pmp.h     | 20 +++++---------------
 target/riscv/machine.c | 10 +++++-----
 target/riscv/pmp.c     | 10 ++++++----
 4 files changed, 28 insertions(+), 24 deletions(-)

diff --git a/target/riscv/csr.h b/target/riscv/csr.h
index 552e6c5de5..3752a0ef43 100644
--- a/target/riscv/csr.h
+++ b/target/riscv/csr.h
@@ -78,4 +78,16 @@ void riscv_set_csr_ops(int csrno, const riscv_csr_operations *ops);
 /* In th_csr.c */
 extern const RISCVCSR th_csr_list[];
 
+/* PMP CSRs, defined in pmp.c */
+void pmpcfg_csr_write(CPURISCVState *env, uint32_t reg_index,
+                      target_ulong val);
+target_ulong pmpcfg_csr_read(CPURISCVState *env, uint32_t reg_index);
+
+void mseccfg_csr_write(CPURISCVState *env, uint64_t val);
+uint64_t mseccfg_csr_read(CPURISCVState *env);
+
+void pmpaddr_csr_write(CPURISCVState *env, uint32_t addr_index,
+                       target_ulong val);
+target_ulong pmpaddr_csr_read(CPURISCVState *env, uint32_t addr_index);
+
 #endif /* RISCV_CSR_H */
diff --git a/target/riscv/pmp.h b/target/riscv/pmp.h
index e322904637..f5d6ec2bbf 100644
--- a/target/riscv/pmp.h
+++ b/target/riscv/pmp.h
@@ -22,8 +22,6 @@
 #ifndef RISCV_PMP_H
 #define RISCV_PMP_H
 
-#include "cpu.h"
-
 typedef enum {
     PMP_READ  = 1 << 0,
     PMP_WRITE = 1 << 1,
@@ -50,7 +48,7 @@ typedef enum {
 } mseccfg_field_t;
 
 typedef struct {
-    target_ulong addr_reg;
+    uint64_t addr_reg;
     uint8_t  cfg_reg;
 } pmp_entry_t;
 
@@ -65,21 +63,13 @@ typedef struct {
     uint32_t num_rules;
 } pmp_table_t;
 
-void pmpcfg_csr_write(CPURISCVState *env, uint32_t reg_index,
-                      target_ulong val);
-target_ulong pmpcfg_csr_read(CPURISCVState *env, uint32_t reg_index);
-
-void mseccfg_csr_write(CPURISCVState *env, uint64_t val);
-uint64_t mseccfg_csr_read(CPURISCVState *env);
+typedef struct CPUArchState CPURISCVState;
 
-void pmpaddr_csr_write(CPURISCVState *env, uint32_t addr_index,
-                       target_ulong val);
-target_ulong pmpaddr_csr_read(CPURISCVState *env, uint32_t addr_index);
 bool pmp_hart_has_privs(CPURISCVState *env, hwaddr addr,
-                        target_ulong size, pmp_priv_t privs,
+                        int size, pmp_priv_t privs,
                         pmp_priv_t *allowed_privs,
-                        target_ulong mode);
-target_ulong pmp_get_tlb_size(CPURISCVState *env, hwaddr addr);
+                        privilege_mode_t mode);
+uint64_t pmp_get_tlb_size(CPURISCVState *env, hwaddr addr);
 void pmp_update_rule_addr(CPURISCVState *env, uint32_t pmp_index);
 void pmp_update_rule_nums(CPURISCVState *env);
 uint32_t pmp_get_num_rules(CPURISCVState *env);
diff --git a/target/riscv/machine.c b/target/riscv/machine.c
index e86fc58e43..eab3adec4d 100644
--- a/target/riscv/machine.c
+++ b/target/riscv/machine.c
@@ -48,10 +48,10 @@ static int pmp_post_load(void *opaque, int version_id)
 
 static const VMStateDescription vmstate_pmp_entry = {
     .name = "cpu/pmp/entry",
-    .version_id = 1,
-    .minimum_version_id = 1,
+    .version_id = 2,
+    .minimum_version_id = 2,
     .fields = (const VMStateField[]) {
-        VMSTATE_UINTTL(addr_reg, pmp_entry_t),
+        VMSTATE_UINT64(addr_reg, pmp_entry_t),
         VMSTATE_UINT8(cfg_reg, pmp_entry_t),
         VMSTATE_END_OF_LIST()
     }
@@ -59,8 +59,8 @@ static const VMStateDescription vmstate_pmp_entry = {
 
 static const VMStateDescription vmstate_pmp = {
     .name = "cpu/pmp",
-    .version_id = 1,
-    .minimum_version_id = 1,
+    .version_id = 2,
+    .minimum_version_id = 2,
     .needed = pmp_needed,
     .post_load = pmp_post_load,
     .fields = (const VMStateField[]) {
diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
index 85199c7387..0839a23086 100644
--- a/target/riscv/pmp.c
+++ b/target/riscv/pmp.c
@@ -23,6 +23,7 @@
 #include "qemu/log.h"
 #include "qapi/error.h"
 #include "cpu.h"
+#include "csr.h"
 #include "trace.h"
 #include "exec/cputlb.h"
 #include "exec/page-protection.h"
@@ -272,7 +273,7 @@ static int pmp_is_in_range(CPURISCVState *env, int pmp_index, hwaddr addr)
  */
 static bool pmp_hart_has_privs_default(CPURISCVState *env, pmp_priv_t privs,
                                        pmp_priv_t *allowed_privs,
-                                       target_ulong mode)
+                                       privilege_mode_t mode)
 {
     bool ret;
 
@@ -331,8 +332,9 @@ static bool pmp_hart_has_privs_default(CPURISCVState *env, pmp_priv_t privs,
  * Return false if no match
  */
 bool pmp_hart_has_privs(CPURISCVState *env, hwaddr addr,
-                        target_ulong size, pmp_priv_t privs,
-                        pmp_priv_t *allowed_privs, target_ulong mode)
+                        int size, pmp_priv_t privs,
+                        pmp_priv_t *allowed_privs,
+                        privilege_mode_t mode)
 {
     int i = 0;
     int pmp_size = 0;
@@ -662,7 +664,7 @@ uint64_t mseccfg_csr_read(CPURISCVState *env)
  * To avoid this we return a size of 1 (which means no caching) if the PMP
  * region only covers partial of the TLB page.
  */
-target_ulong pmp_get_tlb_size(CPURISCVState *env, hwaddr addr)
+uint64_t pmp_get_tlb_size(CPURISCVState *env, hwaddr addr)
 {
     hwaddr pmp_sa;
     hwaddr pmp_ea;
-- 
2.51.0


Re: [PATCH v3 34/34] target/riscv: Make pmp.h target_ulong agnostic
Posted by Alistair Francis 3 months, 3 weeks ago
On Wed, Oct 15, 2025 at 6:38 AM Anton Johansson via
<qemu-devel@nongnu.org> wrote:
>
> The pmp.h header is exposed through cpu.h.  pmp_table_t is also used in
> CPUArchState.  CSR declarations are only used in target/ and are moved to
> csr.h.  In pmp.h, addr_reg is widened to 64 bits and the privilege mode
> parameter is fixed to 8 bits, similar to previous commits.
>
> Note, the cpu/pmp/entry and cpu/pmp VMSTATE versions are bumped, breaking
> migration from older versions.
>
> Signed-off-by: Anton Johansson <anjo@rev.ng>
> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

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

Alistair

> ---
>  target/riscv/csr.h     | 12 ++++++++++++
>  target/riscv/pmp.h     | 20 +++++---------------
>  target/riscv/machine.c | 10 +++++-----
>  target/riscv/pmp.c     | 10 ++++++----
>  4 files changed, 28 insertions(+), 24 deletions(-)
>
> diff --git a/target/riscv/csr.h b/target/riscv/csr.h
> index 552e6c5de5..3752a0ef43 100644
> --- a/target/riscv/csr.h
> +++ b/target/riscv/csr.h
> @@ -78,4 +78,16 @@ void riscv_set_csr_ops(int csrno, const riscv_csr_operations *ops);
>  /* In th_csr.c */
>  extern const RISCVCSR th_csr_list[];
>
> +/* PMP CSRs, defined in pmp.c */
> +void pmpcfg_csr_write(CPURISCVState *env, uint32_t reg_index,
> +                      target_ulong val);
> +target_ulong pmpcfg_csr_read(CPURISCVState *env, uint32_t reg_index);
> +
> +void mseccfg_csr_write(CPURISCVState *env, uint64_t val);
> +uint64_t mseccfg_csr_read(CPURISCVState *env);
> +
> +void pmpaddr_csr_write(CPURISCVState *env, uint32_t addr_index,
> +                       target_ulong val);
> +target_ulong pmpaddr_csr_read(CPURISCVState *env, uint32_t addr_index);
> +
>  #endif /* RISCV_CSR_H */
> diff --git a/target/riscv/pmp.h b/target/riscv/pmp.h
> index e322904637..f5d6ec2bbf 100644
> --- a/target/riscv/pmp.h
> +++ b/target/riscv/pmp.h
> @@ -22,8 +22,6 @@
>  #ifndef RISCV_PMP_H
>  #define RISCV_PMP_H
>
> -#include "cpu.h"
> -
>  typedef enum {
>      PMP_READ  = 1 << 0,
>      PMP_WRITE = 1 << 1,
> @@ -50,7 +48,7 @@ typedef enum {
>  } mseccfg_field_t;
>
>  typedef struct {
> -    target_ulong addr_reg;
> +    uint64_t addr_reg;
>      uint8_t  cfg_reg;
>  } pmp_entry_t;
>
> @@ -65,21 +63,13 @@ typedef struct {
>      uint32_t num_rules;
>  } pmp_table_t;
>
> -void pmpcfg_csr_write(CPURISCVState *env, uint32_t reg_index,
> -                      target_ulong val);
> -target_ulong pmpcfg_csr_read(CPURISCVState *env, uint32_t reg_index);
> -
> -void mseccfg_csr_write(CPURISCVState *env, uint64_t val);
> -uint64_t mseccfg_csr_read(CPURISCVState *env);
> +typedef struct CPUArchState CPURISCVState;
>
> -void pmpaddr_csr_write(CPURISCVState *env, uint32_t addr_index,
> -                       target_ulong val);
> -target_ulong pmpaddr_csr_read(CPURISCVState *env, uint32_t addr_index);
>  bool pmp_hart_has_privs(CPURISCVState *env, hwaddr addr,
> -                        target_ulong size, pmp_priv_t privs,
> +                        int size, pmp_priv_t privs,
>                          pmp_priv_t *allowed_privs,
> -                        target_ulong mode);
> -target_ulong pmp_get_tlb_size(CPURISCVState *env, hwaddr addr);
> +                        privilege_mode_t mode);
> +uint64_t pmp_get_tlb_size(CPURISCVState *env, hwaddr addr);
>  void pmp_update_rule_addr(CPURISCVState *env, uint32_t pmp_index);
>  void pmp_update_rule_nums(CPURISCVState *env);
>  uint32_t pmp_get_num_rules(CPURISCVState *env);
> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> index e86fc58e43..eab3adec4d 100644
> --- a/target/riscv/machine.c
> +++ b/target/riscv/machine.c
> @@ -48,10 +48,10 @@ static int pmp_post_load(void *opaque, int version_id)
>
>  static const VMStateDescription vmstate_pmp_entry = {
>      .name = "cpu/pmp/entry",
> -    .version_id = 1,
> -    .minimum_version_id = 1,
> +    .version_id = 2,
> +    .minimum_version_id = 2,
>      .fields = (const VMStateField[]) {
> -        VMSTATE_UINTTL(addr_reg, pmp_entry_t),
> +        VMSTATE_UINT64(addr_reg, pmp_entry_t),
>          VMSTATE_UINT8(cfg_reg, pmp_entry_t),
>          VMSTATE_END_OF_LIST()
>      }
> @@ -59,8 +59,8 @@ static const VMStateDescription vmstate_pmp_entry = {
>
>  static const VMStateDescription vmstate_pmp = {
>      .name = "cpu/pmp",
> -    .version_id = 1,
> -    .minimum_version_id = 1,
> +    .version_id = 2,
> +    .minimum_version_id = 2,
>      .needed = pmp_needed,
>      .post_load = pmp_post_load,
>      .fields = (const VMStateField[]) {
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index 85199c7387..0839a23086 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -23,6 +23,7 @@
>  #include "qemu/log.h"
>  #include "qapi/error.h"
>  #include "cpu.h"
> +#include "csr.h"
>  #include "trace.h"
>  #include "exec/cputlb.h"
>  #include "exec/page-protection.h"
> @@ -272,7 +273,7 @@ static int pmp_is_in_range(CPURISCVState *env, int pmp_index, hwaddr addr)
>   */
>  static bool pmp_hart_has_privs_default(CPURISCVState *env, pmp_priv_t privs,
>                                         pmp_priv_t *allowed_privs,
> -                                       target_ulong mode)
> +                                       privilege_mode_t mode)
>  {
>      bool ret;
>
> @@ -331,8 +332,9 @@ static bool pmp_hart_has_privs_default(CPURISCVState *env, pmp_priv_t privs,
>   * Return false if no match
>   */
>  bool pmp_hart_has_privs(CPURISCVState *env, hwaddr addr,
> -                        target_ulong size, pmp_priv_t privs,
> -                        pmp_priv_t *allowed_privs, target_ulong mode)
> +                        int size, pmp_priv_t privs,
> +                        pmp_priv_t *allowed_privs,
> +                        privilege_mode_t mode)
>  {
>      int i = 0;
>      int pmp_size = 0;
> @@ -662,7 +664,7 @@ uint64_t mseccfg_csr_read(CPURISCVState *env)
>   * To avoid this we return a size of 1 (which means no caching) if the PMP
>   * region only covers partial of the TLB page.
>   */
> -target_ulong pmp_get_tlb_size(CPURISCVState *env, hwaddr addr)
> +uint64_t pmp_get_tlb_size(CPURISCVState *env, hwaddr addr)
>  {
>      hwaddr pmp_sa;
>      hwaddr pmp_ea;
> --
> 2.51.0
>
>