[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Qemu-riscv] [PATCH 2/2] riscv: sifive_u: Update the pl
From: |
Bin Meng |
Subject: |
Re: [Qemu-devel] [Qemu-riscv] [PATCH 2/2] riscv: sifive_u: Update the plic hart config to support multicore |
Date: |
Sun, 14 Jul 2019 11:22:41 +0800 |
Hi Fabien,
On Tue, Jul 9, 2019 at 12:31 AM Fabien Chouteau <address@hidden> 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