Re: [Qemu-devel] [PATCH v8 03/23] RISC-V CPU Core Definition

[Michael Clark]

On Mon, Mar 5, 2018 at 10:44 PM, Igor Mammedov <address@hidden> wrote:

> On Sat,  3 Mar 2018 02:51:31 +1300
> Michael Clark <address@hidden> wrote:
>
> > Add CPU state header, CPU definitions and initialization routines
> >
> > Reviewed-by: Richard Henderson <address@hidden>
> > Signed-off-by: Sagar Karandikar <address@hidden>
> > Signed-off-by: Michael Clark <address@hidden>
> > ---
> >  target/riscv/cpu.c      | 432 ++++++++++++++++++++++++++++++
> ++++++++++++++++++
> >  target/riscv/cpu.h      | 296 +++++++++++++++++++++++++++++++++
> >  target/riscv/cpu_bits.h | 411 ++++++++++++++++++++++++++++++
> +++++++++++++++
> >  3 files changed, 1139 insertions(+)
> >  create mode 100644 target/riscv/cpu.c
> >  create mode 100644 target/riscv/cpu.h
> >  create mode 100644 target/riscv/cpu_bits.h
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > new file mode 100644
> > index 0000000..4851890
> > --- /dev/null
> > +++ b/target/riscv/cpu.c
> [...]
>
> > +
> > +typedef struct RISCVCPUInfo {
> > +    const int bit_widths;
> > +    const char *name;
> > +    void (*initfn)(Object *obj);
> > +} RISCVCPUInfo;
> > +
> [...]
>
> > +static const RISCVCPUInfo riscv_cpus[] = {
> > +    { 96, TYPE_RISCV_CPU_ANY,              riscv_any_cpu_init },
> > +    { 32, TYPE_RISCV_CPU_RV32GCSU_V1_09_1,
> rv32gcsu_priv1_09_1_cpu_init },
> > +    { 32, TYPE_RISCV_CPU_RV32GCSU_V1_10_0,
> rv32gcsu_priv1_10_0_cpu_init },
> > +    { 32, TYPE_RISCV_CPU_RV32IMACU_NOMMU,  rv32imacu_nommu_cpu_init },
> > +    { 32, TYPE_RISCV_CPU_SIFIVE_E31,       rv32imacu_nommu_cpu_init },
> > +    { 32, TYPE_RISCV_CPU_SIFIVE_U34,       rv32gcsu_priv1_10_0_cpu_init
> },
> > +    { 64, TYPE_RISCV_CPU_RV64GCSU_V1_09_1,
> rv64gcsu_priv1_09_1_cpu_init },
> > +    { 64, TYPE_RISCV_CPU_RV64GCSU_V1_10_0,
> rv64gcsu_priv1_10_0_cpu_init },
> > +    { 64, TYPE_RISCV_CPU_RV64IMACU_NOMMU,  rv64imacu_nommu_cpu_init },
> > +    { 64, TYPE_RISCV_CPU_SIFIVE_E51,       rv64imacu_nommu_cpu_init },
> > +    { 64, TYPE_RISCV_CPU_SIFIVE_U54,       rv64gcsu_priv1_10_0_cpu_init
> },
> > +    { 0, NULL, NULL }
> > +};
> > +
> [...]
>
> > +static void cpu_register(const RISCVCPUInfo *info)
> > +{
> > +    TypeInfo type_info = {
> > +        .name = info->name,
> > +        .parent = TYPE_RISCV_CPU,
> > +        .instance_size = sizeof(RISCVCPU),
> > +        .instance_init = info->initfn,
> > +    };
> > +
> > +    type_register(&type_info);
> > +}
> [...]
>
> > +void riscv_cpu_list(FILE *f, fprintf_function cpu_fprintf)
> > +{
> > +    const RISCVCPUInfo *info = riscv_cpus;
> > +
> > +    while (info->name) {
> > +        if (info->bit_widths & TARGET_LONG_BITS) {
> > +            (*cpu_fprintf)(f, "%s\n", info->name);
> > +        }
> > +        info++;
> > +    }
> > +}
> > +
> > +static void riscv_cpu_register_types(void)
> > +{
> > +    const RISCVCPUInfo *info = riscv_cpus;
> > +
> > +    type_register_static(&riscv_cpu_type_info);
> > +
> > +    while (info->name) {
> > +        if (info->bit_widths & TARGET_LONG_BITS) {
> > +            cpu_register(info);
> > +        }
> > +        info++;
> > +    }
> > +}
> > +
> > +type_init(riscv_cpu_register_types)
> This still isn't fixed as requested
>  http://lists.gnu.org/archive/html/qemu-devel/2018-02/msg06412.html


It's possibly because I explicitly requested a clarification. Pointing at a
commit and being asked to infer what the desired change is, is not what I
would call reasonable feedback. The code has already been reviewed. We have
just expanded on it in a manner consistent with how the ARM port handled
cpu initialization.

I'm happy to comply if you give me detailed instructions on what is wrong,
why, and how to fix it versus infer your problem from this commit to
another architecture.

Apologies if i'm a bit slow, but I really don't understand the change you
intend us to make.