Re: [Qemu-devel] [PATCH v1 3/5] xlnx-zcu102: Specify the valid CPUs

[Eduardo Habkost]

On Wed, Oct 04, 2017 at 01:12:32PM +0200, Igor Mammedov wrote:
> On Tue, 3 Oct 2017 14:41:17 -0700
> Alistair Francis <address@hidden> wrote:
> 
> > On Tue, Oct 3, 2017 at 1:36 PM, Eduardo Habkost <address@hidden> wrote:
> > > On Tue, Oct 03, 2017 at 01:05:13PM -0700, Alistair Francis wrote:  
> > >> List all possible valid CPU options.
> > >>
> > >> Signed-off-by: Alistair Francis <address@hidden>
> > >> ---
> > >>
> > >>  hw/arm/xlnx-zcu102.c         | 10 ++++++++++
> > >>  hw/arm/xlnx-zynqmp.c         | 16 +++++++++-------
> > >>  include/hw/arm/xlnx-zynqmp.h |  1 +
> > >>  3 files changed, 20 insertions(+), 7 deletions(-)
> > >>
> > >> diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c
> > >> index 519a16ed98..039649e522 100644
> > >> --- a/hw/arm/xlnx-zcu102.c
> > >> +++ b/hw/arm/xlnx-zcu102.c
> > >> @@ -98,6 +98,8 @@ static void xlnx_zynqmp_init(XlnxZCU102 *s, 
> > >> MachineState *machine)
> > >>      object_property_add_child(OBJECT(machine), "soc", OBJECT(&s->soc),
> > >>                                &error_abort);
> > >>
> > >> +    object_property_set_str(OBJECT(&s->soc), machine->cpu_type, 
> > >> "cpu-type",
> > >> +                            &error_fatal);  
> > >
> > > Do you have plans to support other CPU types to xlnx_zynqmp in
> > > the future?  If not, I wouldn't bother adding the cpu-type
> > > property and the extra boilerplate code if it's always going to
> > > be set to cortex-a53.  
> > 
> > No, it'll always be A53.
> > 
> > I did think of that, but I also wanted to use the new option! I also
> > think there is an advantage in sanely handling users '-cpu' option,
> > before now we just ignored it, so I think it still does give a
> > benefit. That'll be especially important on the Xilinx tree (sometimes
> > people use our machines with a different CPU to 'benchmark' or test
> > other CPUs with our CoSimulation setup). So I think it does make sense
> > to keep in.
> if cpu isn't user settable, one could just outright die if cpu_type
> is not NULL and say that user's CLI is wrong.
> (i.e. don't give users illusion that they allowed to use '-cpu')

Isn't it exactly what this patch does, by setting:
    mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a53");
    mc->valid_cpu_types = xlnx_zynqmp_valid_cpus;
?

Except that "-cpu cortex-a53" won't die, which is a good thing.

-- 
Eduardo