[Question] Regarding containers "unattached/peripheral/anonymous" - their relation with hot(un)plug of devices

[Question] Regarding containers "unattached/peripheral/anonymous" - their relation with hot(un)plug of devices

[Salil Mehta]

Hello,
I am working on vCPU Hotplug feature for ARM64 and I am in mid of understanding some aspect of device_add/device_del interface of the QEMU.

Observations:
1. Any object initialised by qmp_device_add() gets into /machine/unattached container. I traced the flow to code leg inside  device_set_realized()
2. I could see the reverse qmp_device_del() expects the device to be in  /machine/peripheral container.
3. I could see any object initially added to unattached container did not had their parents until object_add_property_child() was called further in the leg.
    which effectively meant a new property was created and property table populated and child was parented.
4. Generally, container  /machine/peripheral was being used wherever DEVICE(dev)->id was present and non-null.

Question:
1. Wanted to confirm my understanding about the use of having separate containers like unattached, peripheral and anonymous.
2. At init time all the vcpus goes under *unattached* container. Now, qmp_device_del() cannot be used to unplug them. I am wondering
   if all the hotplug devices need to go under the *peripheral* container while they are hotplugged and during object init time as well?
3. I could not see any device being place under *anonymous* container during init time. What is the use of this container?

I would be thankful for your valuable insights and answers and help in highlighting any gap in my understanding.

Thanks in anticipation!

Best Regards
Salil

Re: [Question] Regarding containers "unattached/peripheral/anonymous" - their relation with hot(un)plug of devices

[Igor Mammedov]

On Fri, 24 Jan 2020 11:20:15 +0000
Salil Mehta <salil.mehta@huawei.com> wrote:

> Hello,
> I am working on vCPU Hotplug feature for ARM64 and I am in mid of understanding some aspect of device_add/device_del interface of the QEMU.
> 
> Observations:
> 1. Any object initialised by qmp_device_add() gets into /machine/unattached container. I traced the flow to code leg inside  device_set_realized()
> 2. I could see the reverse qmp_device_del() expects the device to be in  /machine/peripheral container.
> 3. I could see any object initially added to unattached container did not had their parents until object_add_property_child() was called further in the leg.
>     which effectively meant a new property was created and property table populated and child was parented.
> 4. Generally, container  /machine/peripheral was being used wherever DEVICE(dev)->id was present and non-null.
> 
> Question:
> 1. Wanted to confirm my understanding about the use of having separate containers like unattached, peripheral and anonymous.

> 2. At init time all the vcpus goes under *unattached* container. Now, qmp_device_del() cannot be used to unplug them. I am wondering

device is put into 'unattached' in case it wasn't assigned a parent.
Usually it happens when board creates device directly.

>    if all the hotplug devices need to go under the *peripheral* container while they are hotplugged and during object init time as well?

theoretically device_del may use QOM path (the later users can get with query-hotpluggable-cpus),
but I think it's mostly debugging feature.

users are supposed to specify 'id' during -device/device_add if they are going to manage that device
afterwards (like unplugging it). Then they could use that 'id' in other commands (including device_del)

So 'id'-ed devices end up in 'peripheral' container

> 3. I could not see any device being place under *anonymous* container during init time. What is the use of this container?

if I recall it right, devices created with help of device_add but without 'id' go to this container


> 
> I would be thankful for your valuable insights and answers and help in highlighting any gap in my understanding.
> 
> Thanks in anticipation!
> 
> Best Regards
> Salil
> 

RE: [Question] Regarding containers "unattached/peripheral/anonymous" - their relation with hot(un)plug of devices

[Salil Mehta]

> From: Igor Mammedov [mailto:imammedo@redhat.com]
> Sent: Friday, January 24, 2020 1:54 PM
> To: Salil Mehta <salil.mehta@huawei.com>
> 
> On Fri, 24 Jan 2020 11:20:15 +0000
> Salil Mehta <salil.mehta@huawei.com> wrote:
> 
> > Hello,
> > I am working on vCPU Hotplug feature for ARM64 and I am in mid of understanding
> > some aspect of device_add/device_del interface of the QEMU.
> >
> > Observations:
> > 1. Any object initialised by qmp_device_add() gets into /machine/unattached
> > container. I traced the flow to code leg inside  device_set_realized()
> > 2. I could see the reverse qmp_device_del() expects the device to be in
> > /machine/peripheral container.
> > 3. I could see any object initially added to unattached container did not had
> > their parents until object_add_property_child() was called further in the leg.
> > which effectively meant a new property was created and property table
> > populated and child was parented.
> > 4. Generally, container  /machine/peripheral was being used wherever
> > DEVICE(dev)->id was present and non-null.
> >
> > Question:
> > 1. Wanted to confirm my understanding about the use of having separate
> > containers like unattached, peripheral and anonymous.
> 
> > 2. At init time all the vcpus goes under *unattached* container. Now,
> > qmp_device_del() cannot be used to unplug them. I am wondering
> 
> device is put into 'unattached' in case it wasn't assigned a parent.
> Usually it happens when board creates device directly.


Sure, but if we decide that certain number(N) of vcpus are hotplugabble
and certain subset of N (say 'n' < 'N') should be allowed to be present
or cold-plugged at the init time then I wonder which of the following
is correct approach:

1. Bring all of N vcpus at boot time under "peripheral" container
                                   OR
2. Just bring subset 'n' of 'N' under "peripheral" container and rest
    under "unattached" container? And later as and when rest of the
    vcpus are hotplugged they should be transferred from "unattached"
    container to "peripheral" container?


> >    if all the hotplug devices need to go under the *peripheral* container while
> > they are hotplugged and during object init time as well?
> 
> theoretically device_del may use QOM path (the later users can get with
> query-hotpluggable-cpus),
> but I think it's mostly debugging feature.


Sure.


> users are supposed to specify 'id' during -device/device_add if they are going
> to manage that device.
> afterwards (like unplugging it). Then they could use that 'id' in other commands
> (including device_del)
> 
> So 'id'-ed devices end up in 'peripheral' container.


Sure, what if hotplugged device is removed and then added again? It looks 
qmp_device_add() interface will again end up calling the device_set_realized()
which eventually would put hotplugged devices under "unattached" container?


> > 3. I could not see any device being place under *anonymous* container during
> init time. What is the use of this container?
> 
> if I recall it right, devices created with help of device_add but without 'id'
> go to this container


Any examples on top of your head where such an interface might be of use?


Many thanks
Salil.

Re: [Question] Regarding containers "unattached/peripheral/anonymous" - their relation with hot(un)plug of devices

[Igor Mammedov]

On Fri, 24 Jan 2020 15:02:10 +0000
Salil Mehta <salil.mehta@huawei.com> wrote:

> > From: Igor Mammedov [mailto:imammedo@redhat.com]
> > Sent: Friday, January 24, 2020 1:54 PM
> > To: Salil Mehta <salil.mehta@huawei.com>
> > 
> > On Fri, 24 Jan 2020 11:20:15 +0000
> > Salil Mehta <salil.mehta@huawei.com> wrote:
> >   
> > > Hello,
> > > I am working on vCPU Hotplug feature for ARM64 and I am in mid of understanding
> > > some aspect of device_add/device_del interface of the QEMU.
> > >
> > > Observations:
> > > 1. Any object initialised by qmp_device_add() gets into /machine/unattached
> > > container. I traced the flow to code leg inside  device_set_realized()
> > > 2. I could see the reverse qmp_device_del() expects the device to be in
> > > /machine/peripheral container.
> > > 3. I could see any object initially added to unattached container did not had
> > > their parents until object_add_property_child() was called further in the leg.
> > > which effectively meant a new property was created and property table
> > > populated and child was parented.
> > > 4. Generally, container  /machine/peripheral was being used wherever
> > > DEVICE(dev)->id was present and non-null.
> > >
> > > Question:
> > > 1. Wanted to confirm my understanding about the use of having separate
> > > containers like unattached, peripheral and anonymous.  
> >   
> > > 2. At init time all the vcpus goes under *unattached* container. Now,
> > > qmp_device_del() cannot be used to unplug them. I am wondering  
> > 
> > device is put into 'unattached' in case it wasn't assigned a parent.
> > Usually it happens when board creates device directly.  
> 
> 
> Sure, but if we decide that certain number(N) of vcpus are hotplugabble
> and certain subset of N (say 'n' < 'N') should be allowed to be present
> or cold-plugged at the init time then I wonder which of the following
> is correct approach:
> 
> 1. Bring all of N vcpus at boot time under "peripheral" container
>                                    OR
> 2. Just bring subset 'n' of 'N' under "peripheral" container and rest
>     under "unattached" container? And later as and when rest of the
>     vcpus are hotplugged they should be transferred from "unattached"
>     container to "peripheral" container?

