[Question] x86/microvm: why has_hotpluggable_cpus = false but hot(ub)plug APIs exist?

[Question] x86/microvm: why has_hotpluggable_cpus = false but hot(ub)plug APIs exist?

[Salil Mehta]

Hello,

Came across below code excerpt in x86/microvm code and wanted to know 
why 'has_hotpluggable_cpus' flag has been set to 'false' while various 
hot(un)plug APIs have been defined?

static void microvm_class_init(ObjectClass *oc, void *data)
{
     X86MachineClass *x86mc = X86_MACHINE_CLASS(oc);
     MachineClass *mc = MACHINE_CLASS(oc);
     HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);

     mc->init = microvm_machine_state_init;

     mc->family = "microvm_i386";
     [...]
     mc->max_cpus = 288;
     mc->has_hotpluggable_cpus = false;  --------> This one
     [...]

     /* hotplug (for cpu coldplug) */
     mc->get_hotplug_handler = microvm_get_hotplug_handler;
     hc->pre_plug = microvm_device_pre_plug_cb;
     hc->plug = microvm_device_plug_cb;
     hc->unplug_request = microvm_device_unplug_request_cb;
     hc->unplug = microvm_device_unplug_cb;
     [...]
}


Best regards
Salil

Re: [Question] x86/microvm: why has_hotpluggable_cpus = false but hot(ub)plug APIs exist?

[Alex Bennée]

Salil Mehta <salil.mehta@opnsrc.net> writes:

> Hello,
>
> Came across below code excerpt in x86/microvm code and wanted to know
> why 'has_hotpluggable_cpus' flag has been set to 'false' while various
> hot(un)plug APIs have been defined?
>
> static void microvm_class_init(ObjectClass *oc, void *data)
> {
>     X86MachineClass *x86mc = X86_MACHINE_CLASS(oc);
>     MachineClass *mc = MACHINE_CLASS(oc);
>     HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
>
>     mc->init = microvm_machine_state_init;
>
>     mc->family = "microvm_i386";
>     [...]
>     mc->max_cpus = 288;
>     mc->has_hotpluggable_cpus = false;  --------> This one
>     [...]

From the original commit that added it:

  It's a minimalist machine type without PCI nor ACPI support, designed
  for short-lived guests. microvm also establishes a baseline for
  benchmarking and optimizing both QEMU and guest operating systems,
  since it is optimized for both boot time and footprint.

Generally hotplug requires a dance between the VMM and the firmware to
properly shutdown and restart hotplug devices. The principle
communication mechanism for this is ACPI.

>
>     /* hotplug (for cpu coldplug) */
>     mc->get_hotplug_handler = microvm_get_hotplug_handler;
>     hc->pre_plug = microvm_device_pre_plug_cb;
>     hc->plug = microvm_device_plug_cb;
>     hc->unplug_request = microvm_device_unplug_request_cb;
>     hc->unplug = microvm_device_unplug_cb;
>     [...]
> }
>
>
> Best regards
> Salil


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [Question] x86/microvm: why has_hotpluggable_cpus = false but hot(ub)plug APIs exist?

[Salil Mehta]

Hi Alex,

On 18/10/2023 16:41, Alex Bennée wrote:
> 
> Salil Mehta <salil.mehta@opnsrc.net> writes:
> 
>> Hello,
>>
>> Came across below code excerpt in x86/microvm code and wanted to know
>> why 'has_hotpluggable_cpus' flag has been set to 'false' while various
>> hot(un)plug APIs have been defined?
>>
>> static void microvm_class_init(ObjectClass *oc, void *data)
>> {
>>      X86MachineClass *x86mc = X86_MACHINE_CLASS(oc);
>>      MachineClass *mc = MACHINE_CLASS(oc);
>>      HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
>>
>>      mc->init = microvm_machine_state_init;
>>
>>      mc->family = "microvm_i386";
>>      [...]
>>      mc->max_cpus = 288;
>>      mc->has_hotpluggable_cpus = false;  --------> This one
>>      [...]
> 
>  From the original commit that added it:
> 
>    It's a minimalist machine type without PCI nor ACPI support, designed
>    for short-lived guests. microvm also establishes a baseline for
>    benchmarking and optimizing both QEMU and guest operating systems,
>    since it is optimized for both boot time and footprint.


Agreed. It looks like ACPI is supported but neither CPU/Memory Hotplug 
is supported for this minimalist machine type.


static void microvm_devices_init(MicrovmMachineState *mms)
{
     const char *default_firmware;
     X86MachineState *x86ms = X86_MACHINE(mms);

    [...]

     /* Optional and legacy devices */
     if (x86_machine_is_acpi_enabled(x86ms)) {
         DeviceState *dev = qdev_new(TYPE_ACPI_GED_X86);
         qdev_prop_set_uint32(dev, "ged-event", ACPI_GED_PWR_DOWN_EVT);
         sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, GED_MMIO_BASE);
      /* sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, GED_MMIO_BASE_MEMHP); */

         [...]

         sysbus_realize(SYS_BUS_DEVICE(dev), &error_fatal);
         x86ms->acpi_dev = HOTPLUG_HANDLER(dev);
     }
    [...]
}



> 
> Generally hotplug requires a dance between the VMM and the firmware to
> properly shutdown and restart hotplug devices. The principle
> communication mechanism for this is ACPI.

Agreed.



> 
>>
>>      /* hotplug (for cpu coldplug) */
>>      mc->get_hotplug_handler = microvm_get_hotplug_handler;
>>      hc->pre_plug = microvm_device_pre_plug_cb;
>>      hc->plug = microvm_device_plug_cb;
>>      hc->unplug_request = microvm_device_unplug_request_cb;
>>      hc->unplug = microvm_device_unplug_cb;

sorry, I also missed the definitions of the last 2 functions which says 
that unplug is not supported so perhaps these functions are only 
required to support cold plugging which corroborates with the comment as 
well.

Thanks
Salil.

Re: [Question] x86/microvm: why has_hotpluggable_cpus = false but hot(ub)plug APIs exist?

[Igor Mammedov]

On Wed, 18 Oct 2023 17:48:36 +0100
Salil Mehta <salil.mehta@opnsrc.net> wrote:

