Re: [Qemu-devel] [PATCH v8 13/23] RISC-V HART Array

[Michael Clark]

Got it. I can add to the post merge cleanup patch series. BTW the code
currently creates a homogeneous array or cores but the intent is that it
supports a heterogeneous array, in the future. It might evolve into an SOC
base class. We need a method to give it an array or cpu_models. For example
the U54-MC has an e51 monitor core and 4 u54 application cores, but they
are part of a heterogeneous core complex. This code is essentially
“scaffolding”. All of the boards currently have a CLINT, so we could move
that in here too, and perhaps rename it.

Evolution... it works as it is at present. Heterogenous signals the future
intent.

On Sat, 10 Mar 2018 at 1:52 AM, Philippe Mathieu-Daudé 
wrote:

> On 03/02/2018 02:51 PM, Michael Clark wrote:
> > Holds the state of a heterogenous array of RISC-V hardware threads.
>
> heterogeneous
>
> >
> > Reviewed-by: Richard Henderson 
> > Signed-off-by: Sagar Karandikar 
> > Signed-off-by: Michael Clark 
> > ---
> >  hw/riscv/riscv_hart.c | 89
> +++
> >  include/hw/riscv/riscv_hart.h | 39 +++
> >  2 files changed, 128 insertions(+)
> >  create mode 100644 hw/riscv/riscv_hart.c
> >  create mode 100644 include/hw/riscv/riscv_hart.h
> >
> > diff --git a/hw/riscv/riscv_hart.c b/hw/riscv/riscv_hart.c
> > new file mode 100644
> > index 000..14e3c18
> > --- /dev/null
> > +++ b/hw/riscv/riscv_hart.c
> > @@ -0,0 +1,89 @@
> > +/*
> > + * QEMU RISCV Hart Array
> > + *
> > + * Copyright (c) 2017 SiFive, Inc.
> > + *
> > + * Holds the state of a heterogenous array of RISC-V 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,
> > + * version 2 or later, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope it will be useful, but
> WITHOUT
> > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
> License for
> > + * more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> along with
> > + * this program.  If not, see .
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "qapi/error.h"
> > +#include "hw/sysbus.h"
> > +#include "target/riscv/cpu.h"
> > +#include "hw/riscv/riscv_hart.h"
> > +
> > +static Property riscv_harts_props[] = {
> > +DEFINE_PROP_UINT32("num-harts", RISCVHartArrayState, num_harts, 1),
> > +DEFINE_PROP_STRING("cpu-type", RISCVHartArrayState, cpu_type),
> > +DEFINE_PROP_END_OF_LIST(),
> > +};
> > +
> > +static void riscv_harts_cpu_reset(void *opaque)
> > +{
> > +RISCVCPU *cpu = opaque;
> > +cpu_reset(CPU(cpu));
> > +}
> > +
> > +static void riscv_harts_realize(DeviceState *dev, Error **errp)
> > +{
> > +RISCVHartArrayState *s = RISCV_HART_ARRAY(dev);
> > +Error *err = NULL;
> > +int n;
> > +
> > +s->harts = g_new0(RISCVCPU, s->num_harts);
> > +
> > +for (n = 0; n < s->num_harts; n++) {
> > +
> > +object_initialize(>harts[n], sizeof(RISCVCPU), s->cpu_type);
> > +s->harts[n].env.mhartid = n;
> > +object_property_add_child(OBJECT(s), "harts[*]",
> OBJECT(>harts[n]),
> > +  _abort);
> > +qemu_register_reset(riscv_harts_cpu_reset, >harts[n]);
> > +object_property_set_bool(OBJECT(>harts[n]), true,
> > + "realized", );
> > +if (err) {
> > +error_propagate(errp, err);
> > +return;
> > +}
> > +}
> > +}
> > +
> > +static void riscv_harts_class_init(ObjectClass *klass, void *data)
> > +{
> > +DeviceClass *dc = DEVICE_CLASS(klass);
> > +
> > +dc->props = riscv_harts_props;
> > +dc->realize = riscv_harts_realize;
> > +}
> > +
> > +static void riscv_harts_init(Object *obj)
> > +{
> > +/* RISCVHartArrayState *s = SIFIVE_COREPLEX(obj); */
>
> This seems an old comment, now this would be RISCV_HART_ARRAY(obj).
>
> Maybe better remove this riscv_harts_init(), ...
>
> > +}
> > +
> > +static const TypeInfo riscv_harts_info = {
> > +.name  = TYPE_RISCV_HART_ARRAY,
> > +.parent= TYPE_SYS_BUS_DEVICE,
> > +.instance_size = sizeof(RISCVHartArrayState),
> > +.instance_init = riscv_harts_init,
>
> ... and drop this line?
>
> > +.class_init= riscv_harts_class_init,
> > +};
> > +
> > +static void riscv_harts_register_types(void)
> > +{
> > +type_register_static(_harts_info);
> > +}
> > +
> > +type_init(riscv_harts_register_types)
> > diff --git a/include/hw/riscv/riscv_hart.h
> b/include/hw/riscv/riscv_hart.h
> > new file mode 100644
> > index 000..0671d88
> > --- /dev/null
> > +++ b/include/hw/riscv/riscv_hart.h
> > @@ -0,0 +1,39 @@
> > +/*
> > + * QEMU RISC-V Hart 

Re: [Qemu-devel] [PATCH v8 13/23] RISC-V HART Array

[Philippe Mathieu-Daudé]

On 03/02/2018 02:51 PM, Michael Clark wrote:
> Holds the state of a heterogenous array of RISC-V hardware threads.

heterogeneous

> 
> Reviewed-by: Richard Henderson 
> Signed-off-by: Sagar Karandikar 
> Signed-off-by: Michael Clark 
> ---
>  hw/riscv/riscv_hart.c | 89 
> +++
>  include/hw/riscv/riscv_hart.h | 39 +++
>  2 files changed, 128 insertions(+)
>  create mode 100644 hw/riscv/riscv_hart.c
>  create mode 100644 include/hw/riscv/riscv_hart.h
> 
> diff --git a/hw/riscv/riscv_hart.c b/hw/riscv/riscv_hart.c
> new file mode 100644
> index 000..14e3c18
> --- /dev/null
> +++ b/hw/riscv/riscv_hart.c
> @@ -0,0 +1,89 @@
> +/*
> + * QEMU RISCV Hart Array
> + *
> + * Copyright (c) 2017 SiFive, Inc.
> + *
> + * Holds the state of a heterogenous array of RISC-V 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,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along 
> with
> + * this program.  If not, see .
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "hw/sysbus.h"
> +#include "target/riscv/cpu.h"
> +#include "hw/riscv/riscv_hart.h"
> +
> +static Property riscv_harts_props[] = {
> +DEFINE_PROP_UINT32("num-harts", RISCVHartArrayState, num_harts, 1),
> +DEFINE_PROP_STRING("cpu-type", RISCVHartArrayState, cpu_type),
> +DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void riscv_harts_cpu_reset(void *opaque)
> +{
> +RISCVCPU *cpu = opaque;
> +cpu_reset(CPU(cpu));
> +}
> +
> +static void riscv_harts_realize(DeviceState *dev, Error **errp)
> +{
> +RISCVHartArrayState *s = RISCV_HART_ARRAY(dev);
> +Error *err = NULL;
> +int n;
> +
> +s->harts = g_new0(RISCVCPU, s->num_harts);
> +
> +for (n = 0; n < s->num_harts; n++) {
> +
> +object_initialize(>harts[n], sizeof(RISCVCPU), s->cpu_type);
> +s->harts[n].env.mhartid = n;
> +object_property_add_child(OBJECT(s), "harts[*]", 
> OBJECT(>harts[n]),
> +  _abort);
> +qemu_register_reset(riscv_harts_cpu_reset, >harts[n]);
> +object_property_set_bool(OBJECT(>harts[n]), true,
> + "realized", );
> +if (err) {
> +error_propagate(errp, err);
> +return;
> +}
> +}
> +}
> +
> +static void riscv_harts_class_init(ObjectClass *klass, void *data)
> +{
> +DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +dc->props = riscv_harts_props;
> +dc->realize = riscv_harts_realize;
> +}
> +
> +static void riscv_harts_init(Object *obj)
> +{
> +/* RISCVHartArrayState *s = SIFIVE_COREPLEX(obj); */

