From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39834) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1agaOi-0008Tu-3N for qemu-devel@nongnu.org; Thu, 17 Mar 2016 12:03:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1agaOf-0003Pd-4M for qemu-devel@nongnu.org; Thu, 17 Mar 2016 12:03:40 -0400 Received: from e36.co.us.ibm.com ([32.97.110.154]:37570) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1agaOe-0003OI-KA for qemu-devel@nongnu.org; Thu, 17 Mar 2016 12:03:37 -0400 Received: from localhost by e36.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 17 Mar 2016 10:03:34 -0600 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Michael Roth In-Reply-To: <1458016736-10544-2-git-send-email-bharata@linux.vnet.ibm.com> References: <1458016736-10544-1-git-send-email-bharata@linux.vnet.ibm.com> <1458016736-10544-2-git-send-email-bharata@linux.vnet.ibm.com> Message-ID: <20160317160321.32479.66833@loki> Date: Thu, 17 Mar 2016 11:03:21 -0500 Subject: Re: [Qemu-devel] [RFC PATCH v2 1/2] spapr: Add DRC count indexed hotplug identifier type List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bharata B Rao , qemu-devel@nongnu.org Cc: thuth@redhat.com, qemu-ppc@nongnu.org, imammedo@redhat.com, nfont@linux.vnet.ibm.com, david@gibson.dropbear.id.au Quoting Bharata B Rao (2016-03-14 23:38:55) > Add support for DRC count indexed hotplug ID type which is primarily > needed for memory hot unplug. This type allows for specifying the > number of DRs that should be plugged/unplugged starting from a given > DRC index. > = > NOTE: This new hotplug identifier type is not yet part of PAPR. > = > Signed-off-by: Bharata B Rao > --- > hw/ppc/spapr_events.c | 57 +++++++++++++++++++++++++++++++++++++-------= ------ > include/hw/ppc/spapr.h | 2 ++ > 2 files changed, 45 insertions(+), 14 deletions(-) > = > diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c > index 39f4682..5d1d13d 100644 > --- a/hw/ppc/spapr_events.c > +++ b/hw/ppc/spapr_events.c > @@ -171,6 +171,16 @@ struct epow_log_full { > struct rtas_event_log_v6_epow epow; > } QEMU_PACKED; > = > +union drc_id { > + uint32_t index; > + uint32_t count; > + struct count_index { > + uint32_t index; > + uint32_t count; The current version of the spec proposal is actually count followed by index. I kind of wish it was in the opposite order, and it's probably not too late to change this if there's pressing reason, but that's how things stand atm. > + } count_index; > + char name[1]; > +} QEMU_PACKED; > + > struct rtas_event_log_v6_hp { > #define RTAS_LOG_V6_SECTION_ID_HOTPLUG 0x4850 /* HP */ > struct rtas_event_log_v6_section_header hdr; > @@ -187,12 +197,9 @@ struct rtas_event_log_v6_hp { > #define RTAS_LOG_V6_HP_ID_DRC_NAME 1 > #define RTAS_LOG_V6_HP_ID_DRC_INDEX 2 > #define RTAS_LOG_V6_HP_ID_DRC_COUNT 3 > +#define RTAS_LOG_V6_HP_ID_DRC_COUNT_INDEXED 4 > uint8_t reserved; > - union { > - uint32_t index; > - uint32_t count; > - char name[1]; > - } drc; > + union drc_id drc_id; > } QEMU_PACKED; > = > struct hp_log_full { > @@ -389,7 +396,7 @@ static void spapr_powerdown_req(Notifier *n, void *op= aque) > = > static void spapr_hotplug_req_event(uint8_t hp_id, uint8_t hp_action, > sPAPRDRConnectorType drc_type, > - uint32_t drc) > + union drc_id *drc_id) > { > sPAPRMachineState *spapr =3D SPAPR_MACHINE(qdev_get_machine()); > struct hp_log_full *new_hp; > @@ -446,9 +453,12 @@ static void spapr_hotplug_req_event(uint8_t hp_id, u= int8_t hp_action, > } > = > if (hp_id =3D=3D RTAS_LOG_V6_HP_ID_DRC_COUNT) { > - hp->drc.count =3D cpu_to_be32(drc); > + hp->drc_id.count =3D cpu_to_be32(drc_id->count); > } else if (hp_id =3D=3D RTAS_LOG_V6_HP_ID_DRC_INDEX) { > - hp->drc.index =3D cpu_to_be32(drc); > + hp->drc_id.index =3D cpu_to_be32(drc_id->index); > + } else if (hp_id =3D=3D RTAS_LOG_V6_HP_ID_DRC_COUNT_INDEXED) { > + hp->drc_id.count_index.count =3D cpu_to_be32(drc_id->count_index= .count); > + hp->drc_id.count_index.index =3D cpu_to_be32(drc_id->count_index= .index); > } > = > rtas_event_log_queue(RTAS_LOG_TYPE_HOTPLUG, new_hp, true); > @@ -460,34 +470,53 @@ void spapr_hotplug_req_add_by_index(sPAPRDRConnecto= r *drc) > { > sPAPRDRConnectorClass *drck =3D SPAPR_DR_CONNECTOR_GET_CLASS(drc); > sPAPRDRConnectorType drc_type =3D drck->get_type(drc); > - uint32_t index =3D drck->get_index(drc); > + union drc_id drc_id; I'd rather we used 'union drc_id id' or something. Having the typename and variable names be identical is a little confusing. > + drc_id.index =3D drck->get_index(drc); > = > spapr_hotplug_req_event(RTAS_LOG_V6_HP_ID_DRC_INDEX, > - RTAS_LOG_V6_HP_ACTION_ADD, drc_type, index); > + RTAS_LOG_V6_HP_ACTION_ADD, drc_type, &drc_id= ); > } > = > void spapr_hotplug_req_remove_by_index(sPAPRDRConnector *drc) > { > sPAPRDRConnectorClass *drck =3D SPAPR_DR_CONNECTOR_GET_CLASS(drc); > sPAPRDRConnectorType drc_type =3D drck->get_type(drc); > - uint32_t index =3D drck->get_index(drc); > + union drc_id drc_id; > + drc_id.index =3D drck->get_index(drc); > = > spapr_hotplug_req_event(RTAS_LOG_V6_HP_ID_DRC_INDEX, > - RTAS_LOG_V6_HP_ACTION_REMOVE, drc_type, inde= x); > + RTAS_LOG_V6_HP_ACTION_REMOVE, drc_type, &drc= _id); > } > = > void spapr_hotplug_req_add_by_count(sPAPRDRConnectorType drc_type, > uint32_t count) > { > + union drc_id drc_id; > + drc_id.count =3D count; > + > spapr_hotplug_req_event(RTAS_LOG_V6_HP_ID_DRC_COUNT, > - RTAS_LOG_V6_HP_ACTION_ADD, drc_type, count); > + RTAS_LOG_V6_HP_ACTION_ADD, drc_type, &drc_id= ); > } > = > void spapr_hotplug_req_remove_by_count(sPAPRDRConnectorType drc_type, > uint32_t count) > { > + union drc_id drc_id; > + drc_id.count =3D count; > + > spapr_hotplug_req_event(RTAS_LOG_V6_HP_ID_DRC_COUNT, > - RTAS_LOG_V6_HP_ACTION_REMOVE, drc_type, coun= t); > + RTAS_LOG_V6_HP_ACTION_REMOVE, drc_type, &drc= _id); > +} > + > +void spapr_hotplug_req_remove_by_count_indexed(sPAPRDRConnectorType drc_= type, > + uint32_t count, uint32_t = index) > +{ > + union drc_id drc_id; > + drc_id.count_index.count =3D count; > + drc_id.count_index.index =3D index; > + > + spapr_hotplug_req_event(RTAS_LOG_V6_HP_ID_DRC_COUNT_INDEXED, > + RTAS_LOG_V6_HP_ACTION_REMOVE, drc_type, &drc= _id); > } > = > static void check_exception(PowerPCCPU *cpu, sPAPRMachineState *spapr, > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > index 098d85d..f0c426b 100644 > --- a/include/hw/ppc/spapr.h > +++ b/include/hw/ppc/spapr.h > @@ -585,6 +585,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); > +void spapr_hotplug_req_remove_by_count_indexed(sPAPRDRConnectorType drc_= type, > + uint32_t count, uint32_t = index); > = > /* rtas-configure-connector state */ > struct sPAPRConfigureConnectorState { > -- = > 2.1.0 >=20