| Message ID | 20230926100436.28284-30-salil.mehta@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Support of Virtual CPU Hotplug for ARMv8 Arch | expand |
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
Hi Gavin, > From: Gavin Shan <gshan@redhat.com> > Sent: Friday, September 29, 2023 1:31 AM > To: Salil Mehta <salil.mehta@huawei.com>; qemu-devel@nongnu.org; qemu-arm@nongnu.org > Cc: maz@kernel.org; jean-philippe@linaro.org; Jonathan Cameron > <jonathan.cameron@huawei.com>; lpieralisi@kernel.org; > peter.maydell@linaro.org; richard.henderson@linaro.org; > imammedo@redhat.com; andrew.jones@linux.dev; david@redhat.com; > philmd@linaro.org; eric.auger@redhat.com; will@kernel.org; ardb@kernel.org; > oliver.upton@linux.dev; pbonzini@redhat.com; mst@redhat.com; > rafael@kernel.org; borntraeger@linux.ibm.com; alex.bennee@linaro.org; > linux@armlinux.org.uk; darren@os.amperecomputing.com; > ilkka@os.amperecomputing.com; vishnu@os.amperecomputing.com; > karl.heubaum@oracle.com; miguel.luis@oracle.com; salil.mehta@opnsrc.net; > zhukeqian <zhukeqian1@huawei.com>; wangxiongfeng (C) > <wangxiongfeng2@huawei.com>; wangyanan (Y) <wangyanan55@huawei.com>; > jiakernel2@gmail.com; maobibo@loongson.cn; lixianglai@loongson.cn > Subject: Re: [PATCH RFC V2 29/37] arm/virt: Update the guest(via GED) about > CPU hot-(un)plug events > > 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> > > --- [...] > > @@ -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; > } Agreed. Indeed we can remove goto. [...] > > @@ -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. Agreed. We can remove goto [...] > > @@ -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. Agreed. Will fix. Thanks Salil.
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); } 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); } 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); } static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,