This seems an old comment, now this would be RISCV_HART_ARRAY(obj).

Maybe better remove this riscv_harts_init(), ...

> +}
> +
> +static const TypeInfo riscv_harts_info = {
> +.name  = TYPE_RISCV_HART_ARRAY,
> +.parent= TYPE_SYS_BUS_DEVICE,
> +.instance_size = sizeof(RISCVHartArrayState),
> +.instance_init = riscv_harts_init,

... and drop this line?

> +.class_init= riscv_harts_class_init,
> +};
> +
> +static void riscv_harts_register_types(void)
> +{
> +type_register_static(_harts_info);
> +}
> +
> +type_init(riscv_harts_register_types)
> diff --git a/include/hw/riscv/riscv_hart.h b/include/hw/riscv/riscv_hart.h
> new file mode 100644
> index 000..0671d88
> --- /dev/null
> +++ b/include/hw/riscv/riscv_hart.h
> @@ -0,0 +1,39 @@
> +/*
> + * QEMU RISC-V Hart Array interface
> + *
> + * Copyright (c) 2017 SiFive, Inc.
> + *
> + * Holds the state of a heterogenous array of RISC-V 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,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along 
> with
> + * this program.  If not, see .
> + */
> +
> +#ifndef HW_RISCV_HART_H
> +#define HW_RISCV_HART_H
> +
> +#define TYPE_RISCV_HART_ARRAY "riscv.hart_array"
> +
> +#define RISCV_HART_ARRAY(obj) \
> +

