diff mbox series

hw/riscv: Add support to change default RISCV hart memory region

Message ID 20221211052745.24738-1-vysakhpillai@embeddedinn.xyz
State New
Headers show
Series hw/riscv: Add support to change default RISCV hart memory region | expand

Commit Message

Vysakh P Pillai Dec. 11, 2022, 5:27 a.m. UTC
Add support to optionally specify a memory region container
to be used to override the default system memory used
by the the RISCV harts when they are realized. Additional
memory regions can be added as sub-regions of this container
to dynamically control the memory regions and mappings visible
from the hart.

Signed-off-by: Vysakh P Pillai <vysakhpillai@embeddedinn.xyz>
---
 hw/riscv/riscv_hart.c         | 5 +++++
 include/hw/riscv/riscv_hart.h | 1 +
 2 files changed, 6 insertions(+)

Comments

Bin Meng Dec. 15, 2022, 1:09 p.m. UTC | #1
On Sun, Dec 11, 2022 at 1:29 PM Vysakh P Pillai
<vysakhpillai@embeddedinn.xyz> wrote:
>
> Add support to optionally specify a memory region container
> to be used to override the default system memory used
> by the the RISCV harts when they are realized. Additional
> memory regions can be added as sub-regions of this container
> to dynamically control the memory regions and mappings visible
> from the hart.

Could you please specify what user case are you trying to address with
this patch?

