diff mbox series

[v3,14/16] hw/riscv: Add property to hart array to allow private memory

Message ID 20260903055131.257903-15-joel@jms.id.au
State New
Headers show
Series hw/riscv: Socify Atlantis | expand

Commit Message

Joel Stanley Sept. 3, 2026, 5:51 a.m. UTC 
From: Portia Stephens <portias@oss.tenstorrent.com>

There are platforms where CPUs have different mapping of system memory.
The global system memory does not allow for this. This adds an optional
property on the hart array to assign a specific memory region. The array
passes the region to each hart it creates, so a machine can alias the
harts' memory to system memory, while the harts view memory from their
own memory region.

Signed-off-by: Portia Stephens <portias@oss.tenstorrent.com>
Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 include/hw/riscv/riscv_hart.h | 2 ++
 hw/riscv/riscv_hart.c         | 8 ++++++++
 2 files changed, 10 insertions(+)

Comments

Philippe Mathieu-Daudé Sept. 4, 2026, 10:42 a.m. UTC | #1 
On 3/9/26 07:51, Joel Stanley wrote:
> From: Portia Stephens <portias@oss.tenstorrent.com>
> 
> There are platforms where CPUs have different mapping of system memory.
> The global system memory does not allow for this. This adds an optional
> property on the hart array to assign a specific memory region. The array
> passes the region to each hart it creates, so a machine can alias the
> harts' memory to system memory, while the harts view memory from their
> own memory region.
> 
> Signed-off-by: Portia Stephens <portias@oss.tenstorrent.com>
> Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>   include/hw/riscv/riscv_hart.h | 2 ++
>   hw/riscv/riscv_hart.c         | 8 ++++++++
>   2 files changed, 10 insertions(+)
> 
> diff --git a/include/hw/riscv/riscv_hart.h b/include/hw/riscv/riscv_hart.h
> index 197fa1623115..c3cd05a2c877 100644
> --- a/include/hw/riscv/riscv_hart.h
> +++ b/include/hw/riscv/riscv_hart.h
> @@ -42,6 +42,8 @@ struct RISCVHartArrayState {
>       uint64_t *rnmi_irqvec;
>       uint32_t num_rnmi_excpvec;
>       uint64_t *rnmi_excpvec;
> +    /* Optional private memory region for use instead of system memory */
> +    MemoryRegion *memory;
>       RISCVCPU *harts;
>   };
>   
> diff --git a/hw/riscv/riscv_hart.c b/hw/riscv/riscv_hart.c
> index 747754be6158..32fd39737ff6 100644
> --- a/hw/riscv/riscv_hart.c
> +++ b/hw/riscv/riscv_hart.c
> @@ -53,6 +53,8 @@ static const Property riscv_harts_props[] = {
>       DEFINE_PROP_ARRAY("rnmi-exception-vector", RISCVHartArrayState,
>                         num_rnmi_excpvec, rnmi_excpvec, qdev_prop_uint64,
>                         uint64_t),
> +    DEFINE_PROP_LINK("memory", RISCVHartArrayState, memory,
> +                     TYPE_MEMORY_REGION, MemoryRegion *),
>   };
>   
>   static void riscv_harts_cpu_reset(void *opaque)
> @@ -117,6 +119,12 @@ static bool riscv_hart_realize(RISCVHartArrayState *s, int idx,
>       object_initialize_child(OBJECT(s), "harts[*]", &s->harts[idx], cpu_type);
>       qdev_prop_set_uint64(DEVICE(&s->harts[idx]), "resetvec", s->resetvec);
>   
> +    /* Use private memory instead of system_memory if provided */
> +    if (s->memory) {
> +        object_property_set_link(OBJECT(&s->harts[idx]), "memory",
> +                                 OBJECT(s->memory), &error_abort);

IMHO this logic should be inverted: we want explicit memory bus mapping
by default, and if not set then take the global memory. Personally I'd
even go further and assert it is set (otherwise programming error), with
a preliminary patch setting global system memory to current machines.

> +    }
> +
>       if (s->harts[idx].cfg.ext_smrnmi) {
>           if (idx < s->num_rnmi_irqvec) {
>               qdev_prop_set_uint64(DEVICE(&s->harts[idx]),
Joel Stanley Sept. 7, 2026, 7:27 a.m. UTC | #2 
On Fri, 4 Sept 2026 at 20:13, Philippe Mathieu-Daudé
<philmd@oss.qualcomm.com> wrote:
> > @@ -117,6 +119,12 @@ static bool riscv_hart_realize(RISCVHartArrayState *s, int idx,
> >       object_initialize_child(OBJECT(s), "harts[*]", &s->harts[idx], cpu_type);
> >       qdev_prop_set_uint64(DEVICE(&s->harts[idx]), "resetvec", s->resetvec);
> >
> > +    /* Use private memory instead of system_memory if provided */
> > +    if (s->memory) {
> > +        object_property_set_link(OBJECT(&s->harts[idx]), "memory",
> > +                                 OBJECT(s->memory), &error_abort);
>
> IMHO this logic should be inverted: we want explicit memory bus mapping
> by default, and if not set then take the global memory. Personally I'd
> even go further and assert it is set (otherwise programming error), with
> a preliminary patch setting global system memory to current machines.

I'm on board with this idea, but I couldn't follow what you meant by
"if not set then take the global memory"? That's what happens
currently?

Are you saying always set the memory link to the provided MemoryRegion
(with a fallback to system_memory, or in the stronger case with an
assert if not set)?

Then we could drop cpu_exec_init.
diff mbox series

Patch

diff --git a/include/hw/riscv/riscv_hart.h b/include/hw/riscv/riscv_hart.h
index 197fa1623115..c3cd05a2c877 100644
--- a/include/hw/riscv/riscv_hart.h
+++ b/include/hw/riscv/riscv_hart.h
@@ -42,6 +42,8 @@  struct RISCVHartArrayState {
     uint64_t *rnmi_irqvec;
     uint32_t num_rnmi_excpvec;
     uint64_t *rnmi_excpvec;
+    /* Optional private memory region for use instead of system memory */
+    MemoryRegion *memory;
     RISCVCPU *harts;
 };
 
diff --git a/hw/riscv/riscv_hart.c b/hw/riscv/riscv_hart.c
index 747754be6158..32fd39737ff6 100644
--- a/hw/riscv/riscv_hart.c
+++ b/hw/riscv/riscv_hart.c
@@ -53,6 +53,8 @@  static const Property riscv_harts_props[] = {
     DEFINE_PROP_ARRAY("rnmi-exception-vector", RISCVHartArrayState,
                       num_rnmi_excpvec, rnmi_excpvec, qdev_prop_uint64,
                       uint64_t),
+    DEFINE_PROP_LINK("memory", RISCVHartArrayState, memory,
+                     TYPE_MEMORY_REGION, MemoryRegion *),
 };
 
 static void riscv_harts_cpu_reset(void *opaque)
@@ -117,6 +119,12 @@  static bool riscv_hart_realize(RISCVHartArrayState *s, int idx,
     object_initialize_child(OBJECT(s), "harts[*]", &s->harts[idx], cpu_type);
     qdev_prop_set_uint64(DEVICE(&s->harts[idx]), "resetvec", s->resetvec);
 
+    /* Use private memory instead of system_memory if provided */
+    if (s->memory) {
+        object_property_set_link(OBJECT(&s->harts[idx]), "memory",
+                                 OBJECT(s->memory), &error_abort);
+    }
+
     if (s->harts[idx].cfg.ext_smrnmi) {
         if (idx < s->num_rnmi_irqvec) {
             qdev_prop_set_uint64(DEVICE(&s->harts[idx]),