| Message ID | 1567786819-22142-19-git-send-email-bmeng.cn@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | riscv: sifive_u: Improve the emulation fidelity of sifive_u machine | expand |
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; } DEFINE_MACHINE("sifive_u", riscv_sifive_u_machine_init)
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; ? > } > > DEFINE_MACHINE("sifive_u", riscv_sifive_u_machine_init) Regards, Bin
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. > > ? > >> } >> >> DEFINE_MACHINE("sifive_u", riscv_sifive_u_machine_init) > > Regards, > Bin
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". Regards, Bin
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.
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. > >
On Sun, 15 Sep 2019 10:39:38 PDT (-0700), fintelia@gmail.com wrote: > 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? It works for me. > > 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. >> >>
Hi Jonathan, On Mon, Sep 16, 2019 at 1:40 AM Jonathan Behrens <fintelia@gmail.com> wrote: > > 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? > No, U-Boot is using DTB that was passed from previous stage firmware - OpenSBI. On a real board this is 4 + 1. On QEMU, DTB is dynamically generated per "-smp n" settings. So there should be no problem. Regards, Bin
On Sun, Sep 15, 2019 at 6:07 AM Bin Meng <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". Agreed! Alistair > > Regards, > Bin >
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