>
> Signed-off-by: Vysakh P Pillai <vysakhpillai@embeddedinn.xyz>
> ---
>  hw/riscv/riscv_hart.c         | 5 +++++
>  include/hw/riscv/riscv_hart.h | 1 +
>  2 files changed, 6 insertions(+)
>
> diff --git a/hw/riscv/riscv_hart.c b/hw/riscv/riscv_hart.c
> index 613ea2aaa0..7a8dcab7e7 100644
> --- a/hw/riscv/riscv_hart.c
> +++ b/hw/riscv/riscv_hart.c
> @@ -33,6 +33,8 @@ static Property riscv_harts_props[] = {
>      DEFINE_PROP_STRING("cpu-type", RISCVHartArrayState, cpu_type),
>      DEFINE_PROP_UINT64("resetvec", RISCVHartArrayState, resetvec,
>                         DEFAULT_RSTVEC),
> +    DEFINE_PROP_UINT64("cpu-memory", RISCVHartArrayState,
> +                       cpu_memory,NULL),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>
> @@ -49,6 +51,9 @@ static bool riscv_hart_realize(RISCVHartArrayState *s, int idx,
>      qdev_prop_set_uint64(DEVICE(&s->harts[idx]), "resetvec", s->resetvec);
>      s->harts[idx].env.mhartid = s->hartid_base + idx;
>      qemu_register_reset(riscv_harts_cpu_reset, &s->harts[idx]);
> +    if (s->cpu_memory) {
> +        object_property_set_link(OBJECT(&s->harts[idx].parent_obj), "memory",OBJECT(s->cpu_memory), &error_abort);
> +    }
>      return qdev_realize(DEVICE(&s->harts[idx]), NULL, errp);
>  }
>
> diff --git a/include/hw/riscv/riscv_hart.h b/include/hw/riscv/riscv_hart.h
> index bbc21cdc9a..3e5dfeeaae 100644
> --- a/include/hw/riscv/riscv_hart.h
> +++ b/include/hw/riscv/riscv_hart.h
> @@ -38,6 +38,7 @@ struct RISCVHartArrayState {
>      uint32_t hartid_base;
>      char *cpu_type;
>      uint64_t resetvec;
> +    uint64_t cpu_memory;
>      RISCVCPU *harts;
>  };
>

Regards,
Bin
Vysakh P Pillai Dec. 20, 2022, 3:27 p.m. UTC | #2
In the machine model that I am trying to create, there are two CPU clusters. The management cluster has full access to the system resources through a global address space while the application cluster has access to a subset of the resources through a cluster local address space. The current implementation always attaches the system memory to the clusters and there is no means to isolate access to peripherals. With this patch, I am able to create re-mapped aliases to the global address space and attach it as sub-regions to the application cluster container. This approach also provides a capability to dynamically attach and detach sub-regions at runtime.  ---- On Thu, 15 Dec 2022 05:09:53 -0800  bmeng.cn@gmail.com  wrote ----On Sun, Dec 11, 2022 at 1:29 PM Vysakh P Pillai
<vysakhpillai@embeddedinn.xyz> wrote:
>
> Add support to optionally specify a memory region container
> to be used to override the default system memory used
> by the the RISCV harts when they are realized. Additional
> memory regions can be added as sub-regions of this container
> to dynamically control the memory regions and mappings visible
> from the hart.

Could you please specify what user case are you trying to address with
this patch?

>
> Signed-off-by: Vysakh P Pillai <vysakhpillai@embeddedinn.xyz>
> ---
>  hw/riscv/riscv_hart.c         | 5 +++++
>  include/hw/riscv/riscv_hart.h | 1 +
>  2 files changed, 6 insertions(+)
>
> diff --git a/hw/riscv/riscv_hart.c b/hw/riscv/riscv_hart.c
> index 613ea2aaa0..7a8dcab7e7 100644
> --- a/hw/riscv/riscv_hart.c
> +++ b/hw/riscv/riscv_hart.c
> @@ -33,6 +33,8 @@ static Property riscv_harts_props[] = {
>      DEFINE_PROP_STRING("cpu-type", RISCVHartArrayState, cpu_type),
>      DEFINE_PROP_UINT64("resetvec", RISCVHartArrayState, resetvec,
>                         DEFAULT_RSTVEC),
> +    DEFINE_PROP_UINT64("cpu-memory", RISCVHartArrayState,
> +                       cpu_memory,NULL),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>
> @@ -49,6 +51,9 @@ static bool riscv_hart_realize(RISCVHartArrayState *s, int idx,
>      qdev_prop_set_uint64(DEVICE(&s->harts[idx]), "resetvec", s->resetvec);
>      s->harts[idx].env.mhartid = s->hartid_base + idx;
>      qemu_register_reset(riscv_harts_cpu_reset, &s->harts[idx]);
> +    if (s->cpu_memory) {
> +        object_property_set_link(OBJECT(&s->harts[idx].parent_obj), "memory",OBJECT(s->cpu_memory), &error_abort);
> +    }
>      return qdev_realize(DEVICE(&s->harts[idx]), NULL, errp);
>  }
>
> diff --git a/include/hw/riscv/riscv_hart.h b/include/hw/riscv/riscv_hart.h
> index bbc21cdc9a..3e5dfeeaae 100644
> --- a/include/hw/riscv/riscv_hart.h
> +++ b/include/hw/riscv/riscv_hart.h
> @@ -38,6 +38,7 @@ struct RISCVHartArrayState {
>      uint32_t hartid_base;
>      char *cpu_type;
>      uint64_t resetvec;
> +    uint64_t cpu_memory;
>      RISCVCPU *harts;
>  };
>

Regards,
Bin
Alistair Francis Jan. 20, 2023, midnight UTC | #3
On Sun, Dec 11, 2022 at 3:29 PM Vysakh P Pillai
<vysakhpillai@embeddedinn.xyz> wrote:
>
> Add support to optionally specify a memory region container
> to be used to override the default system memory used
> by the the RISCV harts when they are realized. Additional
> memory regions can be added as sub-regions of this container
> to dynamically control the memory regions and mappings visible
> from the hart.

Thanks for the patch.

I think it might make more sense to send this with the series adding
your board. It's a little difficult to picture how this is going to be
used otherwise.

>
> Signed-off-by: Vysakh P Pillai <vysakhpillai@embeddedinn.xyz>
> ---
>  hw/riscv/riscv_hart.c         | 5 +++++
>  include/hw/riscv/riscv_hart.h | 1 +
>  2 files changed, 6 insertions(+)
>
> diff --git a/hw/riscv/riscv_hart.c b/hw/riscv/riscv_hart.c
> index 613ea2aaa0..7a8dcab7e7 100644
> --- a/hw/riscv/riscv_hart.c
> +++ b/hw/riscv/riscv_hart.c
> @@ -33,6 +33,8 @@ static Property riscv_harts_props[] = {
>      DEFINE_PROP_STRING("cpu-type", RISCVHartArrayState, cpu_type),
>      DEFINE_PROP_UINT64("resetvec", RISCVHartArrayState, resetvec,
>                         DEFAULT_RSTVEC),
> +    DEFINE_PROP_UINT64("cpu-memory", RISCVHartArrayState,
> +                       cpu_memory,NULL),

I'm not sure I follow, this is a uint64_t but the default value is NULL?

I assume you are using this as a pointer then?

Alistair

>      DEFINE_PROP_END_OF_LIST(),
>  };
>
> @@ -49,6 +51,9 @@ static bool riscv_hart_realize(RISCVHartArrayState *s, int idx,
>      qdev_prop_set_uint64(DEVICE(&s->harts[idx]), "resetvec", s->resetvec);
>      s->harts[idx].env.mhartid = s->hartid_base + idx;
>      qemu_register_reset(riscv_harts_cpu_reset, &s->harts[idx]);
> +    if (s->cpu_memory) {
> +        object_property_set_link(OBJECT(&s->harts[idx].parent_obj), "memory",OBJECT(s->cpu_memory), &error_abort);
> +    }
>      return qdev_realize(DEVICE(&s->harts[idx]), NULL, errp);
>  }
>
> diff --git a/include/hw/riscv/riscv_hart.h b/include/hw/riscv/riscv_hart.h
> index bbc21cdc9a..3e5dfeeaae 100644
> --- a/include/hw/riscv/riscv_hart.h
> +++ b/include/hw/riscv/riscv_hart.h
> @@ -38,6 +38,7 @@ struct RISCVHartArrayState {
>      uint32_t hartid_base;
>      char *cpu_type;
>      uint64_t resetvec;
> +    uint64_t cpu_memory;
>      RISCVCPU *harts;
>  };
>
> --
> 2.34.1
>
>
>
diff mbox series

Patch

diff --git a/hw/riscv/riscv_hart.c b/hw/riscv/riscv_hart.c
index 613ea2aaa0..7a8dcab7e7 100644
--- a/hw/riscv/riscv_hart.c
+++ b/hw/riscv/riscv_hart.c
@@ -33,6 +33,8 @@  static Property riscv_harts_props[] = {
     DEFINE_PROP_STRING("cpu-type", RISCVHartArrayState, cpu_type),
     DEFINE_PROP_UINT64("resetvec", RISCVHartArrayState, resetvec,
                        DEFAULT_RSTVEC),
+    DEFINE_PROP_UINT64("cpu-memory", RISCVHartArrayState, 
+                       cpu_memory,NULL),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -49,6 +51,9 @@  static bool riscv_hart_realize(RISCVHartArrayState *s, int idx,
     qdev_prop_set_uint64(DEVICE(&s->harts[idx]), "resetvec", s->resetvec);
     s->harts[idx].env.mhartid = s->hartid_base + idx;
     qemu_register_reset(riscv_harts_cpu_reset, &s->harts[idx]);
+    if (s->cpu_memory) {
+        object_property_set_link(OBJECT(&s->harts[idx].parent_obj), "memory",OBJECT(s->cpu_memory), &error_abort);
+    }
     return qdev_realize(DEVICE(&s->harts[idx]), NULL, errp);
 }
 
diff --git a/include/hw/riscv/riscv_hart.h b/include/hw/riscv/riscv_hart.h
index bbc21cdc9a..3e5dfeeaae 100644
--- a/include/hw/riscv/riscv_hart.h
+++ b/include/hw/riscv/riscv_hart.h
@@ -38,6 +38,7 @@  struct RISCVHartArrayState {
     uint32_t hartid_base;
     char *cpu_type;
     uint64_t resetvec;
+    uint64_t cpu_memory;
     RISCVCPU *harts;
 };