Re: [Qemu-devel] [PATCH 07/12] target-i386: Add ICC_BUS and attach apic, kvmvapic and cpu to it

[Igor Mammedov <imammedo@redhat.com>]

On Fri, 29 Mar 2013 15:22:47 +0800
li guang <lig.fnst@cn.fujitsu.com> wrote:

> 在 2013-03-28四的 11:55 +0100，Igor Mammedov写道：
> > On Wed, 27 Mar 2013 11:57:53 +0100
> > Paolo Bonzini <pbonzini@redhat.com> wrote:
> > 
> > > Il 21/03/2013 15:28, Igor Mammedov ha scritto:  
> > > > Introduce hot-pluggable ICC_BUS and convert APIC devices from SysBusDevice
> > > > to ICCDevice wich has ICC_BUS as default one.
> > > > 
> > > >  * attach APIC and kvmvapic to ICC_BUS during creation
> > > >  * use memory API directly instead of using SysBus proxy functions
> > > >  * introduce get_icc_bus() for getting access to system wide ICC_BUS
> > > >  * make ICC_BUS default one for X86CPU class, so that device_add would
> > > >    set it for CPU instead of SysBus
> > > >  * attach cold-pluged CPUs to icc_bus in softmmu mode, so they could be
> > > >    uplugged in future
> > > >  * if kvmvapic init() fails return -1 instead of aborting.
> [...]
> > > > +};
> > > > +
> > > > +static BusState *icc_bus;
> > > > +
> > > > +BusState *get_icc_bus(void)
> > > > +{
> > > > +    if (icc_bus == NULL) {
> > > > +        icc_bus = g_malloc0(icc_bus_info.instance_size);
> > > > +        qbus_create_inplace(icc_bus, TYPE_ICC_BUS, NULL, "icc-bus");
> > > > +        icc_bus->allow_hotplug = 1;
> > > > +        OBJECT(icc_bus)->free = g_free;  
> > > 
> > > You can just use qbus_create.
> > >   
> > > > +        object_property_add_child(container_get(qdev_get_machine(),
> > > > +                                                "/unattached"),  
> > > 
> > > Please put it straight under /machine, not /unattached.  
> > 
> > sure
> > 
> > > 
> > > But actually, you lack a device that instantiates the bus.  That device
> > > would be a simple container device similar hw/a15mpcore.c, and would  
> > 
> > About a year ago something like that device was proposed "cpu_sockets", but
> 
> what about link<Socket> implementation which was mentioned by Andreas
> few days ago at a thread discussion for acpi memory hotplug?
> (http://list-archives.org/2012/12/18/qemu-devel-nongnu-org/rfc-patch-v4-00-30-acpi-memory-hotplug/f/4115902514)
> do consider to implement a link<> property between CPU and cpu_socket?
since CPU slowly moving towards being able to be added via device_add,
there is a limited usefulness in link<> property. It still could be used
to expose possible APIC IDs to user so that user won't have calculate
them by itself. But it requires fixing device_realize first so I haven't
included it in this series.

> 
> > there was suggestion to drop it:
> > http://lists.gnu.org/archive/html/qemu-devel/2012-02/msg02157.html
> > So I've opted in favor of parent-less bus, but I could create it
> > explicitly at board level as you suggest.
> > 
> > However having parent sysbus device for icc-bus will allow to see it via
> > monitor info qtree and reset on SysBus could be propagated through it as well,
> > it may be good to add it later.
> > 
> > > also take care of registering the MSI memory region.  Create that device  
> > 
> > it probably impossible to do, because [kvm]apic sets its private callbacks
> > to it when initializing memory region, and CPU just maps it at specified
> > address, or this region and APIC might not even exists at all if qemu started
> > with -apic feature flag.
> > 
> > > in the boards, and you won't need a special object_property_add_child.
> > > 
> > > get_icc_bus() would then become simply
> > > object_resolve_path_type("icc-bus", TYPE_ICC_BUS, NULL) or something
> > > like that.
> > > 
> > > Paolo
> > >   
> > > > +                                  "icc-bus", OBJECT(icc_bus), NULL);
> > > > +    }
> > > > +    return BUS(icc_bus);
> > > > +}
> > > > +
> 
> 
> 


-- 
Regards,
  Igor