issue with that is that to put device into "peripheral" container,
'the user' must provide 'id'. (currently QEMU isn't able to do it on its own [1])

But it doesn't mean that cold-plugged CPUs can't be unpluged.
What current users could do is start QEMU like this (simplified version):

 $QEMU -smp 1,maxcpus=N -device foo-cpu-type,id=CPU00 -device foo-cpu-type,id=CPU01 ...

i.e. 1st CPU is not manageable due to lack if 'id' and is created by board code,
the rest have 'id' and could be managed.


Question is:
  why you are looking into 'what container' is used for CPUs?


1) here is what I could find on IDs topic
   https://lists.gnu.org/archive/html/qemu-block/2015-09/msg00011.html
 
> > >    if all the hotplug devices need to go under the *peripheral* container while
> > > they are hotplugged and during object init time as well?  
> > 
> > theoretically device_del may use QOM path (the later users can get with
> > query-hotpluggable-cpus),
> > but I think it's mostly debugging feature.  
> 
> 
> Sure.
> 
> 
> > users are supposed to specify 'id' during -device/device_add if they are going
> > to manage that device.
> > afterwards (like unplugging it). Then they could use that 'id' in other commands
> > (including device_del)
> > 
> > So 'id'-ed devices end up in 'peripheral' container.  
> 
> 
> Sure, what if hotplugged device is removed and then added again? It looks 
> qmp_device_add() interface will again end up calling the device_set_realized()
> which eventually would put hotplugged devices under "unattached" container?

it won't, see call chain:

  qmp_device_add()
      -> qdev_device_add()
          -> qdev_set_id()

 
> > > 3. I could not see any device being place under *anonymous* container during  
> > init time. What is the use of this container?
> > 
> > if I recall it right, devices created with help of device_add but without 'id'
> > go to this container  
> 
> 
> Any examples on top of your head where such an interface might be of use?

ex:
one could use -device/device_add without any ids if such devices aren't planned
to be unplugged during runtime or for unpluggable devices

> 
> 
> Many thanks
> Salil.
>

RE: [Question] Regarding containers "unattached/peripheral/anonymous" - their relation with hot(un)plug of devices

[Salil Mehta]

> From: Igor Mammedov [mailto:imammedo@redhat.com]
> Sent: Friday, January 24, 2020 4:07 PM
> 
> On Fri, 24 Jan 2020 15:02:10 +0000
> Salil Mehta <salil.mehta@huawei.com> wrote:
> 
> > > From: Igor Mammedov [mailto:imammedo@redhat.com]
> > > Sent: Friday, January 24, 2020 1:54 PM
> > > To: Salil Mehta <salil.mehta@huawei.com>
> > >
> > > On Fri, 24 Jan 2020 11:20:15 +0000
> > > Salil Mehta <salil.mehta@huawei.com> wrote:
> > >
> > > > Hello,
> > > > I am working on vCPU Hotplug feature for ARM64 and I am in mid of understanding
> > > > some aspect of device_add/device_del interface of the QEMU.
> > > >
> > > > Observations:
> > > > 1. Any object initialised by qmp_device_add() gets into /machine/unattached
> > > > container. I traced the flow to code leg inside  device_set_realized()
> > > > 2. I could see the reverse qmp_device_del() expects the device to be in
> > > > /machine/peripheral container.
> > > > 3. I could see any object initially added to unattached container did not had
> > > > their parents until object_add_property_child() was called further in the leg.
> > > > which effectively meant a new property was created and property table
> > > > populated and child was parented.
> > > > 4. Generally, container  /machine/peripheral was being used wherever
> > > > DEVICE(dev)->id was present and non-null.
> > > >
> > > > Question:
> > > > 1. Wanted to confirm my understanding about the use of having separate
> > > > containers like unattached, peripheral and anonymous.
> > >
> > > > 2. At init time all the vcpus goes under *unattached* container. Now,
> > > > qmp_device_del() cannot be used to unplug them. I am wondering
> > >
> > > device is put into 'unattached' in case it wasn't assigned a parent.
> > > Usually it happens when board creates device directly.
> >
> >
> > Sure, but if we decide that certain number(N) of vcpus are hotplugabble
> > and certain subset of N (say 'n' < 'N') should be allowed to be present
> > or cold-plugged at the init time then I wonder which of the following
> > is correct approach:
> >
> > 1. Bring all of N vcpus at boot time under "peripheral" container
> >                                    OR
> > 2. Just bring subset 'n' of 'N' under "peripheral" container and rest
> >     under "unattached" container? And later as and when rest of the
> >     vcpus are hotplugged they should be transferred from "unattached"
> >     container to "peripheral" container?
> 
> issue with that is that to put device into "peripheral" container,
> 'the user' must provide 'id'. (currently QEMU isn't able to do it on its own
> [1])
> 
> But it doesn't mean that cold-plugged CPUs can't be unpluged.
> What current users could do is start QEMU like this (simplified version):
> 
>  $QEMU -smp 1,maxcpus=N -device foo-cpu-type,id=CPU00 -device
> foo-cpu-type,id=CPU01 ...
> 
> i.e. 1st CPU is not manageable due to lack if 'id' and is created by board code,
> the rest have 'id' and could be managed.


I understand, that we can somehow assign ids from the QMP interface but
above will not push vcpus into "peripheral" container. They will appear
in "unattached" container but with specified names and as-far-as I can
see in the code 'device_del' can only delete objects/devices from the
'peripheral' container?

Plus, having those many ids specified for over large number of vcpus
does not looks very practical solution. We need interface like auto
Generation of ids even at the boot time. I could see from the link you
have shared that it is already being used by ID_BLOCK subsystem. Can we
create a new subsystem for cpus under this and do the auto Generation
of vcpu ids as well?


> Question is:
>   why you are looking into 'what container' is used for CPUs?


Idea is to be able to use 'device_del' interface to unplug vcpus
both for the cold-plugged and hot-plugged cases using the standard
'device_add' interface.

Plus, there is another unique requirement specifically for realizing
vcpu hotplug for ARM64.

Summary:
Right now ARM architecture does not allows reinitializing the GIC
after VM has booted. Therefore, we are planning to pre-size the GIC
interfaces at init time by initializing all of the possible vcpus
and keep them in 'realized' but 'unavailable' state to the Guest
VM. They shall be made available as-and-when vcpus are hot-plugged
later-on. Therefore, current efforts are to be able to plug and
unplug from the qemu QOM without destroying the existing state of
the devices/objects representing vcpus. These all possible vcpus
shall be created once at the boot time of the VM. The vcpus which
are not available to the Guest VM can be Parked. 

Once the vcpus are hot-(un)plug'ged only the (pre-)plug/unplug(-request)
interfaces are used to convey this even information to the Guest
VM.

