On Fri, Apr 24, 2020 at 12:12 PM Corey Wharton <core...@fb.com> wrote: > > > > > -----Original Message----- > > From: Alistair Francis <alistai...@gmail.com> > > Sent: Friday, April 24, 2020 9:04 AM > > To: Corey Wharton <core...@fb.com> > > Cc: qemu-devel@nongnu.org Developers <qemu-devel@nongnu.org>; open > > list:RISC-V <qemu-ri...@nongnu.org>; Sagar Karandikar > > <sag...@eecs.berkeley.edu>; Bastian Koppelmann <kbast...@mail.uni- > > paderborn.de>; Alistair Francis <alistair.fran...@wdc.com>; Palmer Dabbelt > > <pal...@dabbelt.com>; Bin Meng <bmeng...@gmail.com> > > Subject: Re: [PATCH v2 1/2] riscv: sifive_e: Support changing CPU type > > > > On Fri, Mar 13, 2020 at 12:35 PM Corey Wharton <core...@fb.com> wrote: > > > > > > Allows the CPU to be changed from the default via the -cpu command > > > line option. > > > > > > Signed-off-by: Corey Wharton <core...@fb.com> > > > Reviewed-by: Bin Meng <bmeng...@gmail.com> > > > Reviewed-by: Alistair Francis <alistair.fran...@wdc.com> > > > > Thanks for the patch. > > > > Unfortunately this fails `make check`. > > > > The problem is that the machine has the `default_cpu_type` set but then you > > set "cpu-type" from the SoC. > > > > This diff fixes the make check failure for me: > > > > diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c index > > 1fd08f325c..b53109521e 100644 > > --- a/hw/riscv/sifive_e.c > > +++ b/hw/riscv/sifive_e.c > > @@ -123,8 +123,6 @@ static void riscv_sifive_e_soc_init(Object *obj) > > object_initialize_child(obj, "cpus", &s->cpus, > > sizeof(s->cpus), TYPE_RISCV_HART_ARRAY, > > &error_abort, NULL); > > - object_property_set_str(OBJECT(&s->cpus), ms->cpu_type, "cpu-type", > > - &error_abort); > > object_property_set_int(OBJECT(&s->cpus), ms->smp.cpus, "num-harts", > > &error_abort); > > sysbus_init_child_obj(obj, "riscv.sifive.e.gpio0", @@ -141,6 +139,8 @@ > > static void riscv_sifive_e_soc_realize(DeviceState > > *dev, Error **errp) > > SiFiveESoCState *s = RISCV_E_SOC(dev); > > MemoryRegion *sys_mem = get_system_memory(); > > > > + object_property_set_str(OBJECT(&s->cpus), ms->cpu_type, "cpu-type", > > + &error_abort); > > object_property_set_bool(OBJECT(&s->cpus), true, "realized", > > &error_abort); > > > > > > I'm happy to just squash that into the patch. Let me know if you don't want > > me to do that and I'll drop these patches and let you re-send them. > > > > Alistair > > > > Thanks for fixing this issue. I tested your patch and it seems to work as > Intended and I'm fine with you squashing it into the patch.
Great! I'll send this patch as part of the PR after 5.0 then. I also realised that my justification was wrong. It's not because of the machine/SoC split, but because of the order between init/realise. Alistair > > Corey > > > > --- > > > hw/riscv/sifive_e.c | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c index > > > a254cad489..b0a611adb9 100644 > > > --- a/hw/riscv/sifive_e.c > > > +++ b/hw/riscv/sifive_e.c > > > @@ -123,7 +123,7 @@ static void riscv_sifive_e_soc_init(Object *obj) > > > object_initialize_child(obj, "cpus", &s->cpus, > > > sizeof(s->cpus), TYPE_RISCV_HART_ARRAY, > > > &error_abort, NULL); > > > - object_property_set_str(OBJECT(&s->cpus), SIFIVE_E_CPU, "cpu-type", > > > + object_property_set_str(OBJECT(&s->cpus), ms->cpu_type, > > > + "cpu-type", > > > &error_abort); > > > object_property_set_int(OBJECT(&s->cpus), ms->smp.cpus, "num- > > harts", > > > &error_abort); @@ -220,6 +220,7 @@ static > > > void riscv_sifive_e_machine_init(MachineClass *mc) > > > mc->desc = "RISC-V Board compatible with SiFive E SDK"; > > > mc->init = riscv_sifive_e_init; > > > mc->max_cpus = 1; > > > + mc->default_cpu_type = SIFIVE_E_CPU; > > > } > > > > > > DEFINE_MACHINE("sifive_e", riscv_sifive_e_machine_init) > > > -- > > > 2.21.1 > > > > > >