From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52822) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZXlJR-0000Zc-JI for qemu-devel@nongnu.org; Fri, 04 Sep 2015 03:21:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZXlJO-0000uH-Oe for qemu-devel@nongnu.org; Fri, 04 Sep 2015 03:21:29 -0400 Date: Fri, 4 Sep 2015 17:20:10 +1000 From: David Gibson Message-ID: <20150904072010.GD6537@voom.redhat.com> References: <1439967371-15870-1-git-send-email-bharata@linux.vnet.ibm.com> <1439967371-15870-4-git-send-email-bharata@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="k8ylJtLa18RSEDYA" Content-Disposition: inline In-Reply-To: <1439967371-15870-4-git-send-email-bharata@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [RFC PATCH v0 3/3] spapr: Memory hot-unplug support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bharata B Rao Cc: mdroth@linux.vnet.ibm.com, agraf@suse.de, qemu-devel@nongnu.org, qemu-ppc@nongnu.org, tyreld@linux.vnet.ibm.com, nfont@linux.vnet.ibm.com, imammedo@redhat.com --k8ylJtLa18RSEDYA Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Aug 19, 2015 at 12:26:11PM +0530, Bharata B Rao wrote: > Add support to hot remove pc-dimm memory devices. >=20 > Signed-off-by: Bharata B Rao > --- > hw/ppc/spapr.c | 114 +++++++++++++++++++++++++++++++++++++++++++= +++++- > hw/ppc/spapr_drc.c | 21 +++++++++ > include/hw/ppc/spapr.h | 2 + > 3 files changed, 136 insertions(+), 1 deletion(-) >=20 > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 06d000d..441012d 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -2110,6 +2110,109 @@ out: > error_propagate(errp, local_err); > } > =20 > +typedef struct sPAPRDIMMState { > + uint32_t nr_lmbs; > +} sPAPRDIMMState; > + > +/* > + * Called from spapr_drc.c: set_isolation_state(). > + * > + * If the drc is being marked as ISOLATED, ensure that the corresponding > + * LMB is part of the DIMM device which is being deleted. > + */ > +int spapr_lmb_in_removable_dimm(sPAPRDRConnector *drc, > + sPAPRDRIsolationState state) > +{ > + DeviceState *dev =3D drc->dev; > + PCDIMMDevice *dimm =3D PC_DIMM(dev); > + > + if (state !=3D SPAPR_DR_ISOLATION_STATE_ISOLATED) { > + return 0; > + } This check doesn't really seem to fit with the function's name. Would it be clearer if you moved the state test to the caller? > + > + if (!dimm->delete_pending) { > + return -1; > + } > + > + return 0; > +} > + > +static void spapr_lmb_release(DeviceState *dev, void *opaque) > +{ > + sPAPRDIMMState *ds =3D (sPAPRDIMMState *)opaque; > + HotplugHandler *hotplug_ctrl =3D NULL; > + Error *local_err =3D NULL; > + > + if (--ds->nr_lmbs) { > + return; > + } > + > + g_free(ds); > + > + /* > + * Now that all the LMBs have been removed by the guest, call the > + * pc-dimm unplug handler to cleanup up the pc-dimm device. > + */ > + hotplug_ctrl =3D qdev_get_hotplug_handler(dev); > + hotplug_handler_unplug(hotplug_ctrl, dev, &local_err); > +} > + > +static void spapr_del_lmbs(DeviceState *dev, uint64_t addr, uint64_t siz= e, > + Error **errp) > +{ > + sPAPRDRConnector *drc; > + sPAPRDRConnectorClass *drck; > + uint32_t nr_lmbs =3D size/SPAPR_MEMORY_BLOCK_SIZE; > + Error *local_err =3D NULL; > + int i; > + sPAPRDIMMState *ds =3D g_malloc0(sizeof(sPAPRDIMMState)); > + > + ds->nr_lmbs =3D nr_lmbs; > + for (i =3D 0; i < nr_lmbs; i++) { > + drc =3D spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_LMB, > + addr/SPAPR_MEMORY_BLOCK_SIZE); > + g_assert(drc); > + > + drck =3D SPAPR_DR_CONNECTOR_GET_CLASS(drc); > + drck->detach(drc, dev, spapr_lmb_release, ds, &local_err); > + addr +=3D SPAPR_MEMORY_BLOCK_SIZE; > + } > + spapr_hotplug_req_remove_by_count(SPAPR_DR_CONNECTOR_TYPE_LMB, nr_lm= bs); > +} > + > +static void spapr_memory_unplug(HotplugHandler *hotplug_dev, DeviceState= *dev, > + Error **errp) > +{ > + sPAPRMachineState *ms =3D SPAPR_MACHINE(hotplug_dev); > + PCDIMMDevice *dimm =3D PC_DIMM(dev); > + PCDIMMDeviceClass *ddc =3D PC_DIMM_GET_CLASS(dimm); > + MemoryRegion *mr =3D ddc->get_memory_region(dimm); > + > + pc_dimm_memory_unplug(dev, &ms->hotplug_memory, mr); > + object_unparent(OBJECT(dev)); > +} > + > +static void spapr_memory_unplug_request(HotplugHandler *hotplug_dev, > + DeviceState *dev, Error **errp) > +{ > + Error *local_err =3D NULL; > + PCDIMMDevice *dimm =3D PC_DIMM(dev); > + PCDIMMDeviceClass *ddc =3D PC_DIMM_GET_CLASS(dimm); > + MemoryRegion *mr =3D ddc->get_memory_region(dimm); > + uint64_t size =3D memory_region_size(mr); > + uint64_t addr; > + > + addr =3D object_property_get_int(OBJECT(dimm), PC_DIMM_ADDR_PROP, &l= ocal_err); > + if (local_err) { > + goto out; > + } > + > + dimm->delete_pending =3D true; > + spapr_del_lmbs(dev, addr, size, &local_err); > +out: > + error_propagate(errp, local_err); > +} > + > static void spapr_machine_device_plug(HotplugHandler *hotplug_dev, > DeviceState *dev, Error **errp) > { > @@ -2157,7 +2260,15 @@ static void spapr_machine_device_unplug(HotplugHan= dler *hotplug_dev, > DeviceState *dev, Error **errp) > { > if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { > - error_setg(errp, "Memory hot unplug not supported by sPAPR"); > + spapr_memory_unplug(hotplug_dev, dev, errp); > + } > +} > + > +static void spapr_machine_device_unplug_request(HotplugHandler *hotplug_= dev, > + DeviceState *dev, Error = **errp) > +{ > + if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { > + spapr_memory_unplug_request(hotplug_dev, dev, errp); > } > } > =20 > @@ -2191,6 +2302,7 @@ static void spapr_machine_class_init(ObjectClass *o= c, void *data) > mc->get_hotplug_handler =3D spapr_get_hotpug_handler; > hc->plug =3D spapr_machine_device_plug; > hc->unplug =3D spapr_machine_device_unplug; > + hc->unplug_request =3D spapr_machine_device_unplug_request; > =20 > smc->dr_lmb_enabled =3D false; > fwc->get_dev_path =3D spapr_get_fw_dev_path; > diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c > index 8cbcf4d..b9d7c71 100644 > --- a/hw/ppc/spapr_drc.c > +++ b/hw/ppc/spapr_drc.c > @@ -11,6 +11,7 @@ > */ > =20 > #include "hw/ppc/spapr_drc.h" > +#include "hw/ppc/spapr.h" > #include "qom/object.h" > #include "hw/qdev.h" > #include "qapi/visitor.h" > @@ -63,9 +64,29 @@ static int set_isolation_state(sPAPRDRConnector *drc, > sPAPRDRIsolationState state) > { > sPAPRDRConnectorClass *drck =3D SPAPR_DR_CONNECTOR_GET_CLASS(drc); > + int ret; > =20 > DPRINTFN("drc: %x, set_isolation_state: %x", get_index(drc), state); > =20 > + /* > + * Fail any requests to ISOLATE the LMB DRC if this LMB doesn't > + * belong to a DIMM device that is marked for removal. > + * > + * Currently the guest userspace tool drmgr that drives the memory > + * hotplug/unplug will just try to remove a set of 'removable' LMBs > + * in response to a hot unplug request that is based on drc-count. > + * If the LMB being removed doesn't belong to a DIMM device that is > + * actually being unplugged, fail the isolation request here. > + * > + * TODO: Calling out from spapr_drc.c like this doesn't look good. > + */ > + if (drc->type =3D=3D SPAPR_DR_CONNECTOR_TYPE_LMB) { > + ret =3D spapr_lmb_in_removable_dimm(drc, state); > + if (ret) { > + return RTAS_OUT_HW_ERROR; > + } > + } > + > drc->isolation_state =3D state; > =20 > if (drc->isolation_state =3D=3D SPAPR_DR_ISOLATION_STATE_ISOLATED) { > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > index d87c6d4..2039ca1 100644 > --- a/include/hw/ppc/spapr.h > +++ b/include/hw/ppc/spapr.h > @@ -594,6 +594,8 @@ void spapr_hotplug_req_add_by_count(sPAPRDRConnectorT= ype drc_type, > uint32_t count); > void spapr_hotplug_req_remove_by_count(sPAPRDRConnectorType drc_type, > uint32_t count); > +int spapr_lmb_in_removable_dimm(sPAPRDRConnector *drc, > + sPAPRDRIsolationState state); > =20 > /* rtas-configure-connector state */ > struct sPAPRConfigureConnectorState { --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --k8ylJtLa18RSEDYA Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJV6UYqAAoJEGw4ysog2bOS4GYP/jbLJijL4pfF4ozOvjroj93d zdps+nDkqxa24aoWKI8nO/YEp4Or1PyMbELJbEW+ghblbLUGspPk5m9W/AV0gbeR +5kBCE+tfv8PZkXzp7Qgqetp6sm1zyRF/iDQ8HPPOOYPOT0JZLStw3GzLADkffZT tXD9d6M/1DcbJiSu+KzHhoO2N1oo7erjRJpcEuh2CmPPy6y/hz+f45tUb3wPEKKn F1odS18MzDBvadFnnvyJiIO2NOOmKxsRd4hkdzBoh18OcygQ9ntDZwQ8ZH05UePg tKWdOmIaL69VFOgmLmRcrG9uHiVfr1GJd12hQjYjbJL7vQ8+MJN4+0l+gcZ7Bj4G 9T9aH6fUA942ruq+a/9tdklPQsu05v68TPic9p/ZDAMZ4DopZR0AaZBJRE3NWFuy RFBrURSYCbzApRAuh6C+oIiNnHt7wUXNlAeE/9kVgfVjdQ6hY1tfzAOBXI8J47B3 cWFxjE0UAoko2tOvQbXOjKW3SqNOBRSaX8UDyQPA5dnmF/e2r1t1K21UksgTILcV E+uFv5uJ/PbyelPjBliPwAMMbkKIpJPCtw8ZDG9YnNRYIN/oca8NDzjYaUMVTNSU eOJlGpH/wcwhWhc7vdiQNZP+jAKp99O8aJwyA3KVUvCdAh2N3jQaaZvj7iNJkJFo 61uNeODJG1WGleZjJE4k =TwRe -----END PGP SIGNATURE----- --k8ylJtLa18RSEDYA--