I have tested this solution and it works but I wanted to make sure
that I am not doing anything which breaks any of the existing Qemu
QOM interfaces and basic fundamental idea behind being able to attach
and detach from the Qemu QOM is okay?

Any suggestion are welcome in this regard?


NOTE: I plan to share the patches with the community which includes
both the changes of the Linux Kernel and the QEMU in near future.



> 1) here is what I could find on IDs topic
>    https://lists.gnu.org/archive/html/qemu-block/2015-09/msg00011.html
> 
> > > >    if all the hotplug devices need to go under the *peripheral* container while
> > > > they are hotplugged and during object init time as well?
> > >
> > > theoretically device_del may use QOM path (the later users can get with
> > > query-hotpluggable-cpus),
> > > but I think it's mostly debugging feature.
> >
> >
> > Sure.
> >
> >
> > > users are supposed to specify 'id' during -device/device_add if they are going
> > > to manage that device.
> > > afterwards (like unplugging it). Then they could use that 'id' in other commands
> > > (including device_del)
> > >
> > > So 'id'-ed devices end up in 'peripheral' container.
> >
> >
> > Sure, what if hotplugged device is removed and then added again? It looks
> > qmp_device_add() interface will again end up calling the device_set_realized()
> > which eventually would put hotplugged devices under "unattached" container?
> 
> it won't, see call chain:
> 
>   qmp_device_add()
>       -> qdev_device_add()
>           -> qdev_set_id()

Ok, sure. I did see the qdev_set_id() interface. Infact, earlier I was actually
trying to play with it by making it more generic and adding even the 'unattached'
container handling to it(which is missing right now) and calling it inside the
device_set_realized()  instead of below code:

        if (!obj->parent) {
            gchar *name = g_strdup_printf("device[%d]", unattached_count++);

            object_property_add_child(container_get(qdev_get_machine(),
                                                    "/unattached"),
                                      name, obj, &error_abort);
            unattached_parent = true;
            g_free(name);
        }

Idea of above dabbling was to have common interface for 'unattached' container
and call it from virt.c from machvirt_init() where possible vcpus are being
created. Force them to be located either inside 'unttached' or 'peripheral'
container akin to the example you had given.

If we look at qdev_device_add() function, after setting dev->id using
qdev_set_id() (which would also result in parenting of an object under
'peripheral' container), it calls the function to 'realize' the device/object
which would end up in hitting above shared code excerpt and now because
it will have the parent already set, the object won't go into 'unattached'
container.

Currently, later cannot be controlled for the cold-lugged vcpus. Therefore,
before this discussion my initial thought process was to either make
qdev_set_id() universal (i.e. include handling for all type of containers
unattached/peripheral/anonymous in qdev_set_id()). Then call it from
machvirt_init() just before the vcpus have been 'realized' so that
cold-plugged cpus could get into 'peripheral' container. This would help
in hot-unplugging using the standard 'device_del' interface.

Earlier, I was not sure if there was any special significance to the
'unattached' container - which by the discussion it looks there is not
any and we could actually choose to place all of the cold-plugged vpus
as well in the 'peripheral' container. Please correct me here if I
have mis-understood anything here?

Now, assuming we are allowed to push all of the vcpus in the 'peripheral'
container then I could simply call unmodified qdev_set_id() just before
the 'realization' of the cold-plugged vcpus. And maybe we could generate
the cpus-ids using auto generate function which has been mentioned in the
link you shared and just like it is being done for the block devices.

Thoughts?


> > > > 3. I could not see any device being place under *anonymous* container during
> > > init time. What is the use of this container?
> > >
> > > if I recall it right, devices created with help of device_add but without
> 'id'
> > > go to this container
> >
> >
> > Any examples on top of your head where such an interface might be of use?
> 
> ex:
> one could use -device/device_add without any ids if such devices aren't planned
> to be unplugged during runtime or for unpluggable devices

Ok. Thanks!


Best Regards
Salil

Re: [Question] Regarding containers "unattached/peripheral/anonymous" - their relation with hot(un)plug of devices

[Igor Mammedov]

On Fri, 24 Jan 2020 18:44:16 +0000
Salil Mehta <salil.mehta@huawei.com> wrote:

> > From: Igor Mammedov [mailto:imammedo@redhat.com]
> > Sent: Friday, January 24, 2020 4:07 PM
> > 
> > On Fri, 24 Jan 2020 15:02:10 +0000
> > Salil Mehta <salil.mehta@huawei.com> wrote:
> >   
> > > > From: Igor Mammedov [mailto:imammedo@redhat.com]
> > > > Sent: Friday, January 24, 2020 1:54 PM
> > > > To: Salil Mehta <salil.mehta@huawei.com>
> > > >
> > > > On Fri, 24 Jan 2020 11:20:15 +0000
> > > > Salil Mehta <salil.mehta@huawei.com> wrote:
> > > >  
> > > > > Hello,
> > > > > I am working on vCPU Hotplug feature for ARM64 and I am in mid of understanding
> > > > > some aspect of device_add/device_del interface of the QEMU.
> > > > >
> > > > > Observations:
> > > > > 1. Any object initialised by qmp_device_add() gets into /machine/unattached
> > > > > container. I traced the flow to code leg inside  device_set_realized()
> > > > > 2. I could see the reverse qmp_device_del() expects the device to be in
> > > > > /machine/peripheral container.
> > > > > 3. I could see any object initially added to unattached container did not had
> > > > > their parents until object_add_property_child() was called further in the leg.
> > > > > which effectively meant a new property was created and property table
> > > > > populated and child was parented.
> > > > > 4. Generally, container  /machine/peripheral was being used wherever
> > > > > DEVICE(dev)->id was present and non-null.
> > > > >
> > > > > Question:
> > > > > 1. Wanted to confirm my understanding about the use of having separate
> > > > > containers like unattached, peripheral and anonymous.  
> > > >  
> > > > > 2. At init time all the vcpus goes under *unattached* container. Now,
> > > > > qmp_device_del() cannot be used to unplug them. I am wondering  
> > > >
> > > > device is put into 'unattached' in case it wasn't assigned a parent.
> > > > Usually it happens when board creates device directly.  
> > >
> > >
> > > Sure, but if we decide that certain number(N) of vcpus are hotplugabble
> > > and certain subset of N (say 'n' < 'N') should be allowed to be present
> > > or cold-plugged at the init time then I wonder which of the following
> > > is correct approach:
> > >
> > > 1. Bring all of N vcpus at boot time under "peripheral" container
> > >                                    OR
> > > 2. Just bring subset 'n' of 'N' under "peripheral" container and rest
> > >     under "unattached" container? And later as and when rest of the
> > >     vcpus are hotplugged they should be transferred from "unattached"
> > >     container to "peripheral" container?  
> > 
> > issue with that is that to put device into "peripheral" container,
> > 'the user' must provide 'id'. (currently QEMU isn't able to do it on its own
> > [1])
> > 
> > But it doesn't mean that cold-plugged CPUs can't be unpluged.
> > What current users could do is start QEMU like this (simplified version):
> > 
> >  $QEMU -smp 1,maxcpus=N -device foo-cpu-type,id=CPU00 -device
> > foo-cpu-type,id=CPU01 ...
> > 
> > i.e. 1st CPU is not manageable due to lack if 'id' and is created by board code,
> > the rest have 'id' and could be managed.  
> 
> 
> I understand, that we can somehow assign ids from the QMP interface but
> above will not push vcpus into "peripheral" container. They will appear
> in "unattached" container but with specified names and as-far-as I can
> see in the code 'device_del' can only delete objects/devices from the
> 'peripheral' container?

qemu-system-x86_64 -monitor stdio \
    -smp 1,maxcpus=3 \
    -device qemu64-x86_64-cpu,id=foo,socket-id=1,core-id=0,thread-id=0 \
    -device qemu64-x86_64-cpu,socket-id=2,core-id=0,thread-id=0

