From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45362) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bUqpL-0006Id-Tj for qemu-devel@nongnu.org; Wed, 03 Aug 2016 03:42:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bUqpI-00056O-MB for qemu-devel@nongnu.org; Wed, 03 Aug 2016 03:42:55 -0400 Received: from gate.crashing.org ([63.228.1.57]:34833) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bUqpI-00056J-C2 for qemu-devel@nongnu.org; Wed, 03 Aug 2016 03:42:52 -0400 Message-ID: <1470210160.12584.64.camel@kernel.crashing.org> From: Benjamin Herrenschmidt Date: Wed, 03 Aug 2016 17:42:40 +1000 In-Reply-To: <1469493760-4205-1-git-send-email-rth@twiddle.net> References: <1469493760-4205-1-git-send-email-rth@twiddle.net> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] tcg: Merge GETPC and GETRA List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Richard Henderson , qemu-devel@nongnu.org Cc: peter.maydell@linaro.org, leon.alrae@imgtec.com On Tue, 2016-07-26 at 06:12 +0530, Richard Henderson wrote: > The return address argument to the softmmu template helpers was > confused.=C2=A0=C2=A0In the legacy case, we wanted to indicate that the= re > is no return address, and so passed in NULL.=C2=A0=C2=A0However, we the= n > immediately subtracted GETPC_ADJ from NULL, resulting in a non-zero > value, indicating the presence of an (invalid) return address. >=20 > Push the GETPC_ADJ subtraction down to the only point it's required: > immediately before use within cpu_restore_state, after all NULL > pointer > checks have been completed.=C2=A0=C2=A0This makes GETPC and GETRA ident= ical. >=20 > Remove GETRA as the lesser used macro, replacing all uses with GETPC. >=20 > Signed-off-by: Richard Henderson > --- This causes a problem with linux-user. We have cases where a store instruction from the guest turns into *one* store instruction in the host. The PC we get from the host segfault is exact, so the -2 in cpu_restore_state() takes us to the previous instruction. We thus end up reporting the wrong instruction in the signal I've tentatively fixed it with the hack below. It's not pretty since we were trying to get rid of all those GETPC_ADJ sprinkled all over but I don't see any obvious better way. I'm not sure if there are other cases with softmmu for example, where we do get an exact host PC and thus where the -2 might take us back too far. I don't see your patch upstream yet so feel free to fold my change into yours. @@ -103,13 +102,16 @@ static inline int handle_cpu_signal(uintptr_t pc, u= nsigned long address, =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0return 0; /* not an= MMU fault */ =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0} =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0if (ret =3D=3D 0) { =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0return 1; /* the MM= U fault was handled without causing real CPU fault */ =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0} -=C2=A0=C2=A0=C2=A0=C2=A0/* now we have a real cpu fault */ -=C2=A0=C2=A0=C2=A0=C2=A0cpu_restore_state(cpu, pc); - +=C2=A0=C2=A0=C2=A0=C2=A0/* Now we have a real cpu fault. We must undo th= e adjustment +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* done by cpu_restore_state() since we are= n't pointing to the +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* next instruction but to the faulting one= and going back +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* can make us report the wrong guest PC. +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0*/ +=C2=A0=C2=A0=C2=A0=C2=A0cpu_restore_state(cpu, pc + GETPC_ADJ); =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0sigprocmask(SIG_SETMASK, old_set, NULL); =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0cpu_loop_exit(cpu); =C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0/* never comes here */ =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0return 1;