All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Alex Bennée" <alex.bennee@linaro.org>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: "Pavel Dovgalyuk" <Pavel.Dovgaluk@ispras.ru>,
	"QEMU Developers" <qemu-devel@nongnu.org>,
	maria.klimushenkova@ispras.ru,
	"Pavel Dovgalyuk" <dovgaluk@ispras.ru>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Lluís Vilanova" <vilanova@ac.upc.edu>
Subject: Re: [Qemu-devel] [RFC PATCH v2 0/7] QEMU binary instrumentation prototype
Date: Mon, 10 Sep 2018 10:01:12 +0100	[thread overview]
Message-ID: <87h8ixd8vb.fsf@linaro.org> (raw)
In-Reply-To: <CAFEAcA-iTfGnjc+0ndTMxZ=RYRCK3_RRmE1RCNgDnTVL_brrXQ@mail.gmail.com>


Peter Maydell <peter.maydell@linaro.org> writes:

<snip>
>> Now I can see arguments against it from an interface complexity point of
>> view but I think plugins should get access to the TCG functions so they
>> can generate their own op sequences if need be and not have to call a
>> helper at all if they don't need to.
>
> I strongly disagree -- plugins should not have access to details
> of QEMU internals that might change from version to version, and
> definitely not access to generating their own TCG code.

The problem is I think a simple single helper with a PC and a CPU env is
fairly inflexible and inefficient. It would also make the API simpler if
we could leave the decision about installing the helper to the
instrumentation function. However I take your point that exposing the
whole TCG API might be a bit too much. Perhaps the qmeulib functions
could wrap enough so we could have an instrumentation function like
this:

  void plugins_instrument_before_insn(target_ulong disas_pc)
  {
      TCGv t_pc = qemulib_make_const(pc);
      qemulib_call_helper1(pc);
  }

By marshalling through qemulib we could keep track of temps allocated
and do the clean-up for the plugin or take the approach that plugin
writers need to have some idea of what they are doing.

In terms of the wider problem about exposing internals that might change
from version to version I wonder if we should care? Is the plugin API
one where we should provide ABI stability or should we take the approach
that Linux does and say if your code lives out-of-tree then it's your
problem to keep up if/when we change.

--
Alex Bennée

  reply	other threads:[~2018-09-10  9:01 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-05 10:39 [Qemu-devel] [RFC PATCH v2 0/7] QEMU binary instrumentation prototype Pavel Dovgalyuk
2018-06-05 10:39 ` [Qemu-devel] [RFC PATCH v2 1/7] tcg: add headers for non-target helpers Pavel Dovgalyuk
2018-06-05 13:07   ` Thomas Huth
2018-06-06  7:30     ` Pavel Dovgalyuk
2018-09-07 12:16   ` Alex Bennée
2018-06-05 10:39 ` [Qemu-devel] [RFC PATCH v2 2/7] Add plugin support Pavel Dovgalyuk
2018-09-07 10:11   ` Alex Bennée
2018-09-13  6:40     ` Pavel Dovgalyuk
2018-09-07 12:34   ` Alex Bennée
2018-09-10  8:30     ` Pavel Dovgalyuk
2018-09-07 14:14   ` Alex Bennée
2018-09-10 11:41     ` Pavel Dovgalyuk
2018-06-05 10:39 ` [Qemu-devel] [RFC PATCH v2 3/7] plugins: provide helper functions for plugins Pavel Dovgalyuk
2018-09-07 13:06   ` Alex Bennée
2018-06-05 10:39 ` [Qemu-devel] [RFC PATCH v2 4/7] tcg: add instrumenting module Pavel Dovgalyuk
2018-09-07 13:36   ` Alex Bennée
2018-09-13  6:55     ` Pavel Dovgalyuk
2018-06-05 10:39 ` [Qemu-devel] [RFC PATCH v2 5/7] plugins: add plugin template Pavel Dovgalyuk
2018-09-07 13:41   ` Alex Bennée
2018-06-05 10:39 ` [Qemu-devel] [RFC PATCH v2 6/7] plugin: add instruction execution logger Pavel Dovgalyuk
2018-09-07 13:59   ` Alex Bennée
2018-06-05 10:39 ` [Qemu-devel] [RFC PATCH v2 7/7] plugins: add syscall logging plugin sample Pavel Dovgalyuk
2018-09-07 14:06   ` Alex Bennée
2018-09-10  9:18     ` Pavel Dovgalyuk
2018-09-10 13:58       ` Alex Bennée
2018-06-05 10:49 ` [Qemu-devel] [RFC PATCH v2 0/7] QEMU binary instrumentation prototype Peter Maydell
2018-06-05 11:56   ` Pavel Dovgalyuk
2018-06-25  5:46     ` Pavel Dovgalyuk
2018-06-25  9:06       ` Peter Maydell
2018-09-07 14:10       ` Alex Bennée
2018-07-10 13:06     ` Stefan Hajnoczi
2018-07-11  6:02       ` Pavel Dovgalyuk
2018-07-30 13:26         ` Pavel Dovgalyuk
2018-08-29  5:39       ` Pavel Dovgalyuk
2018-08-29 19:57         ` Peter Maydell
2018-08-30  4:03           ` Alex Bennée
2018-06-06  8:52 ` no-reply
2018-06-06  9:21 ` no-reply
2018-06-06 10:45 ` no-reply
2018-09-07 14:39 ` Alex Bennée
2018-09-08  0:57   ` Peter Maydell
2018-09-10  9:01     ` Alex Bennée [this message]
2018-09-10 11:44       ` Pavel Dovgalyuk

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=87h8ixd8vb.fsf@linaro.org \
    --to=alex.bennee@linaro.org \
    --cc=Pavel.Dovgaluk@ispras.ru \
    --cc=dovgaluk@ispras.ru \
    --cc=maria.klimushenkova@ispras.ru \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=vilanova@ac.upc.edu \
    /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.