[Qemu-devel] [PATCH v8 13/23] RISC-V HART Array

[Michael Clark]

Holds the state of a heterogenous array of RISC-V hardware threads.

Reviewed-by: Richard Henderson 
Signed-off-by: Sagar Karandikar 
Signed-off-by: Michael Clark 
---
 hw/riscv/riscv_hart.c | 89 +++
 include/hw/riscv/riscv_hart.h | 39 +++
 2 files changed, 128 insertions(+)
 create mode 100644 hw/riscv/riscv_hart.c
 create mode 100644 include/hw/riscv/riscv_hart.h

diff --git a/hw/riscv/riscv_hart.c b/hw/riscv/riscv_hart.c
new file mode 100644
index 000..14e3c18
--- /dev/null
+++ b/hw/riscv/riscv_hart.c
@@ -0,0 +1,89 @@
+/*
+ * QEMU RISCV Hart Array
+ *
+ * Copyright (c) 2017 SiFive, Inc.
+ *
+ * Holds the state of a heterogenous array of RISC-V 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,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see .
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "hw/sysbus.h"
+#include "target/riscv/cpu.h"
+#include "hw/riscv/riscv_hart.h"
+
+static Property riscv_harts_props[] = {
+DEFINE_PROP_UINT32("num-harts", RISCVHartArrayState, num_harts, 1),
+DEFINE_PROP_STRING("cpu-type", RISCVHartArrayState, cpu_type),
+DEFINE_PROP_END_OF_LIST(),
+};
+
+static void riscv_harts_cpu_reset(void *opaque)
+{
+RISCVCPU *cpu = opaque;
+cpu_reset(CPU(cpu));
+}
+
+static void riscv_harts_realize(DeviceState *dev, Error **errp)
+{
+RISCVHartArrayState *s = RISCV_HART_ARRAY(dev);
+Error *err = NULL;
+int n;
+
+s->harts = g_new0(RISCVCPU, s->num_harts);
+
+for (n = 0; n < s->num_harts; n++) {
+
+object_initialize(>harts[n], sizeof(RISCVCPU), s->cpu_type);
+s->harts[n].env.mhartid = n;
+object_property_add_child(OBJECT(s), "harts[*]", OBJECT(>harts[n]),
+  _abort);
+qemu_register_reset(riscv_harts_cpu_reset, >harts[n]);
+object_property_set_bool(OBJECT(>harts[n]), true,
+ "realized", );
+if (err) {
+error_propagate(errp, err);
+return;
+}
+}
+}
+
+static void riscv_harts_class_init(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+
+dc->props = riscv_harts_props;
+dc->realize = riscv_harts_realize;
+}
+
+static void riscv_harts_init(Object *obj)
+{
+/* RISCVHartArrayState *s = SIFIVE_COREPLEX(obj); */
+}
+
+static const TypeInfo riscv_harts_info = {
+.name  = TYPE_RISCV_HART_ARRAY,
+.parent= TYPE_SYS_BUS_DEVICE,
+.instance_size = sizeof(RISCVHartArrayState),
+.instance_init = riscv_harts_init,
+.class_init= riscv_harts_class_init,
+};
+
+static void riscv_harts_register_types(void)
+{
+type_register_static(_harts_info);
+}
+
+type_init(riscv_harts_register_types)
diff --git a/include/hw/riscv/riscv_hart.h b/include/hw/riscv/riscv_hart.h
new file mode 100644
index 000..0671d88
--- /dev/null
+++ b/include/hw/riscv/riscv_hart.h
@@ -0,0 +1,39 @@
+/*
+ * QEMU RISC-V Hart Array interface
+ *
+ * Copyright (c) 2017 SiFive, Inc.
+ *
+ * Holds the state of a heterogenous array of RISC-V 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,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see .
+ */
+
+#ifndef HW_RISCV_HART_H
+#define HW_RISCV_HART_H
+
+#define TYPE_RISCV_HART_ARRAY "riscv.hart_array"
+
+#define RISCV_HART_ARRAY(obj) \
+OBJECT_CHECK(RISCVHartArrayState, (obj), TYPE_RISCV_HART_ARRAY)
+
+typedef struct RISCVHartArrayState {
+/*< private >*/
+SysBusDevice parent_obj;
+
+/*< public >*/
+uint32_t num_harts;
+char *cpu_type;
+RISCVCPU *harts;
+} RISCVHartArrayState;
+
+#endif
-- 
2.7.0