> Hi Alex,
> 
> On 18/10/2023 16:41, Alex Bennée wrote:
> > 
> > Salil Mehta <salil.mehta@opnsrc.net> writes:
> >   
> >> Hello,
> >>
> >> Came across below code excerpt in x86/microvm code and wanted to know
> >> why 'has_hotpluggable_cpus' flag has been set to 'false' while various
> >> hot(un)plug APIs have been defined?
> >>
> >> static void microvm_class_init(ObjectClass *oc, void *data)
> >> {
> >>      X86MachineClass *x86mc = X86_MACHINE_CLASS(oc);
> >>      MachineClass *mc = MACHINE_CLASS(oc);
> >>      HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
> >>
> >>      mc->init = microvm_machine_state_init;
> >>
> >>      mc->family = "microvm_i386";
> >>      [...]
> >>      mc->max_cpus = 288;
> >>      mc->has_hotpluggable_cpus = false;  --------> This one
> >>      [...]  
> > 
> >  From the original commit that added it:
> > 
> >    It's a minimalist machine type without PCI nor ACPI support, designed
> >    for short-lived guests. microvm also establishes a baseline for
> >    benchmarking and optimizing both QEMU and guest operating systems,
> >    since it is optimized for both boot time and footprint.  
> 
> 
> Agreed. It looks like ACPI is supported but neither CPU/Memory Hotplug 
> is supported for this minimalist machine type.
> 
> 
> static void microvm_devices_init(MicrovmMachineState *mms)
> {
>      const char *default_firmware;
>      X86MachineState *x86ms = X86_MACHINE(mms);
> 
>     [...]
> 
>      /* Optional and legacy devices */
>      if (x86_machine_is_acpi_enabled(x86ms)) {
>          DeviceState *dev = qdev_new(TYPE_ACPI_GED_X86);
>          qdev_prop_set_uint32(dev, "ged-event", ACPI_GED_PWR_DOWN_EVT);
>          sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, GED_MMIO_BASE);
>       /* sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, GED_MMIO_BASE_MEMHP); */
> 
>          [...]
> 
>          sysbus_realize(SYS_BUS_DEVICE(dev), &error_fatal);
>          x86ms->acpi_dev = HOTPLUG_HANDLER(dev);
>      }
>     [...]
> }
> 
> 
> 
> > 
> > Generally hotplug requires a dance between the VMM and the firmware to
> > properly shutdown and restart hotplug devices. The principle
> > communication mechanism for this is ACPI.  

firmware part in cpu/mem hoptlug usually provided by QEMU by the way of
ACPI tables (which may contain AML code that handles dance with QEMU
while exposing standard interface to guest OS to handle hotplug)

> 
> Agreed.
> 
> 
> 
> >   
> >>
> >>      /* hotplug (for cpu coldplug) */
> >>      mc->get_hotplug_handler = microvm_get_hotplug_handler;
> >>      hc->pre_plug = microvm_device_pre_plug_cb;
> >>      hc->plug = microvm_device_plug_cb;
> >>      hc->unplug_request = microvm_device_unplug_request_cb;
> >>      hc->unplug = microvm_device_unplug_cb;  
> 
> sorry, I also missed the definitions of the last 2 functions which says 
> that unplug is not supported so perhaps these functions are only 
> required to support cold plugging which corroborates with the comment as 
> well.

this function are usually used for both cold and hotplug of bus-less devices.
They provide an opt-in way for board to wire up such devices (incl. CPU).

> 
> Thanks
> Salil.
> 

RE: [Question] x86/microvm: why has_hotpluggable_cpus = false but hot(ub)plug APIs exist?

[Salil Mehta via]

Hi Igor,

> From: Igor Mammedov <imammedo@redhat.com>
> Sent: Tuesday, October 24, 2023 9:06 AM
> To: Salil Mehta <salil.mehta@opnsrc.net>
> 
> On Wed, 18 Oct 2023 17:48:36 +0100
> Salil Mehta <salil.mehta@opnsrc.net> wrote:
> 
> > Hi Alex,
> >
> > On 18/10/2023 16:41, Alex Bennée wrote:
> > >
> > > Salil Mehta <salil.mehta@opnsrc.net> writes:
> > >
> > >> Hello,
> > >>
> > >> Came across below code excerpt in x86/microvm code and wanted to know
> > >> why 'has_hotpluggable_cpus' flag has been set to 'false' while various
> > >> hot(un)plug APIs have been defined?
> > >>
> > >> static void microvm_class_init(ObjectClass *oc, void *data)
> > >> {
> > >>      X86MachineClass *x86mc = X86_MACHINE_CLASS(oc);
> > >>      MachineClass *mc = MACHINE_CLASS(oc);
> > >>      HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
> > >>
> > >>      mc->init = microvm_machine_state_init;
> > >>
> > >>      mc->family = "microvm_i386";
> > >>      [...]
> > >>      mc->max_cpus = 288;
> > >>      mc->has_hotpluggable_cpus = false;  --------> This one
> > >>      [...]
> > >
> > >  From the original commit that added it:
> > >
> > >    It's a minimalist machine type without PCI nor ACPI support, designed
> > >    for short-lived guests. microvm also establishes a baseline for
> > >    benchmarking and optimizing both QEMU and guest operating systems,
> > >    since it is optimized for both boot time and footprint.
> >
> >
> > Agreed. It looks like ACPI is supported but neither CPU/Memory Hotplug
> > is supported for this minimalist machine type.
> >
> >
> > static void microvm_devices_init(MicrovmMachineState *mms)
> > {
> >      const char *default_firmware;
> >      X86MachineState *x86ms = X86_MACHINE(mms);
> >
> >     [...]
> >
> >      /* Optional and legacy devices */
> >      if (x86_machine_is_acpi_enabled(x86ms)) {
> >          DeviceState *dev = qdev_new(TYPE_ACPI_GED_X86);
> >          qdev_prop_set_uint32(dev, "ged-event", ACPI_GED_PWR_DOWN_EVT);
> >          sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, GED_MMIO_BASE);
> >       /* sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, GED_MMIO_BASE_MEMHP); */
> >
> >          [...]
> >
> >          sysbus_realize(SYS_BUS_DEVICE(dev), &error_fatal);
> >          x86ms->acpi_dev = HOTPLUG_HANDLER(dev);
> >      }
> >     [...]
> > }
> >
> > >
> > > Generally hotplug requires a dance between the VMM and the firmware to
> > > properly shutdown and restart hotplug devices. The principle
> > > communication mechanism for this is ACPI.
> 
> firmware part in cpu/mem hoptlug usually provided by QEMU by the way of
> ACPI tables (which may contain AML code that handles dance with QEMU
> while exposing standard interface to guest OS to handle hotplug)
> 
> >
> > Agreed.
> >
> > >>      /* hotplug (for cpu coldplug) */
> > >>      mc->get_hotplug_handler = microvm_get_hotplug_handler;
> > >>      hc->pre_plug = microvm_device_pre_plug_cb;
> > >>      hc->plug = microvm_device_plug_cb;
> > >>      hc->unplug_request = microvm_device_unplug_request_cb;
> > >>      hc->unplug = microvm_device_unplug_cb;
> >
> > sorry, I also missed the definitions of the last 2 functions which says
> > that unplug is not supported so perhaps these functions are only
> > required to support cold plugging which corroborates with the comment as
> > well.
> 
> this function are usually used for both cold and hotplug of bus-less devices.
> They provide an opt-in way for board to wire up such devices (incl. CPU).


Sure. I understand but microvm does not support hotplug so presence of
unplug{_request} versions brought a doubt in my mind but I realized later
that their definitions were empty. Cold-plug does not require unplug
versions. 

Was there any plan to support hot-plug with microvm in future?


Thanks
Salil.

RE: [Question] x86/microvm: why has_hotpluggable_cpus = false but hot(ub)plug APIs exist?

[Salil Mehta]

Hi Igor,

