Re: [PATCH v2 3/4] target/i386: Make translator stop before the end of a page

From: Richard Henderson <richard.henderson@linaro.org>
To: "Ilya Leoshkevich" <iii@linux.ibm.com>,
	"Laurent Vivier" <laurent@vivier.eu>,
	"Eduardo Habkost" <eduardo@habkost.net>,
	"Marcel Apfelbaum" <marcel.apfelbaum@gmail.com>,
	"Philippe Mathieu-Daudé" <f4bug@amsat.org>,
	"Yanan Wang" <wangyanan55@huawei.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"David Hildenbrand" <david@redhat.com>,
	"Peter Maydell" <peter.maydell@linaro.org>
Cc: qemu-devel@nongnu.org, qemu-s390x@nongnu.org,
	Christian Borntraeger <borntraeger@linux.ibm.com>
Subject: Re: [PATCH v2 3/4] target/i386: Make translator stop before the end of a page
Date: Fri, 5 Aug 2022 13:19:43 -0700	[thread overview]
Message-ID: <574ceb2a-8e5d-dabc-b019-6f0af7b70714@linaro.org> (raw)
In-Reply-To: <20220805160914.1106091-4-iii@linux.ibm.com>

On 8/5/22 09:09, Ilya Leoshkevich wrote:
> @@ -4568,9 +4598,19 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
>       s->rip_offset = 0; /* for relative ip address */
>       s->vex_l = 0;
>       s->vex_v = 0;
> -    if (sigsetjmp(s->jmpbuf, 0) != 0) {
> +    disas_save(&snapshot, s);
> +    switch (sigsetjmp(s->jmpbuf, 0)) {
> +    case 0:
> +        break;
> +    case 1:
>           gen_exception_gpf(s);
>           return s->pc;
> +    case 2:
> +        disas_restore(&snapshot, s);
> +        s->base.is_jmp = DISAS_TOO_MANY;
> +        return pc_start;
> +    default:

Similarly, this is too late for DISAS_TOO_MANY.

It will be more difficult to fix this for x86, since unlike s390x (or arm for that 
matter), we cannot probe for the total insn length within the first few bits of the insn.

The simplest possibility would to force single-stepping when we're within 16 bytes of the 
end of the page, as that's a hard maximum on the number of bytes within an x86 insn.

We could probably still use this sort of longjmp thing, but we'd need to unwind more than 
you're doing here.  We'd need to discard the insn_start opcode (which is before the 
last_op that you're currently saving), and decrement s->base.num_insns.

r~