Hi Salil,
On 9/26/23 20:04, Salil Mehta wrote:
> During any vCPU hot-(un)plug, running guest VM needs to be intimated about the
> new vCPU being added or request the deletion of the vCPU which is already part
> of the guest VM. This is done using the ACPI GED event which eventually gets
> demultiplexed to a CPU hotplug event and further to specific hot-(un)plug event
> of a particular vCPU.
>
> This change adds the ACPI calls to the existing hot-(un)plug hooks to trigger
> ACPI GED events from QEMU to guest VM.
>
> Co-developed-by: Salil Mehta <salil.mehta@huawei.com>
> Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
> Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
> Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> ---
> hw/arm/virt.c | 33 ++++++++++++++++++++++++++++++---
> 1 file changed, 30 insertions(+), 3 deletions(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index b447e86fb6..6f5ee4a1c6 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -3157,6 +3157,7 @@ static void virt_cpu_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
> VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
> MachineState *ms = MACHINE(hotplug_dev);
> CPUState *cs = CPU(dev);
> + Error *local_err = NULL;
> CPUArchId *cpu_slot;
>
> /* insert the cold/hot-plugged vcpu in the slot */
> @@ -3169,12 +3170,20 @@ static void virt_cpu_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
> * plugged, guest is also notified.
> */
> if (vms->acpi_dev) {
> - /* TODO: update acpi hotplug state. Send cpu hotplug event to guest */
> + HotplugHandlerClass *hhc;
> + /* update acpi hotplug state and send cpu hotplug event to guest */
> + hhc = HOTPLUG_HANDLER_GET_CLASS(vms->acpi_dev);
> + hhc->plug(HOTPLUG_HANDLER(vms->acpi_dev), dev, &local_err);
> + if (local_err) {
> + goto fail;
> + }
> /* TODO: register cpu for reset & update F/W info for the next boot */
> }
>
> cs->disabled = false;
> return;
> +fail:
> + error_propagate(errp, local_err);
> }
>
'fail' tag isn't needed since it's used for once. we can bail early:
if (local_err) {
error_propagate(errp, local_err);
return;
}
> static void virt_cpu_unplug_request(HotplugHandler *hotplug_dev,
> @@ -3182,8 +3191,10 @@ static void virt_cpu_unplug_request(HotplugHandler *hotplug_dev,
> {
> MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
> VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
> + HotplugHandlerClass *hhc;
> ARMCPU *cpu = ARM_CPU(dev);
> CPUState *cs = CPU(dev);
> + Error *local_err = NULL;
>
> if (!vms->acpi_dev || !dev->realized) {
> error_setg(errp, "GED does not exists or device is not realized!");
> @@ -3202,9 +3213,16 @@ static void virt_cpu_unplug_request(HotplugHandler *hotplug_dev,
> return;
> }
>
> - /* TODO: request cpu hotplug from guest */
> + /* request cpu hotplug from guest */
> + hhc = HOTPLUG_HANDLER_GET_CLASS(vms->acpi_dev);
> + hhc->unplug_request(HOTPLUG_HANDLER(vms->acpi_dev), dev, &local_err);
> + if (local_err) {
> + goto fail;
> + }
>
> return;
> +fail:
> + error_propagate(errp, local_err);
> }
>
Same as above, 'fail' tag isn't needed. Besides, 'return' isn't needed.
> static void virt_cpu_unplug(HotplugHandler *hotplug_dev, DeviceState *dev,
> @@ -3212,7 +3230,9 @@ static void virt_cpu_unplug(HotplugHandler *hotplug_dev, DeviceState *dev,
> {
> VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
> MachineState *ms = MACHINE(hotplug_dev);
> + HotplugHandlerClass *hhc;
> CPUState *cs = CPU(dev);
> + Error *local_err = NULL;
> CPUArchId *cpu_slot;
>
> if (!vms->acpi_dev || !dev->realized) {
> @@ -3222,7 +3242,12 @@ static void virt_cpu_unplug(HotplugHandler *hotplug_dev, DeviceState *dev,
>
> cpu_slot = virt_find_cpu_slot(ms, cs->cpu_index);
>
> - /* TODO: update the acpi cpu hotplug state for cpu hot-unplug */
> + /* update the acpi cpu hotplug state for cpu hot-unplug */
> + hhc = HOTPLUG_HANDLER_GET_CLASS(vms->acpi_dev);
> + hhc->unplug(HOTPLUG_HANDLER(vms->acpi_dev), dev, &local_err);
> + if (local_err) {
> + goto fail;
> + }
>
> unwire_gic_cpu_irqs(vms, cs);
> virt_update_gic(vms, cs);
> @@ -3236,6 +3261,8 @@ static void virt_cpu_unplug(HotplugHandler *hotplug_dev, DeviceState *dev,
> cs->disabled = true;
>
> return;
> +fail:
> + error_propagate(errp, local_err);
> }
>
Same as above.
> static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
Thanks,
Gavin