From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39498) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bSEsZ-0002ju-KL for qemu-devel@nongnu.org; Tue, 26 Jul 2016 22:47:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bSEsV-0001mi-2G for qemu-devel@nongnu.org; Tue, 26 Jul 2016 22:47:26 -0400 Date: Wed, 27 Jul 2016 12:04:43 +1000 From: David Gibson Message-ID: <20160727020443.GT17429@voom.fritz.box> References: <1469571686-7284-1-git-send-email-benh@kernel.crashing.org> <1469571686-7284-13-git-send-email-benh@kernel.crashing.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="9hshNW4m6zn79FF/" Content-Disposition: inline In-Reply-To: <1469571686-7284-13-git-send-email-benh@kernel.crashing.org> Subject: Re: [Qemu-devel] [PATCH 13/32] ppc: Don't update NIP in lswi/lswx/stswi/stswx List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Benjamin Herrenschmidt Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org --9hshNW4m6zn79FF/ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Jul 27, 2016 at 08:21:07AM +1000, Benjamin Herrenschmidt wrote: > Instead, pass GETPC() result to the corresponding helpers. This > requires a bit of fiddling to get the PC (hopefully) right in > the case where we generate a program check, though the hacks there > are temporary, a subsequent patch will clean this all up by always > having the nip already set to the right instruction when taking > the fault. Where are cpu_ldub_data() and cpu_ldub_data_ra() defined? I assume it's via macro somewhere, since I couldn't find it with a grep. >=20 > Signed-off-by: Benjamin Herrenschmidt > --- > target-ppc/excp_helper.c | 8 ++++++++ > target-ppc/mem_helper.c | 26 ++++++++++++++++---------- > target-ppc/translate.c | 18 ++++++++---------- > 3 files changed, 32 insertions(+), 20 deletions(-) >=20 > diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c > index 91fdf4b..563c7bc 100644 > --- a/target-ppc/excp_helper.c > +++ b/target-ppc/excp_helper.c > @@ -285,6 +285,10 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int= excp_model, int excp) > LOG_EXCP("Invalid instruction at " TARGET_FMT_lx "\n", env->= nip); > msr |=3D 0x00080000; > env->spr[SPR_BOOKE_ESR] =3D ESR_PIL; > + /* Some invalids will have the PC in the right place already= */ > + if (env->error_code & POWERPC_EXCP_INVAL_LSWX) { > + goto store_next; > + } > break; > case POWERPC_EXCP_PRIV: > msr |=3D 0x00040000; > @@ -306,6 +310,10 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int= excp_model, int excp) > srr1 =3D SPR_HSRR1; > new_msr |=3D (target_ulong)MSR_HVB; > new_msr |=3D env->msr & ((target_ulong)1 << MSR_RI); > + /* Some invalids will have the PC in the right place already */ > + if (env->error_code =3D=3D (POWERPC_EXCP_INVAL|POWERPC_EXCP_INVA= L_LSWX)) { > + goto store_next; > + } > goto store_current; > case POWERPC_EXCP_FPU: /* Floating-point unavailable exception= */ > goto store_current; > diff --git a/target-ppc/mem_helper.c b/target-ppc/mem_helper.c > index e4ed377..de96c91 100644 > --- a/target-ppc/mem_helper.c > +++ b/target-ppc/mem_helper.c > @@ -77,23 +77,30 @@ void helper_stmw(CPUPPCState *env, target_ulong addr,= uint32_t reg) > } > } > =20 > -void helper_lsw(CPUPPCState *env, target_ulong addr, uint32_t nb, uint32= _t reg) > +static void do_lsw(CPUPPCState *env, target_ulong addr, uint32_t nb, > + uint32_t reg, uintptr_t raddr) > { > int sh; > =20 > for (; nb > 3; nb -=3D 4) { > - env->gpr[reg] =3D cpu_ldl_data(env, addr); > + env->gpr[reg] =3D cpu_ldl_data_ra(env, addr, raddr); > reg =3D (reg + 1) % 32; > addr =3D addr_add(env, addr, 4); > } > if (unlikely(nb > 0)) { > env->gpr[reg] =3D 0; > for (sh =3D 24; nb > 0; nb--, sh -=3D 8) { > - env->gpr[reg] |=3D cpu_ldub_data(env, addr) << sh; > + env->gpr[reg] |=3D cpu_ldub_data_ra(env, addr, raddr) << sh; > addr =3D addr_add(env, addr, 1); > } > } > } > + > +void helper_lsw(CPUPPCState *env, target_ulong addr, uint32_t nb, uint32= _t reg) > +{ > + do_lsw(env, addr, nb, reg, GETPC()); > +} > + > /* PPC32 specification says we must generate an exception if > * rA is in the range of registers to be loaded. > * In an other hand, IBM says this is valid, but rA won't be loaded. > @@ -106,12 +113,11 @@ void helper_lswx(CPUPPCState *env, target_ulong add= r, uint32_t reg, > int num_used_regs =3D (xer_bc + 3) / 4; > if (unlikely((ra !=3D 0 && lsw_reg_in_range(reg, num_used_regs, = ra)) || > lsw_reg_in_range(reg, num_used_regs, rb))) { > - env->nip +=3D 4; /* Compensate the "nip - 4" from gen_ls= wx() */ > - helper_raise_exception_err(env, POWERPC_EXCP_PROGRAM, > - POWERPC_EXCP_INVAL | > - POWERPC_EXCP_INVAL_LSWX); > + raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM, > + POWERPC_EXCP_INVAL | > + POWERPC_EXCP_INVAL_LSWX, GETPC()); > } else { > - helper_lsw(env, addr, xer_bc, reg); > + do_lsw(env, addr, xer_bc, reg, GETPC()); > } > } > } > @@ -122,13 +128,13 @@ void helper_stsw(CPUPPCState *env, target_ulong add= r, uint32_t nb, > int sh; > =20 > for (; nb > 3; nb -=3D 4) { > - cpu_stl_data(env, addr, env->gpr[reg]); > + cpu_stl_data_ra(env, addr, env->gpr[reg], GETPC()); > reg =3D (reg + 1) % 32; > addr =3D addr_add(env, addr, 4); > } > if (unlikely(nb > 0)) { > for (sh =3D 24; nb > 0; nb--, sh -=3D 8) { > - cpu_stb_data(env, addr, (env->gpr[reg] >> sh) & 0xFF); > + cpu_stb_data_ra(env, addr, (env->gpr[reg] >> sh) & 0xFF, GET= PC()); > addr =3D addr_add(env, addr, 1); > } > } > diff --git a/target-ppc/translate.c b/target-ppc/translate.c > index a05fed7..9d2e923 100644 > --- a/target-ppc/translate.c > +++ b/target-ppc/translate.c > @@ -2701,12 +2701,16 @@ static void gen_lswi(DisasContext *ctx) > nb =3D 32; > nr =3D (nb + 3) / 4; > if (unlikely(lsw_reg_in_range(start, nr, ra))) { > + /* The handler expects the PC to point to *this* instruction, > + * so setting ctx->exception here prevents it from being > + * improperly updated again by gen_inval_exception > + */ > + gen_update_nip(ctx, ctx->nip - 4); > + ctx->exception =3D POWERPC_EXCP_HV_EMU; > gen_inval_exception(ctx, POWERPC_EXCP_INVAL_LSWX); > return; > } > gen_set_access_type(ctx, ACCESS_INT); > - /* NIP cannot be restored if the memory exception comes from an help= er */ > - gen_update_nip(ctx, ctx->nip - 4); > t0 =3D tcg_temp_new(); > gen_addr_register(ctx, t0); > t1 =3D tcg_const_i32(nb); > @@ -2723,8 +2727,6 @@ static void gen_lswx(DisasContext *ctx) > TCGv t0; > TCGv_i32 t1, t2, t3; > gen_set_access_type(ctx, ACCESS_INT); > - /* NIP cannot be restored if the memory exception comes from an help= er */ > - gen_update_nip(ctx, ctx->nip - 4); > t0 =3D tcg_temp_new(); > gen_addr_reg_index(ctx, t0); > t1 =3D tcg_const_i32(rD(ctx->opcode)); > @@ -2744,8 +2746,6 @@ static void gen_stswi(DisasContext *ctx) > TCGv_i32 t1, t2; > int nb =3D NB(ctx->opcode); > gen_set_access_type(ctx, ACCESS_INT); > - /* NIP cannot be restored if the memory exception comes from an help= er */ > - gen_update_nip(ctx, ctx->nip - 4); > t0 =3D tcg_temp_new(); > gen_addr_register(ctx, t0); > if (nb =3D=3D 0) > @@ -2764,8 +2764,6 @@ static void gen_stswx(DisasContext *ctx) > TCGv t0; > TCGv_i32 t1, t2; > gen_set_access_type(ctx, ACCESS_INT); > - /* NIP cannot be restored if the memory exception comes from an help= er */ > - gen_update_nip(ctx, ctx->nip - 4); > t0 =3D tcg_temp_new(); > gen_addr_reg_index(ctx, t0); > t1 =3D tcg_temp_new_i32(); > @@ -3846,7 +3844,7 @@ static void gen_dcbz(DisasContext *ctx) > static void gen_dst(DisasContext *ctx) > { > if (rA(ctx->opcode) =3D=3D 0) { > - gen_inval_exception(ctx, POWERPC_EXCP_INVAL_LSWX); > + gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL); > } else { > /* interpreted as no-op */ > } > @@ -3856,7 +3854,7 @@ static void gen_dst(DisasContext *ctx) > static void gen_dstst(DisasContext *ctx) > { > if (rA(ctx->opcode) =3D=3D 0) { > - gen_inval_exception(ctx, POWERPC_EXCP_INVAL_LSWX); > + gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL); > } else { > /* interpreted as no-op */ > } --=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 --9hshNW4m6zn79FF/ Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJXmBa7AAoJEGw4ysog2bOSz1MP/Ru0t/DvlQauDkBSmtwoftRa PEJzb8E1d1vkou57Rd9KatxBTjOWCp0LibeF9C3VhvXug7A3nhX8v2pL1QU0xIU9 xegMjZ4tgEh0Yta9KYcsKAoJMYPxFe25JcC8E9a5bDm14yOurl4KkU4RQ2xjJ4uk put52dmNPTFcAO2QUw9JbFJv8+QUoWX5fI8l5gTgXJKKS2FRRGMGXr0a5m3HJMhL qFP8VQyas4p3Vn/SBBElpLpOe7kuiQdfWt5748OXvGxc4FqI6bonOT69JGrLCWwB jwkXsGFTNFPTS8KnKfIlM7yMmgqm9kBt7ii+IasAFBJySBnaSeE68TWb6bG9InZ+ Z0mPEJKjLhXcu/l/iuS9sK/TfSBA5oPFmFfGnyAq9bPQ7+RUjQFMM+538uMZj/AQ NVecmxEGJu6WoJ6JW3OPOWGuyhfwuZ8njmH+9q0c8BLMAegoqTprCfNobQiWtxoh f13H7KJ18Tr1uiNjiogK9kEutkBK+NkJasXv357gOTrTcPW2lpq27ht4H0LjJ5an fV56VUaYtZ8LqGNqf5wUA4k9T4MHyvTnu6/WJgcNZb75Cxyo2cx3ow62ksw1Fewc aDjE5KQBOyuRRr2NB0O/c4B9r+UjcrR58omkskcKTX/nLwccW8/DSH2gFKGOOVdY YDrzGE+xSH4GzAGdxpOC =5kQH -----END PGP SIGNATURE----- --9hshNW4m6zn79FF/--