On Sat, Jul 13, 2019 at 8:23 PM Bin Meng <bmeng...@gmail.com> wrote:
>
> 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.
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
>
