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
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); >
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,
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
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.
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,
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,
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, >
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
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 >
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
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
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!