All of lore.kernel.org
 help / color / mirror / Atom feed
From: Richard Henderson <richard.henderson@linaro.org>
To: "Alex Bennée" <alex.bennee@linaro.org>
Cc: "open list:S390 TCG CPUs" <qemu-s390x@nongnu.org>,
	Thomas Huth <thuth@redhat.com>, Cornelia Huck <cohuck@redhat.com>,
	qemu-devel@nongnu.org, David Hildenbrand <david@redhat.com>
Subject: Re: [RFC PATCH] target/s390x: don't double ld_code() when reading instructions
Date: Tue, 12 Oct 2021 08:38:29 -0700	[thread overview]
Message-ID: <615facec-1751-778d-2187-b6a93afff9eb@linaro.org> (raw)
In-Reply-To: <87lf2ywap4.fsf@linaro.org>

On 10/12/21 7:52 AM, Alex Bennée wrote:
>> I think the plugin stuff could be more clever, knowing where the read
>> occurs within the sequence.  Otherwise, we should simplify the
>> interface so that it is not possible to make this mistake.
> 
> It's plugin_insn_append which is doing the tracking here so we could
> extend the interface to include the current pc of the load and make the
> appropriate adjustments. That said it's a bunch hoops to jump every
> instruction when we could just as easily add an assert and fix up any
> cases where we do. I guess it comes down to how prevalent double dipping
> in the instruction stream is when constructing a translation?

Yes, which is why I suggested simplifying the interface to translate_ld*.  It currently 
takes the DisasContextBase; it could potentially read from pc_next, and increment it.  It 
would completely eliminate the problem you're encountering.

> What happens if the protection of the code area changes half way through
> a translation? Could a mapping change in flight?

No, we hold mmap_lock.

r~


      reply	other threads:[~2021-10-12 15:39 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-12  9:31 [RFC PATCH] target/s390x: don't double ld_code() when reading instructions Alex Bennée
2021-10-12 12:07 ` Richard Henderson
2021-10-12 12:10 ` Richard Henderson
2021-10-12 14:52   ` Alex Bennée
2021-10-12 15:38     ` Richard Henderson [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=615facec-1751-778d-2187-b6a93afff9eb@linaro.org \
    --to=richard.henderson@linaro.org \
    --cc=alex.bennee@linaro.org \
    --cc=cohuck@redhat.com \
    --cc=david@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-s390x@nongnu.org \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.