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.