From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44582) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c4L3V-00056s-PU for qemu-devel@nongnu.org; Wed, 09 Nov 2016 00:04:15 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c4L3R-0003z4-Hh for qemu-devel@nongnu.org; Wed, 09 Nov 2016 00:04:13 -0500 Date: Wed, 9 Nov 2016 14:52:29 +1100 From: David Gibson Message-ID: <20161109035229.GD427@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> <20161108051847.GS28688@umbus.fritz.box> <40dfd693-9f80-4492-4c96-8e608d3527d7@ozlabs.ru> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="k4f25fnPtRuIRUb3" Content-Disposition: inline In-Reply-To: <40dfd693-9f80-4492-4c96-8e608d3527d7@ozlabs.ru> 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 --k4f25fnPtRuIRUb3 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Nov 09, 2016 at 12:27:47PM +1100, Alexey Kardashevskiy wrote: > On 08/11/16 16:18, David Gibson wrote: > > 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. > >>> > >>> We put it in target-ppc/compat.c - it makes as much sense at the CPU = level > >>> as it does at the machine level. We also move the cpu_synchronize_st= ate() > >>> into ppc_set_compat(), since it doesn't really make any sense to call= that > >>> without synchronizing state. > >>> > >>> 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(-) > >>> > >>> 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, = sPAPRMachineState *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 = *spapr, > >>> target_ulong opcod= e, > >>> @@ -902,7 +888,6 @@ static target_ulong h_client_architecture_support= (PowerPCCPU *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_suppo= rt(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 com= pat_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 co= mpat_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 *e= nv, bool ifetch) > >>> bool ppc_check_compat(PowerPCCPU *cpu, uint32_t compat_pvr, > >>> uint32_t min_compat_pvr, uint32_t max_compat_p= vr); > >>> void ppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr, Error **er= rp); > >>> +#if !defined(CONFIG_USER_ONLY) > >>> +void ppc_set_compat_all(uint32_t compat_pvr, Error **errp); > >>> +#endif > >> > >> I would put all ppc*compat*() under #if defined(CONFIG_USER_ONLY) && > >> defined(TARGET_PPC64) (or even moved this to target-ppc/Makefile.objs). > >=20 > > I was originally going to do that, but decided against it. > >=20 > >> 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?). > >=20 > > 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 > Compatibility modes are designed to disable sets of instructions to keep > working old userspace software which relies on some opcodes to be invalid. >=20 > linux-user is TCG, right? The user can pick any CPU he likes if there is > need to run such an old software, why on earth would anyone bother with > this compat mode in linux-user? True, I can't really see any reason to do that. On the other hand, compat mode does at least make theoretical sense, whereas, for example, compat mode on powernv is fundamentally nonsense. At this point I'm not terribly include to take away the (token) user-only support unless there's a compelling reason *not* to include it. --=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 --k4f25fnPtRuIRUb3 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJYIp18AAoJEGw4ysog2bOSj0AP/2H9XjFhMWSew7SUG7Uhv0V2 khtEehL1038TssPux5pMT/3jArMvGWUPDcgjtwUXKHl/e9RzX7Y+xWIqcPqakUXM MtAPcH4mBPe3mCOunGDJnUGPxxS0lcUvRb5twDLgaPF8QPdIbEBoOoEqQAldPAdW pnA8FUnaqEtsjh5yNOYwCqCeXFaXV1d5NWy3lx4JPPgmWU64BgMvcqhvI+9XCBWa EkTmX7O3l4dKWrz9G0FvH5+tMTnzWWgn0ujslvoFqgimUYeOyk4FKreZJBjQ8aoO BKSk/367qHnMSvzaxeCd2/XhGiibJ2/gmpP97IbJjWexOhfbZ/j6/lGdIlch+4qr v0NgJvGHstmQkoclWt0rXo1ZV4L7Eq8eZ9Q/Q01sXp8u0roJiig96i9m/GplHJlX 6fyl9Scg/LtrMW9Xy0bKqeMDZn5RX5cSJBHjGyk8SZ2w2yro7Z6rlnPW2/+0T5a8 2b1PXlZljVsTj6TZLi5GjtQxFWXH34iWNQacsVvN/K87xqYXgH9S8Ix4ahuOyaO9 4GlQctOXq00xeClNsYQ+c9Ul0B2Hb7IBpdrT57CDxufHtppjtDm1GkmTqzuOuH+O eHn36HKMcMB7L4Lx17JNcEhD3HMQCUbPa9TYHipL3NUhW1bIbSDOjvXVODfeCiQe iH2ZJ2iwoSHbLt7hmalv =N9QZ -----END PGP SIGNATURE----- --k4f25fnPtRuIRUb3--