Message ID | 7d590a446dfe29a1815c3cad2a93f7d6b0d8a214.1508279421.git.alistair.francis@xilinx.com |
---|---|
State | New |
Headers | show |
Series | Add a valid_cpu_types property | expand |
On Tue, Oct 17, 2017 at 03:31:19PM -0700, Alistair Francis wrote: > List all possible valid CPU options. > > Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> > --- > > An implementation for single CPU machines is still being discussed. A > solution proposed by Eduardo is this: > > 1) Change the default on TYPE_MACHINE to: > mc->valid_cpu_types = { TYPE_CPU, NULL }; > > This will keep the existing behavior for all boards. > > 2) mc->valid_cpu_types=NULL be interpreted as "no CPU model > except the default is accepted" or "-cpu is not accepted" in > machine_run_board_init() (I prefer the former, but both > options would be correct) > > 3) Boards like xlnx_zynqmp could then just do this: > > static void xxx_class_init(...) { > mc->default_cpu_type = MY_CPU_TYPE; > /* Reason: XXX_init() is hardcoded to MY_CPU_TYPE */ > mc->valid_cpu_types = NULL; > } > > V3: > - Make variable static > V2: > - Don't use the users -cpu > - Fixup allignment > > hw/arm/xlnx-zcu102.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c > index 519a16ed98..d5a5425356 100644 > --- a/hw/arm/xlnx-zcu102.c > +++ b/hw/arm/xlnx-zcu102.c > @@ -160,6 +160,11 @@ static void xlnx_zynqmp_init(XlnxZCU102 *s, MachineState *machine) > arm_load_kernel(s->soc.boot_cpu_ptr, &xlnx_zcu102_binfo); > } > > +static const char *xlnx_zynqmp_valid_cpus[] = { > + ARM_CPU_TYPE_NAME("cortex-a53"), > + NULL > + }; > + > static void xlnx_ep108_init(MachineState *machine) > { > XlnxZCU102 *s = EP108_MACHINE(machine); > @@ -185,6 +190,12 @@ static void xlnx_ep108_machine_class_init(ObjectClass *oc, void *data) > mc->block_default_type = IF_IDE; > mc->units_per_default_bus = 1; > mc->ignore_memory_transaction_failures = true; > + mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a53"); > + /* The ZynqMP SoC is always a Cortex-A53. We add this here to give > + * users a sane error if they specify a different CPU, but we never > + * use their CPU choice. > + */ > + mc->valid_cpu_types = xlnx_zynqmp_valid_cpus; > } > > static const TypeInfo xlnx_ep108_machine_init_typeinfo = { > @@ -240,6 +251,12 @@ static void xlnx_zcu102_machine_class_init(ObjectClass *oc, void *data) > mc->block_default_type = IF_IDE; > mc->units_per_default_bus = 1; > mc->ignore_memory_transaction_failures = true; > + mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a53"); > + /* The ZynqMP SoC is always a Cortex-A53. We add this here to give > + * users a sane error if they specify a different CPU, but we never > + * use their CPU choice. > + */ > + mc->valid_cpu_types = xlnx_zynqmp_valid_cpus; > } > > static const TypeInfo xlnx_zcu102_machine_init_typeinfo = { > -- > 2.11.0 >
On 10/17/2017 07:31 PM, Alistair Francis wrote: > List all possible valid CPU options. > > Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> > --- > > An implementation for single CPU machines is still being discussed. A > solution proposed by Eduardo is this: > > 1) Change the default on TYPE_MACHINE to: > mc->valid_cpu_types = { TYPE_CPU, NULL }; > > This will keep the existing behavior for all boards. > > 2) mc->valid_cpu_types=NULL be interpreted as "no CPU model > except the default is accepted" or "-cpu is not accepted" in > machine_run_board_init() (I prefer the former, but both > options would be correct) > > 3) Boards like xlnx_zynqmp could then just do this: > > static void xxx_class_init(...) { > mc->default_cpu_type = MY_CPU_TYPE; > /* Reason: XXX_init() is hardcoded to MY_CPU_TYPE */ > mc->valid_cpu_types = NULL; > } > > V3: > - Make variable static > V2: > - Don't use the users -cpu > - Fixup allignment > > hw/arm/xlnx-zcu102.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c > index 519a16ed98..d5a5425356 100644 > --- a/hw/arm/xlnx-zcu102.c > +++ b/hw/arm/xlnx-zcu102.c > @@ -160,6 +160,11 @@ static void xlnx_zynqmp_init(XlnxZCU102 *s, MachineState *machine) > arm_load_kernel(s->soc.boot_cpu_ptr, &xlnx_zcu102_binfo); > } > > +static const char *xlnx_zynqmp_valid_cpus[] = { > + ARM_CPU_TYPE_NAME("cortex-a53"), > + NULL > + }; Why so many spaces? :) Maybe Peter can clean it when applying. static const char *xlnx_zynqmp_valid_cpus[] = { ARM_CPU_TYPE_NAME("cortex-a53"), NULL }; Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > + > static void xlnx_ep108_init(MachineState *machine) > { > XlnxZCU102 *s = EP108_MACHINE(machine); > @@ -185,6 +190,12 @@ static void xlnx_ep108_machine_class_init(ObjectClass *oc, void *data) > mc->block_default_type = IF_IDE; > mc->units_per_default_bus = 1; > mc->ignore_memory_transaction_failures = true; > + mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a53"); > + /* The ZynqMP SoC is always a Cortex-A53. We add this here to give > + * users a sane error if they specify a different CPU, but we never > + * use their CPU choice. > + */ > + mc->valid_cpu_types = xlnx_zynqmp_valid_cpus; > } > > static const TypeInfo xlnx_ep108_machine_init_typeinfo = { > @@ -240,6 +251,12 @@ static void xlnx_zcu102_machine_class_init(ObjectClass *oc, void *data) > mc->block_default_type = IF_IDE; > mc->units_per_default_bus = 1; > mc->ignore_memory_transaction_failures = true; > + mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a53"); > + /* The ZynqMP SoC is always a Cortex-A53. We add this here to give > + * users a sane error if they specify a different CPU, but we never > + * use their CPU choice. > + */ > + mc->valid_cpu_types = xlnx_zynqmp_valid_cpus; > } > > static const TypeInfo xlnx_zcu102_machine_init_typeinfo = { >
On Mon, Oct 23, 2017 at 1:14 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > On 10/17/2017 07:31 PM, Alistair Francis wrote: >> List all possible valid CPU options. >> >> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> >> --- >> >> An implementation for single CPU machines is still being discussed. A >> solution proposed by Eduardo is this: >> >> 1) Change the default on TYPE_MACHINE to: >> mc->valid_cpu_types = { TYPE_CPU, NULL }; >> >> This will keep the existing behavior for all boards. >> >> 2) mc->valid_cpu_types=NULL be interpreted as "no CPU model >> except the default is accepted" or "-cpu is not accepted" in >> machine_run_board_init() (I prefer the former, but both >> options would be correct) >> >> 3) Boards like xlnx_zynqmp could then just do this: >> >> static void xxx_class_init(...) { >> mc->default_cpu_type = MY_CPU_TYPE; >> /* Reason: XXX_init() is hardcoded to MY_CPU_TYPE */ >> mc->valid_cpu_types = NULL; >> } >> >> V3: >> - Make variable static >> V2: >> - Don't use the users -cpu >> - Fixup allignment >> >> hw/arm/xlnx-zcu102.c | 17 +++++++++++++++++ >> 1 file changed, 17 insertions(+) >> >> diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c >> index 519a16ed98..d5a5425356 100644 >> --- a/hw/arm/xlnx-zcu102.c >> +++ b/hw/arm/xlnx-zcu102.c >> @@ -160,6 +160,11 @@ static void xlnx_zynqmp_init(XlnxZCU102 *s, MachineState *machine) >> arm_load_kernel(s->soc.boot_cpu_ptr, &xlnx_zcu102_binfo); >> } >> >> +static const char *xlnx_zynqmp_valid_cpus[] = { >> + ARM_CPU_TYPE_NAME("cortex-a53"), >> + NULL >> + }; > > Why so many spaces? :) Maybe Peter can clean it when applying. > > static const char *xlnx_zynqmp_valid_cpus[] = { > ARM_CPU_TYPE_NAME("cortex-a53"), > NULL > }; The spaces are there to keep the elements under the array (while being less then 80 characters). Alistair > > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > >> + >> static void xlnx_ep108_init(MachineState *machine) >> { >> XlnxZCU102 *s = EP108_MACHINE(machine); >> @@ -185,6 +190,12 @@ static void xlnx_ep108_machine_class_init(ObjectClass *oc, void *data) >> mc->block_default_type = IF_IDE; >> mc->units_per_default_bus = 1; >> mc->ignore_memory_transaction_failures = true; >> + mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a53"); >> + /* The ZynqMP SoC is always a Cortex-A53. We add this here to give >> + * users a sane error if they specify a different CPU, but we never >> + * use their CPU choice. >> + */ >> + mc->valid_cpu_types = xlnx_zynqmp_valid_cpus; >> } >> >> static const TypeInfo xlnx_ep108_machine_init_typeinfo = { >> @@ -240,6 +251,12 @@ static void xlnx_zcu102_machine_class_init(ObjectClass *oc, void *data) >> mc->block_default_type = IF_IDE; >> mc->units_per_default_bus = 1; >> mc->ignore_memory_transaction_failures = true; >> + mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a53"); >> + /* The ZynqMP SoC is always a Cortex-A53. We add this here to give >> + * users a sane error if they specify a different CPU, but we never >> + * use their CPU choice. >> + */ >> + mc->valid_cpu_types = xlnx_zynqmp_valid_cpus; >> } >> >> static const TypeInfo xlnx_zcu102_machine_init_typeinfo = { >>
On Thu, 26 Oct 2017 11:59:16 +0200 Alistair Francis <alistair.francis@xilinx.com> wrote: > On Mon, Oct 23, 2017 at 1:14 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > > On 10/17/2017 07:31 PM, Alistair Francis wrote: > >> List all possible valid CPU options. > >> > >> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> > >> --- > >> > >> An implementation for single CPU machines is still being discussed. A > >> solution proposed by Eduardo is this: > >> > >> 1) Change the default on TYPE_MACHINE to: > >> mc->valid_cpu_types = { TYPE_CPU, NULL }; > >> > >> This will keep the existing behavior for all boards. > >> > >> 2) mc->valid_cpu_types=NULL be interpreted as "no CPU model > >> except the default is accepted" or "-cpu is not accepted" in > >> machine_run_board_init() (I prefer the former, but both > >> options would be correct) > >> > >> 3) Boards like xlnx_zynqmp could then just do this: > >> > >> static void xxx_class_init(...) { > >> mc->default_cpu_type = MY_CPU_TYPE; > >> /* Reason: XXX_init() is hardcoded to MY_CPU_TYPE */ > >> mc->valid_cpu_types = NULL; > >> } > >> > >> V3: > >> - Make variable static > >> V2: > >> - Don't use the users -cpu > >> - Fixup allignment > >> > >> hw/arm/xlnx-zcu102.c | 17 +++++++++++++++++ > >> 1 file changed, 17 insertions(+) > >> > >> diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c > >> index 519a16ed98..d5a5425356 100644 > >> --- a/hw/arm/xlnx-zcu102.c > >> +++ b/hw/arm/xlnx-zcu102.c > >> @@ -160,6 +160,11 @@ static void xlnx_zynqmp_init(XlnxZCU102 *s, MachineState *machine) > >> arm_load_kernel(s->soc.boot_cpu_ptr, &xlnx_zcu102_binfo); > >> } > >> > >> +static const char *xlnx_zynqmp_valid_cpus[] = { > >> + ARM_CPU_TYPE_NAME("cortex-a53"), > >> + NULL > >> + }; > > > > Why so many spaces? :) Maybe Peter can clean it when applying. > > > > static const char *xlnx_zynqmp_valid_cpus[] = { > > ARM_CPU_TYPE_NAME("cortex-a53"), > > NULL > > }; +1 to what Philippe is suggesting, this is common pattern in QEMU code. > The spaces are there to keep the elements under the array (while being > less then 80 characters). it's not aligned on 4SP from the beginning or 4SP after {} so it's not exactly matching codding style and catches eye. Also in case one would need to add an extra cpu there with longer name beyond 80chr, one would need to move the rest elements to new alignment. the same applies for 4-5/5, I'd would repost fixed up series. > > Alistair > > > > > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > > > >> + > >> static void xlnx_ep108_init(MachineState *machine) > >> { > >> XlnxZCU102 *s = EP108_MACHINE(machine); > >> @@ -185,6 +190,12 @@ static void xlnx_ep108_machine_class_init(ObjectClass *oc, void *data) > >> mc->block_default_type = IF_IDE; > >> mc->units_per_default_bus = 1; > >> mc->ignore_memory_transaction_failures = true; > >> + mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a53"); > >> + /* The ZynqMP SoC is always a Cortex-A53. We add this here to give > >> + * users a sane error if they specify a different CPU, but we never > >> + * use their CPU choice. > >> + */ > >> + mc->valid_cpu_types = xlnx_zynqmp_valid_cpus; > >> } > >> > >> static const TypeInfo xlnx_ep108_machine_init_typeinfo = { > >> @@ -240,6 +251,12 @@ static void xlnx_zcu102_machine_class_init(ObjectClass *oc, void *data) > >> mc->block_default_type = IF_IDE; > >> mc->units_per_default_bus = 1; > >> mc->ignore_memory_transaction_failures = true; > >> + mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a53"); > >> + /* The ZynqMP SoC is always a Cortex-A53. We add this here to give > >> + * users a sane error if they specify a different CPU, but we never > >> + * use their CPU choice. > >> + */ > >> + mc->valid_cpu_types = xlnx_zynqmp_valid_cpus; > >> } > >> > >> static const TypeInfo xlnx_zcu102_machine_init_typeinfo = { > >>
diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c index 519a16ed98..d5a5425356 100644 --- a/hw/arm/xlnx-zcu102.c +++ b/hw/arm/xlnx-zcu102.c @@ -160,6 +160,11 @@ static void xlnx_zynqmp_init(XlnxZCU102 *s, MachineState *machine) arm_load_kernel(s->soc.boot_cpu_ptr, &xlnx_zcu102_binfo); } +static const char *xlnx_zynqmp_valid_cpus[] = { + ARM_CPU_TYPE_NAME("cortex-a53"), + NULL + }; + static void xlnx_ep108_init(MachineState *machine) { XlnxZCU102 *s = EP108_MACHINE(machine); @@ -185,6 +190,12 @@ static void xlnx_ep108_machine_class_init(ObjectClass *oc, void *data) mc->block_default_type = IF_IDE; mc->units_per_default_bus = 1; mc->ignore_memory_transaction_failures = true; + mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a53"); + /* The ZynqMP SoC is always a Cortex-A53. We add this here to give + * users a sane error if they specify a different CPU, but we never + * use their CPU choice. + */ + mc->valid_cpu_types = xlnx_zynqmp_valid_cpus; } static const TypeInfo xlnx_ep108_machine_init_typeinfo = { @@ -240,6 +251,12 @@ static void xlnx_zcu102_machine_class_init(ObjectClass *oc, void *data) mc->block_default_type = IF_IDE; mc->units_per_default_bus = 1; mc->ignore_memory_transaction_failures = true; + mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a53"); + /* The ZynqMP SoC is always a Cortex-A53. We add this here to give + * users a sane error if they specify a different CPU, but we never + * use their CPU choice. + */ + mc->valid_cpu_types = xlnx_zynqmp_valid_cpus; } static const TypeInfo xlnx_zcu102_machine_init_typeinfo = {
List all possible valid CPU options. Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> --- An implementation for single CPU machines is still being discussed. A solution proposed by Eduardo is this: 1) Change the default on TYPE_MACHINE to: mc->valid_cpu_types = { TYPE_CPU, NULL }; This will keep the existing behavior for all boards. 2) mc->valid_cpu_types=NULL be interpreted as "no CPU model except the default is accepted" or "-cpu is not accepted" in machine_run_board_init() (I prefer the former, but both options would be correct) 3) Boards like xlnx_zynqmp could then just do this: static void xxx_class_init(...) { mc->default_cpu_type = MY_CPU_TYPE; /* Reason: XXX_init() is hardcoded to MY_CPU_TYPE */ mc->valid_cpu_types = NULL; } V3: - Make variable static V2: - Don't use the users -cpu - Fixup allignment hw/arm/xlnx-zcu102.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)