Re: [RFC v1] arm/virt: Add memory hot remove support
Hi Shameer, On 3/26/20 12:14 PM, Shameerali Kolothum Thodi wrote: > Hi Eric, > >> -Original Message- >> From: Auger Eric [mailto:eric.au...@redhat.com] >> Sent: 26 March 2020 11:01 >> To: Shameerali Kolothum Thodi ; >> qemu-devel@nongnu.org; qemu-...@nongnu.org >> Cc: imamm...@redhat.com; peter.mayd...@linaro.org; m...@redhat.com; >> xuwei (O) ; Zengtao (B) ; >> Linuxarm ; Anshuman Khandual >> >> 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:fe00137d1f40 refcount:1 mapcount:0 >> mapping:0004ea9f20b1 index:0xaaab11c6e >> [ 79.335927] anon flags: 0x17880024(uptodate|active|swapbacked) >> [ 79.337571] raw: 17880024 dead0100 >> dead0122 >> 0004ea9f20b1 >> [ 79.339502] raw: 000aaab11c6e 0001 >> 0004fd4e3000 >> [ 79.341701] page dumped because: unmovable page >> [ 79.342887] page->mem_cgroup:0004fd4e3000 >> [ 79.354729] page:fe00137d1f40 refcount:1 mapcount:0 >> mapping:0004ea9f20b1 index:0xaaab11c6e >> [ 79.357012] anon flags: 0x17880024(uptodate|active|swapbacked) >> [ 79.358658] raw: 17880024 dead0100 >> dead0122 >> 0004ea9f20b1 >> [ 79.360611] raw: 000aaab11c6e 0001 >> 0004fd4e3000 >> [ 79.362560] page dumped because: unmovable page >> [ 79.363742] page->mem_cgroup:0004fd4e3000 >> [ 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 >>> --- >>> -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 *hotp
RE: [RFC v1] arm/virt: Add memory hot remove support
Hi Eric, > -Original Message- > From: Auger Eric [mailto:eric.au...@redhat.com] > Sent: 26 March 2020 11:01 > To: Shameerali Kolothum Thodi ; > qemu-devel@nongnu.org; qemu-...@nongnu.org > Cc: imamm...@redhat.com; peter.mayd...@linaro.org; m...@redhat.com; > xuwei (O) ; Zengtao (B) ; > Linuxarm ; Anshuman Khandual > > 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:fe00137d1f40 refcount:1 mapcount:0 > mapping:0004ea9f20b1 index:0xaaab11c6e > [ 79.335927] anon flags: 0x17880024(uptodate|active|swapbacked) > [ 79.337571] raw: 17880024 dead0100 > dead0122 > 0004ea9f20b1 > [ 79.339502] raw: 000aaab11c6e 0001 > 0004fd4e3000 > [ 79.341701] page dumped because: unmovable page > [ 79.342887] page->mem_cgroup:0004fd4e3000 > [ 79.354729] page:fe00137d1f40 refcount:1 mapcount:0 > mapping:0004ea9f20b1 index:0xaaab11c6e > [ 79.357012] anon flags: 0x17880024(uptodate|active|swapbacked) > [ 79.358658] raw: 17880024 dead0100 > dead0122 > 0004ea9f20b1 > [ 79.360611] raw: 000aaab11c6e 0001 > 0004fd4e3000 > [ 79.362560] page dumped because: unmovable page > [ 79.363742] page->mem_cgroup:0004fd4e3000 > [ 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 > > --- > > -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, > >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)) { > > +
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:fe00137d1f40 refcount:1 mapcount:0 mapping:0004ea9f20b1 index:0xaaab11c6e [ 79.335927] anon flags: 0x17880024(uptodate|active|swapbacked) [ 79.337571] raw: 17880024 dead0100 dead0122 0004ea9f20b1 [ 79.339502] raw: 000aaab11c6e 0001 0004fd4e3000 [ 79.341701] page dumped because: unmovable page [ 79.342887] page->mem_cgroup:0004fd4e3000 [ 79.354729] page:fe00137d1f40 refcount:1 mapcount:0 mapping:0004ea9f20b1 index:0xaaab11c6e [ 79.357012] anon flags: 0x17880024(uptodate|active|swapbacked) [ 79.358658] raw: 17880024 dead0100 dead0122 0004ea9f20b1 [ 79.360611] raw: 000aaab11c6e 0001 0004fd4e3000 [ 79.362560] page dumped because: unmovable page [ 79.363742] page->mem_cgroup:0004fd4e3000 [ 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 > --- > -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, >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(>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 = _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, > +
Re: [RFC v1] arm/virt: Add memory hot remove support
On Wed, 18 Mar 2020 12:37:22 + Shameer Kolothum wrote: > This adds support for memory hot remove on arm/virt that > uses acpi ged device. > > Signed-off-by: Shameer Kolothum 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, >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(>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 = _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, > + _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, _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) >