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.