Hi Alex,

On 18/10/2023 16:41, Alex Bennée wrote:

Salil Mehta <salil.me...@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.


Reply via email to