From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56861) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bHOSL-0006UL-48 for qemu-devel@nongnu.org; Mon, 27 Jun 2016 00:47:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bHOSI-0001wC-2P for qemu-devel@nongnu.org; Mon, 27 Jun 2016 00:47:33 -0400 Date: Mon, 27 Jun 2016 14:26:30 +1000 From: David Gibson Message-ID: <20160627042630.GH4242@voom.fritz.box> References: <1466704050-15108-1-git-send-email-nikunj@linux.vnet.ibm.com> <1466704050-15108-10-git-send-email-nikunj@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="HuscSE0D68UGttcd" Content-Disposition: inline In-Reply-To: <1466704050-15108-10-git-send-email-nikunj@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [PATCH v1 09/11] ppc/xics: Split ICS into ics-base and ics class List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Nikunj A Dadhania Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org, clg@kaod.org, Benjamin Herrenschmidt --HuscSE0D68UGttcd Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Jun 23, 2016 at 11:17:28PM +0530, Nikunj A Dadhania wrote: > From: Benjamin Herrenschmidt >=20 > The existing implementation remains same and ics-base is introduced. >=20 > This will allow different implementations for the source controllers > such as the MSI support of PHB3 on Power8 which uses in-memory state > tables for example. >=20 > Signed-off-by: Benjamin Herrenschmidt > Signed-off-by: Nikunj A Dadhania > --- > hw/intc/xics.c | 101 +++++++++++++++++++++++++++++++++-----------= ------ > hw/intc/xics_spapr.c | 36 ++++++++++-------- > include/hw/ppc/xics.h | 11 +++++- > 3 files changed, 97 insertions(+), 51 deletions(-) >=20 > diff --git a/hw/intc/xics.c b/hw/intc/xics.c > index 326d21f..e2aa48d 100644 > --- a/hw/intc/xics.c > +++ b/hw/intc/xics.c > @@ -220,9 +220,32 @@ static const TypeInfo xics_common_info =3D { > #define XISR(ss) (((ss)->xirr) & XISR_MASK) > #define CPPR(ss) (((ss)->xirr) >> 24) > =20 > -static void ics_reject(ICSState *ics, int nr); > -static void ics_resend(ICSState *ics); > -static void ics_eoi(ICSState *ics, int nr); > +static void ics_base_reject(ICSState *ics, uint32_t nr) AFICT these will actually work for any of the derived classes, since they call the function pointer. So I thin the original name was better than ics_base_*(). > +{ > + ICSStateClass *k =3D ICS_GET_CLASS(ics); > + > + if (k->reject) { > + k->reject(ics, nr); > + } > +} > + > +static void ics_base_resend(ICSState *ics) > +{ > + ICSStateClass *k =3D ICS_GET_CLASS(ics); > + > + if (k->resend) { > + k->resend(ics); > + } > +} > + > +static void ics_base_eoi(ICSState *ics, int nr) > +{ > + ICSStateClass *k =3D ICS_GET_CLASS(ics); > + > + if (k->eoi) { > + k->eoi(ics, nr); > + } > +} > =20 > static void icp_check_ipi(ICPState *ss, int server) > { > @@ -233,7 +256,7 @@ static void icp_check_ipi(ICPState *ss, int server) > trace_xics_icp_check_ipi(server, ss->mfrr); > =20 > if (XISR(ss) && ss->xirr_owner) { > - ics_reject(ss->xirr_owner, XISR(ss)); > + ics_base_reject(ss->xirr_owner, XISR(ss)); > } > =20 > ss->xirr =3D (ss->xirr & ~XISR_MASK) | XICS_IPI; > @@ -251,7 +274,7 @@ static void icp_resend(XICSState *xics, int server) > icp_check_ipi(ss, server); > } > QLIST_FOREACH(ics, &xics->ics, list) { > - ics_resend(ics); > + ics_base_resend(ics); > } > } > =20 > @@ -271,7 +294,7 @@ void icp_set_cppr(XICSState *xics, int server, uint8_= t cppr) > ss->pending_priority =3D 0xff; > qemu_irq_lower(ss->output); > if (ss->xirr_owner) { > - ics_reject(ss->xirr_owner, old_xisr); > + ics_base_reject(ss->xirr_owner, old_xisr); > ss->xirr_owner =3D NULL; > } > } > @@ -325,8 +348,8 @@ void icp_eoi(XICSState *xics, int server, uint32_t xi= rr) > trace_xics_icp_eoi(server, xirr, ss->xirr); > irq =3D xirr & XISR_MASK; > QLIST_FOREACH(ics, &xics->ics, list) { > - if (ics_valid_irq(ics, irq)) { > - ics_eoi(ics, irq); > + if (ics_base_valid_irq(ics, irq)) { > + ics_base_eoi(ics, irq); > } > } > if (!XISR(ss)) { > @@ -343,10 +366,10 @@ static void icp_irq(ICSState *ics, int server, int = nr, uint8_t priority) > =20 > if ((priority >=3D CPPR(ss)) > || (XISR(ss) && (ss->pending_priority <=3D priority))) { > - ics_reject(ics, nr); > + ics_base_reject(ics, nr); > } else { > if (XISR(ss) && ss->xirr_owner) { > - ics_reject(ss->xirr_owner, XISR(ss)); > + ics_base_reject(ss->xirr_owner, XISR(ss)); > ss->xirr_owner =3D NULL; > } > ss->xirr =3D (ss->xirr & ~XISR_MASK) | (nr & XISR_MASK); > @@ -425,7 +448,7 @@ static const TypeInfo icp_info =3D { > /* > * ICS: Source layer > */ > -static void resend_msi(ICSState *ics, int srcno) > +static void ics_resend_msi(ICSState *ics, int srcno) > { > ICSIRQState *irq =3D ics->irqs + srcno; > =20 > @@ -438,7 +461,7 @@ static void resend_msi(ICSState *ics, int srcno) > } > } > =20 > -static void resend_lsi(ICSState *ics, int srcno) > +static void ics_resend_lsi(ICSState *ics, int srcno) > { > ICSIRQState *irq =3D ics->irqs + srcno; > =20 > @@ -450,7 +473,7 @@ static void resend_lsi(ICSState *ics, int srcno) > } > } > =20 > -static void set_irq_msi(ICSState *ics, int srcno, int val) > +static void ics_set_irq_msi(ICSState *ics, int srcno, int val) > { > ICSIRQState *irq =3D ics->irqs + srcno; > =20 > @@ -466,7 +489,7 @@ static void set_irq_msi(ICSState *ics, int srcno, int= val) > } > } > =20 > -static void set_irq_lsi(ICSState *ics, int srcno, int val) > +static void ics_set_irq_lsi(ICSState *ics, int srcno, int val) > { > ICSIRQState *irq =3D ics->irqs + srcno; > =20 > @@ -476,7 +499,7 @@ static void set_irq_lsi(ICSState *ics, int srcno, int= val) > } else { > irq->status &=3D ~XICS_STATUS_ASSERTED; > } > - resend_lsi(ics, srcno); > + ics_resend_lsi(ics, srcno); > } > =20 > static void ics_set_irq(void *opaque, int srcno, int val) > @@ -484,13 +507,13 @@ static void ics_set_irq(void *opaque, int srcno, in= t val) > ICSState *ics =3D (ICSState *)opaque; > =20 > if (ics->irqs[srcno].flags & XICS_FLAGS_IRQ_LSI) { > - set_irq_lsi(ics, srcno, val); > + ics_set_irq_lsi(ics, srcno, val); > } else { > - set_irq_msi(ics, srcno, val); > + ics_set_irq_msi(ics, srcno, val); > } > } > =20 > -static void write_xive_msi(ICSState *ics, int srcno) > +static void ics_write_xive_msi(ICSState *ics, int srcno) > { > ICSIRQState *irq =3D ics->irqs + srcno; > =20 > @@ -503,31 +526,30 @@ static void write_xive_msi(ICSState *ics, int srcno) > icp_irq(ics, irq->server, srcno + ics->offset, irq->priority); > } > =20 > -static void write_xive_lsi(ICSState *ics, int srcno) > +static void ics_write_xive_lsi(ICSState *ics, int srcno) > { > - resend_lsi(ics, srcno); > + ics_resend_lsi(ics, srcno); > } > =20 > -void ics_write_xive(ICSState *ics, int nr, int server, > - uint8_t priority, uint8_t saved_priority) > +void ics_write_xive(ICSState *ics, int srcno, int server, > + uint8_t priority, uint8_t saved_priority) > { > - int srcno =3D nr - ics->offset; > ICSIRQState *irq =3D ics->irqs + srcno; > =20 > irq->server =3D server; > irq->priority =3D priority; > irq->saved_priority =3D saved_priority; > =20 > - trace_xics_ics_write_xive(nr, srcno, server, priority); > + trace_xics_ics_write_xive(ics->offset + srcno, srcno, server, priori= ty); > =20 > if (ics->irqs[srcno].flags & XICS_FLAGS_IRQ_LSI) { > - write_xive_lsi(ics, srcno); > + ics_write_xive_lsi(ics, srcno); > } else { > - write_xive_msi(ics, srcno); > + ics_write_xive_msi(ics, srcno); > } > } > =20 > -static void ics_reject(ICSState *ics, int nr) > +static void ics_reject(ICSState *ics, uint32_t nr) > { > ICSIRQState *irq =3D ics->irqs + nr - ics->offset; > =20 > @@ -543,14 +565,14 @@ static void ics_resend(ICSState *ics) > for (i =3D 0; i < ics->nr_irqs; i++) { > /* FIXME: filter by server#? */ > if (ics->irqs[i].flags & XICS_FLAGS_IRQ_LSI) { > - resend_lsi(ics, i); > + ics_resend_lsi(ics, i); > } else { > - resend_msi(ics, i); > + ics_resend_msi(ics, i); > } > } > } > =20 > -static void ics_eoi(ICSState *ics, int nr) > +static void ics_eoi(ICSState *ics, uint32_t nr) > { > int srcno =3D nr - ics->offset; > ICSIRQState *irq =3D ics->irqs + srcno; > @@ -639,7 +661,8 @@ static const VMStateDescription vmstate_ics =3D { > VMSTATE_UINT32_EQUAL(nr_irqs, ICSState), > =20 > VMSTATE_STRUCT_VARRAY_POINTER_UINT32(irqs, ICSState, nr_irqs, > - vmstate_ics_irq, ICSIRQStat= e), > + vmstate_ics_irq, > + ICSIRQState), > VMSTATE_END_OF_LIST() > }, > }; > @@ -672,17 +695,28 @@ static void ics_class_init(ObjectClass *klass, void= *data) > dc->vmsd =3D &vmstate_ics; > dc->reset =3D ics_reset; > isc->post_load =3D ics_post_load; > + isc->reject =3D ics_reject; > + isc->resend =3D ics_resend; > + isc->eoi =3D ics_eoi; > } > =20 > static const TypeInfo ics_info =3D { > .name =3D TYPE_ICS, > - .parent =3D TYPE_DEVICE, > + .parent =3D TYPE_ICS_BASE, > .instance_size =3D sizeof(ICSState), > .class_init =3D ics_class_init, > .class_size =3D sizeof(ICSStateClass), > .instance_init =3D ics_initfn, > }; > =20 > +static const TypeInfo ics_base_info =3D { > + .name =3D TYPE_ICS_BASE, > + .parent =3D TYPE_DEVICE, > + .abstract =3D true, > + .instance_size =3D sizeof(ICSState), > + .class_size =3D sizeof(ICSStateClass), > +}; > + > /* > * Exported functions > */ > @@ -691,7 +725,7 @@ ICSState *xics_find_source(XICSState *xics, int irq) > ICSState *ics; > =20 > QLIST_FOREACH(ics, &xics->ics, list) { > - if (ics_valid_irq(ics, irq)) { > + if (ics_base_valid_irq(ics, irq)) { > return ics; > } > } > @@ -721,6 +755,7 @@ static void xics_register_types(void) > { > type_register_static(&xics_common_info); > type_register_static(&ics_info); > + type_register_static(&ics_base_info); > type_register_static(&icp_info); > } > =20 > diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c > index 8acafd9..113b067 100644 > --- a/hw/intc/xics_spapr.c > +++ b/hw/intc/xics_spapr.c > @@ -114,7 +114,7 @@ static void rtas_set_xive(PowerPCCPU *cpu, sPAPRMachi= neState *spapr, > uint32_t nret, target_ulong rets) > { > ICSState *ics =3D QLIST_FIRST(&spapr->xics->ics); > - uint32_t nr, server, priority; > + uint32_t nr, srcno, server, priority; > =20 > if ((nargs !=3D 3) || (nret !=3D 1) || !ics) { > rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); > @@ -125,13 +125,14 @@ static void rtas_set_xive(PowerPCCPU *cpu, sPAPRMac= hineState *spapr, > server =3D get_cpu_index_by_dt_id(rtas_ld(args, 1)); > priority =3D rtas_ld(args, 2); > =20 > - if (!ics_valid_irq(ics, nr) || (server >=3D ics->xics->nr_servers) > + if (!ics_base_valid_irq(ics, nr) || (server >=3D ics->xics->nr_serve= rs) > || (priority > 0xff)) { > rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); > return; > } > =20 > - ics_write_xive(ics, nr, server, priority, priority); > + srcno =3D nr - ics->offset; > + ics_write_xive(ics, srcno, server, priority, priority); > =20 > rtas_st(rets, 0, RTAS_OUT_SUCCESS); > } > @@ -142,7 +143,7 @@ static void rtas_get_xive(PowerPCCPU *cpu, sPAPRMachi= neState *spapr, > uint32_t nret, target_ulong rets) > { > ICSState *ics =3D QLIST_FIRST(&spapr->xics->ics); > - uint32_t nr; > + uint32_t nr, srcno; > =20 > if ((nargs !=3D 1) || (nret !=3D 3) || !ics) { > rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); > @@ -151,14 +152,15 @@ static void rtas_get_xive(PowerPCCPU *cpu, sPAPRMac= hineState *spapr, > =20 > nr =3D rtas_ld(args, 0); > =20 > - if (!ics_valid_irq(ics, nr)) { > + if (!ics_base_valid_irq(ics, nr)) { > rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); > return; > } > =20 > rtas_st(rets, 0, RTAS_OUT_SUCCESS); > - rtas_st(rets, 1, ics->irqs[nr - ics->offset].server); > - rtas_st(rets, 2, ics->irqs[nr - ics->offset].priority); > + srcno =3D nr - ics->offset; > + rtas_st(rets, 1, ics->irqs[srcno].server); > + rtas_st(rets, 2, ics->irqs[srcno].priority); > } > =20 > static void rtas_int_off(PowerPCCPU *cpu, sPAPRMachineState *spapr, > @@ -167,7 +169,7 @@ static void rtas_int_off(PowerPCCPU *cpu, sPAPRMachin= eState *spapr, > uint32_t nret, target_ulong rets) > { > ICSState *ics =3D QLIST_FIRST(&spapr->xics->ics); > - uint32_t nr; > + uint32_t nr, srcno; > =20 > if ((nargs !=3D 1) || (nret !=3D 1) || !ics) { > rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); > @@ -176,13 +178,14 @@ static void rtas_int_off(PowerPCCPU *cpu, sPAPRMach= ineState *spapr, > =20 > nr =3D rtas_ld(args, 0); > =20 > - if (!ics_valid_irq(ics, nr)) { > + if (!ics_base_valid_irq(ics, nr)) { > rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); > return; > } > =20 > - ics_write_xive(ics, nr, ics->irqs[nr - ics->offset].server, 0xff, > - ics->irqs[nr - ics->offset].priority); > + srcno =3D nr - ics->offset; > + ics_write_xive(ics, srcno, ics->irqs[srcno].server, 0xff, > + ics->irqs[srcno].priority); > =20 > rtas_st(rets, 0, RTAS_OUT_SUCCESS); > } > @@ -193,7 +196,7 @@ static void rtas_int_on(PowerPCCPU *cpu, sPAPRMachine= State *spapr, > uint32_t nret, target_ulong rets) > { > ICSState *ics =3D QLIST_FIRST(&spapr->xics->ics); > - uint32_t nr; > + uint32_t nr, srcno; > =20 > if ((nargs !=3D 1) || (nret !=3D 1) || !ics) { > rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); > @@ -202,14 +205,15 @@ static void rtas_int_on(PowerPCCPU *cpu, sPAPRMachi= neState *spapr, > =20 > nr =3D rtas_ld(args, 0); > =20 > - if (!ics_valid_irq(ics, nr)) { > + if (!ics_base_valid_irq(ics, nr)) { > rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); > return; > } > =20 > - ics_write_xive(ics, nr, ics->irqs[nr - ics->offset].server, > - ics->irqs[nr - ics->offset].saved_priority, > - ics->irqs[nr - ics->offset].saved_priority); > + srcno =3D nr - ics->offset; > + ics_write_xive(ics, srcno, ics->irqs[srcno].server, > + ics->irqs[srcno].saved_priority, > + ics->irqs[srcno].saved_priority); > =20 > rtas_st(rets, 0, RTAS_OUT_SUCCESS); > } > diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h > index ee0fce2..6fb1cb4 100644 > --- a/include/hw/ppc/xics.h > +++ b/include/hw/ppc/xics.h > @@ -117,6 +117,10 @@ struct ICPState { > bool cap_irq_xics_enabled; > }; > =20 > +#define TYPE_ICS_BASE "ics-base" > +#define ICS_BASE(obj) OBJECT_CHECK(ICSState, (obj), TYPE_ICS_SIMPLE) > + > +/* Retain ics for sPAPR for migration from existing sPAPR guests */ > #define TYPE_ICS "ics" > #define ICS(obj) OBJECT_CHECK(ICSState, (obj), TYPE_ICS) > =20 > @@ -133,6 +137,9 @@ struct ICSStateClass { > =20 > void (*pre_save)(ICSState *s); > int (*post_load)(ICSState *s, int version_id); > + void (*reject)(ICSState *s, uint32_t irq); > + void (*resend)(ICSState *s); > + void (*eoi)(ICSState *s, uint32_t irq); > }; > =20 > struct ICSState { > @@ -147,7 +154,7 @@ struct ICSState { > QLIST_ENTRY(ICSState) list; > }; > =20 > -static inline bool ics_valid_irq(ICSState *ics, uint32_t nr) > +static inline bool ics_base_valid_irq(ICSState *ics, uint32_t nr) > { > return (ics->offset !=3D 0) && (nr >=3D ics->offset) > && (nr < (ics->offset + ics->nr_irqs)); > @@ -190,7 +197,7 @@ uint32_t icp_ipoll(ICPState *ss, uint32_t *mfrr); > void icp_eoi(XICSState *icp, int server, uint32_t xirr); > =20 > void ics_write_xive(ICSState *ics, int nr, int server, > - uint8_t priority, uint8_t saved_priority); > + uint8_t priority, uint8_t saved_priority); > =20 > void ics_set_irq_type(ICSState *ics, int srcno, bool lsi); > =20 --=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 --HuscSE0D68UGttcd Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJXcKr2AAoJEGw4ysog2bOSwswP+weOjZqgIM1v5lHIR9azUBDA hSgzaQzGLQA48ULHSq3mPeqPuzdt0HRWN0Rjr4+AOXxUoCcZVHBoEYr/0n/3kO2w 4UAJG+7eh4RenrhANzceS9N7NK95PCC/I8Yb/frEDG9C/zv4HKek21EJHkmDEoIo gGtLYRnVvSL/FlW4Lvd/yDMfpKh7WttSCv26LfbxJDkboxxICQZ+IJJCire79sKt agfpt1VAE5ZgXPQ7PzEE4sdSbEqcyEWL+vxrK+vM4kwujVytGpLjHYmgxii42sTG 8K/OJqhd3pGoFmdGz6uKy/d7OSv3LFkhfenKTZhppiXL+uoN7IL43I7HCiLzldCt n75KgpTBwE5Kmv+lOpB1ZyoJJzIOaQ0VAdlwMwL2dLyu9/8P6YJDnkFNGHEeWJOr Qax/+Cl38mOdLtIpyklN6clJxmdoDV+dZa4pzHsR029ul0JJKhkeMNEMSjOEUBXQ aQbwAYTDNhg88y7tTRsnvtaTVe/UMUEYij6G0kWXBGY0jBDcVjoVZLmiA2JIjZn/ JNMXIgZh6iBjOcsWI7JrxHDXLBZKjnXa8eG/fevtTxDDb7TUUk87Ho1WinVPizNJ TJZMKcw7uWd5Ps3HNmyaoy2Dn1lWIq+3P+SKwEWVQm9ePv7PcT6YS13ZgdFDjtAV DMi1BttGcNAgnMDUuyG2 =0ICH -----END PGP SIGNATURE----- --HuscSE0D68UGttcd--