> From: Igor Mammedov <imammedo@redhat.com>
> Sent: Tuesday, October 24, 2023 9:06 AM
> To: Salil Mehta <salil.mehta@opnsrc.net>
> 
> On Wed, 18 Oct 2023 17:48:36 +0100
> Salil Mehta <salil.mehta@opnsrc.net> wrote:
> 
> > Hi Alex,
> >
> > On 18/10/2023 16:41, Alex Bennée wrote:
> > >
> > > Salil Mehta <salil.mehta@opnsrc.net> writes:
> > >
> > >> Hello,
> > >>
> > >> Came across below code excerpt in x86/microvm code and wanted to know
> > >> why 'has_hotpluggable_cpus' flag has been set to 'false' while various
> > >> hot(un)plug APIs have been defined?
> > >>
> > >> static void microvm_class_init(ObjectClass *oc, void *data)
> > >> {
> > >>      X86MachineClass *x86mc = X86_MACHINE_CLASS(oc);
> > >>      MachineClass *mc = MACHINE_CLASS(oc);
> > >>      HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
> > >>
> > >>      mc->init = microvm_machine_state_init;
> > >>
> > >>      mc->family = "microvm_i386";
> > >>      [...]
> > >>      mc->max_cpus = 288;
> > >>      mc->has_hotpluggable_cpus = false;  --------> This one
> > >>      [...]
> > >
> > >  From the original commit that added it:
> > >
> > >    It's a minimalist machine type without PCI nor ACPI support, designed
> > >    for short-lived guests. microvm also establishes a baseline for
> > >    benchmarking and optimizing both QEMU and guest operating systems,
> > >    since it is optimized for both boot time and footprint.
> >
> >
> > Agreed. It looks like ACPI is supported but neither CPU/Memory Hotplug
> > is supported for this minimalist machine type.
> >
> >
> > static void microvm_devices_init(MicrovmMachineState *mms)
> > {
> >      const char *default_firmware;
> >      X86MachineState *x86ms = X86_MACHINE(mms);
> >
> >     [...]
> >
> >      /* Optional and legacy devices */
> >      if (x86_machine_is_acpi_enabled(x86ms)) {
> >          DeviceState *dev = qdev_new(TYPE_ACPI_GED_X86);
> >          qdev_prop_set_uint32(dev, "ged-event", ACPI_GED_PWR_DOWN_EVT);
> >          sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, GED_MMIO_BASE);
> >       /* sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, GED_MMIO_BASE_MEMHP); */
> >
> >          [...]
> >
> >          sysbus_realize(SYS_BUS_DEVICE(dev), &error_fatal);
> >          x86ms->acpi_dev = HOTPLUG_HANDLER(dev);
> >      }
> >     [...]
> > }
> >
> > >
> > > Generally hotplug requires a dance between the VMM and the firmware to
> > > properly shutdown and restart hotplug devices. The principle
> > > communication mechanism for this is ACPI.
> 
> firmware part in cpu/mem hoptlug usually provided by QEMU by the way of
> ACPI tables (which may contain AML code that handles dance with QEMU
> while exposing standard interface to guest OS to handle hotplug)
> 
> >
> > Agreed.
> >
> > >>      /* hotplug (for cpu coldplug) */
> > >>      mc->get_hotplug_handler = microvm_get_hotplug_handler;
> > >>      hc->pre_plug = microvm_device_pre_plug_cb;
> > >>      hc->plug = microvm_device_plug_cb;
> > >>      hc->unplug_request = microvm_device_unplug_request_cb;
> > >>      hc->unplug = microvm_device_unplug_cb;
> >
> > sorry, I also missed the definitions of the last 2 functions which says
> > that unplug is not supported so perhaps these functions are only
> > required to support cold plugging which corroborates with the comment as
> > well.
> 
> this function are usually used for both cold and hotplug of bus-less devices.
> They provide an opt-in way for board to wire up such devices (incl. CPU).


Sure. I understand but microvm does not support hotplug so presence of
unplug{_request} versions brought a doubt in my mind but I realized later
that their definitions were empty. Cold-plug does not require unplug
versions. 

Was there any plan to support hot-plug with microvm in future?


Thanks
Salil.

Re: [Question] x86/microvm: why has_hotpluggable_cpus = false but hot(ub)plug APIs exist?

[David Hildenbrand]

On 25.10.23 11:16, Salil Mehta wrote:
> Hi Igor,
> 
>> From: Igor Mammedov <imammedo@redhat.com>
>> Sent: Tuesday, October 24, 2023 9:06 AM
>> To: Salil Mehta <salil.mehta@opnsrc.net>
>>
>> On Wed, 18 Oct 2023 17:48:36 +0100
>> Salil Mehta <salil.mehta@opnsrc.net> wrote:
>>
>>> Hi Alex,
>>>
>>> On 18/10/2023 16:41, Alex Bennée wrote:
>>>>
>>>> Salil Mehta <salil.mehta@opnsrc.net> writes:
>>>>
>>>>> Hello,
>>>>>
>>>>> Came across below code excerpt in x86/microvm code and wanted to know
>>>>> why 'has_hotpluggable_cpus' flag has been set to 'false' while various
>>>>> hot(un)plug APIs have been defined?
>>>>>
>>>>> static void microvm_class_init(ObjectClass *oc, void *data)
>>>>> {
>>>>>       X86MachineClass *x86mc = X86_MACHINE_CLASS(oc);
>>>>>       MachineClass *mc = MACHINE_CLASS(oc);
>>>>>       HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
>>>>>
>>>>>       mc->init = microvm_machine_state_init;
>>>>>
>>>>>       mc->family = "microvm_i386";
>>>>>       [...]
>>>>>       mc->max_cpus = 288;
>>>>>       mc->has_hotpluggable_cpus = false;  --------> This one
>>>>>       [...]
>>>>
>>>>   From the original commit that added it:
>>>>
>>>>     It's a minimalist machine type without PCI nor ACPI support, designed
>>>>     for short-lived guests. microvm also establishes a baseline for
>>>>     benchmarking and optimizing both QEMU and guest operating systems,
>>>>     since it is optimized for both boot time and footprint.
>>>
>>>
>>> Agreed. It looks like ACPI is supported but neither CPU/Memory Hotplug
>>> is supported for this minimalist machine type.
>>>
>>>
>>> static void microvm_devices_init(MicrovmMachineState *mms)
>>> {
>>>       const char *default_firmware;
>>>       X86MachineState *x86ms = X86_MACHINE(mms);
>>>
>>>      [...]
>>>
>>>       /* Optional and legacy devices */
>>>       if (x86_machine_is_acpi_enabled(x86ms)) {
>>>           DeviceState *dev = qdev_new(TYPE_ACPI_GED_X86);
>>>           qdev_prop_set_uint32(dev, "ged-event", ACPI_GED_PWR_DOWN_EVT);
>>>           sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, GED_MMIO_BASE);
>>>        /* sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, GED_MMIO_BASE_MEMHP); */
>>>
>>>           [...]
>>>
>>>           sysbus_realize(SYS_BUS_DEVICE(dev), &error_fatal);
>>>           x86ms->acpi_dev = HOTPLUG_HANDLER(dev);
>>>       }
>>>      [...]
>>> }
>>>
>>>>
>>>> Generally hotplug requires a dance between the VMM and the firmware to
>>>> properly shutdown and restart hotplug devices. The principle
>>>> communication mechanism for this is ACPI.
>>
>> firmware part in cpu/mem hoptlug usually provided by QEMU by the way of
>> ACPI tables (which may contain AML code that handles dance with QEMU
>> while exposing standard interface to guest OS to handle hotplug)
>>
>>>
>>> Agreed.
>>>
>>>>>       /* hotplug (for cpu coldplug) */
>>>>>       mc->get_hotplug_handler = microvm_get_hotplug_handler;
>>>>>       hc->pre_plug = microvm_device_pre_plug_cb;
>>>>>       hc->plug = microvm_device_plug_cb;
>>>>>       hc->unplug_request = microvm_device_unplug_request_cb;
>>>>>       hc->unplug = microvm_device_unplug_cb;
>>>
>>> sorry, I also missed the definitions of the last 2 functions which says
>>> that unplug is not supported so perhaps these functions are only
>>> required to support cold plugging which corroborates with the comment as
>>> well.
>>
>> this function are usually used for both cold and hotplug of bus-less devices.
>> They provide an opt-in way for board to wire up such devices (incl. CPU).
> 
> 
> Sure. I understand but microvm does not support hotplug so presence of
> unplug{_request} versions brought a doubt in my mind but I realized later
> that their definitions were empty. Cold-plug does not require unplug
> versions.
> 
> Was there any plan to support hot-plug with microvm in future?

