Hi Fabien, On Tue, Jul 9, 2019 at 12:31 AM Fabien Chouteau <chout...@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