qemu-arm
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-arm] [Qemu-devel] [PATCH 4/4] hw/arm/virt: Use new cpu_generic


From: Eduardo Habkost
Subject: Re: [Qemu-arm] [Qemu-devel] [PATCH 4/4] hw/arm/virt: Use new cpu_generic_new()
Date: Tue, 21 Feb 2017 13:09:12 -0300
User-agent: Mutt/1.7.1 (2016-10-04)

On Mon, Feb 20, 2017 at 08:10:29PM +0100, Igor Mammedov wrote:
> On Mon, 13 Feb 2017 14:28:19 +0000
> Peter Maydell <address@hidden> wrote:
> 
> > Use the new cpu_generic_new() rather than calling
> > parse_features by hand.
> > 
> > Signed-off-by: Peter Maydell <address@hidden>
> > ---
> >  hw/arm/virt.c | 24 ++----------------------
> >  1 file changed, 2 insertions(+), 22 deletions(-)
> > 
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index f3440f2..bcb9a6d 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -172,7 +172,6 @@ static const char *valid_cpus[] = {
> >  static bool cpuname_valid(const char *cpu)
> >  {
> >      int i;
> > -
> >      for (i = 0; i < ARRAY_SIZE(valid_cpus); i++) {
> >          if (strcmp(cpu, valid_cpus[i]) == 0) {
> >              return true;
> > @@ -1206,10 +1205,6 @@ static void machvirt_init(MachineState *machine)
> >      MemoryRegion *ram = g_new(MemoryRegion, 1);
> >      const char *cpu_model = machine->cpu_model;
> >      char **cpustr;
> > -    ObjectClass *oc;
> > -    const char *typename;
> > -    CPUClass *cc;
> > -    Error *err = NULL;
> >      bool firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0);
> >      uint8_t clustersz;
> >  
> > @@ -1240,6 +1235,7 @@ static void machvirt_init(MachineState *machine)
> >          error_report("mach-virt: CPU %s not supported", cpustr[0]);
> >          exit(1);
> >      }
> > +    g_strfreev(cpustr);
> >  
> >      /* If we have an EL3 boot ROM then the assumption is that it will
> >       * implement PSCI itself, so disable QEMU's internal implementation
> > @@ -1309,24 +1305,8 @@ static void machvirt_init(MachineState *machine)
> >  
> >      create_fdt(vms);
> >  
> > -    oc = cpu_class_by_name(TYPE_ARM_CPU, cpustr[0]);
> > -    if (!oc) {
> > -        error_report("Unable to find CPU definition");
> > -        exit(1);
> > -    }
> > -    typename = object_class_get_name(oc);
> > -
> > -    /* convert -smp CPU options specified by the user into global props */
> > -    cc = CPU_CLASS(oc);
> > -    cc->parse_features(typename, cpustr[1], &err);
> > -    g_strfreev(cpustr);
> > -    if (err) {
> > -        error_report_err(err);
> > -        exit(1);
> > -    }
> > -
> >      for (n = 0; n < smp_cpus; n++) {
> > -        Object *cpuobj = object_new(typename);
> > +        Object *cpuobj = OBJECT(cpu_generic_new(TYPE_ARM_CPU, cpu_model));
> It unnecessarily pushes cpu_model parsing farther down the call stack
> when all we need for CPU creation is type name.

I don't see a problem in pushing it down the call stack.  It is
just replacing logic that is duplicated on lots of boards to a
wrapper. When we refactor the parsing logic, it will be easier to
do it if parsing is inside a single wrapper insted of being
duplicated on multiple boards.

> 
> Since I'm looking at adding '-device cpu' support to virt-arm,
> I would prefer to leave object_new() here so that creation logic
> would be similar to device_add and not mix '-cpu' parsing to typename
> + global properties that could be moved to generic machine code
> with actual CPU creation.

We can still move parsing to generic machine code, with the new
wrapper. While we don't do that, we can have a helper that makes
the current API easier to use. Once we move parsing to generic
machine code, we can easily replace cpu_generic_new() with
object_new().

> 
> 
> >          if (!vmc->disallow_affinity_adjustment) {
> >              /* Adjust MPIDR like 64-bit KVM hosts, which incorporate the
> >               * GIC's target-list limitations. 32-bit KVM hosts currently
> 

-- 
Eduardo



reply via email to

[Prev in Thread] Current Thread [Next in Thread]