| Message ID | 20200318123722.19736-1-shameerali.kolothum.thodi@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC,v1] arm/virt: Add memory hot remove support | expand |
On Wed, 18 Mar 2020 12:37:22 +0000 Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> wrote: > This adds support for memory hot remove on arm/virt that > uses acpi ged device. > > Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> Looks fine to me, please repost once 5.0 is released. > --- > -RFC because linux kernel support for mem hot remove is just queued > for 5.7[1]. > -Tested with guest kernel 5.6-rc5 + [1] > > 1. https://patchwork.kernel.org/cover/11419301/ > --- > hw/acpi/generic_event_device.c | 28 +++++++++++++++++ > hw/arm/virt.c | 56 ++++++++++++++++++++++++++++++++-- > 2 files changed, 82 insertions(+), 2 deletions(-) > > diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c > index 021ed2bf23..3e28c110fa 100644 > --- a/hw/acpi/generic_event_device.c > +++ b/hw/acpi/generic_event_device.c > @@ -182,6 +182,32 @@ static void acpi_ged_device_plug_cb(HotplugHandler *hotplug_dev, > } > } > > +static void acpi_ged_unplug_request_cb(HotplugHandler *hotplug_dev, > + DeviceState *dev, Error **errp) > +{ > + AcpiGedState *s = ACPI_GED(hotplug_dev); > + > + if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { > + acpi_memory_unplug_request_cb(hotplug_dev, &s->memhp_state, dev, errp); > + } else { > + error_setg(errp, "acpi: device unplug request for unsupported device" > + " type: %s", object_get_typename(OBJECT(dev))); > + } > +} > + > +static void acpi_ged_unplug_cb(HotplugHandler *hotplug_dev, > + DeviceState *dev, Error **errp) > +{ > + AcpiGedState *s = ACPI_GED(hotplug_dev); > + > + if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { > + acpi_memory_unplug_cb(&s->memhp_state, dev, errp); > + } else { > + error_setg(errp, "acpi: device unplug for unsupported device" > + " type: %s", object_get_typename(OBJECT(dev))); > + } > +} > + > static void acpi_ged_send_event(AcpiDeviceIf *adev, AcpiEventStatusBits ev) > { > AcpiGedState *s = ACPI_GED(adev); > @@ -286,6 +312,8 @@ static void acpi_ged_class_init(ObjectClass *class, void *data) > dc->vmsd = &vmstate_acpi_ged; > > hc->plug = acpi_ged_device_plug_cb; > + hc->unplug_request = acpi_ged_unplug_request_cb; > + hc->unplug = acpi_ged_unplug_cb; > > adevc->send_event = acpi_ged_send_event; > } > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index 94f93dda54..91974e4e80 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -2096,11 +2096,62 @@ static void virt_machine_device_plug_cb(HotplugHandler *hotplug_dev, > } > } > > +static void virt_dimm_unplug_request(HotplugHandler *hotplug_dev, > + DeviceState *dev, Error **errp) > +{ > + VirtMachineState *vms = VIRT_MACHINE(hotplug_dev); > + Error *local_err = NULL; > + > + if (!vms->acpi_dev) { > + error_setg(errp, > + "memory hotplug is not enabled: missing acpi-ged device"); > + goto out; > + } > + > + hotplug_handler_unplug_request(HOTPLUG_HANDLER(vms->acpi_dev), dev, > + &local_err); > +out: > + error_propagate(errp, local_err); > +} > + > +static void virt_dimm_unplug(HotplugHandler *hotplug_dev, > + DeviceState *dev, Error **errp) > +{ > + VirtMachineState *vms = VIRT_MACHINE(hotplug_dev); > + Error *local_err = NULL; > + > + hotplug_handler_unplug(HOTPLUG_HANDLER(vms->acpi_dev), dev, &local_err); > + if (local_err) { > + goto out; > + } > + > + pc_dimm_unplug(PC_DIMM(dev), MACHINE(vms)); > + object_property_set_bool(OBJECT(dev), false, "realized", NULL); > + > + out: > + error_propagate(errp, local_err); > +} > + > static void virt_machine_device_unplug_request_cb(HotplugHandler *hotplug_dev, > DeviceState *dev, Error **errp) > { > - error_setg(errp, "device unplug request for unsupported device" > - " type: %s", object_get_typename(OBJECT(dev))); > + if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { > + virt_dimm_unplug_request(hotplug_dev, dev, errp); > + } else { > + error_setg(errp, "device unplug request for unsupported device" > + " type: %s", object_get_typename(OBJECT(dev))); > + } > +} > + > +static void virt_machine_device_unplug_cb(HotplugHandler *hotplug_dev, > + DeviceState *dev, Error **errp) > +{ > + if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { > + virt_dimm_unplug(hotplug_dev, dev, errp); > + } else { > + error_setg(errp, "virt: device unplug for unsupported device" > + " type: %s", object_get_typename(OBJECT(dev))); > + } > } > > static HotplugHandler *virt_machine_get_hotplug_handler(MachineState *machine, > @@ -2181,6 +2232,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data) > hc->pre_plug = virt_machine_device_pre_plug_cb; > hc->plug = virt_machine_device_plug_cb; > hc->unplug_request = virt_machine_device_unplug_request_cb; > + hc->unplug = virt_machine_device_unplug_cb; > mc->numa_mem_supported = true; > mc->auto_enable_numa_with_memhp = true; > mc->default_ram_id = "mach-virt.ram";
Hi Shameer, On 3/18/20 1:37 PM, Shameer Kolothum wrote: > This adds support for memory hot remove on arm/virt that > uses acpi ged device. I gave this a try and it works fine if the PCDIMM slot was initially hotplugged: (QEMU) object-add qom-type=memory-backend-ram id=mem1 props.size=4294967296 {"return": {}} (QEMU) device_add driver=pc-dimm id=pcdimm1 memdev=mem1 (QEMU) device_del id=pcdimm1 {"return": {}} on guest I can see: [ 82.466321] Offlined Pages 262144 [ 82.541712] Offlined Pages 262144 [ 82.589236] Offlined Pages 262144 [ 82.969166] Offlined Pages 262144 However I noticed that if qemu is launched directly with -m 16G,maxmem=32G,slots=2 \ -object memory-backend-ram,id=mem1,size=4G \ -device pc-dimm,memdev=mem1,id=dimm1,driver=pc-dimm -device and then in the qmp shell: (QEMU) device_del id=dimm1 the hot-unplug fails in guest: [ 78.897407] Offlined Pages 262144 [ 79.260811] Offlined Pages 262144 [ 79.308105] Offlined Pages 262144 [ 79.333675] page:fffffe00137d1f40 refcount:1 mapcount:0 mapping:ffff0004ea9f20b1 index:0xaaab11c6e [ 79.335927] anon flags: 0x17ffff8000080024(uptodate|active|swapbacked) [ 79.337571] raw: 17ffff8000080024 dead000000000100 dead000000000122 ffff0004ea9f20b1 [ 79.339502] raw: 0000000aaab11c6e 0000000000000000 00000001ffffffff ffff0004fd4e3000 [ 79.341701] page dumped because: unmovable page [ 79.342887] page->mem_cgroup:ffff0004fd4e3000 [ 79.354729] page:fffffe00137d1f40 refcount:1 mapcount:0 mapping:ffff0004ea9f20b1 index:0xaaab11c6e [ 79.357012] anon flags: 0x17ffff8000080024(uptodate|active|swapbacked) [ 79.358658] raw: 17ffff8000080024 dead000000000100 dead000000000122 ffff0004ea9f20b1 [ 79.360611] raw: 0000000aaab11c6e 0000000000000000 00000001ffffffff ffff0004fd4e3000 [ 79.362560] page dumped because: unmovable page [ 79.363742] page->mem_cgroup:ffff0004fd4e3000 [ 79.368636] memory memory20: Offline failed. I did not expect this. The PCDIMM slot in that case does not seem to be interpreted as a hot-unpluggable one (?). I added Anshuman in cc. Thanks Eric > > Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> > --- > -RFC because linux kernel support for mem hot remove is just queued > for 5.7[1]. > -Tested with guest kernel 5.6-rc5 + [1] > > 1. https://patchwork.kernel.org/cover/11419301/ > --- > hw/acpi/generic_event_device.c | 28 +++++++++++++++++ > hw/arm/virt.c | 56 ++++++++++++++++++++++++++++++++-- > 2 files changed, 82 insertions(+), 2 deletions(-) > > diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c > index 021ed2bf23..3e28c110fa 100644 > --- a/hw/acpi/generic_event_device.c > +++ b/hw/acpi/generic_event_device.c > @@ -182,6 +182,32 @@ static void acpi_ged_device_plug_cb(HotplugHandler *hotplug_dev, > } > } > > +static void acpi_ged_unplug_request_cb(HotplugHandler *hotplug_dev, > + DeviceState *dev, Error **errp) > +{ > + AcpiGedState *s = ACPI_GED(hotplug_dev); > + > + if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { > + acpi_memory_unplug_request_cb(hotplug_dev, &s->memhp_state, dev, errp); > + } else { > + error_setg(errp, "acpi: device unplug request for unsupported device" > + " type: %s", object_get_typename(OBJECT(dev))); > + } > +} > + > +static void acpi_ged_unplug_cb(HotplugHandler *hotplug_dev, > + DeviceState *dev, Error **errp) > +{ > + AcpiGedState *s = ACPI_GED(hotplug_dev); > + > + if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { > + acpi_memory_unplug_cb(&s->memhp_state, dev, errp); > + } else { > + error_setg(errp, "acpi: device unplug for unsupported device" > + " type: %s", object_get_typename(OBJECT(dev))); > + } > +} > + > static void acpi_ged_send_event(AcpiDeviceIf *adev, AcpiEventStatusBits ev) > { > AcpiGedState *s = ACPI_GED(adev); > @@ -286,6 +312,8 @@ static void acpi_ged_class_init(ObjectClass *class, void *data) > dc->vmsd = &vmstate_acpi_ged; > > hc->plug = acpi_ged_device_plug_cb; > + hc->unplug_request = acpi_ged_unplug_request_cb; > + hc->unplug = acpi_ged_unplug_cb; > > adevc->send_event = acpi_ged_send_event; > } > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index 94f93dda54..91974e4e80 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -2096,11 +2096,62 @@ static void virt_machine_device_plug_cb(HotplugHandler *hotplug_dev, > } > } > > +static void virt_dimm_unplug_request(HotplugHandler *hotplug_dev, > + DeviceState *dev, Error **errp) > +{ > + VirtMachineState *vms = VIRT_MACHINE(hotplug_dev); > + Error *local_err = NULL; > + > + if (!vms->acpi_dev) { > + error_setg(errp, > + "memory hotplug is not enabled: missing acpi-ged device"); > + goto out; > + } > + > + hotplug_handler_unplug_request(HOTPLUG_HANDLER(vms->acpi_dev), dev, > + &local_err); > +out: > + error_propagate(errp, local_err); > +} > + > +static void virt_dimm_unplug(HotplugHandler *hotplug_dev, > + DeviceState *dev, Error **errp) > +{ > + VirtMachineState *vms = VIRT_MACHINE(hotplug_dev); > + Error *local_err = NULL; > + > + hotplug_handler_unplug(HOTPLUG_HANDLER(vms->acpi_dev), dev, &local_err); > + if (local_err) { > + goto out; > + } > + > + pc_dimm_unplug(PC_DIMM(dev), MACHINE(vms)); > + object_property_set_bool(OBJECT(dev), false, "realized", NULL); > + > + out: > + error_propagate(errp, local_err); > +} > + > static void virt_machine_device_unplug_request_cb(HotplugHandler *hotplug_dev, > DeviceState *dev, Error **errp) > { > - error_setg(errp, "device unplug request for unsupported device" > - " type: %s", object_get_typename(OBJECT(dev))); > + if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { > + virt_dimm_unplug_request(hotplug_dev, dev, errp); > + } else { > + error_setg(errp, "device unplug request for unsupported device" > + " type: %s", object_get_typename(OBJECT(dev))); > + } > +} > + > +static void virt_machine_device_unplug_cb(HotplugHandler *hotplug_dev, > + DeviceState *dev, Error **errp) > +{ > + if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { > + virt_dimm_unplug(hotplug_dev, dev, errp); > + } else { > + error_setg(errp, "virt: device unplug for unsupported device" > + " type: %s", object_get_typename(OBJECT(dev))); > + } > } > > static HotplugHandler *virt_machine_get_hotplug_handler(MachineState *machine, > @@ -2181,6 +2232,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data) > hc->pre_plug = virt_machine_device_pre_plug_cb; > hc->plug = virt_machine_device_plug_cb; > hc->unplug_request = virt_machine_device_unplug_request_cb; > + hc->unplug = virt_machine_device_unplug_cb; > mc->numa_mem_supported = true; > mc->auto_enable_numa_with_memhp = true; > mc->default_ram_id = "mach-virt.ram"; >
Hi Eric, > -----Original Message----- > From: Auger Eric [mailto:eric.auger@redhat.com] > Sent: 26 March 2020 11:01 > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>; > qemu-devel@nongnu.org; qemu-arm@nongnu.org > Cc: imammedo@redhat.com; peter.maydell@linaro.org; mst@redhat.com; > xuwei (O) <xuwei5@huawei.com>; Zengtao (B) <prime.zeng@hisilicon.com>; > Linuxarm <linuxarm@huawei.com>; Anshuman Khandual > <anshuman.khandual@arm.com> > Subject: Re: [RFC v1] arm/virt: Add memory hot remove support > > Hi Shameer, > > On 3/18/20 1:37 PM, Shameer Kolothum wrote: > > This adds support for memory hot remove on arm/virt that > > uses acpi ged device. > > I gave this a try and it works fine if the PCDIMM slot was initially > hotplugged: > (QEMU) object-add qom-type=memory-backend-ram id=mem1 > props.size=4294967296 > {"return": {}} > (QEMU) device_add driver=pc-dimm id=pcdimm1 memdev=mem1 > (QEMU) device_del id=pcdimm1 > {"return": {}} > > on guest I can see: > [ 82.466321] Offlined Pages 262144 > [ 82.541712] Offlined Pages 262144 > [ 82.589236] Offlined Pages 262144 > [ 82.969166] Offlined Pages 262144 > > However I noticed that if qemu is launched directly with > > -m 16G,maxmem=32G,slots=2 \ > -object memory-backend-ram,id=mem1,size=4G \ > -device pc-dimm,memdev=mem1,id=dimm1,driver=pc-dimm -device > > and then in the qmp shell: > (QEMU) device_del id=dimm1 > > the hot-unplug fails in guest: > > [ 78.897407] Offlined Pages 262144 > [ 79.260811] Offlined Pages 262144 > [ 79.308105] Offlined Pages 262144 > [ 79.333675] page:fffffe00137d1f40 refcount:1 mapcount:0 > mapping:ffff0004ea9f20b1 index:0xaaab11c6e > [ 79.335927] anon flags: 0x17ffff8000080024(uptodate|active|swapbacked) > [ 79.337571] raw: 17ffff8000080024 dead000000000100 > dead000000000122 > ffff0004ea9f20b1 > [ 79.339502] raw: 0000000aaab11c6e 0000000000000000 00000001ffffffff > ffff0004fd4e3000 > [ 79.341701] page dumped because: unmovable page > [ 79.342887] page->mem_cgroup:ffff0004fd4e3000 > [ 79.354729] page:fffffe00137d1f40 refcount:1 mapcount:0 > mapping:ffff0004ea9f20b1 index:0xaaab11c6e > [ 79.357012] anon flags: 0x17ffff8000080024(uptodate|active|swapbacked) > [ 79.358658] raw: 17ffff8000080024 dead000000000100 > dead000000000122 > ffff0004ea9f20b1 > [ 79.360611] raw: 0000000aaab11c6e 0000000000000000 00000001ffffffff > ffff0004fd4e3000 > [ 79.362560] page dumped because: unmovable page > [ 79.363742] page->mem_cgroup:ffff0004fd4e3000 > [ 79.368636] memory memory20: Offline failed. > > I did not expect this. The PCDIMM slot in that case does not seem to be > interpreted as a hot-unpluggable one (?). I added Anshuman in cc. Could you please try adding "movable_node" to qemu guest kernel command line params. This will prevent any kernel allocation from hotplugable memory nodes which I think is causing the behavior you are seeing. Thanks, Shameer > Thanks > > Eric > > > > > > > Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> > > --- > > -RFC because linux kernel support for mem hot remove is just queued > > for 5.7[1]. > > -Tested with guest kernel 5.6-rc5 + [1] > > > > 1. https://patchwork.kernel.org/cover/11419301/ > > --- > > hw/acpi/generic_event_device.c | 28 +++++++++++++++++ > > hw/arm/virt.c | 56 > ++++++++++++++++++++++++++++++++-- > > 2 files changed, 82 insertions(+), 2 deletions(-) > > > > diff --git a/hw/acpi/generic_event_device.c > b/hw/acpi/generic_event_device.c > > index 021ed2bf23..3e28c110fa 100644 > > --- a/hw/acpi/generic_event_device.c > > +++ b/hw/acpi/generic_event_device.c > > @@ -182,6 +182,32 @@ static void > acpi_ged_device_plug_cb(HotplugHandler *hotplug_dev, > > } > > } > > > > +static void acpi_ged_unplug_request_cb(HotplugHandler *hotplug_dev, > > + DeviceState *dev, Error > **errp) > > +{ > > + AcpiGedState *s = ACPI_GED(hotplug_dev); > > + > > + if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { > > + acpi_memory_unplug_request_cb(hotplug_dev, > &s->memhp_state, dev, errp); > > + } else { > > + error_setg(errp, "acpi: device unplug request for unsupported > device" > > + " type: %s", object_get_typename(OBJECT(dev))); > > + } > > +} > > + > > +static void acpi_ged_unplug_cb(HotplugHandler *hotplug_dev, > > + DeviceState *dev, Error **errp) > > +{ > > + AcpiGedState *s = ACPI_GED(hotplug_dev); > > + > > + if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { > > + acpi_memory_unplug_cb(&s->memhp_state, dev, errp); > > + } else { > > + error_setg(errp, "acpi: device unplug for unsupported device" > > + " type: %s", object_get_typename(OBJECT(dev))); > > + } > > +} > > + > > static void acpi_ged_send_event(AcpiDeviceIf *adev, AcpiEventStatusBits > ev) > > { > > AcpiGedState *s = ACPI_GED(adev); > > @@ -286,6 +312,8 @@ static void acpi_ged_class_init(ObjectClass *class, > void *data) > > dc->vmsd = &vmstate_acpi_ged; > > > > hc->plug = acpi_ged_device_plug_cb; > > + hc->unplug_request = acpi_ged_unplug_request_cb; > > + hc->unplug = acpi_ged_unplug_cb; > > > > adevc->send_event = acpi_ged_send_event; > > } > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > > index 94f93dda54..91974e4e80 100644 > > --- a/hw/arm/virt.c > > +++ b/hw/arm/virt.c > > @@ -2096,11 +2096,62 @@ static void > virt_machine_device_plug_cb(HotplugHandler *hotplug_dev, > > } > > } > > > > +static void virt_dimm_unplug_request(HotplugHandler *hotplug_dev, > > + DeviceState *dev, Error > **errp) > > +{ > > + VirtMachineState *vms = VIRT_MACHINE(hotplug_dev); > > + Error *local_err = NULL; > > + > > + if (!vms->acpi_dev) { > > + error_setg(errp, > > + "memory hotplug is not enabled: missing acpi-ged > device"); > > + goto out; > > + } > > + > > + hotplug_handler_unplug_request(HOTPLUG_HANDLER(vms->acpi_dev), > dev, > > + &local_err); > > +out: > > + error_propagate(errp, local_err); > > +} > > + > > +static void virt_dimm_unplug(HotplugHandler *hotplug_dev, > > + DeviceState *dev, Error **errp) > > +{ > > + VirtMachineState *vms = VIRT_MACHINE(hotplug_dev); > > + Error *local_err = NULL; > > + > > + hotplug_handler_unplug(HOTPLUG_HANDLER(vms->acpi_dev), dev, > &local_err); > > + if (local_err) { > > + goto out; > > + } > > + > > + pc_dimm_unplug(PC_DIMM(dev), MACHINE(vms)); > > + object_property_set_bool(OBJECT(dev), false, "realized", NULL); > > + > > + out: > > + error_propagate(errp, local_err); > > +} > > + > > static void virt_machine_device_unplug_request_cb(HotplugHandler > *hotplug_dev, > > DeviceState *dev, Error > **errp) > > { > > - error_setg(errp, "device unplug request for unsupported device" > > - " type: %s", object_get_typename(OBJECT(dev))); > > + if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { > > + virt_dimm_unplug_request(hotplug_dev, dev, errp); > > + } else { > > + error_setg(errp, "device unplug request for unsupported device" > > + " type: %s", object_get_typename(OBJECT(dev))); > > + } > > +} > > + > > +static void virt_machine_device_unplug_cb(HotplugHandler *hotplug_dev, > > + DeviceState *dev, Error > **errp) > > +{ > > + if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { > > + virt_dimm_unplug(hotplug_dev, dev, errp); > > + } else { > > + error_setg(errp, "virt: device unplug for unsupported device" > > + " type: %s", object_get_typename(OBJECT(dev))); > > + } > > } > > > > static HotplugHandler *virt_machine_get_hotplug_handler(MachineState > *machine, > > @@ -2181,6 +2232,7 @@ static void virt_machine_class_init(ObjectClass > *oc, void *data) > > hc->pre_plug = virt_machine_device_pre_plug_cb; > > hc->plug = virt_machine_device_plug_cb; > > hc->unplug_request = virt_machine_device_unplug_request_cb; > > + hc->unplug = virt_machine_device_unplug_cb; > > mc->numa_mem_supported = true; > > mc->auto_enable_numa_with_memhp = true; > > mc->default_ram_id = "mach-virt.ram"; > >
Hi Shameer, On 3/26/20 12:14 PM, Shameerali Kolothum Thodi wrote: > Hi Eric, > >> -----Original Message----- >> From: Auger Eric [mailto:eric.auger@redhat.com] >> Sent: 26 March 2020 11:01 >> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>; >> qemu-devel@nongnu.org; qemu-arm@nongnu.org >> Cc: imammedo@redhat.com; peter.maydell@linaro.org; mst@redhat.com; >> xuwei (O) <xuwei5@huawei.com>; Zengtao (B) <prime.zeng@hisilicon.com>; >> Linuxarm <linuxarm@huawei.com>; Anshuman Khandual >> <anshuman.khandual@arm.com> >> Subject: Re: [RFC v1] arm/virt: Add memory hot remove support >> >> Hi Shameer, >> >> On 3/18/20 1:37 PM, Shameer Kolothum wrote: >>> This adds support for memory hot remove on arm/virt that >>> uses acpi ged device. >> >> I gave this a try and it works fine if the PCDIMM slot was initially >> hotplugged: >> (QEMU) object-add qom-type=memory-backend-ram id=mem1 >> props.size=4294967296 >> {"return": {}} >> (QEMU) device_add driver=pc-dimm id=pcdimm1 memdev=mem1 >> (QEMU) device_del id=pcdimm1 >> {"return": {}} >> >> on guest I can see: >> [ 82.466321] Offlined Pages 262144 >> [ 82.541712] Offlined Pages 262144 >> [ 82.589236] Offlined Pages 262144 >> [ 82.969166] Offlined Pages 262144 >> >> However I noticed that if qemu is launched directly with >> >> -m 16G,maxmem=32G,slots=2 \ >> -object memory-backend-ram,id=mem1,size=4G \ >> -device pc-dimm,memdev=mem1,id=dimm1,driver=pc-dimm -device >> >> and then in the qmp shell: >> (QEMU) device_del id=dimm1 >> >> the hot-unplug fails in guest: >> >> [ 78.897407] Offlined Pages 262144 >> [ 79.260811] Offlined Pages 262144 >> [ 79.308105] Offlined Pages 262144 >> [ 79.333675] page:fffffe00137d1f40 refcount:1 mapcount:0 >> mapping:ffff0004ea9f20b1 index:0xaaab11c6e >> [ 79.335927] anon flags: 0x17ffff8000080024(uptodate|active|swapbacked) >> [ 79.337571] raw: 17ffff8000080024 dead000000000100 >> dead000000000122 >> ffff0004ea9f20b1 >> [ 79.339502] raw: 0000000aaab11c6e 0000000000000000 00000001ffffffff >> ffff0004fd4e3000 >> [ 79.341701] page dumped because: unmovable page >> [ 79.342887] page->mem_cgroup:ffff0004fd4e3000 >> [ 79.354729] page:fffffe00137d1f40 refcount:1 mapcount:0 >> mapping:ffff0004ea9f20b1 index:0xaaab11c6e >> [ 79.357012] anon flags: 0x17ffff8000080024(uptodate|active|swapbacked) >> [ 79.358658] raw: 17ffff8000080024 dead000000000100 >> dead000000000122 >> ffff0004ea9f20b1 >> [ 79.360611] raw: 0000000aaab11c6e 0000000000000000 00000001ffffffff >> ffff0004fd4e3000 >> [ 79.362560] page dumped because: unmovable page >> [ 79.363742] page->mem_cgroup:ffff0004fd4e3000 >> [ 79.368636] memory memory20: Offline failed. >> >> I did not expect this. The PCDIMM slot in that case does not seem to be >> interpreted as a hot-unpluggable one (?). I added Anshuman in cc. > > Could you please try adding "movable_node" to qemu guest kernel command line params. > This will prevent any kernel allocation from hotplugable memory nodes which I think is > causing the behavior you are seeing. Effectively, when adding the movable_node option in the guest kernel parameters, I get the following traces: [ 29.581418] Offlined Pages 262144 [ 29.663605] Offlined Pages 262144 [ 29.714225] Offlined Pages 262144 [ 30.222953] Offlined Pages 262144 [ 30.314288] Built 1 zonelists, mobility grouping on. Total pages: 4076898 [ 30.316067] Policy zone: Normal Thanks Eric > > Thanks, > Shameer > > >> Thanks >> >> Eric >> >> >> >>> >>> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> >>> --- >>> -RFC because linux kernel support for mem hot remove is just queued >>> for 5.7[1]. >>> -Tested with guest kernel 5.6-rc5 + [1] >>> >>> 1. https://patchwork.kernel.org/cover/11419301/ >>> --- >>> hw/acpi/generic_event_device.c | 28 +++++++++++++++++ >>> hw/arm/virt.c | 56 >> ++++++++++++++++++++++++++++++++-- >>> 2 files changed, 82 insertions(+), 2 deletions(-) >>> >>> diff --git a/hw/acpi/generic_event_device.c >> b/hw/acpi/generic_event_device.c >>> index 021ed2bf23..3e28c110fa 100644 >>> --- a/hw/acpi/generic_event_device.c >>> +++ b/hw/acpi/generic_event_device.c >>> @@ -182,6 +182,32 @@ static void >> acpi_ged_device_plug_cb(HotplugHandler *hotplug_dev, >>> } >>> } >>> >>> +static void acpi_ged_unplug_request_cb(HotplugHandler *hotplug_dev, >>> + DeviceState *dev, Error >> **errp) >>> +{ >>> + AcpiGedState *s = ACPI_GED(hotplug_dev); >>> + >>> + if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { >>> + acpi_memory_unplug_request_cb(hotplug_dev, >> &s->memhp_state, dev, errp); >>> + } else { >>> + error_setg(errp, "acpi: device unplug request for unsupported >> device" >>> + " type: %s", object_get_typename(OBJECT(dev))); >>> + } >>> +} >>> + >>> +static void acpi_ged_unplug_cb(HotplugHandler *hotplug_dev, >>> + DeviceState *dev, Error **errp) >>> +{ >>> + AcpiGedState *s = ACPI_GED(hotplug_dev); >>> + >>> + if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { >>> + acpi_memory_unplug_cb(&s->memhp_state, dev, errp); >>> + } else { >>> + error_setg(errp, "acpi: device unplug for unsupported device" >>> + " type: %s", object_get_typename(OBJECT(dev))); >>> + } >>> +} >>> + >>> static void acpi_ged_send_event(AcpiDeviceIf *adev, AcpiEventStatusBits >> ev) >>> { >>> AcpiGedState *s = ACPI_GED(adev); >>> @@ -286,6 +312,8 @@ static void acpi_ged_class_init(ObjectClass *class, >> void *data) >>> dc->vmsd = &vmstate_acpi_ged; >>> >>> hc->plug = acpi_ged_device_plug_cb; >>> + hc->unplug_request = acpi_ged_unplug_request_cb; >>> + hc->unplug = acpi_ged_unplug_cb; >>> >>> adevc->send_event = acpi_ged_send_event; >>> } >>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c >>> index 94f93dda54..91974e4e80 100644 >>> --- a/hw/arm/virt.c >>> +++ b/hw/arm/virt.c >>> @@ -2096,11 +2096,62 @@ static void >> virt_machine_device_plug_cb(HotplugHandler *hotplug_dev, >>> } >>> } >>> >>> +static void virt_dimm_unplug_request(HotplugHandler *hotplug_dev, >>> + DeviceState *dev, Error >> **errp) >>> +{ >>> + VirtMachineState *vms = VIRT_MACHINE(hotplug_dev); >>> + Error *local_err = NULL; >>> + >>> + if (!vms->acpi_dev) { >>> + error_setg(errp, >>> + "memory hotplug is not enabled: missing acpi-ged >> device"); >>> + goto out; >>> + } >>> + >>> + hotplug_handler_unplug_request(HOTPLUG_HANDLER(vms->acpi_dev), >> dev, >>> + &local_err); >>> +out: >>> + error_propagate(errp, local_err); >>> +} >>> + >>> +static void virt_dimm_unplug(HotplugHandler *hotplug_dev, >>> + DeviceState *dev, Error **errp) >>> +{ >>> + VirtMachineState *vms = VIRT_MACHINE(hotplug_dev); >>> + Error *local_err = NULL; >>> + >>> + hotplug_handler_unplug(HOTPLUG_HANDLER(vms->acpi_dev), dev, >> &local_err); >>> + if (local_err) { >>> + goto out; >>> + } >>> + >>> + pc_dimm_unplug(PC_DIMM(dev), MACHINE(vms)); >>> + object_property_set_bool(OBJECT(dev), false, "realized", NULL); >>> + >>> + out: >>> + error_propagate(errp, local_err); >>> +} >>> + >>> static void virt_machine_device_unplug_request_cb(HotplugHandler >> *hotplug_dev, >>> DeviceState *dev, Error >> **errp) >>> { >>> - error_setg(errp, "device unplug request for unsupported device" >>> - " type: %s", object_get_typename(OBJECT(dev))); >>> + if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { >>> + virt_dimm_unplug_request(hotplug_dev, dev, errp); >>> + } else { >>> + error_setg(errp, "device unplug request for unsupported device" >>> + " type: %s", object_get_typename(OBJECT(dev))); >>> + } >>> +} >>> + >>> +static void virt_machine_device_unplug_cb(HotplugHandler *hotplug_dev, >>> + DeviceState *dev, Error >> **errp) >>> +{ >>> + if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { >>> + virt_dimm_unplug(hotplug_dev, dev, errp); >>> + } else { >>> + error_setg(errp, "virt: device unplug for unsupported device" >>> + " type: %s", object_get_typename(OBJECT(dev))); >>> + } >>> } >>> >>> static HotplugHandler *virt_machine_get_hotplug_handler(MachineState >> *machine, >>> @@ -2181,6 +2232,7 @@ static void virt_machine_class_init(ObjectClass >> *oc, void *data) >>> hc->pre_plug = virt_machine_device_pre_plug_cb; >>> hc->plug = virt_machine_device_plug_cb; >>> hc->unplug_request = virt_machine_device_unplug_request_cb; >>> + hc->unplug = virt_machine_device_unplug_cb; >>> mc->numa_mem_supported = true; >>> mc->auto_enable_numa_with_memhp = true; >>> mc->default_ram_id = "mach-virt.ram"; >>> >
diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c index 021ed2bf23..3e28c110fa 100644 --- a/hw/acpi/generic_event_device.c +++ b/hw/acpi/generic_event_device.c @@ -182,6 +182,32 @@ static void acpi_ged_device_plug_cb(HotplugHandler *hotplug_dev, } } +static void acpi_ged_unplug_request_cb(HotplugHandler *hotplug_dev, + DeviceState *dev, Error **errp) +{ + AcpiGedState *s = ACPI_GED(hotplug_dev); + + if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { + acpi_memory_unplug_request_cb(hotplug_dev, &s->memhp_state, dev, errp); + } else { + error_setg(errp, "acpi: device unplug request for unsupported device" + " type: %s", object_get_typename(OBJECT(dev))); + } +} + +static void acpi_ged_unplug_cb(HotplugHandler *hotplug_dev, + DeviceState *dev, Error **errp) +{ + AcpiGedState *s = ACPI_GED(hotplug_dev); + + if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { + acpi_memory_unplug_cb(&s->memhp_state, dev, errp); + } else { + error_setg(errp, "acpi: device unplug for unsupported device" + " type: %s", object_get_typename(OBJECT(dev))); + } +} + static void acpi_ged_send_event(AcpiDeviceIf *adev, AcpiEventStatusBits ev) { AcpiGedState *s = ACPI_GED(adev); @@ -286,6 +312,8 @@ static void acpi_ged_class_init(ObjectClass *class, void *data) dc->vmsd = &vmstate_acpi_ged; hc->plug = acpi_ged_device_plug_cb; + hc->unplug_request = acpi_ged_unplug_request_cb; + hc->unplug = acpi_ged_unplug_cb; adevc->send_event = acpi_ged_send_event; } diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 94f93dda54..91974e4e80 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -2096,11 +2096,62 @@ static void virt_machine_device_plug_cb(HotplugHandler *hotplug_dev, } } +static void virt_dimm_unplug_request(HotplugHandler *hotplug_dev, + DeviceState *dev, Error **errp) +{ + VirtMachineState *vms = VIRT_MACHINE(hotplug_dev); + Error *local_err = NULL; + + if (!vms->acpi_dev) { + error_setg(errp, + "memory hotplug is not enabled: missing acpi-ged device"); + goto out; + } + + hotplug_handler_unplug_request(HOTPLUG_HANDLER(vms->acpi_dev), dev, + &local_err); +out: + error_propagate(errp, local_err); +} + +static void virt_dimm_unplug(HotplugHandler *hotplug_dev, + DeviceState *dev, Error **errp) +{ + VirtMachineState *vms = VIRT_MACHINE(hotplug_dev); + Error *local_err = NULL; + + hotplug_handler_unplug(HOTPLUG_HANDLER(vms->acpi_dev), dev, &local_err); + if (local_err) { + goto out; + } + + pc_dimm_unplug(PC_DIMM(dev), MACHINE(vms)); + object_property_set_bool(OBJECT(dev), false, "realized", NULL); + + out: + error_propagate(errp, local_err); +} + static void virt_machine_device_unplug_request_cb(HotplugHandler *hotplug_dev, DeviceState *dev, Error **errp) { - error_setg(errp, "device unplug request for unsupported device" - " type: %s", object_get_typename(OBJECT(dev))); + if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { + virt_dimm_unplug_request(hotplug_dev, dev, errp); + } else { + error_setg(errp, "device unplug request for unsupported device" + " type: %s", object_get_typename(OBJECT(dev))); + } +} + +static void virt_machine_device_unplug_cb(HotplugHandler *hotplug_dev, + DeviceState *dev, Error **errp) +{ + if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { + virt_dimm_unplug(hotplug_dev, dev, errp); + } else { + error_setg(errp, "virt: device unplug for unsupported device" + " type: %s", object_get_typename(OBJECT(dev))); + } } static HotplugHandler *virt_machine_get_hotplug_handler(MachineState *machine, @@ -2181,6 +2232,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data) hc->pre_plug = virt_machine_device_pre_plug_cb; hc->plug = virt_machine_device_plug_cb; hc->unplug_request = virt_machine_device_unplug_request_cb; + hc->unplug = virt_machine_device_unplug_cb; mc->numa_mem_supported = true; mc->auto_enable_numa_with_memhp = true; mc->default_ram_id = "mach-virt.ram";
This adds support for memory hot remove on arm/virt that uses acpi ged device. Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> --- -RFC because linux kernel support for mem hot remove is just queued for 5.7[1]. -Tested with guest kernel 5.6-rc5 + [1] 1. https://patchwork.kernel.org/cover/11419301/ --- hw/acpi/generic_event_device.c | 28 +++++++++++++++++ hw/arm/virt.c | 56 ++++++++++++++++++++++++++++++++-- 2 files changed, 82 insertions(+), 2 deletions(-)