From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43995) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1buwWT-0002CR-Hg for qemu-devel@nongnu.org; Fri, 14 Oct 2016 03:03:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1buwWQ-0005Dg-PJ for qemu-devel@nongnu.org; Fri, 14 Oct 2016 03:03:16 -0400 Date: Fri, 14 Oct 2016 16:34:04 +1100 From: David Gibson Message-ID: <20161014053404.GN28562@umbus> References: <1475479496-16158-1-git-send-email-clg@kaod.org> <1475479496-16158-14-git-send-email-clg@kaod.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="D9sZ58tf58331Q5M" Content-Disposition: inline In-Reply-To: <1475479496-16158-14-git-send-email-clg@kaod.org> Subject: Re: [Qemu-devel] [PATCH v4 13/20] ppc/xics: introduce helpers to find an ICP from some (CPU) index List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?iso-8859-1?Q?C=E9dric?= Le Goater Cc: qemu-ppc@nongnu.org, Benjamin Herrenschmidt , qemu-devel@nongnu.org --D9sZ58tf58331Q5M Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Oct 03, 2016 at 09:24:49AM +0200, C=E9dric Le Goater wrote: > Today, the Interrupt Presentation Controllers (ICP) are stored in an > array under the base class XICSState. They are simply indexed using > the CPU index field of CPUState. This made sense for the current > derived classes, spapr and kvm, as the CPU index are contiguous. > Nevertheless some problems have started to occur with CPU hotplug. >=20 > With the PowerNV platform CPUs, this is not true anymore. Real HW ids > are being used and they are not contiguous. So we need a way to > customize the lookups in the array. Below is a proposal for this > purpose. A couple of helpers are introduced to hide the nature of the > underlying ICP array and also a new XICSStateClass method 'find_icp' > to let the derived classes customize the ICP lookups. >=20 > A followup patch would be to let the derived class decide on the ICP > storage. They could use a hash table for instance. We would need to > introduce a new class method 'get_icp' for that. Or simply, change the > ICP array for a hash table and let the derived class decide on the key > to use. >=20 > Signed-off-by: C=E9dric Le Goater Uuuh.. IIRC this series has now been reworked (as suggested) to keep cpu_index contiguous and have a separate hardware id. Doesn't that mean this patch can go away (though the callers might need to use a hwid->cpu_index helper). > --- > hw/intc/xics.c | 48 ++++++++++++++++++++++++++++++++++++---------= --- > hw/intc/xics_kvm.c | 7 ++----- > hw/intc/xics_spapr.c | 12 ++++++------ > include/hw/ppc/xics.h | 2 ++ > 4 files changed, 46 insertions(+), 23 deletions(-) >=20 > diff --git a/hw/intc/xics.c b/hw/intc/xics.c > index 3bbbcc847791..876c472aaa69 100644 > --- a/hw/intc/xics.c > +++ b/hw/intc/xics.c > @@ -48,12 +48,32 @@ int xics_get_cpu_index_by_dt_id(int cpu_dt_id) > return -1; > } > =20 > +ICPState *xics_find_icp(XICSState *xics, int cpu_index) > +{ > + XICSStateClass *xsc =3D XICS_COMMON_GET_CLASS(xics); > + ICPState *icp =3D xsc->find_icp(xics, cpu_index); > + > + assert(icp); > + > + return icp; > +} > + > +static ICPState *xics_get_icp(XICSState *xics, CPUState *cs) > +{ > + ICPState *ss; > + > + assert(cs->cpu_index < xics->nr_servers); > + > + ss =3D &xics->ss[cs->cpu_index]; > + ss->cs =3D cs; > + return ss; > +} > + > void xics_cpu_destroy(XICSState *xics, PowerPCCPU *cpu) > { > CPUState *cs =3D CPU(cpu); > - ICPState *ss =3D &xics->ss[cs->cpu_index]; > + ICPState *ss =3D xics_find_icp(xics, cs->cpu_index); > =20 > - assert(cs->cpu_index < xics->nr_servers); > assert(cs =3D=3D ss->cs); > =20 > ss->output =3D NULL; > @@ -64,13 +84,9 @@ void xics_cpu_setup(XICSState *xics, PowerPCCPU *cpu) > { > CPUState *cs =3D CPU(cpu); > CPUPPCState *env =3D &cpu->env; > - ICPState *ss =3D &xics->ss[cs->cpu_index]; > + ICPState *ss =3D xics_get_icp(xics, cs); > XICSStateClass *info =3D XICS_COMMON_GET_CLASS(xics); > =20 > - assert(cs->cpu_index < xics->nr_servers); > - > - ss->cs =3D cs; > - > if (info->cpu_setup) { > info->cpu_setup(xics, cpu); > } > @@ -94,6 +110,12 @@ void xics_cpu_setup(XICSState *xics, PowerPCCPU *cpu) > /* > * XICS Common class - parent for emulated XICS and KVM-XICS > */ > + > +static ICPState *xics_common_find_icp(XICSState *xics, int cpu_index) > +{ > + return &xics->ss[cpu_index]; > +} > + > static void xics_common_reset(DeviceState *d) > { > XICSState *xics =3D XICS_COMMON(d); > @@ -191,8 +213,10 @@ static void xics_common_initfn(Object *obj) > static void xics_common_class_init(ObjectClass *oc, void *data) > { > DeviceClass *dc =3D DEVICE_CLASS(oc); > + XICSStateClass *xsc =3D XICS_COMMON_CLASS(oc); > =20 > dc->reset =3D xics_common_reset; > + xsc->find_icp =3D xics_common_find_icp; > } > =20 > static const TypeInfo xics_common_info =3D { > @@ -261,7 +285,7 @@ static void icp_check_ipi(ICPState *ss) > =20 > static void icp_resend(XICSState *xics, int server) > { > - ICPState *ss =3D xics->ss + server; > + ICPState *ss =3D xics_find_icp(xics, server); > ICSState *ics; > =20 > if (ss->mfrr < CPPR(ss)) { > @@ -274,7 +298,7 @@ static void icp_resend(XICSState *xics, int server) > =20 > void icp_set_cppr(XICSState *xics, int server, uint8_t cppr) > { > - ICPState *ss =3D xics->ss + server; > + ICPState *ss =3D xics_find_icp(xics, server); > uint8_t old_cppr; > uint32_t old_xisr; > =20 > @@ -301,7 +325,7 @@ void icp_set_cppr(XICSState *xics, int server, uint8_= t cppr) > =20 > void icp_set_mfrr(XICSState *xics, int server, uint8_t mfrr) > { > - ICPState *ss =3D xics->ss + server; > + ICPState *ss =3D xics_find_icp(xics, server); > =20 > ss->mfrr =3D mfrr; > if (mfrr < CPPR(ss)) { > @@ -333,7 +357,7 @@ uint32_t icp_ipoll(ICPState *ss, uint32_t *mfrr) > =20 > void icp_eoi(XICSState *xics, int server, uint32_t xirr) > { > - ICPState *ss =3D xics->ss + server; > + ICPState *ss =3D xics_find_icp(xics, server); > ICSState *ics; > uint32_t irq; > =20 > @@ -354,7 +378,7 @@ void icp_eoi(XICSState *xics, int server, uint32_t xi= rr) > static void icp_irq(ICSState *ics, int server, int nr, uint8_t priority) > { > XICSState *xics =3D ics->xics; > - ICPState *ss =3D xics->ss + server; > + ICPState *ss =3D xics_find_icp(xics, server); > =20 > trace_xics_icp_irq(server, nr, priority); > =20 > diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c > index 9c2f198fd142..b666bb59fc24 100644 > --- a/hw/intc/xics_kvm.c > +++ b/hw/intc/xics_kvm.c > @@ -326,14 +326,11 @@ static const TypeInfo ics_kvm_info =3D { > */ > static void xics_kvm_cpu_setup(XICSState *xics, PowerPCCPU *cpu) > { > - CPUState *cs; > - ICPState *ss; > + CPUState *cs =3D CPU(cpu); > + ICPState *ss =3D xics_find_icp(xics, cs->cpu_index); > KVMXICSState *xicskvm =3D XICS_SPAPR_KVM(xics); > int ret; > =20 > - cs =3D CPU(cpu); > - ss =3D &xics->ss[cs->cpu_index]; > - > assert(cs->cpu_index < xics->nr_servers); > if (xicskvm->kernel_xics_fd =3D=3D -1) { > abort(); > diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c > index e8d0623c2cb5..af29998b1255 100644 > --- a/hw/intc/xics_spapr.c > +++ b/hw/intc/xics_spapr.c > @@ -67,9 +67,9 @@ static target_ulong h_xirr(PowerPCCPU *cpu, sPAPRMachin= eState *spapr, > target_ulong opcode, target_ulong *args) > { > CPUState *cs =3D CPU(cpu); > - uint32_t xirr =3D icp_accept(spapr->xics->ss + cs->cpu_index); > + ICPState *ss =3D xics_find_icp(spapr->xics, cs->cpu_index); > =20 > - args[0] =3D xirr; > + args[0] =3D icp_accept(ss); > return H_SUCCESS; > } > =20 > @@ -77,10 +77,9 @@ static target_ulong h_xirr_x(PowerPCCPU *cpu, sPAPRMac= hineState *spapr, > target_ulong opcode, target_ulong *args) > { > CPUState *cs =3D CPU(cpu); > - ICPState *ss =3D &spapr->xics->ss[cs->cpu_index]; > - uint32_t xirr =3D icp_accept(ss); > + ICPState *ss =3D xics_find_icp(spapr->xics, cs->cpu_index); > =20 > - args[0] =3D xirr; > + args[0] =3D icp_accept(ss); > args[1] =3D cpu_get_host_ticks(); > return H_SUCCESS; > } > @@ -99,8 +98,9 @@ static target_ulong h_ipoll(PowerPCCPU *cpu, sPAPRMachi= neState *spapr, > target_ulong opcode, target_ulong *args) > { > CPUState *cs =3D CPU(cpu); > + ICPState *ss =3D xics_find_icp(spapr->xics, cs->cpu_index); > uint32_t mfrr; > - uint32_t xirr =3D icp_ipoll(spapr->xics->ss + cs->cpu_index, &mfrr); > + uint32_t xirr =3D icp_ipoll(ss, &mfrr); > =20 > args[0] =3D xirr; > args[1] =3D mfrr; > diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h > index ca9f8da542e0..52c426d409c9 100644 > --- a/include/hw/ppc/xics.h > +++ b/include/hw/ppc/xics.h > @@ -76,6 +76,7 @@ struct XICSStateClass { > void (*cpu_setup)(XICSState *icp, PowerPCCPU *cpu); > void (*set_nr_irqs)(XICSState *icp, uint32_t nr_irqs, Error **errp); > void (*set_nr_servers)(XICSState *icp, uint32_t nr_servers, Error **= errp); > + ICPState *(*find_icp)(XICSState *xics, int cpu_index); > }; > =20 > struct XICSState { > @@ -206,5 +207,6 @@ void ics_set_irq_type(ICSState *ics, int srcno, bool = lsi); > ICSState *xics_find_source(XICSState *icp, int irq); > =20 > void xics_hmp_info_pic(Monitor *mon, const QDict *qdict); > +ICPState *xics_find_icp(XICSState *xics, int cpu_index); > =20 > #endif /* XICS_H */ --=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 --D9sZ58tf58331Q5M Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJYAG5LAAoJEGw4ysog2bOSqaUP/26GncSVU3eYXFc0fg7JkKPQ TQSGFObqFctnV2koDFXu4Zkt9ldYq8BR/Y/7onxTDxCeQNy3+TTFZsXLROZzbKVT gTQBBSG0UiSRD5CJ7R12oRWW29TTly/XduD+mc68rkPYBrRuJmUY5R1mbdKMVGyT ZQTVpB4JPpVdeKVBlSIBNCYvs2AelSc5tci1Z06Ft67Z28s4WdrIikbcfQWbH7ce y7K3dGjwOMh3DtWL+W9A5tgr18XNhR1jcevc5yT0bkL8SD46srBRbkjXj1cVTzml 3l2CpmRibxeNmIemhwsTRbFygBn/+J18k0M/3gbCHHlg1F6ZpmyDMrJfXUIhHaVO tiq+ocQm2IWgZmRplkdRweJKkcH/G9Qu2HtbNngA0L9aD9uIEVGR65WLF6yIa3H5 KXC4tjxG0j9Dh6hXT+UkUSQpVnTMck1B9Oc/c667QoHQ8hWzBYJu8XiKQ3l8FHuw 7wfFo9lGkqtXBeOJi3x21rh18xURsphWhb5b7BwDOiCzrtVzWhGMCY3BCRW2mc9A 1T3lptDQ3/NNJ3Mq0WTtb9iFXLBkEq2ZJfx5UYNbwSqDtYea4f9ZXUOMNcKkotc/ Zbsmp31V/RgXFPJ2aCvt5Rqej+em5YfSqE2wScNUhvmkntPUGqVdnfLdKqPVq5vH nmjc8du3pv4+jBfUhoOM =IWCz -----END PGP SIGNATURE----- --D9sZ58tf58331Q5M--