From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34696) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c3z4Z-0000KB-UL for qemu-devel@nongnu.org; Tue, 08 Nov 2016 00:35:53 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c3z4X-0002GX-CK for qemu-devel@nongnu.org; Tue, 08 Nov 2016 00:35:51 -0500 Date: Tue, 8 Nov 2016 16:18:47 +1100 From: David Gibson Message-ID: <20161108051847.GS28688@umbus.fritz.box> References: <1477825928-10803-1-git-send-email-david@gibson.dropbear.id.au> <1477825928-10803-12-git-send-email-david@gibson.dropbear.id.au> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="QHhm1I6mwQR20oIa" Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [RFC 11/17] ppc: Add ppc_set_compat_all() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy Cc: nikunj@linux.vnet.ibm.com, mdroth@linux.vnet.ibm.com, thuth@redhat.com, lvivier@redhat.com, qemu-ppc@nongnu.org, qemu-devel@nongnu.org --QHhm1I6mwQR20oIa Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Nov 04, 2016 at 03:01:40PM +1100, Alexey Kardashevskiy wrote: > On 30/10/16 22:12, David Gibson wrote: > > Once a compatiblity mode is negotiated with the guest, > > h_client_architecture_support() uses run_on_cpu() to update each CPU to > > the new mode. We're going to want this logic somewhere else shortly, > > so make a helper function to do this global update. > >=20 > > We put it in target-ppc/compat.c - it makes as much sense at the CPU le= vel > > as it does at the machine level. We also move the cpu_synchronize_stat= e() > > into ppc_set_compat(), since it doesn't really make any sense to call t= hat > > without synchronizing state. > >=20 > > Signed-off-by: David Gibson > > --- > > hw/ppc/spapr_hcall.c | 31 +++++-------------------------- > > target-ppc/compat.c | 36 ++++++++++++++++++++++++++++++++++++ > > target-ppc/cpu.h | 3 +++ > > 3 files changed, 44 insertions(+), 26 deletions(-) > >=20 > > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c > > index 3bd6d06..4eaf9a6 100644 > > --- a/hw/ppc/spapr_hcall.c > > +++ b/hw/ppc/spapr_hcall.c > > @@ -881,20 +881,6 @@ static target_ulong h_set_mode(PowerPCCPU *cpu, sP= APRMachineState *spapr, > > return ret; > > } > > =20 > > -typedef struct { > > - uint32_t compat_pvr; > > - Error *err; > > -} SetCompatState; > > - > > -static void do_set_compat(CPUState *cs, void *arg) > > -{ > > - PowerPCCPU *cpu =3D POWERPC_CPU(cs); > > - SetCompatState *s =3D arg; > > - > > - cpu_synchronize_state(cs); > > - ppc_set_compat(cpu, s->compat_pvr, &s->err); > > -} > > - > > static target_ulong h_client_architecture_support(PowerPCCPU *cpu, > > sPAPRMachineState *s= papr, > > target_ulong opcode, > > @@ -902,7 +888,6 @@ static target_ulong h_client_architecture_support(P= owerPCCPU *cpu, > > { > > target_ulong list =3D ppc64_phys_to_real(args[0]); > > target_ulong ov_table; > > - CPUState *cs; > > bool explicit_match =3D false; /* Matched the CPU's real PVR */ > > uint32_t max_compat =3D cpu->max_compat; > > uint32_t best_compat =3D 0; > > @@ -949,18 +934,12 @@ static target_ulong h_client_architecture_support= (PowerPCCPU *cpu, > > =20 > > /* Update CPUs */ > > if (cpu->compat_pvr !=3D best_compat) { > > - CPU_FOREACH(cs) { > > - SetCompatState s =3D { > > - .compat_pvr =3D best_compat, > > - .err =3D NULL, > > - }; > > + Error *local_err =3D NULL; > > =20 > > - run_on_cpu(cs, do_set_compat, &s); > > - > > - if (s.err) { > > - error_report_err(s.err); > > - return H_HARDWARE; > > - } > > + ppc_set_compat_all(best_compat, &local_err); > > + if (local_err) { > > + error_report_err(local_err); > > + return H_HARDWARE; > > } > > } > > =20 > > diff --git a/target-ppc/compat.c b/target-ppc/compat.c > > index 1059555..0b12b58 100644 > > --- a/target-ppc/compat.c > > +++ b/target-ppc/compat.c > > @@ -124,6 +124,8 @@ void ppc_set_compat(PowerPCCPU *cpu, uint32_t compa= t_pvr, Error **errp) > > pcr =3D compat->pcr; > > } > > =20 > > + cpu_synchronize_state(CPU(cpu)); > > + > > cpu->compat_pvr =3D compat_pvr; > > env->spr[SPR_PCR] =3D pcr & pcc->pcr_mask; > > =20 > > @@ -136,6 +138,40 @@ void ppc_set_compat(PowerPCCPU *cpu, uint32_t comp= at_pvr, Error **errp) > > } > > } > > =20 > > +#if !defined(CONFIG_USER_ONLY) > > +typedef struct { > > + uint32_t compat_pvr; > > + Error *err; > > +} SetCompatState; > > + > > +static void do_set_compat(CPUState *cs, void *arg) > > +{ > > + PowerPCCPU *cpu =3D POWERPC_CPU(cs); > > + SetCompatState *s =3D arg; > > + > > + ppc_set_compat(cpu, s->compat_pvr, &s->err); > > +} > > + > > +void ppc_set_compat_all(uint32_t compat_pvr, Error **errp) > > +{ > > + CPUState *cs; > > + > > + CPU_FOREACH(cs) { > > + SetCompatState s =3D { > > + .compat_pvr =3D compat_pvr, > > + .err =3D NULL, > > + }; > > + > > + run_on_cpu(cs, do_set_compat, &s); > > + > > + if (s.err) { > > + error_propagate(errp, s.err); > > + return; > > + } > > + } > > +} > > +#endif > > + > > int ppc_compat_max_threads(PowerPCCPU *cpu) > > { > > const CompatInfo *compat =3D compat_by_pvr(cpu->compat_pvr); > > diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h > > index 91e8be8..201a655 100644 > > --- a/target-ppc/cpu.h > > +++ b/target-ppc/cpu.h > > @@ -1317,6 +1317,9 @@ static inline int cpu_mmu_index (CPUPPCState *env= , bool ifetch) > > bool ppc_check_compat(PowerPCCPU *cpu, uint32_t compat_pvr, > > uint32_t min_compat_pvr, uint32_t max_compat_pvr= ); > > void ppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr, Error **errp= ); > > +#if !defined(CONFIG_USER_ONLY) > > +void ppc_set_compat_all(uint32_t compat_pvr, Error **errp); > > +#endif >=20 > I would put all ppc*compat*() under #if defined(CONFIG_USER_ONLY) && > defined(TARGET_PPC64) (or even moved this to target-ppc/Makefile.objs). I was originally going to do that, but decided against it. > Otherwise, functions like ppc_check_compat() have #if > !defined(CONFIG_USER_ONLY) which suggests that the rest of > ppc_check_compat() can actually be executed in ppc64-linux-user (while it > cannot, can it?). It won't be, but there's no theoretical reason they couldn't be. User mode, like spapr, doesn't execute hypervisor privilege code, and so the PCR isn't owned by the "guest" (if you can call the user mode executable that). Which means it could make sense to set it from the outside, although that's not something we currently do. --=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 --QHhm1I6mwQR20oIa Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJYIWA3AAoJEGw4ysog2bOSqDUQAKdgbXHZTiJUbBexAytDdBsP H/Bh+CcrvL7iLejCTxHVVmaa/79dCcc+nfexMEPce8J3GlgPNcO/v3+RLMiko3q1 g5uHoaKWezf1X+ytQ98H3gEci3FIRwgmctoF++eP3NqWAcIp4OZogCNnDqERhaGj hEa095gsVRQ6A/wS/LOrG+XvovEKoHh91b6be8Y2pSI55wDT+zh5xgtG+FIOlapY /ia4LBJc9IQ13D+I9y1ti/y/G5Og6HcHEApYWvUHt5o2uUoVmi+XCbgBFkCUlHaD iVONw5Ypo8iIqkcLCl5QgcHS0T0Q4oq49innauhBnIndQhUeH0qC9ag6P+kTN/jg f6Mc5braoavtZT0YRbXhkJsrc7NBYX0ArVLpE2iON7gCQctzB0QZp9IYOTMFmGCy D1en4VWz8ItuCn8QQ4xvSnBALR1AsedQYwl2aZXZu5ecd9miS2FFUgECaYsgXFaw ooA1ozeB4wpCluXwa161uMpj6SNVMZZD8tICGugwg5ARm81iyPQsFM4Lw+r8HCDe QRxGy4HlRcfuchZc5rLTdFCdxLUGDk8iT/gzq4uQMoai8Y5AxQXBGxk5miQTtqBW mAmYqiVLjkGbKDK4JvVZIMfM5wf1IPj7onnbWyU+KeB6VKbDz8R5XGcUYeMECqVp 3jb/MTNtNA14BsC4VeX6 =KFUP -----END PGP SIGNATURE----- --QHhm1I6mwQR20oIa--