At least virtio-mem for memory hotplug should be fairly straight-forward 
to wire-up I guess. The relation to ACPI are minimal: we currently only 
use ACPI SRAT to expose the maximum GPA range that e.g., Linux requires 
early during boot to properly prepare for memory hotplug (size the 
virtual memory space for the kernel accordingly). One could use 
alternative (paravirtualized) interfaces for that.

The question is whether any form of hotplug is "in the spirit" of microvm.

-- 
Cheers,

David / dhildenb

RE: [Question] x86/microvm: why has_hotpluggable_cpus = false but hot(ub)plug APIs exist?

[Salil Mehta via]

> From: David Hildenbrand <david@redhat.com>
> Sent: Wednesday, October 25, 2023 10:32 AM
> To: Salil Mehta <salil.mehta@huawei.com>; Igor Mammedov
> <imammedo@redhat.com>; Salil Mehta <salil.mehta@opnsrc.net>
> 
> On 25.10.23 11:16, Salil Mehta wrote:
> > Hi Igor,
> >
> >> From: Igor Mammedov <imammedo@redhat.com>
> >> Sent: Tuesday, October 24, 2023 9:06 AM
> >> To: Salil Mehta <salil.mehta@opnsrc.net>
> >>
> >> On Wed, 18 Oct 2023 17:48:36 +0100
> >> Salil Mehta <salil.mehta@opnsrc.net> wrote:
> >>
> >>> Hi Alex,
> >>>
> >>> On 18/10/2023 16:41, Alex Bennée wrote:
> >>>>
> >>>> Salil Mehta <salil.mehta@opnsrc.net> writes:
> >>>>
> >>>>> Hello,
> >>>>>
> >>>>> Came across below code excerpt in x86/microvm code and wanted to know
> >>>>> why 'has_hotpluggable_cpus' flag has been set to 'false' while various
> >>>>> hot(un)plug APIs have been defined?
> >>>>>
> >>>>> static void microvm_class_init(ObjectClass *oc, void *data)
> >>>>> {
> >>>>>       X86MachineClass *x86mc = X86_MACHINE_CLASS(oc);
> >>>>>       MachineClass *mc = MACHINE_CLASS(oc);
> >>>>>       HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
> >>>>>
> >>>>>       mc->init = microvm_machine_state_init;
> >>>>>
> >>>>>       mc->family = "microvm_i386";
> >>>>>       [...]
> >>>>>       mc->max_cpus = 288;
> >>>>>       mc->has_hotpluggable_cpus = false;  --------> This one
> >>>>>       [...]
> >>>>
> >>>>   From the original commit that added it:
> >>>>
> >>>>     It's a minimalist machine type without PCI nor ACPI support, designed
> >>>>     for short-lived guests. microvm also establishes a baseline for
> >>>>     benchmarking and optimizing both QEMU and guest operating systems,
> >>>>     since it is optimized for both boot time and footprint.
> >>>
> >>>
> >>> Agreed. It looks like ACPI is supported but neither CPU/Memory Hotplug
> >>> is supported for this minimalist machine type.
> >>>
> >>>
> >>> static void microvm_devices_init(MicrovmMachineState *mms)
> >>> {
> >>>       const char *default_firmware;
> >>>       X86MachineState *x86ms = X86_MACHINE(mms);
> >>>
> >>>      [...]
> >>>
> >>>       /* Optional and legacy devices */
> >>>       if (x86_machine_is_acpi_enabled(x86ms)) {
> >>>           DeviceState *dev = qdev_new(TYPE_ACPI_GED_X86);
> >>>           qdev_prop_set_uint32(dev, "ged-event", ACPI_GED_PWR_DOWN_EVT);
> >>>           sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, GED_MMIO_BASE);
> >>>        /* sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, GED_MMIO_BASE_MEMHP);
> */
> >>>
> >>>           [...]
> >>>
> >>>           sysbus_realize(SYS_BUS_DEVICE(dev), &error_fatal);
> >>>           x86ms->acpi_dev = HOTPLUG_HANDLER(dev);
> >>>       }
> >>>      [...]
> >>> }
> >>>
> >>>>
> >>>> Generally hotplug requires a dance between the VMM and the firmware to
> >>>> properly shutdown and restart hotplug devices. The principle
> >>>> communication mechanism for this is ACPI.
> >>
> >> firmware part in cpu/mem hoptlug usually provided by QEMU by the way of
> >> ACPI tables (which may contain AML code that handles dance with QEMU
> >> while exposing standard interface to guest OS to handle hotplug)
> >>
> >>>
> >>> Agreed.
> >>>
> >>>>>       /* hotplug (for cpu coldplug) */
> >>>>>       mc->get_hotplug_handler = microvm_get_hotplug_handler;
> >>>>>       hc->pre_plug = microvm_device_pre_plug_cb;
> >>>>>       hc->plug = microvm_device_plug_cb;
> >>>>>       hc->unplug_request = microvm_device_unplug_request_cb;
> >>>>>       hc->unplug = microvm_device_unplug_cb;
> >>>
> >>> sorry, I also missed the definitions of the last 2 functions which says
> >>> that unplug is not supported so perhaps these functions are only
> >>> required to support cold plugging which corroborates with the comment as
> >>> well.
> >>
> >> this function are usually used for both cold and hotplug of bus-less devices.
> >> They provide an opt-in way for board to wire up such devices (incl. CPU).
> >
> >
> > Sure. I understand but microvm does not support hotplug so presence of
> > unplug{_request} versions brought a doubt in my mind but I realized later
> > that their definitions were empty. Cold-plug does not require unplug
> > versions.
> >
> > Was there any plan to support hot-plug with microvm in future?
> 
> At least virtio-mem for memory hotplug should be fairly straight-forward
> to wire-up I guess. The relation to ACPI are minimal: we currently only
> use ACPI SRAT to expose the maximum GPA range that e.g., Linux requires
> early during boot to properly prepare for memory hotplug (size the
> virtual memory space for the kernel accordingly). One could use
> alternative (paravirtualized) interfaces for that.