(qemu) info hotpluggable-cpus 
Hotpluggable CPUs:
  type: "qemu64-x86_64-cpu"
  vcpus_count: "1"
  qom_path: "/machine/peripheral-anon/device[0]"
                      ^^^^^^^^^^^^^^^
  CPUInstance Properties:
    socket-id: "2"
    core-id: "0"
    thread-id: "0"
  type: "qemu64-x86_64-cpu"
  vcpus_count: "1"
  qom_path: "/machine/peripheral/foo"
                      ^^^^^^^^^^

in gist, if device is created with any variant of device_add,
it goes to "peripheral[-anon]" including cold-plugged one.

  CPUInstance Properties:
    socket-id: "1"
    core-id: "0"
    thread-id: "0"
  type: "qemu64-x86_64-cpu"
  vcpus_count: "1"
  qom_path: "/machine/unattached/device[0]"
  CPUInstance Properties:
    socket-id: "0"
    core-id: "0"
    thread-id: "0"





> 
> Plus, having those many ids specified for over large number of vcpus
> does not looks very practical solution. We need interface like auto
number of IDs is not a problem since it's usually handled by management
software just fine (ex: libvirt does it)

> Generation of ids even at the boot time. I could see from the link you
> have shared that it is already being used by ID_BLOCK subsystem. Can we
> create a new subsystem for cpus under this and do the auto Generation
> of vcpu ids as well?

