[Qemu-devel] [PATCH 2/2] riscv: sifive_u: Update the plic hart config to support multicore

[Qemu-devel] [PATCH 2/2] riscv: sifive_u: Update the plic hart config to support multicore

[Bin Meng]

At present the PLIC is instantiated to support only one hart, while
the machine allows at most 4 harts to be created. When more than 1
hart is configured, PLIC needs to instantiated to support multicore,
otherwise an SMP OS does not work.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---

 hw/riscv/sifive_u.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index e2120ac..a416d5d 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -344,6 +344,8 @@ static void riscv_sifive_u_soc_realize(DeviceState *dev, Error **errp)
     MemoryRegion *system_memory = get_system_memory();
     MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
     qemu_irq plic_gpios[SIFIVE_U_PLIC_NUM_SOURCES];
+    char *plic_hart_config;
+    size_t plic_hart_config_len;
     int i;
     Error *err = NULL;
     NICInfo *nd = &nd_table[0];
@@ -357,9 +359,21 @@ static void riscv_sifive_u_soc_realize(DeviceState *dev, Error **errp)
     memory_region_add_subregion(system_memory, memmap[SIFIVE_U_MROM].base,
                                 mask_rom);
 
+    /* create PLIC hart topology configuration string */
+    plic_hart_config_len = (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1) * smp_cpus;
+    plic_hart_config = g_malloc0(plic_hart_config_len);
+    for (i = 0; i < smp_cpus; i++) {
+        if (i != 0) {
+            strncat(plic_hart_config, ",", plic_hart_config_len);
+        }
+        strncat(plic_hart_config, SIFIVE_U_PLIC_HART_CONFIG,
+                plic_hart_config_len);
+        plic_hart_config_len -= (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1);
+    }
+
     /* MMIO */
     s->plic = sifive_plic_create(memmap[SIFIVE_U_PLIC].base,
-        (char *)SIFIVE_U_PLIC_HART_CONFIG,
+        plic_hart_config,
         SIFIVE_U_PLIC_NUM_SOURCES,
         SIFIVE_U_PLIC_NUM_PRIORITIES,
         SIFIVE_U_PLIC_PRIORITY_BASE,
-- 
2.7.4

[Qemu-riscv] [PATCH 2/2] riscv: sifive_u: Update the plic hart config to support multicore

[Bin Meng]

At present the PLIC is instantiated to support only one hart, while
the machine allows at most 4 harts to be created. When more than 1
hart is configured, PLIC needs to instantiated to support multicore,
otherwise an SMP OS does not work.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---

 hw/riscv/sifive_u.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index e2120ac..a416d5d 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -344,6 +344,8 @@ static void riscv_sifive_u_soc_realize(DeviceState *dev, Error **errp)
     MemoryRegion *system_memory = get_system_memory();
     MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
     qemu_irq plic_gpios[SIFIVE_U_PLIC_NUM_SOURCES];
+    char *plic_hart_config;
+    size_t plic_hart_config_len;
     int i;
     Error *err = NULL;
     NICInfo *nd = &nd_table[0];
@@ -357,9 +359,21 @@ static void riscv_sifive_u_soc_realize(DeviceState *dev, Error **errp)
     memory_region_add_subregion(system_memory, memmap[SIFIVE_U_MROM].base,
                                 mask_rom);
 
+    /* create PLIC hart topology configuration string */
+    plic_hart_config_len = (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1) * smp_cpus;
+    plic_hart_config = g_malloc0(plic_hart_config_len);
+    for (i = 0; i < smp_cpus; i++) {
+        if (i != 0) {
+            strncat(plic_hart_config, ",", plic_hart_config_len);
+        }
+        strncat(plic_hart_config, SIFIVE_U_PLIC_HART_CONFIG,
+                plic_hart_config_len);
+        plic_hart_config_len -= (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1);
+    }
+
     /* MMIO */
     s->plic = sifive_plic_create(memmap[SIFIVE_U_PLIC].base,
-        (char *)SIFIVE_U_PLIC_HART_CONFIG,
+        plic_hart_config,
         SIFIVE_U_PLIC_NUM_SOURCES,
         SIFIVE_U_PLIC_NUM_PRIORITIES,
         SIFIVE_U_PLIC_PRIORITY_BASE,
-- 
2.7.4

Re: [Qemu-devel] [PATCH 1/2] riscv: sifive_u: Do not create hard-coded phandles in DT

[Alistair Francis]

On Fri, 2019-05-17 at 08:51 -0700, Bin Meng wrote:
> At present the cpu, plic and ethclk nodes' phandles are hard-coded
> to 1/2/3 in DT. If we configure more than 1 cpu for the machine,
> all cpu nodes' phandles conflict with each other as they are all 1.
> Fix it by removing the hardcode.
> 
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>

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

Alistair

> ---
> 
>  hw/riscv/sifive_u.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index 5ecc47c..e2120ac 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -86,7 +86,7 @@ static void create_fdt(SiFiveUState *s, const
> struct MemmapEntry *memmap,
>      uint32_t *cells;
>      char *nodename;
>      char ethclk_names[] = "pclk\0hclk\0tx_clk";
> -    uint32_t plic_phandle, ethclk_phandle;
> +    uint32_t plic_phandle, ethclk_phandle, phandle = 1;
>  
>      fdt = s->fdt = create_device_tree(&s->fdt_size);
>      if (!fdt) {
> @@ -121,6 +121,7 @@ static void create_fdt(SiFiveUState *s, const
> struct MemmapEntry *memmap,
>      qemu_fdt_setprop_cell(fdt, "/cpus", "#address-cells", 0x1);
>  
>      for (cpu = s->soc.cpus.num_harts - 1; cpu >= 0; cpu--) {
> +        int cpu_phandle = phandle++;
>          nodename = g_strdup_printf("/cpus/cpu@%d", cpu);
>          char *intc = g_strdup_printf("/cpus/cpu@%d/interrupt-
> controller", cpu);
>          char *isa = riscv_isa_string(&s->soc.cpus.harts[cpu]);
> @@ -134,8 +135,8 @@ static void create_fdt(SiFiveUState *s, const
> struct MemmapEntry *memmap,
>          qemu_fdt_setprop_cell(fdt, nodename, "reg", cpu);
>          qemu_fdt_setprop_string(fdt, nodename, "device_type",
> "cpu");
>          qemu_fdt_add_subnode(fdt, intc);
> -        qemu_fdt_setprop_cell(fdt, intc, "phandle", 1);
> -        qemu_fdt_setprop_cell(fdt, intc, "linux,phandle", 1);
> +        qemu_fdt_setprop_cell(fdt, intc, "phandle", cpu_phandle);
> +        qemu_fdt_setprop_cell(fdt, intc, "linux,phandle",
> cpu_phandle);
>          qemu_fdt_setprop_string(fdt, intc, "compatible", "riscv,cpu-
> intc");
>          qemu_fdt_setprop(fdt, intc, "interrupt-controller", NULL,
> 0);
>          qemu_fdt_setprop_cell(fdt, intc, "#interrupt-cells", 1);
> @@ -167,6 +168,7 @@ static void create_fdt(SiFiveUState *s, const
> struct MemmapEntry *memmap,
>      g_free(cells);
>      g_free(nodename);
>  
> +    plic_phandle = phandle++;
>      cells =  g_new0(uint32_t, s->soc.cpus.num_harts * 4);
>      for (cpu = 0; cpu < s->soc.cpus.num_harts; cpu++) {
>          nodename =
> @@ -192,20 +194,21 @@ static void create_fdt(SiFiveUState *s, const
> struct MemmapEntry *memmap,
>      qemu_fdt_setprop_string(fdt, nodename, "reg-names", "control");
>      qemu_fdt_setprop_cell(fdt, nodename, "riscv,max-priority", 7);
>      qemu_fdt_setprop_cell(fdt, nodename, "riscv,ndev", 0x35);
> -    qemu_fdt_setprop_cells(fdt, nodename, "phandle", 2);
> -    qemu_fdt_setprop_cells(fdt, nodename, "linux,phandle", 2);
> +    qemu_fdt_setprop_cells(fdt, nodename, "phandle", plic_phandle);
> +    qemu_fdt_setprop_cells(fdt, nodename, "linux,phandle",
> plic_phandle);
>      plic_phandle = qemu_fdt_get_phandle(fdt, nodename);
>      g_free(cells);
>      g_free(nodename);
>  
> +    ethclk_phandle = phandle++;
>      nodename = g_strdup_printf("/soc/ethclk");
>      qemu_fdt_add_subnode(fdt, nodename);
>      qemu_fdt_setprop_string(fdt, nodename, "compatible", "fixed-
> clock");
>      qemu_fdt_setprop_cell(fdt, nodename, "#clock-cells", 0x0);
>      qemu_fdt_setprop_cell(fdt, nodename, "clock-frequency",
>          SIFIVE_U_GEM_CLOCK_FREQ);
> -    qemu_fdt_setprop_cell(fdt, nodename, "phandle", 3);
> -    qemu_fdt_setprop_cell(fdt, nodename, "linux,phandle", 3);
> +    qemu_fdt_setprop_cell(fdt, nodename, "phandle", ethclk_phandle);
> +    qemu_fdt_setprop_cell(fdt, nodename, "linux,phandle",
> ethclk_phandle);
>      ethclk_phandle = qemu_fdt_get_phandle(fdt, nodename);
>      g_free(nodename);
>  

Re: [Qemu-riscv] [PATCH 1/2] riscv: sifive_u: Do not create hard-coded phandles in DT

[Alistair Francis]

On Fri, 2019-05-17 at 08:51 -0700, Bin Meng wrote:
> At present the cpu, plic and ethclk nodes' phandles are hard-coded
> to 1/2/3 in DT. If we configure more than 1 cpu for the machine,
> all cpu nodes' phandles conflict with each other as they are all 1.
> Fix it by removing the hardcode.
> 
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>

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

Alistair

> ---
> 
>  hw/riscv/sifive_u.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index 5ecc47c..e2120ac 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -86,7 +86,7 @@ static void create_fdt(SiFiveUState *s, const
> struct MemmapEntry *memmap,
>      uint32_t *cells;
>      char *nodename;
>      char ethclk_names[] = "pclk\0hclk\0tx_clk";
> -    uint32_t plic_phandle, ethclk_phandle;
> +    uint32_t plic_phandle, ethclk_phandle, phandle = 1;
>  
>      fdt = s->fdt = create_device_tree(&s->fdt_size);
>      if (!fdt) {
> @@ -121,6 +121,7 @@ static void create_fdt(SiFiveUState *s, const
> struct MemmapEntry *memmap,
>      qemu_fdt_setprop_cell(fdt, "/cpus", "#address-cells", 0x1);
>  
>      for (cpu = s->soc.cpus.num_harts - 1; cpu >= 0; cpu--) {
> +        int cpu_phandle = phandle++;
>          nodename = g_strdup_printf("/cpus/cpu@%d", cpu);
>          char *intc = g_strdup_printf("/cpus/cpu@%d/interrupt-
> controller", cpu);
>          char *isa = riscv_isa_string(&s->soc.cpus.harts[cpu]);
> @@ -134,8 +135,8 @@ static void create_fdt(SiFiveUState *s, const
> struct MemmapEntry *memmap,
>          qemu_fdt_setprop_cell(fdt, nodename, "reg", cpu);
>          qemu_fdt_setprop_string(fdt, nodename, "device_type",
> "cpu");
>          qemu_fdt_add_subnode(fdt, intc);
> -        qemu_fdt_setprop_cell(fdt, intc, "phandle", 1);
> -        qemu_fdt_setprop_cell(fdt, intc, "linux,phandle", 1);
> +        qemu_fdt_setprop_cell(fdt, intc, "phandle", cpu_phandle);
> +        qemu_fdt_setprop_cell(fdt, intc, "linux,phandle",
> cpu_phandle);
>          qemu_fdt_setprop_string(fdt, intc, "compatible", "riscv,cpu-
> intc");
>          qemu_fdt_setprop(fdt, intc, "interrupt-controller", NULL,
> 0);
>          qemu_fdt_setprop_cell(fdt, intc, "#interrupt-cells", 1);
> @@ -167,6 +168,7 @@ static void create_fdt(SiFiveUState *s, const
> struct MemmapEntry *memmap,
>      g_free(cells);
>      g_free(nodename);
>  
> +    plic_phandle = phandle++;
>      cells =  g_new0(uint32_t, s->soc.cpus.num_harts * 4);
>      for (cpu = 0; cpu < s->soc.cpus.num_harts; cpu++) {
>          nodename =
> @@ -192,20 +194,21 @@ static void create_fdt(SiFiveUState *s, const
> struct MemmapEntry *memmap,
>      qemu_fdt_setprop_string(fdt, nodename, "reg-names", "control");
>      qemu_fdt_setprop_cell(fdt, nodename, "riscv,max-priority", 7);
>      qemu_fdt_setprop_cell(fdt, nodename, "riscv,ndev", 0x35);
> -    qemu_fdt_setprop_cells(fdt, nodename, "phandle", 2);
> -    qemu_fdt_setprop_cells(fdt, nodename, "linux,phandle", 2);
> +    qemu_fdt_setprop_cells(fdt, nodename, "phandle", plic_phandle);
> +    qemu_fdt_setprop_cells(fdt, nodename, "linux,phandle",
> plic_phandle);
>      plic_phandle = qemu_fdt_get_phandle(fdt, nodename);
>      g_free(cells);
>      g_free(nodename);
>  
> +    ethclk_phandle = phandle++;
>      nodename = g_strdup_printf("/soc/ethclk");
>      qemu_fdt_add_subnode(fdt, nodename);
>      qemu_fdt_setprop_string(fdt, nodename, "compatible", "fixed-
> clock");
>      qemu_fdt_setprop_cell(fdt, nodename, "#clock-cells", 0x0);
>      qemu_fdt_setprop_cell(fdt, nodename, "clock-frequency",
>          SIFIVE_U_GEM_CLOCK_FREQ);
> -    qemu_fdt_setprop_cell(fdt, nodename, "phandle", 3);
> -    qemu_fdt_setprop_cell(fdt, nodename, "linux,phandle", 3);
> +    qemu_fdt_setprop_cell(fdt, nodename, "phandle", ethclk_phandle);
> +    qemu_fdt_setprop_cell(fdt, nodename, "linux,phandle",
> ethclk_phandle);
>      ethclk_phandle = qemu_fdt_get_phandle(fdt, nodename);
>      g_free(nodename);
>  

Re: [Qemu-devel] [PATCH 2/2] riscv: sifive_u: Update the plic hart config to support multicore

[Alistair Francis]

On Fri, 2019-05-17 at 08:51 -0700, Bin Meng wrote:
> At present the PLIC is instantiated to support only one hart, while
> the machine allows at most 4 harts to be created. When more than 1
> hart is configured, PLIC needs to instantiated to support multicore,
> otherwise an SMP OS does not work.
> 
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>

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

Alistair

> ---
> 
>  hw/riscv/sifive_u.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index e2120ac..a416d5d 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -344,6 +344,8 @@ static void
> riscv_sifive_u_soc_realize(DeviceState *dev, Error **errp)
>      MemoryRegion *system_memory = get_system_memory();
>      MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
>      qemu_irq plic_gpios[SIFIVE_U_PLIC_NUM_SOURCES];
> +    char *plic_hart_config;
> +    size_t plic_hart_config_len;
>      int i;
>      Error *err = NULL;
>      NICInfo *nd = &nd_table[0];
> @@ -357,9 +359,21 @@ static void
> riscv_sifive_u_soc_realize(DeviceState *dev, Error **errp)
>      memory_region_add_subregion(system_memory,
> memmap[SIFIVE_U_MROM].base,
>                                  mask_rom);
>  
> +    /* create PLIC hart topology configuration string */
> +    plic_hart_config_len = (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1) *
> smp_cpus;
> +    plic_hart_config = g_malloc0(plic_hart_config_len);
> +    for (i = 0; i < smp_cpus; i++) {
> +        if (i != 0) {
> +            strncat(plic_hart_config, ",", plic_hart_config_len);
> +        }
> +        strncat(plic_hart_config, SIFIVE_U_PLIC_HART_CONFIG,
> +                plic_hart_config_len);
> +        plic_hart_config_len -= (strlen(SIFIVE_U_PLIC_HART_CONFIG) +
> 1);
> +    }
> +
>      /* MMIO */
>      s->plic = sifive_plic_create(memmap[SIFIVE_U_PLIC].base,
> -        (char *)SIFIVE_U_PLIC_HART_CONFIG,
> +        plic_hart_config,
>          SIFIVE_U_PLIC_NUM_SOURCES,
>          SIFIVE_U_PLIC_NUM_PRIORITIES,
>          SIFIVE_U_PLIC_PRIORITY_BASE,

Re: [Qemu-riscv] [PATCH 2/2] riscv: sifive_u: Update the plic hart config to support multicore

[Alistair Francis]

On Fri, 2019-05-17 at 08:51 -0700, Bin Meng wrote:
> At present the PLIC is instantiated to support only one hart, while
> the machine allows at most 4 harts to be created. When more than 1
> hart is configured, PLIC needs to instantiated to support multicore,
> otherwise an SMP OS does not work.
> 
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>

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

Alistair

> ---
> 
>  hw/riscv/sifive_u.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index e2120ac..a416d5d 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -344,6 +344,8 @@ static void
> riscv_sifive_u_soc_realize(DeviceState *dev, Error **errp)
>      MemoryRegion *system_memory = get_system_memory();
>      MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
>      qemu_irq plic_gpios[SIFIVE_U_PLIC_NUM_SOURCES];
> +    char *plic_hart_config;
> +    size_t plic_hart_config_len;
>      int i;
>      Error *err = NULL;
>      NICInfo *nd = &nd_table[0];
> @@ -357,9 +359,21 @@ static void
> riscv_sifive_u_soc_realize(DeviceState *dev, Error **errp)
>      memory_region_add_subregion(system_memory,
> memmap[SIFIVE_U_MROM].base,
>                                  mask_rom);
>  
> +    /* create PLIC hart topology configuration string */
> +    plic_hart_config_len = (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1) *
> smp_cpus;
> +    plic_hart_config = g_malloc0(plic_hart_config_len);
> +    for (i = 0; i < smp_cpus; i++) {
> +        if (i != 0) {
> +            strncat(plic_hart_config, ",", plic_hart_config_len);
> +        }
> +        strncat(plic_hart_config, SIFIVE_U_PLIC_HART_CONFIG,
> +                plic_hart_config_len);
> +        plic_hart_config_len -= (strlen(SIFIVE_U_PLIC_HART_CONFIG) +
> 1);
> +    }
> +
>      /* MMIO */
>      s->plic = sifive_plic_create(memmap[SIFIVE_U_PLIC].base,
> -        (char *)SIFIVE_U_PLIC_HART_CONFIG,
> +        plic_hart_config,
>          SIFIVE_U_PLIC_NUM_SOURCES,
>          SIFIVE_U_PLIC_NUM_PRIORITIES,
>          SIFIVE_U_PLIC_PRIORITY_BASE,

Re: [Qemu-devel] [PATCH 1/2] riscv: sifive_u: Do not create hard-coded phandles in DT

[Bin Meng]

Hi,

On Sat, May 18, 2019 at 5:34 AM Alistair Francis
<Alistair.Francis@wdc.com> wrote:
>
> On Fri, 2019-05-17 at 08:51 -0700, Bin Meng wrote:
> > At present the cpu, plic and ethclk nodes' phandles are hard-coded
> > to 1/2/3 in DT. If we configure more than 1 cpu for the machine,
> > all cpu nodes' phandles conflict with each other as they are all 1.
> > Fix it by removing the hardcode.
> >
> > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>

Can this go in the 4.1 PR?

Regards,
Bin

Re: [Qemu-riscv] [PATCH 1/2] riscv: sifive_u: Do not create hard-coded phandles in DT

[Bin Meng]

Hi,

On Sat, May 18, 2019 at 5:34 AM Alistair Francis
<Alistair.Francis@wdc.com> wrote:
>
> On Fri, 2019-05-17 at 08:51 -0700, Bin Meng wrote:
> > At present the cpu, plic and ethclk nodes' phandles are hard-coded
> > to 1/2/3 in DT. If we configure more than 1 cpu for the machine,
> > all cpu nodes' phandles conflict with each other as they are all 1.
> > Fix it by removing the hardcode.
> >
> > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>

Can this go in the 4.1 PR?

Regards,
Bin

Re: [Qemu-devel] [PATCH 1/2] riscv: sifive_u: Do not create hard-coded phandles in DT

[Palmer Dabbelt]

On Tue, 25 Jun 2019 18:47:15 PDT (-0700), bmeng.cn@gmail.com wrote:
> Hi,
>
> On Sat, May 18, 2019 at 5:34 AM Alistair Francis
> <Alistair.Francis@wdc.com> wrote:
>>
>> On Fri, 2019-05-17 at 08:51 -0700, Bin Meng wrote:
>> > At present the cpu, plic and ethclk nodes' phandles are hard-coded
>> > to 1/2/3 in DT. If we configure more than 1 cpu for the machine,
>> > all cpu nodes' phandles conflict with each other as they are all 1.
>> > Fix it by removing the hardcode.
>> >
>> > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>>
>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>>
>
> Can this go in the 4.1 PR?

Thanks, I must have dropped them.

Re: [Qemu-riscv] [PATCH 1/2] riscv: sifive_u: Do not create hard-coded phandles in DT

[Palmer Dabbelt]

On Tue, 25 Jun 2019 18:47:15 PDT (-0700), bmeng.cn@gmail.com wrote:
> Hi,
>
> On Sat, May 18, 2019 at 5:34 AM Alistair Francis
> <Alistair.Francis@wdc.com> wrote:
>>
>> On Fri, 2019-05-17 at 08:51 -0700, Bin Meng wrote:
>> > At present the cpu, plic and ethclk nodes' phandles are hard-coded
>> > to 1/2/3 in DT. If we configure more than 1 cpu for the machine,
>> > all cpu nodes' phandles conflict with each other as they are all 1.
>> > Fix it by removing the hardcode.
>> >
>> > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>>
>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>>
>
> Can this go in the 4.1 PR?

Thanks, I must have dropped them.

Re: [Qemu-devel] [PATCH 2/2] riscv: sifive_u: Update the plic hart config to support multicore

[Palmer Dabbelt]

On Fri, 17 May 2019 14:35:56 PDT (-0700), Alistair Francis wrote:
> On Fri, 2019-05-17 at 08:51 -0700, Bin Meng wrote:
>> At present the PLIC is instantiated to support only one hart, while
>> the machine allows at most 4 harts to be created. When more than 1
>> hart is configured, PLIC needs to instantiated to support multicore,
>> otherwise an SMP OS does not work.
>> 
>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> 
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

I got this one as well.

> 
> Alistair
> 
>> ---
>> 
>>  hw/riscv/sifive_u.c | 16 +++++++++++++++-
>>  1 file changed, 15 insertions(+), 1 deletion(-)
>> 
>> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
>> index e2120ac..a416d5d 100644
>> --- a/hw/riscv/sifive_u.c
>> +++ b/hw/riscv/sifive_u.c
>> @@ -344,6 +344,8 @@ static void
>> riscv_sifive_u_soc_realize(DeviceState *dev, Error **errp)
>>      MemoryRegion *system_memory = get_system_memory();
>>      MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
>>      qemu_irq plic_gpios[SIFIVE_U_PLIC_NUM_SOURCES];
>> +    char *plic_hart_config;
>> +    size_t plic_hart_config_len;
>>      int i;
>>      Error *err = NULL;
>>      NICInfo *nd = &nd_table[0];
>> @@ -357,9 +359,21 @@ static void
>> riscv_sifive_u_soc_realize(DeviceState *dev, Error **errp)
>>      memory_region_add_subregion(system_memory,
>> memmap[SIFIVE_U_MROM].base,
>>                                  mask_rom);
>>  
>> +    /* create PLIC hart topology configuration string */
>> +    plic_hart_config_len = (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1) *
>> smp_cpus;
>> +    plic_hart_config = g_malloc0(plic_hart_config_len);
>> +    for (i = 0; i < smp_cpus; i++) {
>> +        if (i != 0) {
>> +            strncat(plic_hart_config, ",", plic_hart_config_len);
>> +        }
>> +        strncat(plic_hart_config, SIFIVE_U_PLIC_HART_CONFIG,
>> +                plic_hart_config_len);
>> +        plic_hart_config_len -= (strlen(SIFIVE_U_PLIC_HART_CONFIG) +
>> 1);
>> +    }
>> +
>>      /* MMIO */
>>      s->plic = sifive_plic_create(memmap[SIFIVE_U_PLIC].base,
>> -        (char *)SIFIVE_U_PLIC_HART_CONFIG,
>> +        plic_hart_config,
>>          SIFIVE_U_PLIC_NUM_SOURCES,
>>          SIFIVE_U_PLIC_NUM_PRIORITIES,
>>          SIFIVE_U_PLIC_PRIORITY_BASE,

Re: [Qemu-riscv] [PATCH 2/2] riscv: sifive_u: Update the plic hart config to support multicore

[Palmer Dabbelt]

On Fri, 17 May 2019 14:35:56 PDT (-0700), Alistair Francis wrote:
> On Fri, 2019-05-17 at 08:51 -0700, Bin Meng wrote:
>> At present the PLIC is instantiated to support only one hart, while
>> the machine allows at most 4 harts to be created. When more than 1
>> hart is configured, PLIC needs to instantiated to support multicore,
>> otherwise an SMP OS does not work.
>> 
>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> 
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

I got this one as well.

> 
> Alistair
> 
>> ---
>> 
>>  hw/riscv/sifive_u.c | 16 +++++++++++++++-
>>  1 file changed, 15 insertions(+), 1 deletion(-)
>> 
>> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
>> index e2120ac..a416d5d 100644
>> --- a/hw/riscv/sifive_u.c
>> +++ b/hw/riscv/sifive_u.c
>> @@ -344,6 +344,8 @@ static void
>> riscv_sifive_u_soc_realize(DeviceState *dev, Error **errp)
>>      MemoryRegion *system_memory = get_system_memory();
>>      MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
>>      qemu_irq plic_gpios[SIFIVE_U_PLIC_NUM_SOURCES];
>> +    char *plic_hart_config;
>> +    size_t plic_hart_config_len;
>>      int i;
>>      Error *err = NULL;
>>      NICInfo *nd = &nd_table[0];
>> @@ -357,9 +359,21 @@ static void
>> riscv_sifive_u_soc_realize(DeviceState *dev, Error **errp)
>>      memory_region_add_subregion(system_memory,
>> memmap[SIFIVE_U_MROM].base,
>>                                  mask_rom);
>>  
>> +    /* create PLIC hart topology configuration string */
>> +    plic_hart_config_len = (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1) *
>> smp_cpus;
>> +    plic_hart_config = g_malloc0(plic_hart_config_len);
>> +    for (i = 0; i < smp_cpus; i++) {
>> +        if (i != 0) {
>> +            strncat(plic_hart_config, ",", plic_hart_config_len);
>> +        }
>> +        strncat(plic_hart_config, SIFIVE_U_PLIC_HART_CONFIG,
>> +                plic_hart_config_len);
>> +        plic_hart_config_len -= (strlen(SIFIVE_U_PLIC_HART_CONFIG) +
>> 1);
>> +    }
>> +
>>      /* MMIO */
>>      s->plic = sifive_plic_create(memmap[SIFIVE_U_PLIC].base,
>> -        (char *)SIFIVE_U_PLIC_HART_CONFIG,
>> +        plic_hart_config,
>>          SIFIVE_U_PLIC_NUM_SOURCES,
>>          SIFIVE_U_PLIC_NUM_PRIORITIES,
>>          SIFIVE_U_PLIC_PRIORITY_BASE,

Re: [Qemu-devel] [Qemu-riscv] [PATCH 2/2] riscv: sifive_u: Update the plic hart config to support multicore

[Fabien Chouteau]

Hi Bin,

Thanks for this patch.

I know I am very late to the game but I have a comment here.

On 17/05/2019 17:51, Bin Meng wrote:
> +    /* create PLIC hart topology configuration string */
> +    plic_hart_config_len = (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1) * smp_cpus;
> +    plic_hart_config = g_malloc0(plic_hart_config_len);
> +    for (i = 0; i < smp_cpus; i++) {
> +        if (i != 0) {
> +            strncat(plic_hart_config, ",", plic_hart_config_len);
> +        }
> +        strncat(plic_hart_config, SIFIVE_U_PLIC_HART_CONFIG,
> +                plic_hart_config_len);
> +        plic_hart_config_len -= (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1);
> +    }
> +

This will create up to 4 MS PLIC devices. However on the Unleashed FU540 the PLICs are M,MS,MS,MS,MS because of the monitor hart #0.

This means a different memory layout than the real hardware.

For instance address 0x0C00_2080 will be hart #0 S-Mode interrupt enables in QEMU, instead of #1 M-Mode interrupt enables for the real hardware.

To fix this I suggest to change this loop to:

    for (i = 0; i < smp_cpus; i++) {
        if (i != 0) {
            strncat(plic_hart_config, "," SIFIVE_U_PLIC_HART_CONFIG,
                    plic_hart_config_len);
        } else {
            strncat(plic_hart_config, "M", plic_hart_config_len);
        }
        plic_hart_config_len -= (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1);
    }

This will make hart #0 PLIC in M mode and the others in MS.

What do you think?

Best regards,

Re: [Qemu-riscv] [PATCH 2/2] riscv: sifive_u: Update the plic hart config to support multicore

[Fabien Chouteau]

Hi Bin,

Thanks for this patch.

I know I am very late to the game but I have a comment here.

On 17/05/2019 17:51, Bin Meng wrote:
> +    /* create PLIC hart topology configuration string */
> +    plic_hart_config_len = (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1) * smp_cpus;
> +    plic_hart_config = g_malloc0(plic_hart_config_len);
> +    for (i = 0; i < smp_cpus; i++) {
> +        if (i != 0) {
> +            strncat(plic_hart_config, ",", plic_hart_config_len);
> +        }
> +        strncat(plic_hart_config, SIFIVE_U_PLIC_HART_CONFIG,
> +                plic_hart_config_len);
> +        plic_hart_config_len -= (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1);
> +    }
> +

This will create up to 4 MS PLIC devices. However on the Unleashed FU540 the PLICs are M,MS,MS,MS,MS because of the monitor hart #0.

This means a different memory layout than the real hardware.

For instance address 0x0C00_2080 will be hart #0 S-Mode interrupt enables in QEMU, instead of #1 M-Mode interrupt enables for the real hardware.

To fix this I suggest to change this loop to:

    for (i = 0; i < smp_cpus; i++) {
        if (i != 0) {
            strncat(plic_hart_config, "," SIFIVE_U_PLIC_HART_CONFIG,
                    plic_hart_config_len);
        } else {
            strncat(plic_hart_config, "M", plic_hart_config_len);
        }
        plic_hart_config_len -= (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1);
    }

This will make hart #0 PLIC in M mode and the others in MS.

What do you think?

Best regards,

Re: [Qemu-devel] [Qemu-riscv] [PATCH 2/2] riscv: sifive_u: Update the plic hart config to support multicore

[Alistair Francis]

On Mon, Jul 8, 2019 at 9:32 AM Fabien Chouteau <chouteau@adacore.com> wrote:
>
> Hi Bin,
>
> Thanks for this patch.
>
> I know I am very late to the game but I have a comment here.
>
> On 17/05/2019 17:51, Bin Meng wrote:
> > +    /* create PLIC hart topology configuration string */
> > +    plic_hart_config_len = (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1) * smp_cpus;
> > +    plic_hart_config = g_malloc0(plic_hart_config_len);
> > +    for (i = 0; i < smp_cpus; i++) {
> > +        if (i != 0) {
> > +            strncat(plic_hart_config, ",", plic_hart_config_len);
> > +        }
> > +        strncat(plic_hart_config, SIFIVE_U_PLIC_HART_CONFIG,
> > +                plic_hart_config_len);
> > +        plic_hart_config_len -= (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1);
> > +    }
> > +
>
> This will create up to 4 MS PLIC devices. However on the Unleashed FU540 the PLICs are M,MS,MS,MS,MS because of the monitor hart #0.
>
> This means a different memory layout than the real hardware.
>
> For instance address 0x0C00_2080 will be hart #0 S-Mode interrupt enables in QEMU, instead of #1 M-Mode interrupt enables for the real hardware.
>
> To fix this I suggest to change this loop to:
>
>     for (i = 0; i < smp_cpus; i++) {
>         if (i != 0) {
>             strncat(plic_hart_config, "," SIFIVE_U_PLIC_HART_CONFIG,
>                     plic_hart_config_len);
>         } else {
>             strncat(plic_hart_config, "M", plic_hart_config_len);
>         }
>         plic_hart_config_len -= (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1);
>     }
>
> This will make hart #0 PLIC in M mode and the others in MS.
>
> What do you think?

I think I understand what you mean, in which case I also think you are
correct. Do you want to send a patch and we can discuss there?

Alistair

>
> Best regards,
>

Re: [Qemu-riscv] [Qemu-devel] [PATCH 2/2] riscv: sifive_u: Update the plic hart config to support multicore

[Alistair Francis]

On Mon, Jul 8, 2019 at 9:32 AM Fabien Chouteau <chouteau@adacore.com> wrote:
>
> Hi Bin,
>
> Thanks for this patch.
>
> I know I am very late to the game but I have a comment here.
>
> On 17/05/2019 17:51, Bin Meng wrote:
> > +    /* create PLIC hart topology configuration string */
> > +    plic_hart_config_len = (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1) * smp_cpus;
> > +    plic_hart_config = g_malloc0(plic_hart_config_len);
> > +    for (i = 0; i < smp_cpus; i++) {
> > +        if (i != 0) {
> > +            strncat(plic_hart_config, ",", plic_hart_config_len);
> > +        }
> > +        strncat(plic_hart_config, SIFIVE_U_PLIC_HART_CONFIG,
> > +                plic_hart_config_len);
> > +        plic_hart_config_len -= (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1);
> > +    }
> > +
>
> This will create up to 4 MS PLIC devices. However on the Unleashed FU540 the PLICs are M,MS,MS,MS,MS because of the monitor hart #0.
>
> This means a different memory layout than the real hardware.
>
> For instance address 0x0C00_2080 will be hart #0 S-Mode interrupt enables in QEMU, instead of #1 M-Mode interrupt enables for the real hardware.
>
> To fix this I suggest to change this loop to:
>
>     for (i = 0; i < smp_cpus; i++) {
>         if (i != 0) {
>             strncat(plic_hart_config, "," SIFIVE_U_PLIC_HART_CONFIG,
>                     plic_hart_config_len);
>         } else {
>             strncat(plic_hart_config, "M", plic_hart_config_len);
>         }
>         plic_hart_config_len -= (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1);
>     }
>
> This will make hart #0 PLIC in M mode and the others in MS.
>
> What do you think?

I think I understand what you mean, in which case I also think you are
correct. Do you want to send a patch and we can discuss there?

Alistair

>
> Best regards,
>

Re: [Qemu-devel] [Qemu-riscv] [PATCH 2/2] riscv: sifive_u: Update the plic hart config to support multicore

[Bin Meng]

Hi Fabien,

On Tue, Jul 9, 2019 at 12:31 AM Fabien Chouteau <chouteau@adacore.com> wrote:
>
> Hi Bin,
>
> Thanks for this patch.
>
> I know I am very late to the game but I have a comment here.
>
> On 17/05/2019 17:51, Bin Meng wrote:
> > +    /* create PLIC hart topology configuration string */
> > +    plic_hart_config_len = (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1) * smp_cpus;
> > +    plic_hart_config = g_malloc0(plic_hart_config_len);
> > +    for (i = 0; i < smp_cpus; i++) {
> > +        if (i != 0) {
> > +            strncat(plic_hart_config, ",", plic_hart_config_len);
> > +        }
> > +        strncat(plic_hart_config, SIFIVE_U_PLIC_HART_CONFIG,
> > +                plic_hart_config_len);
> > +        plic_hart_config_len -= (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1);
> > +    }
> > +
>
> This will create up to 4 MS PLIC devices. However on the Unleashed FU540 the PLICs are M,MS,MS,MS,MS because of the monitor hart #0.
>
> This means a different memory layout than the real hardware.
>
> For instance address 0x0C00_2080 will be hart #0 S-Mode interrupt enables in QEMU, instead of #1 M-Mode interrupt enables for the real hardware.

Thanks for the notes! I agree to better match the real hardware, it
should be modeled like that. However I am not sure what the original
intention was when creating the "sifive_u" machine. Both OpenSBI and
U-Boot list sifive_u as a special target, instead of the real
Unleashed board hence I assume this is a hypothetical target too, like
the "virt", but was created to best match the real Unleashed board
though.

>
> To fix this I suggest to change this loop to:
>
>     for (i = 0; i < smp_cpus; i++) {
>         if (i != 0) {
>             strncat(plic_hart_config, "," SIFIVE_U_PLIC_HART_CONFIG,
>                     plic_hart_config_len);
>         } else {
>             strncat(plic_hart_config, "M", plic_hart_config_len);
>         }
>         plic_hart_config_len -= (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1);
>     }
>
> This will make hart #0 PLIC in M mode and the others in MS.
>
> What do you think?


Regards,
Bin

Re: [Qemu-riscv] [PATCH 2/2] riscv: sifive_u: Update the plic hart config to support multicore

[Bin Meng]

Hi Fabien,

On Tue, Jul 9, 2019 at 12:31 AM Fabien Chouteau <chouteau@adacore.com> wrote:
>
> Hi Bin,
>
> Thanks for this patch.
>
> I know I am very late to the game but I have a comment here.
>
> On 17/05/2019 17:51, Bin Meng wrote:
> > +    /* create PLIC hart topology configuration string */
> > +    plic_hart_config_len = (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1) * smp_cpus;
> > +    plic_hart_config = g_malloc0(plic_hart_config_len);
> > +    for (i = 0; i < smp_cpus; i++) {
> > +        if (i != 0) {
> > +            strncat(plic_hart_config, ",", plic_hart_config_len);
> > +        }
> > +        strncat(plic_hart_config, SIFIVE_U_PLIC_HART_CONFIG,
> > +                plic_hart_config_len);
> > +        plic_hart_config_len -= (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1);
> > +    }
> > +
>
> This will create up to 4 MS PLIC devices. However on the Unleashed FU540 the PLICs are M,MS,MS,MS,MS because of the monitor hart #0.
>
> This means a different memory layout than the real hardware.
>
> For instance address 0x0C00_2080 will be hart #0 S-Mode interrupt enables in QEMU, instead of #1 M-Mode interrupt enables for the real hardware.

Thanks for the notes! I agree to better match the real hardware, it
should be modeled like that. However I am not sure what the original
intention was when creating the "sifive_u" machine. Both OpenSBI and
U-Boot list sifive_u as a special target, instead of the real
Unleashed board hence I assume this is a hypothetical target too, like
the "virt", but was created to best match the real Unleashed board
though.

>
> To fix this I suggest to change this loop to:
>
>     for (i = 0; i < smp_cpus; i++) {
>         if (i != 0) {
>             strncat(plic_hart_config, "," SIFIVE_U_PLIC_HART_CONFIG,
>                     plic_hart_config_len);
>         } else {
>             strncat(plic_hart_config, "M", plic_hart_config_len);
>         }
>         plic_hart_config_len -= (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1);
>     }
>
> This will make hart #0 PLIC in M mode and the others in MS.
>
> What do you think?


Regards,
Bin

Re: [Qemu-devel] [Qemu-riscv] [PATCH 2/2] riscv: sifive_u: Update the plic hart config to support multicore

[Alistair Francis]

On Sat, Jul 13, 2019 at 8:23 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Fabien,
>
> On Tue, Jul 9, 2019 at 12:31 AM Fabien Chouteau <chouteau@adacore.com> wrote:
> >
> > Hi Bin,
> >
> > Thanks for this patch.
> >
> > I know I am very late to the game but I have a comment here.
> >
> > On 17/05/2019 17:51, Bin Meng wrote:
> > > +    /* create PLIC hart topology configuration string */
> > > +    plic_hart_config_len = (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1) * smp_cpus;
> > > +    plic_hart_config = g_malloc0(plic_hart_config_len);
> > > +    for (i = 0; i < smp_cpus; i++) {
> > > +        if (i != 0) {
> > > +            strncat(plic_hart_config, ",", plic_hart_config_len);
> > > +        }
> > > +        strncat(plic_hart_config, SIFIVE_U_PLIC_HART_CONFIG,
> > > +                plic_hart_config_len);
> > > +        plic_hart_config_len -= (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1);
> > > +    }
> > > +
> >
> > This will create up to 4 MS PLIC devices. However on the Unleashed FU540 the PLICs are M,MS,MS,MS,MS because of the monitor hart #0.
> >
> > This means a different memory layout than the real hardware.
> >
> > For instance address 0x0C00_2080 will be hart #0 S-Mode interrupt enables in QEMU, instead of #1 M-Mode interrupt enables for the real hardware.
>
> Thanks for the notes! I agree to better match the real hardware, it
> should be modeled like that. However I am not sure what the original
> intention was when creating the "sifive_u" machine. Both OpenSBI and
> U-Boot list sifive_u as a special target, instead of the real
> Unleashed board hence I assume this is a hypothetical target too, like
> the "virt", but was created to best match the real Unleashed board
> though.

I thought (Palmer correct me if I'm wrong) that the sifive_u machine
*should* match the hardware. The problem is that QEMU doesn't support
everything that the HW supports which is why U-Boot and OpenSBI have
their own targets. The goal is to not require special QEMU targets, so
this is a step in the right direction.

Alistair

>
> >
> > To fix this I suggest to change this loop to:
> >
> >     for (i = 0; i < smp_cpus; i++) {
> >         if (i != 0) {
> >             strncat(plic_hart_config, "," SIFIVE_U_PLIC_HART_CONFIG,
> >                     plic_hart_config_len);
> >         } else {
> >             strncat(plic_hart_config, "M", plic_hart_config_len);
> >         }
> >         plic_hart_config_len -= (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1);
> >     }
> >
> > This will make hart #0 PLIC in M mode and the others in MS.
> >
> > What do you think?
>
>
> Regards,
> Bin
>

Re: [Qemu-riscv] [Qemu-devel] [PATCH 2/2] riscv: sifive_u: Update the plic hart config to support multicore

[Alistair Francis]

On Sat, Jul 13, 2019 at 8:23 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Fabien,
>
> On Tue, Jul 9, 2019 at 12:31 AM Fabien Chouteau <chouteau@adacore.com> wrote:
> >
> > Hi Bin,
> >
> > Thanks for this patch.
> >
> > I know I am very late to the game but I have a comment here.
> >
> > On 17/05/2019 17:51, Bin Meng wrote:
> > > +    /* create PLIC hart topology configuration string */
> > > +    plic_hart_config_len = (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1) * smp_cpus;
> > > +    plic_hart_config = g_malloc0(plic_hart_config_len);
> > > +    for (i = 0; i < smp_cpus; i++) {
> > > +        if (i != 0) {
> > > +            strncat(plic_hart_config, ",", plic_hart_config_len);
> > > +        }
> > > +        strncat(plic_hart_config, SIFIVE_U_PLIC_HART_CONFIG,
> > > +                plic_hart_config_len);
> > > +        plic_hart_config_len -= (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1);
> > > +    }
> > > +
> >
> > This will create up to 4 MS PLIC devices. However on the Unleashed FU540 the PLICs are M,MS,MS,MS,MS because of the monitor hart #0.
> >
> > This means a different memory layout than the real hardware.
> >
> > For instance address 0x0C00_2080 will be hart #0 S-Mode interrupt enables in QEMU, instead of #1 M-Mode interrupt enables for the real hardware.
>
> Thanks for the notes! I agree to better match the real hardware, it
> should be modeled like that. However I am not sure what the original
> intention was when creating the "sifive_u" machine. Both OpenSBI and
> U-Boot list sifive_u as a special target, instead of the real
> Unleashed board hence I assume this is a hypothetical target too, like
> the "virt", but was created to best match the real Unleashed board
> though.

I thought (Palmer correct me if I'm wrong) that the sifive_u machine
*should* match the hardware. The problem is that QEMU doesn't support
everything that the HW supports which is why U-Boot and OpenSBI have
their own targets. The goal is to not require special QEMU targets, so
this is a step in the right direction.

Alistair

>
> >
> > To fix this I suggest to change this loop to:
> >
> >     for (i = 0; i < smp_cpus; i++) {
> >         if (i != 0) {
> >             strncat(plic_hart_config, "," SIFIVE_U_PLIC_HART_CONFIG,
> >                     plic_hart_config_len);
> >         } else {
> >             strncat(plic_hart_config, "M", plic_hart_config_len);
> >         }
> >         plic_hart_config_len -= (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1);
> >     }
> >
> > This will make hart #0 PLIC in M mode and the others in MS.
> >
> > What do you think?
>
>
> Regards,
> Bin
>

Re: [Qemu-devel] [Qemu-riscv] [PATCH 2/2] riscv: sifive_u: Update the plic hart config to support multicore

[Bin Meng]

Hi Alistair,

On Tue, Jul 16, 2019 at 5:33 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Sat, Jul 13, 2019 at 8:23 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Hi Fabien,
> >
> > On Tue, Jul 9, 2019 at 12:31 AM Fabien Chouteau <chouteau@adacore.com> wrote:
> > >
> > > Hi Bin,
> > >
> > > Thanks for this patch.
> > >
> > > I know I am very late to the game but I have a comment here.
> > >
> > > On 17/05/2019 17:51, Bin Meng wrote:
> > > > +    /* create PLIC hart topology configuration string */
> > > > +    plic_hart_config_len = (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1) * smp_cpus;
> > > > +    plic_hart_config = g_malloc0(plic_hart_config_len);
> > > > +    for (i = 0; i < smp_cpus; i++) {
> > > > +        if (i != 0) {
> > > > +            strncat(plic_hart_config, ",", plic_hart_config_len);
> > > > +        }
> > > > +        strncat(plic_hart_config, SIFIVE_U_PLIC_HART_CONFIG,
> > > > +                plic_hart_config_len);
> > > > +        plic_hart_config_len -= (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1);
> > > > +    }
> > > > +
> > >
> > > This will create up to 4 MS PLIC devices. However on the Unleashed FU540 the PLICs are M,MS,MS,MS,MS because of the monitor hart #0.
> > >
> > > This means a different memory layout than the real hardware.
> > >
> > > For instance address 0x0C00_2080 will be hart #0 S-Mode interrupt enables in QEMU, instead of #1 M-Mode interrupt enables for the real hardware.
> >
> > Thanks for the notes! I agree to better match the real hardware, it
> > should be modeled like that. However I am not sure what the original
> > intention was when creating the "sifive_u" machine. Both OpenSBI and
> > U-Boot list sifive_u as a special target, instead of the real
> > Unleashed board hence I assume this is a hypothetical target too, like
> > the "virt", but was created to best match the real Unleashed board
> > though.
>
> I thought (Palmer correct me if I'm wrong) that the sifive_u machine
> *should* match the hardware. The problem is that QEMU doesn't support
> everything that the HW supports which is why U-Boot and OpenSBI have
> their own targets. The goal is to not require special QEMU targets, so
> this is a step in the right direction.
>

I've sent a series that improves the emulation fidelity of sifive_u
machine, so that the upstream OpenSBI, U-Boot and kernel images built
for the SiFive HiFive Unleashed board can be used out of the box
without any special hack.

Please have a look.
http://patchwork.ozlabs.org/project/qemu-devel/list/?series=123386

Regards,
Bin

Re: [Qemu-riscv] [Qemu-devel] [PATCH 2/2] riscv: sifive_u: Update the plic hart config to support multicore

[Bin Meng]

Hi Alistair,

On Tue, Jul 16, 2019 at 5:33 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Sat, Jul 13, 2019 at 8:23 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Hi Fabien,
> >
> > On Tue, Jul 9, 2019 at 12:31 AM Fabien Chouteau <chouteau@adacore.com> wrote:
> > >
> > > Hi Bin,
> > >
> > > Thanks for this patch.
> > >
> > > I know I am very late to the game but I have a comment here.
> > >
> > > On 17/05/2019 17:51, Bin Meng wrote:
> > > > +    /* create PLIC hart topology configuration string */
> > > > +    plic_hart_config_len = (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1) * smp_cpus;
> > > > +    plic_hart_config = g_malloc0(plic_hart_config_len);
> > > > +    for (i = 0; i < smp_cpus; i++) {
> > > > +        if (i != 0) {
> > > > +            strncat(plic_hart_config, ",", plic_hart_config_len);
> > > > +        }
> > > > +        strncat(plic_hart_config, SIFIVE_U_PLIC_HART_CONFIG,
> > > > +                plic_hart_config_len);
> > > > +        plic_hart_config_len -= (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1);
> > > > +    }
> > > > +
> > >
> > > This will create up to 4 MS PLIC devices. However on the Unleashed FU540 the PLICs are M,MS,MS,MS,MS because of the monitor hart #0.
> > >
> > > This means a different memory layout than the real hardware.
> > >
> > > For instance address 0x0C00_2080 will be hart #0 S-Mode interrupt enables in QEMU, instead of #1 M-Mode interrupt enables for the real hardware.
> >
> > Thanks for the notes! I agree to better match the real hardware, it
> > should be modeled like that. However I am not sure what the original
> > intention was when creating the "sifive_u" machine. Both OpenSBI and
> > U-Boot list sifive_u as a special target, instead of the real
> > Unleashed board hence I assume this is a hypothetical target too, like
> > the "virt", but was created to best match the real Unleashed board
> > though.
>
> I thought (Palmer correct me if I'm wrong) that the sifive_u machine
> *should* match the hardware. The problem is that QEMU doesn't support
> everything that the HW supports which is why U-Boot and OpenSBI have
> their own targets. The goal is to not require special QEMU targets, so
> this is a step in the right direction.
>

I've sent a series that improves the emulation fidelity of sifive_u
machine, so that the upstream OpenSBI, U-Boot and kernel images built
for the SiFive HiFive Unleashed board can be used out of the box
without any special hack.

Please have a look.
http://patchwork.ozlabs.org/project/qemu-devel/list/?series=123386

Regards,
Bin

Re: [Qemu-devel] [Qemu-riscv] [PATCH 2/2] riscv: sifive_u: Update the plic hart config to support multicore

[Bin Meng]

Hi Fabien,

On Tue, Jul 9, 2019 at 12:31 AM Fabien Chouteau <chouteau@adacore.com> wrote:
>
> Hi Bin,
>
> Thanks for this patch.
>
> I know I am very late to the game but I have a comment here.
>
> On 17/05/2019 17:51, Bin Meng wrote:
> > +    /* create PLIC hart topology configuration string */
> > +    plic_hart_config_len = (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1) * smp_cpus;
> > +    plic_hart_config = g_malloc0(plic_hart_config_len);
> > +    for (i = 0; i < smp_cpus; i++) {
> > +        if (i != 0) {
> > +            strncat(plic_hart_config, ",", plic_hart_config_len);
> > +        }
> > +        strncat(plic_hart_config, SIFIVE_U_PLIC_HART_CONFIG,
> > +                plic_hart_config_len);
> > +        plic_hart_config_len -= (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1);
> > +    }
> > +
>
> This will create up to 4 MS PLIC devices. However on the Unleashed FU540 the PLICs are M,MS,MS,MS,MS because of the monitor hart #0.
>
> This means a different memory layout than the real hardware.
>
> For instance address 0x0C00_2080 will be hart #0 S-Mode interrupt enables in QEMU, instead of #1 M-Mode interrupt enables for the real hardware.
>
> To fix this I suggest to change this loop to:
>
>     for (i = 0; i < smp_cpus; i++) {
>         if (i != 0) {
>             strncat(plic_hart_config, "," SIFIVE_U_PLIC_HART_CONFIG,
>                     plic_hart_config_len);
>         } else {
>             strncat(plic_hart_config, "M", plic_hart_config_len);
>         }
>         plic_hart_config_len -= (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1);
>     }
>
> This will make hart #0 PLIC in M mode and the others in MS.
>
> What do you think?

Thank you for the suggestion. A patch was created for this:
http://patchwork.ozlabs.org/patch/1142282/

Regards,
Bin

Re: [Qemu-riscv] [PATCH 2/2] riscv: sifive_u: Update the plic hart config to support multicore

[Bin Meng]

Hi Fabien,

On Tue, Jul 9, 2019 at 12:31 AM Fabien Chouteau <chouteau@adacore.com> wrote:
>
> Hi Bin,
>
> Thanks for this patch.
>
> I know I am very late to the game but I have a comment here.
>
> On 17/05/2019 17:51, Bin Meng wrote:
> > +    /* create PLIC hart topology configuration string */
> > +    plic_hart_config_len = (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1) * smp_cpus;
> > +    plic_hart_config = g_malloc0(plic_hart_config_len);
> > +    for (i = 0; i < smp_cpus; i++) {
> > +        if (i != 0) {
> > +            strncat(plic_hart_config, ",", plic_hart_config_len);
> > +        }
> > +        strncat(plic_hart_config, SIFIVE_U_PLIC_HART_CONFIG,
> > +                plic_hart_config_len);
> > +        plic_hart_config_len -= (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1);
> > +    }
> > +
>
> This will create up to 4 MS PLIC devices. However on the Unleashed FU540 the PLICs are M,MS,MS,MS,MS because of the monitor hart #0.
>
> This means a different memory layout than the real hardware.
>
> For instance address 0x0C00_2080 will be hart #0 S-Mode interrupt enables in QEMU, instead of #1 M-Mode interrupt enables for the real hardware.
>
> To fix this I suggest to change this loop to:
>
>     for (i = 0; i < smp_cpus; i++) {
>         if (i != 0) {
>             strncat(plic_hart_config, "," SIFIVE_U_PLIC_HART_CONFIG,
>                     plic_hart_config_len);
>         } else {
>             strncat(plic_hart_config, "M", plic_hart_config_len);
>         }
>         plic_hart_config_len -= (strlen(SIFIVE_U_PLIC_HART_CONFIG) + 1);
>     }
>
> This will make hart #0 PLIC in M mode and the others in MS.
>
> What do you think?

Thank you for the suggestion. A patch was created for this:
http://patchwork.ozlabs.org/patch/1142282/

Regards,
Bin

Re: [Qemu-devel] [Qemu-riscv] [PATCH 2/2] riscv: sifive_u: Update the plic hart config to support multicore

[Fabien Chouteau]

On 05/08/2019 18:10, Bin Meng wrote:
> Thank you for the suggestion. A patch was created for this:
> http://patchwork.ozlabs.org/patch/1142282/

Awesome, thank you Bin!

Re: [Qemu-riscv] [PATCH 2/2] riscv: sifive_u: Update the plic hart config to support multicore

[Fabien Chouteau]

On 05/08/2019 18:10, Bin Meng wrote:
> Thank you for the suggestion. A patch was created for this:
> http://patchwork.ozlabs.org/patch/1142282/

Awesome, thank you Bin!