Ok. Light weight VM more in lines of Firecracker to improve boot-up times?

Also, presence of unplug versions gives a wrong impression about code?

> The question is whether any form of hotplug is "in the spirit" of microvm.

Understand that. BTW, was it ever made to be used with kata-containers?

Thanks
Salil.

RE: [Question] x86/microvm: why has_hotpluggable_cpus = false but hot(ub)plug APIs exist?

[Salil Mehta]

> From: David Hildenbrand <david@redhat.com>
> Sent: Wednesday, October 25, 2023 10:32 AM
> To: Salil Mehta <salil.mehta@huawei.com>; Igor Mammedov
> <imammedo@redhat.com>; Salil Mehta <salil.mehta@opnsrc.net>
> 
> On 25.10.23 11:16, Salil Mehta wrote:
> > Hi Igor,
> >
> >> From: Igor Mammedov <imammedo@redhat.com>
> >> Sent: Tuesday, October 24, 2023 9:06 AM
> >> To: Salil Mehta <salil.mehta@opnsrc.net>
> >>
> >> On Wed, 18 Oct 2023 17:48:36 +0100
> >> Salil Mehta <salil.mehta@opnsrc.net> wrote:
> >>
> >>> Hi Alex,
> >>>
> >>> On 18/10/2023 16:41, Alex Bennée wrote:
> >>>>
> >>>> Salil Mehta <salil.mehta@opnsrc.net> writes:
> >>>>
> >>>>> Hello,
> >>>>>
> >>>>> Came across below code excerpt in x86/microvm code and wanted to know
> >>>>> why 'has_hotpluggable_cpus' flag has been set to 'false' while various
> >>>>> hot(un)plug APIs have been defined?
> >>>>>
> >>>>> static void microvm_class_init(ObjectClass *oc, void *data)
> >>>>> {
> >>>>>       X86MachineClass *x86mc = X86_MACHINE_CLASS(oc);
> >>>>>       MachineClass *mc = MACHINE_CLASS(oc);
> >>>>>       HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
> >>>>>
> >>>>>       mc->init = microvm_machine_state_init;
> >>>>>
> >>>>>       mc->family = "microvm_i386";
> >>>>>       [...]
> >>>>>       mc->max_cpus = 288;
> >>>>>       mc->has_hotpluggable_cpus = false;  --------> This one
> >>>>>       [...]
> >>>>
> >>>>   From the original commit that added it:
> >>>>
> >>>>     It's a minimalist machine type without PCI nor ACPI support, designed
> >>>>     for short-lived guests. microvm also establishes a baseline for
> >>>>     benchmarking and optimizing both QEMU and guest operating systems,
> >>>>     since it is optimized for both boot time and footprint.
> >>>
> >>>
> >>> Agreed. It looks like ACPI is supported but neither CPU/Memory Hotplug
> >>> is supported for this minimalist machine type.
> >>>
> >>>
> >>> static void microvm_devices_init(MicrovmMachineState *mms)
> >>> {
> >>>       const char *default_firmware;
> >>>       X86MachineState *x86ms = X86_MACHINE(mms);
> >>>
> >>>      [...]
> >>>
> >>>       /* Optional and legacy devices */
> >>>       if (x86_machine_is_acpi_enabled(x86ms)) {
> >>>           DeviceState *dev = qdev_new(TYPE_ACPI_GED_X86);
> >>>           qdev_prop_set_uint32(dev, "ged-event", ACPI_GED_PWR_DOWN_EVT);
> >>>           sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, GED_MMIO_BASE);
> >>>        /* sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, GED_MMIO_BASE_MEMHP);
> */
> >>>
> >>>           [...]
> >>>
> >>>           sysbus_realize(SYS_BUS_DEVICE(dev), &error_fatal);
> >>>           x86ms->acpi_dev = HOTPLUG_HANDLER(dev);
> >>>       }
> >>>      [...]
> >>> }
> >>>
> >>>>
> >>>> Generally hotplug requires a dance between the VMM and the firmware to
> >>>> properly shutdown and restart hotplug devices. The principle
> >>>> communication mechanism for this is ACPI.
> >>
> >> firmware part in cpu/mem hoptlug usually provided by QEMU by the way of
> >> ACPI tables (which may contain AML code that handles dance with QEMU
> >> while exposing standard interface to guest OS to handle hotplug)
> >>
> >>>
> >>> Agreed.
> >>>
> >>>>>       /* hotplug (for cpu coldplug) */
> >>>>>       mc->get_hotplug_handler = microvm_get_hotplug_handler;
> >>>>>       hc->pre_plug = microvm_device_pre_plug_cb;
> >>>>>       hc->plug = microvm_device_plug_cb;
> >>>>>       hc->unplug_request = microvm_device_unplug_request_cb;
> >>>>>       hc->unplug = microvm_device_unplug_cb;
> >>>
> >>> sorry, I also missed the definitions of the last 2 functions which says
> >>> that unplug is not supported so perhaps these functions are only
> >>> required to support cold plugging which corroborates with the comment as
> >>> well.
> >>
> >> this function are usually used for both cold and hotplug of bus-less devices.
> >> They provide an opt-in way for board to wire up such devices (incl. CPU).
> >
> >
> > Sure. I understand but microvm does not support hotplug so presence of
> > unplug{_request} versions brought a doubt in my mind but I realized later
> > that their definitions were empty. Cold-plug does not require unplug
> > versions.
> >
> > Was there any plan to support hot-plug with microvm in future?
> 
> At least virtio-mem for memory hotplug should be fairly straight-forward
> to wire-up I guess. The relation to ACPI are minimal: we currently only
> use ACPI SRAT to expose the maximum GPA range that e.g., Linux requires
> early during boot to properly prepare for memory hotplug (size the
> virtual memory space for the kernel accordingly). One could use
> alternative (paravirtualized) interfaces for that.

Ok. Light weight VM more in lines of Firecracker to improve boot-up times?

Also, presence of unplug versions gives a wrong impression about code?

> The question is whether any form of hotplug is "in the spirit" of microvm.

Understand that. BTW, was it ever made to be used with kata-containers?

Thanks
Salil.

Re: [Question] x86/microvm: why has_hotpluggable_cpus = false but hot(ub)plug APIs exist?

[Igor Mammedov]

On Wed, 25 Oct 2023 09:54:07 +0000
Salil Mehta <salil.mehta@huawei.com> wrote:

