Re: [Qemu-devel] [Qemu-riscv] [PATCH v8 18/32] riscv: sifive_u: Set the minimum number of cpus to 2

From: Jonathan Behrens <fintelia@gmail.com>
To: Palmer Dabbelt <palmer@sifive.com>
Cc: Alistair Francis <Alistair.Francis@wdc.com>,
	Bin Meng <bmeng.cn@gmail.com>,
	"open list:RISC-V" <qemu-riscv@nongnu.org>,
	"qemu-devel@nongnu.org Developers" <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] [Qemu-riscv] [PATCH v8 18/32] riscv: sifive_u: Set the minimum number of cpus to 2
Date: Sun, 15 Sep 2019 13:39:38 -0400	[thread overview]
Message-ID: <CANnJOVGRWepwwZPyOmF1O7azqKVYgB2ZSqLrh5qnye10Oi_M7Q@mail.gmail.com> (raw)
In-Reply-To: <mhng-da766f03-2535-4a8c-97aa-1f85f986bee3@palmer-si-x1e>

Has there been testing with "-smp 2"? A while back I thought I read that
the included uboot firmware was using a hard-coded device tree that
indicated 4+1 CPUs, which I would have expected to cause Linux boot issues?

Jonathan

On Sun, Sep 15, 2019 at 1:31 PM Palmer Dabbelt <palmer@sifive.com> wrote:

> On Sun, 15 Sep 2019 06:07:18 PDT (-0700), bmeng.cn@gmail.com wrote:
> > Hi Palmer,
> >
> > On Sun, Sep 15, 2019 at 3:00 AM Palmer Dabbelt <palmer@sifive.com>
> wrote:
> >>
> >> On Fri, 13 Sep 2019 08:25:21 PDT (-0700), bmeng.cn@gmail.com wrote:
> >> > Hi Palmer,
> >> >
> >> > On Fri, Sep 13, 2019 at 10:33 PM Palmer Dabbelt <palmer@sifive.com>
> wrote:
> >> >>
> >> >> On Fri, 06 Sep 2019 09:20:05 PDT (-0700), bmeng.cn@gmail.com wrote:
> >> >> > It is not useful if we only have one management CPU.
> >> >> >
> >> >> > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> >> >> > Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> >> >> >
> >> >> > ---
> >> >> >
> >> >> > Changes in v8: None
> >> >> > Changes in v7: None
> >> >> > Changes in v6: None
> >> >> > Changes in v5: None
> >> >> > Changes in v4: None
> >> >> > Changes in v3:
> >> >> > - use management cpu count + 1 for the min_cpus
> >> >> >
> >> >> > Changes in v2:
> >> >> > - update the file header to indicate at least 2 harts are created
> >> >> >
> >> >> >  hw/riscv/sifive_u.c         | 4 +++-
> >> >> >  include/hw/riscv/sifive_u.h | 2 ++
> >> >> >  2 files changed, 5 insertions(+), 1 deletion(-)
> >> >> >
> >> >> > diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> >> >> > index 2947e06..2023b71 100644
> >> >> > --- a/hw/riscv/sifive_u.c
> >> >> > +++ b/hw/riscv/sifive_u.c
> >> >> > @@ -10,7 +10,8 @@
> >> >> >   * 1) CLINT (Core Level Interruptor)
> >> >> >   * 2) PLIC (Platform Level Interrupt Controller)
> >> >> >   *
> >> >> > - * This board currently uses a hardcoded devicetree that
> indicates one hart.
> >> >> > + * This board currently generates devicetree dynamically that
> indicates at least
> >> >> > + * two harts.
> >> >> >   *
> >> >> >   * This program is free software; you can redistribute it and/or
> modify it
> >> >> >   * under the terms and conditions of the GNU General Public
> License,
> >> >> > @@ -433,6 +434,7 @@ static void
> riscv_sifive_u_machine_init(MachineClass *mc)
> >> >> >       * management CPU.
> >> >> >       */
> >> >> >      mc->max_cpus = 4;
> >> >> > +    mc->min_cpus = SIFIVE_U_MANAGEMENT_CPU_COUNT + 1;
> >> >> >  }
> >> >> >
> >> >> >  DEFINE_MACHINE("sifive_u", riscv_sifive_u_machine_init)
> >> >> > diff --git a/include/hw/riscv/sifive_u.h
> b/include/hw/riscv/sifive_u.h
> >> >> > index f25bad8..6d22741 100644
> >> >> > --- a/include/hw/riscv/sifive_u.h
> >> >> > +++ b/include/hw/riscv/sifive_u.h
> >> >> > @@ -69,6 +69,8 @@ enum {
> >> >> >      SIFIVE_U_GEM_CLOCK_FREQ = 125000000
> >> >> >  };
> >> >> >
> >> >> > +#define SIFIVE_U_MANAGEMENT_CPU_COUNT   1
> >> >> > +
> >> >> >  #define SIFIVE_U_PLIC_HART_CONFIG "MS"
> >> >> >  #define SIFIVE_U_PLIC_NUM_SOURCES 54
> >> >> >  #define SIFIVE_U_PLIC_NUM_PRIORITIES 7
> >> >>
> >> >> This fails "make check", so I'm going to squash in this
> >> >>
> >> >> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> >> >> index ca9f7fea41..adecbf1dd9 100644
> >> >> --- a/hw/riscv/sifive_u.c
> >> >> +++ b/hw/riscv/sifive_u.c
> >> >> @@ -528,6 +528,7 @@ static void
> riscv_sifive_u_machine_init(MachineClass *mc)
> >> >>      mc->init = riscv_sifive_u_init;
> >> >>      mc->max_cpus = SIFIVE_U_MANAGEMENT_CPU_COUNT +
> SIFIVE_U_COMPUTE_CPU_COUNT;
> >> >>      mc->min_cpus = SIFIVE_U_MANAGEMENT_CPU_COUNT + 1;
> >> >> +    mc->default_cpus = mc->max_cpus;
> >> >
> >> > Thank you for fixing the 'make check'. Shouldn't it be:
> >> >
> >> > mc->default_cpus = mc->min_cpus;
> >>
> >> We have 5 harts on the board that this matches, so I figured that'd be
> the
> >> better default.
> >>
> >
> > Per my understanding mc->default_cpus is used when invoking QEMU
> > without passing '-smp n' (that's what 'make check' uses), and with the
> > updated sifive_u machine, '-smp 2' is the actual useful configuration
> > to boot Linux. For consistency with user experience on other machines,
> > without '-smp' means we want a uni-processor machine hence I would
> > suggest we set "mc->default_cpus = mc->min_cpus".
>
> OK, I've spun a v3.  I never sent out v2 but I had tagged it, unless
> there's
> any opposition I'll send this out when I'm back on normal internet.
>
>