Re: [Qemu-devel] [PATCH 3/7 v8] target-i386: Move APIC to ICC bus

[Igor Mammedov]

On Mon, 29 Apr 2013 19:15:56 +0200
Andreas Färber <address@hidden> wrote:

> Am 29.04.2013 19:03, schrieb Igor Mammedov:
> > It allows APIC to be hotplugged.
> > 
> >  * map APIC's mmio at board level if it is present
> >  * do not register mmio region for each APIC, since
> >    only one is used/mapped
> > 
> > Signed-off-by: Igor Mammedov <address@hidden>
> > Signed-off-by: Andreas Färber <address@hidden>
> > ---
> > v3:
> >   - fix compile error caused by mismerged hunk
> > v2:
> >   - use icc-bridge from args instead of resolving it
> 
> Thanks, applied to qom-cpu:
> https://github.com/afaerber/qemu-cpu/commits/qom-cpu
> 
> One question though...
> 
> [...]
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index 658ff6c..6b3faac 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> [...]
> > @@ -929,14 +931,21 @@ void pc_cpus_init(const char *cpu_model, DeviceState 
> > *icc_bridge)
> >      }
> >  
> >      for (i = 0; i < smp_cpus; i++) {
> > -        pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i),
> > -                   icc_bridge, &error);
> > +        cpu = pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i),
> > +                         icc_bridge, &error);
> >          if (error) {
> >              fprintf(stderr, "%s\n", error_get_pretty(error));
> >              error_free(error);
> >              exit(1);
> >          }
> >      }
> > +
> > +    /* map APIC MMIO area if CPU has APIC */
> > +    if (cpu && cpu->env.apic_state) {
> 
> Might sysbus_mmio_get_region(SYS_BUS_DEVICE(icc_bridge), 0) != NULL be a
> more straightforward guard? We're not accessing apic_state at all. :)
it won't be NULL, it's icc_bridge's region that always exists.

> 
> Andreas
> 
> > +        /* XXX: what if the base changes? */
> > +        sysbus_mmio_map_overlap(SYS_BUS_DEVICE(icc_bridge), 0,
> > +                                APIC_DEFAULT_ADDRESS, 0x1000);
> > +    }
> >  }
> >  
> >  void pc_acpi_init(const char *default_dsdt)
> [snip]
> 
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg


-- 
Regards,
  Igor