From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48973) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gYm4R-0004y6-GM for qemu-devel@nongnu.org; Mon, 17 Dec 2018 01:08:04 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gYm4Q-0008UF-1q for qemu-devel@nongnu.org; Mon, 17 Dec 2018 01:08:03 -0500 Date: Mon, 17 Dec 2018 17:01:39 +1100 From: David Gibson Message-ID: <20181217060139.GG5597@umbus.fritz.box> References: <20181211223823.13770-1-clg@kaod.org> <20181211223823.13770-10-clg@kaod.org> <4a9ab4ce-c7d7-9744-e5ea-10df38806e9a@kaod.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="HB4mHL4PVvkpZAgW" Content-Disposition: inline In-Reply-To: <4a9ab4ce-c7d7-9744-e5ea-10df38806e9a@kaod.org> Subject: Re: [Qemu-devel] [PATCH v8 09/12] spapr: set the interrupt presenter at reset 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, qemu-devel@nongnu.org, Benjamin Herrenschmidt --HB4mHL4PVvkpZAgW Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Dec 13, 2018 at 01:52:14PM +0100, C=E9dric Le Goater wrote: > On 12/11/18 11:38 PM, C=E9dric Le Goater wrote: > > Currently, the interrupt presenter of the vCPU is set at realize > > time. Setting it at reset will become necessary when the new machine > > supporting both interrupt modes is introduced. In this machine, the > > interrupt mode is chosen at CAS time and activated after a reset. >=20 > I think we need a second '->intc' pointer specific to XIVE instead.=20 > How about ->intc_xive ? Ah, interesting. If we're going to need that, we might as well go to specific ->icp and ->tctx pointers with the right types. I don't particularly like having those machine specific pointers within the cpu structure, but we can look at fixing that later by reviving my patches to move various machine specific stuff within the cpu into a separate sub-allocation. >=20 > We could just drop this patch and easly get rid of the XiveTCTX object=20 > leak in spapr_unrealize_vcpu(). >=20 > C.=20 >=20 > > Signed-off-by: C=E9dric Le Goater > > --- > >=20 > > Changes since v7: > >=20 > > - introduced spapr_irq_reset_xics().=20 > >=20 > > include/hw/ppc/spapr_cpu_core.h | 2 ++ > > hw/ppc/spapr_cpu_core.c | 26 ++++++++++++++++++++++++++ > > hw/ppc/spapr_irq.c | 13 +++++++++++++ > > 3 files changed, 41 insertions(+) > >=20 > > diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu= _core.h > > index 9e2821e4b31f..fc8ea9021656 100644 > > --- a/include/hw/ppc/spapr_cpu_core.h > > +++ b/include/hw/ppc/spapr_cpu_core.h > > @@ -53,4 +53,6 @@ static inline sPAPRCPUState *spapr_cpu_state(PowerPCC= PU *cpu) > > return (sPAPRCPUState *)cpu->machine_data; > > } > > =20 > > +void spapr_cpu_core_set_intc(PowerPCCPU *cpu, const char *intc_type); > > + > > #endif > > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c > > index 82666436e9b4..afc5d4f4e739 100644 > > --- a/hw/ppc/spapr_cpu_core.c > > +++ b/hw/ppc/spapr_cpu_core.c > > @@ -397,3 +397,29 @@ static const TypeInfo spapr_cpu_core_type_infos[] = =3D { > > }; > > =20 > > DEFINE_TYPES(spapr_cpu_core_type_infos) > > + > > +typedef struct ForeachFindIntCArgs { > > + const char *intc_type; > > + Object *intc; > > +} ForeachFindIntCArgs; > > + > > +static int spapr_cpu_core_find_intc(Object *child, void *opaque) > > +{ > > + ForeachFindIntCArgs *args =3D opaque; > > + > > + if (object_dynamic_cast(child, args->intc_type)) { > > + args->intc =3D child; > > + } > > + > > + return args->intc !=3D NULL; > > +} > > + > > +void spapr_cpu_core_set_intc(PowerPCCPU *cpu, const char *intc_type) > > +{ > > + ForeachFindIntCArgs args =3D { intc_type, NULL }; > > + > > + object_child_foreach(OBJECT(cpu), spapr_cpu_core_find_intc, &args); > > + g_assert(args.intc); > > + > > + cpu->intc =3D args.intc; > > +} > > diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c > > index 0999a2b2d69c..814500f22d34 100644 > > --- a/hw/ppc/spapr_irq.c > > +++ b/hw/ppc/spapr_irq.c > > @@ -12,6 +12,7 @@ > > #include "qemu/error-report.h" > > #include "qapi/error.h" > > #include "hw/ppc/spapr.h" > > +#include "hw/ppc/spapr_cpu_core.h" > > #include "hw/ppc/spapr_xive.h" > > #include "hw/ppc/xics.h" > > #include "sysemu/kvm.h" > > @@ -208,6 +209,15 @@ static int spapr_irq_post_load_xics(sPAPRMachineSt= ate *spapr, int version_id) > > return 0; > > } > > =20 > > +static void spapr_irq_reset_xics(sPAPRMachineState *spapr, Error **err= p) > > +{ > > + CPUState *cs; > > + > > + CPU_FOREACH(cs) { > > + spapr_cpu_core_set_intc(POWERPC_CPU(cs), spapr->icp_type); > > + } > > +} > > + > > #define SPAPR_IRQ_XICS_NR_IRQS 0x1000 > > #define SPAPR_IRQ_XICS_NR_MSIS \ > > (XICS_IRQ_BASE + SPAPR_IRQ_XICS_NR_IRQS - SPAPR_IRQ_MSI) > > @@ -225,6 +235,7 @@ sPAPRIrq spapr_irq_xics =3D { > > .dt_populate =3D spapr_dt_xics, > > .cpu_intc_create =3D spapr_irq_cpu_intc_create_xics, > > .post_load =3D spapr_irq_post_load_xics, > > + .reset =3D spapr_irq_reset_xics, > > }; > > =20 > > /* > > @@ -325,6 +336,8 @@ static void spapr_irq_reset_xive(sPAPRMachineState = *spapr, Error **errp) > > CPU_FOREACH(cs) { > > PowerPCCPU *cpu =3D POWERPC_CPU(cs); > > =20 > > + spapr_cpu_core_set_intc(cpu, TYPE_XIVE_TCTX); > > + > > /* (TCG) Set the OS CAM line of the thread interrupt context. = */ > > spapr_xive_set_tctx_os_cam(XIVE_TCTX(cpu->intc)); > > } > >=20 >=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 --HB4mHL4PVvkpZAgW Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEdfRlhq5hpmzETofcbDjKyiDZs5IFAlwXO8AACgkQbDjKyiDZ s5JcvA//aUiwrVbAn5C650WKBcKAhc5ATu95rMfDQVUo0HOLFpN1+V3CJdLodop6 A0/IXEsMjAGh7FhCA1JVhUooLaUUS4SsekiwpcQ2vQR+NurqvIU+3NIkux8urYD4 AP7UiFDpS59tIlLqp9K2/53a9OTWiJIOw1m+87QVT0B1y1h5xVqxGr2I8uvvyi3V RpeZDPxq9pmFQEgv7EUR3KFGpDMod/JswpUoALIhCfGmr05NK9k4wFyZLuuTgIdv J1DYuVuKU0TieXHWl2n+KkdWV9ZXuNnHVojVgKMeCUbz1P1N8W0zjzB/7zIde2IP /rNai/hFITD/Ic8F4GfB76rwGrVol4S7t0tYcbq+Z12eWGbOMQJlMK5jrECgtbTp Cwb/vZ8jHiiPgpCmde7XNLIMi8awex3w3qfmZNMBhsyUiQm8VtORoPuNnfXxMVcg csff1FrPhHVwJmhsl8SxKCepnkChhzufncnY7At6eTTmDWV9HTUa/JAbnFi7/yyM q0IiJlP00F+cDGCuvHz3PZ34qos28t505uHFRG8aNf7oeNI5mLvSPheAdaGM7AYB XlUjM/NixI9T3gZuHMCxODdLSZLcr9hW7iE0XlTsZdeMsaKkc5sICO7rwyVJHa72 uMQWgH+IuBYPIW5Z1HXnZRQRqu5ELl4v5AemhTXUMCsF3ZW6T/s= =7/nq -----END PGP SIGNATURE----- --HB4mHL4PVvkpZAgW--