> > From: David Hildenbrand <david@redhat.com>
> > Sent: Wednesday, October 25, 2023 10:32 AM
> > To: Salil Mehta <salil.mehta@huawei.com>; Igor Mammedov
> > <imammedo@redhat.com>; Salil Mehta <salil.mehta@opnsrc.net>
> > 
> > On 25.10.23 11:16, Salil Mehta wrote:  
> > > Hi Igor,
> > >  
> > >> From: Igor Mammedov <imammedo@redhat.com>
> > >> Sent: Tuesday, October 24, 2023 9:06 AM
> > >> To: Salil Mehta <salil.mehta@opnsrc.net>
> > >>
> > >> On Wed, 18 Oct 2023 17:48:36 +0100
> > >> Salil Mehta <salil.mehta@opnsrc.net> wrote:
> > >>  
> > >>> Hi Alex,
> > >>>
> > >>> On 18/10/2023 16:41, Alex Bennée wrote:  
> > >>>>
> > >>>> Salil Mehta <salil.mehta@opnsrc.net> writes:
> > >>>>  
> > >>>>> Hello,
> > >>>>>
> > >>>>> Came across below code excerpt in x86/microvm code and wanted to know
> > >>>>> why 'has_hotpluggable_cpus' flag has been set to 'false' while various
> > >>>>> hot(un)plug APIs have been defined?
> > >>>>>
> > >>>>> static void microvm_class_init(ObjectClass *oc, void *data)
> > >>>>> {
> > >>>>>       X86MachineClass *x86mc = X86_MACHINE_CLASS(oc);
> > >>>>>       MachineClass *mc = MACHINE_CLASS(oc);
> > >>>>>       HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
> > >>>>>
> > >>>>>       mc->init = microvm_machine_state_init;
> > >>>>>
> > >>>>>       mc->family = "microvm_i386";
> > >>>>>       [...]
> > >>>>>       mc->max_cpus = 288;
> > >>>>>       mc->has_hotpluggable_cpus = false;  --------> This one
> > >>>>>       [...]  
> > >>>>
> > >>>>   From the original commit that added it:
> > >>>>
> > >>>>     It's a minimalist machine type without PCI nor ACPI support, designed
> > >>>>     for short-lived guests. microvm also establishes a baseline for
> > >>>>     benchmarking and optimizing both QEMU and guest operating systems,
> > >>>>     since it is optimized for both boot time and footprint.  
> > >>>
> > >>>
> > >>> Agreed. It looks like ACPI is supported but neither CPU/Memory Hotplug
> > >>> is supported for this minimalist machine type.
> > >>>
> > >>>
> > >>> static void microvm_devices_init(MicrovmMachineState *mms)
> > >>> {
> > >>>       const char *default_firmware;
> > >>>       X86MachineState *x86ms = X86_MACHINE(mms);
> > >>>
> > >>>      [...]
> > >>>
> > >>>       /* Optional and legacy devices */
> > >>>       if (x86_machine_is_acpi_enabled(x86ms)) {
> > >>>           DeviceState *dev = qdev_new(TYPE_ACPI_GED_X86);
> > >>>           qdev_prop_set_uint32(dev, "ged-event", ACPI_GED_PWR_DOWN_EVT);
> > >>>           sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, GED_MMIO_BASE);
> > >>>        /* sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, GED_MMIO_BASE_MEMHP);  
> > */  
> > >>>
> > >>>           [...]
> > >>>
> > >>>           sysbus_realize(SYS_BUS_DEVICE(dev), &error_fatal);
> > >>>           x86ms->acpi_dev = HOTPLUG_HANDLER(dev);
> > >>>       }
> > >>>      [...]
> > >>> }
> > >>>  
> > >>>>
> > >>>> Generally hotplug requires a dance between the VMM and the firmware to
> > >>>> properly shutdown and restart hotplug devices. The principle
> > >>>> communication mechanism for this is ACPI.  
> > >>
> > >> firmware part in cpu/mem hoptlug usually provided by QEMU by the way of
> > >> ACPI tables (which may contain AML code that handles dance with QEMU
> > >> while exposing standard interface to guest OS to handle hotplug)
> > >>  
> > >>>
> > >>> Agreed.
> > >>>  
> > >>>>>       /* hotplug (for cpu coldplug) */
> > >>>>>       mc->get_hotplug_handler = microvm_get_hotplug_handler;
> > >>>>>       hc->pre_plug = microvm_device_pre_plug_cb;
> > >>>>>       hc->plug = microvm_device_plug_cb;
> > >>>>>       hc->unplug_request = microvm_device_unplug_request_cb;
> > >>>>>       hc->unplug = microvm_device_unplug_cb;  
> > >>>
> > >>> sorry, I also missed the definitions of the last 2 functions which says
> > >>> that unplug is not supported so perhaps these functions are only
> > >>> required to support cold plugging which corroborates with the comment as
> > >>> well.  
> > >>
> > >> this function are usually used for both cold and hotplug of bus-less devices.
> > >> They provide an opt-in way for board to wire up such devices (incl. CPU).  
> > >
> > >
> > > Sure. I understand but microvm does not support hotplug so presence of
> > > unplug{_request} versions brought a doubt in my mind but I realized later
> > > that their definitions were empty. Cold-plug does not require unplug
> > > versions.
> > >
> > > Was there any plan to support hot-plug with microvm in future?  
> > 
> > At least virtio-mem for memory hotplug should be fairly straight-forward
> > to wire-up I guess. The relation to ACPI are minimal: we currently only
> > use ACPI SRAT to expose the maximum GPA range that e.g., Linux requires
> > early during boot to properly prepare for memory hotplug (size the
> > virtual memory space for the kernel accordingly). One could use
> > alternative (paravirtualized) interfaces for that.  
> 
> Ok. Light weight VM more in lines of Firecracker to improve boot-up times?
> 
> Also, presence of unplug versions gives a wrong impression about code?

unplug handlers could be theoretically used to undo what (pre)plug did
during VM deconstruction. (though it's not used this way today unless
hotplug controller requested it).

What is confusing is interface naming (HotplugHandler) which is not
hotplug limited anymore.
 
> > The question is whether any form of hotplug is "in the spirit" of microvm.  
> 
> Understand that. BTW, was it ever made to be used with kata-containers?
> 
> Thanks
> Salil.

RE: [Question] x86/microvm: why has_hotpluggable_cpus = false but hot(ub)plug APIs exist?

[Salil Mehta via]

