1. Patchew
  2. QEMU
  3. [PATCH v2 0/2] Support different CPU types for the sifive_e machine
  4. View patch

[PATCH v2 1/2] riscv: sifive_e: Support changing CPU type

Corey Wharton posted 2 patches 5 years, 11 months ago
Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Sagar Karandikar <sagark@eecs.berkeley.edu>, Bastian Koppelmann <kbastian@mail.uni-paderborn.de>, Alistair Francis <Alistair.Francis@wdc.com>
[PATCH v2 1/2] riscv: sifive_e: Support changing CPU type
Posted by Corey Wharton 5 years, 11 months ago 
Allows the CPU to be changed from the default via the -cpu command
line option.

Signed-off-by: Corey Wharton <coreyw7@fb.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
 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
Re: [PATCH v2 1/2] riscv: sifive_e: Support changing CPU type
Posted by Philippe Mathieu-Daudé 5 years, 11 months ago 
On 3/13/20 8:34 PM, Corey Wharton wrote:
> Allows the CPU to be changed from the default via the -cpu command
> line option.
> 
> Signed-off-by: Corey Wharton <coreyw7@fb.com>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>   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)
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Re: [PATCH v2 1/2] riscv: sifive_e: Support changing CPU type
Posted by Alistair Francis 5 years, 9 months ago 
On Fri, Mar 13, 2020 at 12:35 PM Corey Wharton <coreyw7@fb.com> wrote:
>
> Allows the CPU to be changed from the default via the -cpu command
> line option.
>
> Signed-off-by: Corey Wharton <coreyw7@fb.com>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> Reviewed-by: Alistair Francis <alistair.francis@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

> ---
>  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
>
>
RE: [PATCH v2 1/2] riscv: sifive_e: Support changing CPU type
Posted by Corey Wharton 5 years, 9 months ago 

> -----Original Message-----
> From: Alistair Francis <alistair23@gmail.com>
> Sent: Friday, April 24, 2020 9:04 AM
> To: Corey Wharton <coreyw7@fb.com>
> Cc: qemu-devel@nongnu.org Developers <qemu-devel@nongnu.org>; open
> list:RISC-V <qemu-riscv@nongnu.org>; Sagar Karandikar
> <sagark@eecs.berkeley.edu>; Bastian Koppelmann <kbastian@mail.uni-
> paderborn.de>; Alistair Francis <alistair.francis@wdc.com>; Palmer Dabbelt
> <palmer@dabbelt.com>; Bin Meng <bmeng.cn@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 <coreyw7@fb.com> wrote:
> >
> > Allows the CPU to be changed from the default via the -cpu command
> > line option.
> >
> > Signed-off-by: Corey Wharton <coreyw7@fb.com>
> > Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> > Reviewed-by: Alistair Francis <alistair.francis@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.

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
> >
> >
Re: [PATCH v2 1/2] riscv: sifive_e: Support changing CPU type
Posted by Alistair Francis 5 years, 9 months ago 
On Fri, Apr 24, 2020 at 12:12 PM Corey Wharton <coreyw7@fb.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Alistair Francis <alistair23@gmail.com>
> > Sent: Friday, April 24, 2020 9:04 AM
> > To: Corey Wharton <coreyw7@fb.com>
> > Cc: qemu-devel@nongnu.org Developers <qemu-devel@nongnu.org>; open
> > list:RISC-V <qemu-riscv@nongnu.org>; Sagar Karandikar
> > <sagark@eecs.berkeley.edu>; Bastian Koppelmann <kbastian@mail.uni-
> > paderborn.de>; Alistair Francis <alistair.francis@wdc.com>; Palmer Dabbelt
> > <palmer@dabbelt.com>; Bin Meng <bmeng.cn@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 <coreyw7@fb.com> wrote:
> > >
> > > Allows the CPU to be changed from the default via the -cpu command
> > > line option.
> > >
> > > Signed-off-by: Corey Wharton <coreyw7@fb.com>
> > > Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> > > Reviewed-by: Alistair Francis <alistair.francis@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
> > >
> > >

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.