I'm not sure that auto ids was actually merged.
(I thought it wasn't)

Anyway auto IDs are not directly related to enabling CPU hotplug for ARM,
if you feel they should be generated you can try to propose patches.

> > Question is:
> >   why you are looking into 'what container' is used for CPUs?  
> 
> 
> Idea is to be able to use 'device_del' interface to unplug vcpus
> both for the cold-plugged and hot-plugged cases using the standard
> 'device_add' interface.

As far as CPU devices in QEMU are created with help of -device/device_add
with 'id' provided it should work with current code.

 
> Plus, there is another unique requirement specifically for realizing
> vcpu hotplug for ARM64.
> 
> Summary:
> Right now ARM architecture does not allows reinitializing the GIC
> after VM has booted. Therefore, we are planning to pre-size the GIC
> interfaces at init time by initializing all of the possible vcpus
> and keep them in 'realized' but 'unavailable' state to the Guest
> VM. They shall be made available as-and-when vcpus are hot-plugged
> later-on. Therefore, current efforts are to be able to plug and
> unplug from the qemu QOM without destroying the existing state of
> the devices/objects representing vcpus. These all possible vcpus
> shall be created once at the boot time of the VM. The vcpus which
> are not available to the Guest VM can be Parked. 
>
> Once the vcpus are hot-(un)plug'ged only the (pre-)plug/unplug(-request)
> interfaces are used to convey this even information to the Guest
> VM.
> 
> I have tested this solution and it works but I wanted to make sure
> that I am not doing anything which breaks any of the existing Qemu
> QOM interfaces and basic fundamental idea behind being able to attach
> and detach from the Qemu QOM is okay?
> 
> Any suggestion are welcome in this regard?

From discussion with Drew [CCed], I got that kvm/arm isn't designed
to support vCPU hotplug (and it would require heavy refactoring to
separate GIC and VCPUs, which probably won't be welcomed by maintainers).

But that's only KVM side of the equation. Assuming that we don't
touch KVM much, the only QEMU side is left.

Further lets call
 * vCPU - a kvm's part of CPU
 * CPU - QEMU object which is linked to vCPU via file descriptor.

In QEMU we have CPU devices which optionally might create vCPUs
during device realize time (if QEMU runs with KVM accelerator).

So from design point of view, I'd suggest to dynamically
create/remove CPU devices on demand using existing
 -device/device_add/device_del interface
like we do for other architectures.

But in case of running with KVM accelerator, to accommodate
current non dynamic ARM/KVM, I'd move vCPU creation to "kvm_init()"
time or board init time, so it would pre-create vCPUs in
KVM early in parked state and put them in 'kvm_parked_vcpus' list
but won't create CPU devices for them.

Then later when management adds CPU device either with
'-device' or 'device_add', a new CPU device will pick up
pre-created parked vCPU file descriptor and re-use it.

Parked vCPU infrastructure is already exists in QEMU as we use it
for hot-unplugged CPUs for the same reasons (it needs too much
refactoring on KVM side to really remove vCPU).

So when CPU is hot-unplugged, we put linked vCPU file descriptor
into kvm_parked_vcpus (see: kvm_destroy_vcpu) and completely delete
CPU device on QEMU side. Then when the same CPU is hot-plugged again,
we reuse previously parked vCPU file descriptor (see: kvm_get_vcpu).


> NOTE: I plan to share the patches with the community which includes
> both the changes of the Linux Kernel and the QEMU in near future.
> 
> 
> 
> > 1) here is what I could find on IDs topic
> >    https://lists.gnu.org/archive/html/qemu-block/2015-09/msg00011.html
> >   
> > > > >    if all the hotplug devices need to go under the *peripheral* container while
> > > > > they are hotplugged and during object init time as well?  
> > > >
> > > > theoretically device_del may use QOM path (the later users can get with
> > > > query-hotpluggable-cpus),
> > > > but I think it's mostly debugging feature.  
> > >
> > >
> > > Sure.
> > >
> > >  
> > > > users are supposed to specify 'id' during -device/device_add if they are going
> > > > to manage that device.
> > > > afterwards (like unplugging it). Then they could use that 'id' in other commands
> > > > (including device_del)
> > > >
> > > > So 'id'-ed devices end up in 'peripheral' container.  
> > >
> > >
> > > Sure, what if hotplugged device is removed and then added again? It looks
> > > qmp_device_add() interface will again end up calling the device_set_realized()
> > > which eventually would put hotplugged devices under "unattached" container?  
> > 
> > it won't, see call chain:
> > 
> >   qmp_device_add()  
> >       -> qdev_device_add()
> >           -> qdev_set_id()  
> 
> Ok, sure. I did see the qdev_set_id() interface. Infact, earlier I was actually
> trying to play with it by making it more generic and adding even the 'unattached'
> container handling to it(which is missing right now) and calling it inside the
> device_set_realized()  instead of below code:
> 
>         if (!obj->parent) {
>             gchar *name = g_strdup_printf("device[%d]", unattached_count++);
> 
>             object_property_add_child(container_get(qdev_get_machine(),
>                                                     "/unattached"),
>                                       name, obj, &error_abort);
>             unattached_parent = true;
>             g_free(name);
>         }
> 
> Idea of above dabbling was to have common interface for 'unattached' container
> and call it from virt.c from machvirt_init() where possible vcpus are being
> created. Force them to be located either inside 'unttached' or 'peripheral'
> container akin to the example you had given.
> 
> If we look at qdev_device_add() function, after setting dev->id using
> qdev_set_id() (which would also result in parenting of an object under
> 'peripheral' container), it calls the function to 'realize' the device/object
> which would end up in hitting above shared code excerpt and now because
> it will have the parent already set, the object won't go into 'unattached'
> container.
> 
> Currently, later cannot be controlled for the cold-lugged vcpus. Therefore,
> before this discussion my initial thought process was to either make
> qdev_set_id() universal (i.e. include handling for all type of containers
> unattached/peripheral/anonymous in qdev_set_id()). Then call it from
> machvirt_init() just before the vcpus have been 'realized' so that
> cold-plugged cpus could get into 'peripheral' container. This would help
> in hot-unplugging using the standard 'device_del' interface.

I think we understand cold-plugged vcpus differently.
From QEMU point of view there are 2 kinds of cold-plugged CPU devices.

Ones that are currently created by board directly following pattern

 object_new()
 set properties
 relalize

these are created (lets call them builtin) in amount specified by -smp X
and are not manageable by external applications.

The second kind of cold-plugged CPUs are the ones created on command
line with help of:

 -device cpu-type-foo

these are created by management applications and could be hot-removed
if management supplied 'id'. For example libvirt starts qemu with 1
built-in cpu in paused mode (-s) QEMU, then it hotplugs via QMP N cpus
and lets VM run (it's essentially the same as using -device on CLI).
This way it can remove all CPUs except of 1st one which is good enough
in almost all the cases.

> Earlier, I was not sure if there was any special significance to the
> 'unattached' container - which by the discussion it looks there is not
> any and we could actually choose to place all of the cold-plugged vpus
> as well in the 'peripheral' container. Please correct me here if I
> have mis-understood anything here?
> 
> Now, assuming we are allowed to push all of the vcpus in the 'peripheral'
> container then I could simply call unmodified qdev_set_id() just before
> the 'realization' of the cold-plugged vcpus. And maybe we could generate
> the cpus-ids using auto generate function which has been mentioned in the
> link you shared and just like it is being done for the block devices.

In ARM case, I'd consider to implementing -device/device_add support first.
So it would be on par with other architectures in QEMU that support cpu hotplug.

Then try to implement id auto-generation on top and discuss if making
built-in CPUs hot-removable is worthwhile, as it's a separate issues
and could be done later.

Separating feature on distinct self-sufficient chunks usually makes
reviews/merging easier comparing to trying to do everything at once.

> Thoughts?
> 
> 
> > > > > 3. I could not see any device being place under *anonymous* container during  
> > > > init time. What is the use of this container?
> > > >
> > > > if I recall it right, devices created with help of device_add but without  
> > 'id'  
> > > > go to this container  
> > >
> > >
> > > Any examples on top of your head where such an interface might be of use?  
> > 
> > ex:
> > one could use -device/device_add without any ids if such devices aren't planned
> > to be unplugged during runtime or for unpluggable devices  
> 
> Ok. Thanks!
> 
> 
> Best Regards
> Salil
> 

Re: [Question] Regarding containers "unattached/peripheral/anonymous" - their relation with hot(un)plug of devices

[Michael S. Tsirkin]

On Fri, Jan 24, 2020 at 02:54:04PM +0100, Igor Mammedov wrote:
> On Fri, 24 Jan 2020 11:20:15 +0000
> Salil Mehta <salil.mehta@huawei.com> wrote:
> 
> > Hello,
> > I am working on vCPU Hotplug feature for ARM64 and I am in mid of understanding some aspect of device_add/device_del interface of the QEMU.
> > 
> > Observations:
> > 1. Any object initialised by qmp_device_add() gets into /machine/unattached container. I traced the flow to code leg inside  device_set_realized()
> > 2. I could see the reverse qmp_device_del() expects the device to be in  /machine/peripheral container.
> > 3. I could see any object initially added to unattached container did not had their parents until object_add_property_child() was called further in the leg.
> >     which effectively meant a new property was created and property table populated and child was parented.
> > 4. Generally, container  /machine/peripheral was being used wherever DEVICE(dev)->id was present and non-null.
> > 
> > Question:
> > 1. Wanted to confirm my understanding about the use of having separate containers like unattached, peripheral and anonymous.
> 
> > 2. At init time all the vcpus goes under *unattached* container. Now, qmp_device_del() cannot be used to unplug them. I am wondering
> 
> device is put into 'unattached' in case it wasn't assigned a parent.
> Usually it happens when board creates device directly.
> 
> >    if all the hotplug devices need to go under the *peripheral* container while they are hotplugged and during object init time as well?
> 
> theoretically device_del may use QOM path (the later users can get with query-hotpluggable-cpus),
> but I think it's mostly debugging feature.
> 
> users are supposed to specify 'id' during -device/device_add if they are going to manage that device
> afterwards (like unplugging it). Then they could use that 'id' in other commands (including device_del)
> 
> So 'id'-ed devices end up in 'peripheral' container
> 
> > 3. I could not see any device being place under *anonymous* container during init time. What is the use of this container?
> 
> if I recall it right, devices created with help of device_add but without 'id' go to this container

BTW, ATM hw/acpi/cpu.c creates _EJ0 for all CPUs (except the 1st one).
It might be cleaner to skip it for CPUs which don't have an id - what
do you think?


> 
> > 
> > I would be thankful for your valuable insights and answers and help in highlighting any gap in my understanding.
> > 
> > Thanks in anticipation!
> > 
> > Best Regards
> > Salil
> > 

Re: [Question] Regarding containers "unattached/peripheral/anonymous" - their relation with hot(un)plug of devices

[Igor Mammedov]

On Mon, 3 Feb 2020 05:40:01 -0500
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Fri, Jan 24, 2020 at 02:54:04PM +0100, Igor Mammedov wrote:
> > On Fri, 24 Jan 2020 11:20:15 +0000
> > Salil Mehta <salil.mehta@huawei.com> wrote:
> >   
> > > Hello,
> > > I am working on vCPU Hotplug feature for ARM64 and I am in mid of understanding some aspect of device_add/device_del interface of the QEMU.
> > > 
> > > Observations:
> > > 1. Any object initialised by qmp_device_add() gets into /machine/unattached container. I traced the flow to code leg inside  device_set_realized()
> > > 2. I could see the reverse qmp_device_del() expects the device to be in  /machine/peripheral container.
> > > 3. I could see any object initially added to unattached container did not had their parents until object_add_property_child() was called further in the leg.
> > >     which effectively meant a new property was created and property table populated and child was parented.
> > > 4. Generally, container  /machine/peripheral was being used wherever DEVICE(dev)->id was present and non-null.
> > > 
> > > Question:
> > > 1. Wanted to confirm my understanding about the use of having separate containers like unattached, peripheral and anonymous.  
> >   
> > > 2. At init time all the vcpus goes under *unattached* container. Now, qmp_device_del() cannot be used to unplug them. I am wondering  
> > 
> > device is put into 'unattached' in case it wasn't assigned a parent.
> > Usually it happens when board creates device directly.
> >   
> > >    if all the hotplug devices need to go under the *peripheral* container while they are hotplugged and during object init time as well?  
> > 
> > theoretically device_del may use QOM path (the later users can get with query-hotpluggable-cpus),
> > but I think it's mostly debugging feature.
> > 
> > users are supposed to specify 'id' during -device/device_add if they are going to manage that device
> > afterwards (like unplugging it). Then they could use that 'id' in other commands (including device_del)
> > 
> > So 'id'-ed devices end up in 'peripheral' container
> >   
> > > 3. I could not see any device being place under *anonymous* container during init time. What is the use of this container?  
> > 
> > if I recall it right, devices created with help of device_add but without 'id' go to this container  
> 
> BTW, ATM hw/acpi/cpu.c creates _EJ0 for all CPUs (except the 1st one).
Eject on the 1st is disabled because we can't remove it in current impl.

> It might be cleaner to skip it for CPUs which don't have an id - what
> do you think?

Not sure as it's a bit different.
device_del is mgmt means to ask guest to remove CPU and if mgmt cares about removal
it should use 'id' during -device/device_add.
Where as _EJ0 is guest's means to actually remove cpus either on mgmt request
or manually via some other tooling running on guest side.
Once guest ejects such CPU, slot becomes free and mgmt could plug a CPU into it
later on.

I don't know if it's used in practice, so the only way to find it out is to
break it and see if someone complains.

Disabling _EJ0 on -smp X cpus looks to me like a policy decision (possible config option)
and won't simplify anything in practice (QEMU wise).

(so I'd duck my head into sand due to luck of complains from users)


> >   
> > > 
> > > I would be thankful for your valuable insights and answers and help in highlighting any gap in my understanding.
> > > 
> > > Thanks in anticipation!
> > > 
> > > Best Regards
> > > Salil
> > >   
> 
> 

RE: [Question] Regarding containers "unattached/peripheral/anonymous" - their relation with hot(un)plug of devices

[Salil Mehta]

Hi Igor,
My sincere Apologies, I just realized that I missed to reply this mail.
I was distracted to something else in  the month of the February and
had only resumed working on hotplug in march. But will still reply to
this mail. Also, I have incorporated most of the below points as in the
vcpu hotplug patches as per your suggestions.


> From: Qemu-arm [mailto:qemu-arm-bounces+salil.mehta=huawei.com@nongnu.org]
> On Behalf Of Igor Mammedov
> Sent: Monday, January 27, 2020 3:04 PM
> To: Salil Mehta <salil.mehta@huawei.com>
> 
> On Fri, 24 Jan 2020 18:44:16 +0000
> Salil Mehta <salil.mehta@huawei.com> wrote:
> 
> > > From: Igor Mammedov [mailto:imammedo@redhat.com]
> > > Sent: Friday, January 24, 2020 4:07 PM
> > >
> > > On Fri, 24 Jan 2020 15:02:10 +0000
> > > Salil Mehta <salil.mehta@huawei.com> wrote:
> > >
> > > > > From: Igor Mammedov [mailto:imammedo@redhat.com]
> > > > > Sent: Friday, January 24, 2020 1:54 PM
> > > > > To: Salil Mehta <salil.mehta@huawei.com>
> > > > >
> > > > > On Fri, 24 Jan 2020 11:20:15 +0000
> > > > > Salil Mehta <salil.mehta@huawei.com> wrote:
> > > > >
> > > > > > Hello,
> > > > > > I am working on vCPU Hotplug feature for ARM64 and I am in mid of understanding
> > > > > > some aspect of device_add/device_del interface of the QEMU.
> > > > > >
> > > > > > Observations:
> > > > > > 1. Any object initialised by qmp_device_add() gets into /machine/unattached
> > > > > > container. I traced the flow to code leg inside  device_set_realized()
> > > > > > 2. I could see the reverse qmp_device_del() expects the device to be in
> > > > > > /machine/peripheral container.
> > > > > > 3. I could see any object initially added to unattached container did not had
> > > > > > their parents until object_add_property_child() was called further in the leg.
> > > > > > which effectively meant a new property was created and property table
> > > > > > populated and child was parented.
> > > > > > 4. Generally, container  /machine/peripheral was being used wherever
> > > > > > DEVICE(dev)->id was present and non-null.
> > > > > >
> > > > > > Question:
> > > > > > 1. Wanted to confirm my understanding about the use of having separate
> > > > > > containers like unattached, peripheral and anonymous.
> > > > >
> > > > > > 2. At init time all the vcpus goes under *unattached* container. Now,
> > > > > > qmp_device_del() cannot be used to unplug them. I am wondering
> > > > >
> > > > > device is put into 'unattached' in case it wasn't assigned a parent.
> > > > > Usually it happens when board creates device directly.
> > > >
> > > >
> > > > Sure, but if we decide that certain number(N) of vcpus are hotplugabble
> > > > and certain subset of N (say 'n' < 'N') should be allowed to be present
> > > > or cold-plugged at the init time then I wonder which of the following
> > > > is correct approach:
> > > >
> > > > 1. Bring all of N vcpus at boot time under "peripheral" container
> > > >                                    OR
> > > > 2. Just bring subset 'n' of 'N' under "peripheral" container and rest
> > > >     under "unattached" container? And later as and when rest of the
> > > >     vcpus are hotplugged they should be transferred from "unattached"
> > > >     container to "peripheral" container?
> > >
> > > issue with that is that to put device into "peripheral" container,
> > > 'the user' must provide 'id'. (currently QEMU isn't able to do it on its own
> > > [1])
> > >
> > > But it doesn't mean that cold-plugged CPUs can't be unpluged.
> > > What current users could do is start QEMU like this (simplified version):
> > >
> > >  $QEMU -smp 1,maxcpus=N -device foo-cpu-type,id=CPU00 -device
> > > foo-cpu-type,id=CPU01 ...
> > >
> > > i.e. 1st CPU is not manageable due to lack if 'id' and is created by board code,
> > > the rest have 'id' and could be managed.
> >
> >
> > I understand, that we can somehow assign ids from the QMP interface but
> > above will not push vcpus into "peripheral" container. They will appear
> > in "unattached" container but with specified names and as-far-as I can
> > see in the code 'device_del' can only delete objects/devices from the
> > 'peripheral' container?
> 
> qemu-system-x86_64 -monitor stdio \
>     -smp 1,maxcpus=3 \
>     -device qemu64-x86_64-cpu,id=foo,socket-id=1,core-id=0,thread-id=0 \
>     -device qemu64-x86_64-cpu,socket-id=2,core-id=0,thread-id=0
> 
> (qemu) info hotpluggable-cpus
> Hotpluggable CPUs:
>   type: "qemu64-x86_64-cpu"
>   vcpus_count: "1"
>   qom_path: "/machine/peripheral-anon/device[0]"
>                       ^^^^^^^^^^^^^^^
>   CPUInstance Properties:
>     socket-id: "2"
>     core-id: "0"
>     thread-id: "0"
>   type: "qemu64-x86_64-cpu"
>   vcpus_count: "1"
>   qom_path: "/machine/peripheral/foo"
>                       ^^^^^^^^^^
> 
> in gist, if device is created with any variant of device_add,
> it goes to "peripheral[-anon]" including cold-plugged one.


I am not sure why you said "including cold-plugged one"? I hope by
cold-plug'ging you mean here are the CPUs which already exist at
the boot time of the Guest VM and plugged using -device?


>   CPUInstance Properties:
>     socket-id: "1"
>     core-id: "0"
>     thread-id: "0"
>   type: "qemu64-x86_64-cpu"
>   vcpus_count: "1"
>   qom_path: "/machine/unattached/device[0]"
                          ^^^^^^^^^^^^^

Unless you have pasted wrongly here, above output also shows qom path
as 'unattached' for cold-plugged CPUs. Am I missing something? :)


>   CPUInstance Properties:
>     socket-id: "0"
>     core-id: "0"
>     thread-id: "0"



> > Plus, having those many ids specified for over large number of vcpus
> > does not looks very practical solution. We need interface like auto
> number of IDs is not a problem since it's usually handled by management
> software just fine (ex: libvirt does it)
> 
> > Generation of ids even at the boot time. I could see from the link you
> > have shared that it is already being used by ID_BLOCK subsystem. Can we
> > create a new subsystem for cpus under this and do the auto Generation
> > of vcpu ids as well?
> 
> I'm not sure that auto ids was actually merged.
> (I thought it wasn't)


Well if you were referring to below then it has been part of qemu for quite
long now:

Patch: util - add automated ID generation utility
File: https://github.com/qemu/qemu/blob/master/util/id.c
Commit-id: https://github.com/qemu/qemu/commit/a0f1913637e6


> Anyway auto IDs are not directly related to enabling CPU hotplug for ARM,
> if you feel they should be generated you can try to propose patches.

Sure. 


> > Plus, there is another unique requirement specifically for realizing
> > vcpu hotplug for ARM64.
> >
> > Summary:
> > Right now ARM architecture does not allows reinitializing the GIC
> > after VM has booted. Therefore, we are planning to pre-size the GIC
> > interfaces at init time by initializing all of the possible vcpus
> > and keep them in 'realized' but 'unavailable' state to the Guest
> > VM. They shall be made available as-and-when vcpus are hot-plugged
> > later-on. Therefore, current efforts are to be able to plug and
> > unplug from the qemu QOM without destroying the existing state of
> > the devices/objects representing vcpus. These all possible vcpus
> > shall be created once at the boot time of the VM. The vcpus which
> > are not available to the Guest VM can be Parked.
> >
> > Once the vcpus are hot-(un)plug'ged only the (pre-)plug/unplug(-request)
> > interfaces are used to convey this even information to the Guest
> > VM.
> >
> > I have tested this solution and it works but I wanted to make sure
> > that I am not doing anything which breaks any of the existing Qemu
> > QOM interfaces and basic fundamental idea behind being able to attach
> > and detach from the Qemu QOM is okay?
> >
> > Any suggestion are welcome in this regard?
> 
> From discussion with Drew [CCed], I got that kvm/arm isn't designed
> to support vCPU hotplug (and it would require heavy refactoring to
> separate GIC and VCPUs, which probably won't be welcomed by maintainers).

Agreed. I have pre-sized GIC with possible vcpus. I think Marc did
mention about this earlier.

> 
> But that's only KVM side of the equation. Assuming that we don't
> touch KVM much, the only QEMU side is left.
> 
> Further lets call
>  * vCPU - a kvm's part of CPU
>  * CPU - QEMU object which is linked to vCPU via file descriptor.
> 
> In QEMU we have CPU devices which optionally might create vCPUs
> during device realize time (if QEMU runs with KVM accelerator).
> 
> So from design point of view, I'd suggest to dynamically
> create/remove CPU devices on demand using existing
>  -device/device_add/device_del interface
> like we do for other architectures.
> 
> But in case of running with KVM accelerator, to accommodate
> current non dynamic ARM/KVM, I'd move vCPU creation to "kvm_init()"
> time or board init time, so it would pre-create vCPUs in
> KVM early in parked state and put them in 'kvm_parked_vcpus' list
> but won't create CPU devices for them.


Something similar. But I am doing it from virt machine init.


> Then later when management adds CPU device either with
> '-device' or 'device_add', a new CPU device will pick up
> pre-created parked vCPU file descriptor and re-use it.
> 
> Parked vCPU infrastructure is already exists in QEMU as we use it
> for hot-unplugged CPUs for the same reasons (it needs too much
> refactoring on KVM side to really remove vCPU).

Agreed with respect to KVM change. I have added some new in qemu/kvm
layer but some re-factoring might be needed later to extract common
code. Also, ARM arch specific functions had to be introduced.

> 
> So when CPU is hot-unplugged, we put linked vCPU file descriptor
> into kvm_parked_vcpus (see: kvm_destroy_vcpu) and completely delete
> CPU device on QEMU side. Then when the same CPU is hot-plugged again,
> we reuse previously parked vCPU file descriptor (see: kvm_get_vcpu).


Yes, I have done exactly this and perhaps this approach was discussed
in earlier mail-chains as well involving you. So taken your suggestion
and many thanks for this :)


> > NOTE: I plan to share the patches with the community which includes
> > both the changes of the Linux Kernel and the QEMU in near future.


Plan to share a RFC within a week for sure. 


> > > 1) here is what I could find on IDs topic
> > >    https://lists.gnu.org/archive/html/qemu-block/2015-09/msg00011.html
> > >
> > > > > >    if all the hotplug devices need to go under the *peripheral* container while
> > > > > > they are hotplugged and during object init time as well?
> > > > >
> > > > > theoretically device_del may use QOM path (the later users can get with
> > > > > query-hotpluggable-cpus),
> > > > > but I think it's mostly debugging feature.
> > > >
> > > >
> > > > Sure.
> > > >
> > > >
> > > > > users are supposed to specify 'id' during -device/device_add if they are going
> > > > > to manage that device.
> > > > > afterwards (like unplugging it). Then they could use that 'id' in other commands
> > > > > (including device_del)
> > > > >
> > > > > So 'id'-ed devices end up in 'peripheral' container.
> > > >
> > > >
> > > > Sure, what if hotplugged device is removed and then added again? It looks
> > > > qmp_device_add() interface will again end up calling the device_set_realized()
> > > > which eventually would put hotplugged devices under "unattached" container?
> > >
> > > it won't, see call chain:
> > >
> > >   qmp_device_add()
> > >       -> qdev_device_add()
> > >           -> qdev_set_id()
> >
> > Ok, sure. I did see the qdev_set_id() interface. Infact, earlier I was actually
> > trying to play with it by making it more generic and adding even the 'unattached'
> > container handling to it(which is missing right now) and calling it inside the
> > device_set_realized()  instead of below code:
> >
> >         if (!obj->parent) {
> >             gchar *name = g_strdup_printf("device[%d]", unattached_count++);
> >
> >             object_property_add_child(container_get(qdev_get_machine(),
> >                                                     "/unattached"),
> >                                       name, obj, &error_abort);
> >             unattached_parent = true;
> >             g_free(name);
> >         }
> >
> > Idea of above dabbling was to have common interface for 'unattached' container
> > and call it from virt.c from machvirt_init() where possible vcpus are being
> > created. Force them to be located either inside 'unttached' or 'peripheral'
> > container akin to the example you had given.
> >
> > If we look at qdev_device_add() function, after setting dev->id using
> > qdev_set_id() (which would also result in parenting of an object under
> > 'peripheral' container), it calls the function to 'realize' the device/object
> > which would end up in hitting above shared code excerpt and now because
> > it will have the parent already set, the object won't go into 'unattached'
> > container.
> >
> > Currently, later cannot be controlled for the cold-lugged vcpus. Therefore,
> > before this discussion my initial thought process was to either make
> > qdev_set_id() universal (i.e. include handling for all type of containers
> > unattached/peripheral/anonymous in qdev_set_id()). Then call it from
> > machvirt_init() just before the vcpus have been 'realized' so that
> > cold-plugged cpus could get into 'peripheral' container. This would help
> > in hot-unplugging using the standard 'device_del' interface.
> 
> I think we understand cold-plugged vcpus differently.
> From QEMU point of view there are 2 kinds of cold-plugged CPU devices.
> 
> Ones that are currently created by board directly following pattern
> 
>  object_new()
>  set properties
>  relalize
> 
> these are created (lets call them builtin) in amount specified by -smp X
> and are not manageable by external applications.
> 
> The second kind of cold-plugged CPUs are the ones created on command
> line with help of:
> 
>  -device cpu-type-foo
> 
> these are created by management applications and could be hot-removed
> if management supplied 'id'. For example libvirt starts qemu with 1
> built-in cpu in paused mode (-s) QEMU, then it hotplugs via QMP N cpus
> and lets VM run (it's essentially the same as using -device on CLI).
> This way it can remove all CPUs except of 1st one which is good enough
> in almost all the cases.
> 

Sure, I can see your point. I have tried to incorporate part of it.
Maybe you would like to check the code which will be sent soon.


Many thanks
Salil

Re: [Question] Regarding containers "unattached/peripheral/anonymous" - their relation with hot(un)plug of devices

[Igor Mammedov]

On Wed, 3 Jun 2020 15:13:26 +0000
Salil Mehta <salil.mehta@huawei.com> wrote:

> Hi Igor,
> My sincere Apologies, I just realized that I missed to reply this mail.
> I was distracted to something else in  the month of the February and
> had only resumed working on hotplug in march. But will still reply to
> this mail. Also, I have incorporated most of the below points as in the
> vcpu hotplug patches as per your suggestions.
> 
> 
> > From: Qemu-arm [mailto:qemu-arm-bounces+salil.mehta=huawei.com@nongnu.org]
> > On Behalf Of Igor Mammedov
> > Sent: Monday, January 27, 2020 3:04 PM
> > To: Salil Mehta <salil.mehta@huawei.com>
> > 
> > On Fri, 24 Jan 2020 18:44:16 +0000
> > Salil Mehta <salil.mehta@huawei.com> wrote:
> >   
> > > > From: Igor Mammedov [mailto:imammedo@redhat.com]
> > > > Sent: Friday, January 24, 2020 4:07 PM
> > > >
> > > > On Fri, 24 Jan 2020 15:02:10 +0000
> > > > Salil Mehta <salil.mehta@huawei.com> wrote:
> > > >  
> > > > > > From: Igor Mammedov [mailto:imammedo@redhat.com]
> > > > > > Sent: Friday, January 24, 2020 1:54 PM
> > > > > > To: Salil Mehta <salil.mehta@huawei.com>
> > > > > >
> > > > > > On Fri, 24 Jan 2020 11:20:15 +0000
> > > > > > Salil Mehta <salil.mehta@huawei.com> wrote:
> > > > > >  
> > > > > > > Hello,
> > > > > > > I am working on vCPU Hotplug feature for ARM64 and I am in mid of understanding
> > > > > > > some aspect of device_add/device_del interface of the QEMU.
> > > > > > >
> > > > > > > Observations:
> > > > > > > 1. Any object initialised by qmp_device_add() gets into /machine/unattached
> > > > > > > container. I traced the flow to code leg inside  device_set_realized()
> > > > > > > 2. I could see the reverse qmp_device_del() expects the device to be in
> > > > > > > /machine/peripheral container.
> > > > > > > 3. I could see any object initially added to unattached container did not had
> > > > > > > their parents until object_add_property_child() was called further in the leg.
> > > > > > > which effectively meant a new property was created and property table
> > > > > > > populated and child was parented.
> > > > > > > 4. Generally, container  /machine/peripheral was being used wherever
> > > > > > > DEVICE(dev)->id was present and non-null.
> > > > > > >
> > > > > > > Question:
> > > > > > > 1. Wanted to confirm my understanding about the use of having separate
> > > > > > > containers like unattached, peripheral and anonymous.  
> > > > > >  
> > > > > > > 2. At init time all the vcpus goes under *unattached* container. Now,
> > > > > > > qmp_device_del() cannot be used to unplug them. I am wondering  
> > > > > >
> > > > > > device is put into 'unattached' in case it wasn't assigned a parent.
> > > > > > Usually it happens when board creates device directly.  
> > > > >
> > > > >
> > > > > Sure, but if we decide that certain number(N) of vcpus are hotplugabble
> > > > > and certain subset of N (say 'n' < 'N') should be allowed to be present
> > > > > or cold-plugged at the init time then I wonder which of the following
> > > > > is correct approach:
> > > > >
> > > > > 1. Bring all of N vcpus at boot time under "peripheral" container
> > > > >                                    OR
> > > > > 2. Just bring subset 'n' of 'N' under "peripheral" container and rest
> > > > >     under "unattached" container? And later as and when rest of the
> > > > >     vcpus are hotplugged they should be transferred from "unattached"
> > > > >     container to "peripheral" container?  
> > > >
> > > > issue with that is that to put device into "peripheral" container,
> > > > 'the user' must provide 'id'. (currently QEMU isn't able to do it on its own
> > > > [1])
> > > >
> > > > But it doesn't mean that cold-plugged CPUs can't be unpluged.
> > > > What current users could do is start QEMU like this (simplified version):
> > > >
> > > >  $QEMU -smp 1,maxcpus=N -device foo-cpu-type,id=CPU00 -device
> > > > foo-cpu-type,id=CPU01 ...
> > > >
> > > > i.e. 1st CPU is not manageable due to lack if 'id' and is created by board code,
> > > > the rest have 'id' and could be managed.  
> > >
> > >
> > > I understand, that we can somehow assign ids from the QMP interface but
> > > above will not push vcpus into "peripheral" container. They will appear
> > > in "unattached" container but with specified names and as-far-as I can
> > > see in the code 'device_del' can only delete objects/devices from the
> > > 'peripheral' container?  
> > 
> > qemu-system-x86_64 -monitor stdio \
> >     -smp 1,maxcpus=3 \
> >     -device qemu64-x86_64-cpu,id=foo,socket-id=1,core-id=0,thread-id=0 \
> >     -device qemu64-x86_64-cpu,socket-id=2,core-id=0,thread-id=0
> > 
> > (qemu) info hotpluggable-cpus
> > Hotpluggable CPUs:
> >   type: "qemu64-x86_64-cpu"
> >   vcpus_count: "1"
> >   qom_path: "/machine/peripheral-anon/device[0]"
> >                       ^^^^^^^^^^^^^^^
> >   CPUInstance Properties:
> >     socket-id: "2"
> >     core-id: "0"
> >     thread-id: "0"
> >   type: "qemu64-x86_64-cpu"
> >   vcpus_count: "1"
> >   qom_path: "/machine/peripheral/foo"
> >                       ^^^^^^^^^^
> > 
> > in gist, if device is created with any variant of device_add,
> > it goes to "peripheral[-anon]" including cold-plugged one.  
> 
> 
> I am not sure why you said "including cold-plugged one"? I hope by
> cold-plug'ging you mean here are the CPUs which already exist at
> the boot time of the Guest VM and plugged using -device?

yes, it's about a plugged one with  '-device'

 
> >   CPUInstance Properties:
> >     socket-id: "1"
> >     core-id: "0"
> >     thread-id: "0"
> >   type: "qemu64-x86_64-cpu"
> >   vcpus_count: "1"
> >   qom_path: "/machine/unattached/device[0]"  
>                           ^^^^^^^^^^^^^
> 
> Unless you have pasted wrongly here, above output also shows qom path
> as 'unattached' for cold-plugged CPUs. Am I missing something? :)
> 
> 
> >   CPUInstance Properties:
> >     socket-id: "0"
> >     core-id: "0"
> >     thread-id: "0"  
> 
> 
> 
> > > Plus, having those many ids specified for over large number of vcpus
> > > does not looks very practical solution. We need interface like auto  
> > number of IDs is not a problem since it's usually handled by management
> > software just fine (ex: libvirt does it)
> >   
> > > Generation of ids even at the boot time. I could see from the link you
> > > have shared that it is already being used by ID_BLOCK subsystem. Can we
> > > create a new subsystem for cpus under this and do the auto Generation
> > > of vcpu ids as well?  
> > 
> > I'm not sure that auto ids was actually merged.
> > (I thought it wasn't)  
> 
> 
> Well if you were referring to below then it has been part of qemu for quite
> long now:
> 
> Patch: util - add automated ID generation utility
> File: https://github.com/qemu/qemu/blob/master/util/id.c
> Commit-id: https://github.com/qemu/qemu/commit/a0f1913637e6

Thanks, I didn't know that it eventually got merged.

> 
> > Anyway auto IDs are not directly related to enabling CPU hotplug for ARM,
> > if you feel they should be generated you can try to propose patches.  
> 
> Sure. 
> 
> 
[...]

RE: [Question] Regarding containers "unattached/peripheral/anonymous" - their relation with hot(un)plug of devices

[Salil Mehta]

> From: Igor Mammedov [mailto:imammedo@redhat.com]
> Sent: Thursday, June 4, 2020 10:55 AM
> To: Salil Mehta <salil.mehta@huawei.com>
> Cc: Andrew Jones <drjones@redhat.com>; gshan@redhat.com; mst@redhat.com;
> qemu-devel@nongnu.org; eric.auger@redhat.com; qemu-arm@nongnu.org; pbonzini
> <pbonzini@redhat.com>
> Subject: Re: [Question] Regarding containers "unattached/peripheral/anonymous"
> - their relation with hot(un)plug of devices
> 
> On Wed, 3 Jun 2020 15:13:26 +0000
> Salil Mehta <salil.mehta@huawei.com> wrote:
> 

[...]

> >
> > I am not sure why you said "including cold-plugged one"? I hope by
> > cold-plug'ging you mean here are the CPUs which already exist at
> > the boot time of the Guest VM and plugged using -device?
> 
> yes, it's about a plugged one with  '-device'


Sure, thank you.

Salil.