> From: Igor Mammedov <imammedo@redhat.com>
> Sent: Thursday, October 26, 2023 1:09 PM
> To: Salil Mehta <salil.mehta@huawei.com>
> 
> On Wed, 25 Oct 2023 09:54:07 +0000
> Salil Mehta <salil.mehta@huawei.com> wrote:
> 
> > > From: David Hildenbrand <david@redhat.com>
> > > Sent: Wednesday, October 25, 2023 10:32 AM
> > > To: Salil Mehta <salil.mehta@huawei.com>; Igor Mammedov
> > > <imammedo@redhat.com>; Salil Mehta <salil.mehta@opnsrc.net>
> > >
> > > On 25.10.23 11:16, Salil Mehta wrote:
> > > > Hi Igor,
> > > >
> > > >> From: Igor Mammedov <imammedo@redhat.com>
> > > >> Sent: Tuesday, October 24, 2023 9:06 AM
> > > >> To: Salil Mehta <salil.mehta@opnsrc.net>
> > > >>
> > > >> On Wed, 18 Oct 2023 17:48:36 +0100
> > > >> Salil Mehta <salil.mehta@opnsrc.net> wrote:
> > > >>
> > > >>> Hi Alex,
> > > >>>
> > > >>> On 18/10/2023 16:41, Alex Bennée wrote:
> > > >>>>
> > > >>>> Salil Mehta <salil.mehta@opnsrc.net> writes:
> > > >>>>
> > > >>>>> Hello,
> > > >>>>>
> > > >>>>> Came across below code excerpt in x86/microvm code and wanted to know
> > > >>>>> why 'has_hotpluggable_cpus' flag has been set to 'false' while various
> > > >>>>> hot(un)plug APIs have been defined?
> > > >>>>>
> > > >>>>> static void microvm_class_init(ObjectClass *oc, void *data)
> > > >>>>> {
> > > >>>>>       X86MachineClass *x86mc = X86_MACHINE_CLASS(oc);
> > > >>>>>       MachineClass *mc = MACHINE_CLASS(oc);
> > > >>>>>       HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
> > > >>>>>
> > > >>>>>       mc->init = microvm_machine_state_init;
> > > >>>>>
> > > >>>>>       mc->family = "microvm_i386";
> > > >>>>>       [...]
> > > >>>>>       mc->max_cpus = 288;
> > > >>>>>       mc->has_hotpluggable_cpus = false;  --------> This one
> > > >>>>>       [...]
> > > >>>>
> > > >>>>   From the original commit that added it:
> > > >>>>
> > > >>>>     It's a minimalist machine type without PCI nor ACPI support, designed
> > > >>>>     for short-lived guests. microvm also establishes a baseline for
> > > >>>>     benchmarking and optimizing both QEMU and guest operating systems,
> > > >>>>     since it is optimized for both boot time and footprint.
> > > >>>
> > > >>>
> > > >>> Agreed. It looks like ACPI is supported but neither CPU/Memory Hotplug
> > > >>> is supported for this minimalist machine type.
> > > >>>
> > > >>>
> > > >>> static void microvm_devices_init(MicrovmMachineState *mms)
> > > >>> {
> > > >>>       const char *default_firmware;
> > > >>>       X86MachineState *x86ms = X86_MACHINE(mms);
> > > >>>
> > > >>>      [...]
> > > >>>
> > > >>>       /* Optional and legacy devices */
> > > >>>       if (x86_machine_is_acpi_enabled(x86ms)) {
> > > >>>           DeviceState *dev = qdev_new(TYPE_ACPI_GED_X86);
> > > >>>           qdev_prop_set_uint32(dev, "ged-event", ACPI_GED_PWR_DOWN_EVT);
> > > >>>           sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, GED_MMIO_BASE);
> > > >>>        /* sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, GED_MMIO_BASE_MEMHP);
> > > */
> > > >>>
> > > >>>           [...]
> > > >>>
> > > >>>           sysbus_realize(SYS_BUS_DEVICE(dev), &error_fatal);
> > > >>>           x86ms->acpi_dev = HOTPLUG_HANDLER(dev);
> > > >>>       }
> > > >>>      [...]
> > > >>> }
> > > >>>
> > > >>>>
> > > >>>> Generally hotplug requires a dance between the VMM and the firmware to
> > > >>>> properly shutdown and restart hotplug devices. The principle
> > > >>>> communication mechanism for this is ACPI.
> > > >>
> > > >> firmware part in cpu/mem hoptlug usually provided by QEMU by the way of
> > > >> ACPI tables (which may contain AML code that handles dance with QEMU
> > > >> while exposing standard interface to guest OS to handle hotplug)
> > > >>
> > > >>>
> > > >>> Agreed.
> > > >>>
> > > >>>>>       /* hotplug (for cpu coldplug) */
> > > >>>>>       mc->get_hotplug_handler = microvm_get_hotplug_handler;
> > > >>>>>       hc->pre_plug = microvm_device_pre_plug_cb;
> > > >>>>>       hc->plug = microvm_device_plug_cb;
> > > >>>>>       hc->unplug_request = microvm_device_unplug_request_cb;
> > > >>>>>       hc->unplug = microvm_device_unplug_cb;
> > > >>>
> > > >>> sorry, I also missed the definitions of the last 2 functions which says
> > > >>> that unplug is not supported so perhaps these functions are only
> > > >>> required to support cold plugging which corroborates with the comment as
> > > >>> well.
> > > >>
> > > >> this function are usually used for both cold and hotplug of bus-less devices.
> > > >> They provide an opt-in way for board to wire up such devices (incl. CPU).
> > > >
> > > >
> > > > Sure. I understand but microvm does not support hotplug so presence of
> > > > unplug{_request} versions brought a doubt in my mind but I realized later
> > > > that their definitions were empty. Cold-plug does not require unplug
> > > > versions.
> > > >
> > > > Was there any plan to support hot-plug with microvm in future?
> > >
> > > At least virtio-mem for memory hotplug should be fairly straight- forward
> > > to wire-up I guess. The relation to ACPI are minimal: we currently only
> > > use ACPI SRAT to expose the maximum GPA range that e.g., Linux requires
> > > early during boot to properly prepare for memory hotplug (size the
> > > virtual memory space for the kernel accordingly). One could use
> > > alternative (paravirtualized) interfaces for that.
> >
> > Ok. Light weight VM more in lines of Firecracker to improve boot-up times?
> >
> > Also, presence of unplug versions gives a wrong impression about code?
> 
> unplug handlers could be theoretically used to undo what (pre)plug did
> during VM deconstruction. (though it's not used this way today unless
> hotplug controller requested it).


I understand the intent but it would be better to remove these and bring
then back again if they are required in future. 

  "/* hotplug (for cpu coldplug) */"

Above comment and the code are not exactly consistent even though
definitions of the {request_}unplug are empty.


> What is confusing is interface naming (HotplugHandler) which is not
> hotplug limited anymore.

{pre_}plug routines are common to {hot,cold}-plugged CPUs, therefore, it
makes sense to have them here but I am not sure it is necessary to have
{request_}unplug routines as part of this implementation. Absolutely, it
makes sense to have them part of the design/framework for future
extensibility in case even un-initialization of VM decides to use these.


> > > The question is whether any form of hotplug is "in the spirit" of microvm.
> >
> > Understand that. BTW, was it ever made to be used with kata-containers?
> >
> > Thanks
> > Salil.

RE: [Question] x86/microvm: why has_hotpluggable_cpus = false but hot(ub)plug APIs exist?

[Salil Mehta]

> From: Igor Mammedov <imammedo@redhat.com>
> Sent: Thursday, October 26, 2023 1:09 PM
> To: Salil Mehta <salil.mehta@huawei.com>
> 
> On Wed, 25 Oct 2023 09:54:07 +0000
> Salil Mehta <salil.mehta@huawei.com> wrote:
> 
> > > From: David Hildenbrand <david@redhat.com>
> > > Sent: Wednesday, October 25, 2023 10:32 AM
> > > To: Salil Mehta <salil.mehta@huawei.com>; Igor Mammedov
> > > <imammedo@redhat.com>; Salil Mehta <salil.mehta@opnsrc.net>
> > >
> > > On 25.10.23 11:16, Salil Mehta wrote:
> > > > Hi Igor,
> > > >
> > > >> From: Igor Mammedov <imammedo@redhat.com>
> > > >> Sent: Tuesday, October 24, 2023 9:06 AM
> > > >> To: Salil Mehta <salil.mehta@opnsrc.net>
> > > >>
> > > >> On Wed, 18 Oct 2023 17:48:36 +0100
> > > >> Salil Mehta <salil.mehta@opnsrc.net> wrote:
> > > >>
> > > >>> Hi Alex,
> > > >>>
> > > >>> On 18/10/2023 16:41, Alex Bennée wrote:
> > > >>>>
> > > >>>> Salil Mehta <salil.mehta@opnsrc.net> writes:
> > > >>>>
> > > >>>>> Hello,
> > > >>>>>
> > > >>>>> Came across below code excerpt in x86/microvm code and wanted to know
> > > >>>>> why 'has_hotpluggable_cpus' flag has been set to 'false' while various
> > > >>>>> hot(un)plug APIs have been defined?
> > > >>>>>
> > > >>>>> static void microvm_class_init(ObjectClass *oc, void *data)
> > > >>>>> {
> > > >>>>>       X86MachineClass *x86mc = X86_MACHINE_CLASS(oc);
> > > >>>>>       MachineClass *mc = MACHINE_CLASS(oc);
> > > >>>>>       HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
> > > >>>>>
> > > >>>>>       mc->init = microvm_machine_state_init;
> > > >>>>>
> > > >>>>>       mc->family = "microvm_i386";
> > > >>>>>       [...]
> > > >>>>>       mc->max_cpus = 288;
> > > >>>>>       mc->has_hotpluggable_cpus = false;  --------> This one
> > > >>>>>       [...]
> > > >>>>
> > > >>>>   From the original commit that added it:
> > > >>>>
> > > >>>>     It's a minimalist machine type without PCI nor ACPI support, designed
> > > >>>>     for short-lived guests. microvm also establishes a baseline for
> > > >>>>     benchmarking and optimizing both QEMU and guest operating systems,
> > > >>>>     since it is optimized for both boot time and footprint.
> > > >>>
> > > >>>
> > > >>> Agreed. It looks like ACPI is supported but neither CPU/Memory Hotplug
> > > >>> is supported for this minimalist machine type.
> > > >>>
> > > >>>
> > > >>> static void microvm_devices_init(MicrovmMachineState *mms)
> > > >>> {
> > > >>>       const char *default_firmware;
> > > >>>       X86MachineState *x86ms = X86_MACHINE(mms);
> > > >>>
> > > >>>      [...]
> > > >>>
> > > >>>       /* Optional and legacy devices */
> > > >>>       if (x86_machine_is_acpi_enabled(x86ms)) {
> > > >>>           DeviceState *dev = qdev_new(TYPE_ACPI_GED_X86);
> > > >>>           qdev_prop_set_uint32(dev, "ged-event", ACPI_GED_PWR_DOWN_EVT);
> > > >>>           sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, GED_MMIO_BASE);
> > > >>>        /* sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, GED_MMIO_BASE_MEMHP);
> > > */
> > > >>>
> > > >>>           [...]
> > > >>>
> > > >>>           sysbus_realize(SYS_BUS_DEVICE(dev), &error_fatal);
> > > >>>           x86ms->acpi_dev = HOTPLUG_HANDLER(dev);
> > > >>>       }
> > > >>>      [...]
> > > >>> }
> > > >>>
> > > >>>>
> > > >>>> Generally hotplug requires a dance between the VMM and the firmware to
> > > >>>> properly shutdown and restart hotplug devices. The principle
> > > >>>> communication mechanism for this is ACPI.
> > > >>
> > > >> firmware part in cpu/mem hoptlug usually provided by QEMU by the way of
> > > >> ACPI tables (which may contain AML code that handles dance with QEMU
> > > >> while exposing standard interface to guest OS to handle hotplug)
> > > >>
> > > >>>
> > > >>> Agreed.
> > > >>>
> > > >>>>>       /* hotplug (for cpu coldplug) */
> > > >>>>>       mc->get_hotplug_handler = microvm_get_hotplug_handler;
> > > >>>>>       hc->pre_plug = microvm_device_pre_plug_cb;
> > > >>>>>       hc->plug = microvm_device_plug_cb;
> > > >>>>>       hc->unplug_request = microvm_device_unplug_request_cb;
> > > >>>>>       hc->unplug = microvm_device_unplug_cb;
> > > >>>
> > > >>> sorry, I also missed the definitions of the last 2 functions which says
> > > >>> that unplug is not supported so perhaps these functions are only
> > > >>> required to support cold plugging which corroborates with the comment as
> > > >>> well.
> > > >>
> > > >> this function are usually used for both cold and hotplug of bus-less devices.
> > > >> They provide an opt-in way for board to wire up such devices (incl. CPU).
> > > >
> > > >
> > > > Sure. I understand but microvm does not support hotplug so presence of
> > > > unplug{_request} versions brought a doubt in my mind but I realized later
> > > > that their definitions were empty. Cold-plug does not require unplug
> > > > versions.
> > > >
> > > > Was there any plan to support hot-plug with microvm in future?
> > >
> > > At least virtio-mem for memory hotplug should be fairly straight- forward
> > > to wire-up I guess. The relation to ACPI are minimal: we currently only
> > > use ACPI SRAT to expose the maximum GPA range that e.g., Linux requires
> > > early during boot to properly prepare for memory hotplug (size the
> > > virtual memory space for the kernel accordingly). One could use
> > > alternative (paravirtualized) interfaces for that.
> >
> > Ok. Light weight VM more in lines of Firecracker to improve boot-up times?
> >
> > Also, presence of unplug versions gives a wrong impression about code?
> 
> unplug handlers could be theoretically used to undo what (pre)plug did
> during VM deconstruction. (though it's not used this way today unless
> hotplug controller requested it).


I understand the intent but it would be better to remove these and bring
then back again if they are required in future. 

  "/* hotplug (for cpu coldplug) */"

Above comment and the code are not exactly consistent even though
definitions of the {request_}unplug are empty.


> What is confusing is interface naming (HotplugHandler) which is not
> hotplug limited anymore.

{pre_}plug routines are common to {hot,cold}-plugged CPUs, therefore, it
makes sense to have them here but I am not sure it is necessary to have
{request_}unplug routines as part of this implementation. Absolutely, it
makes sense to have them part of the design/framework for future
extensibility in case even un-initialization of VM decides to use these.


> > > The question is whether any form of hotplug is "in the spirit" of microvm.
> >
> > Understand that. BTW, was it ever made to be used with kata-containers?
> >
> > Thanks
> > Salil.