All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mikhail Tyutin <m.tyutin@yadro.com>
To: "Alex Bennée" <alex.bennee@linaro.org>
Cc: "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	Richard Henderson <richard.henderson@linaro.org>,
	"erdnaxe@crans.org" <erdnaxe@crans.org>,
	"ma.mandourr@gmail.com" <ma.mandourr@gmail.com>
Subject: RE: Instruction virtual address in TCG Plugins
Date: Tue, 21 Nov 2023 16:39:25 +0000	[thread overview]
Message-ID: <f79fe2c0d7e5457ca7172862e96fd886@yadro.com> (raw)
In-Reply-To: <874jhoy54t.fsf@draig.linaro.org>

> >> > I suspect it is because of memory mappings by guest OS that changes virtual addresses for that block.
> >> >
> >> > I also looked at gen_empty_udata_cb() function and considered to extend plugin API to pass a program counter
> >> > value as additional callback argument. I thought it would always give me valid virtual address of an instruction.
> >> > Unfortunately, I didn't find a way to get value of that register in architecture agnostic way (it is 'pc' member in
> >> > CPUArchState structure).
> >>
> >> When we merge the register api you should be able to do that. Although
> >> during testing I realised that PC acted funny compared to everything
> >> else because we don't actually update the shadow register every
> >> instruction.
> >
> > We implemented similar API to read registers (by coincidence, I posted this patch at the same time as the API you
> > mentioned) and I observe similar behavior. As far as I see, CPU state is only updated in between of executed translation
> > blocks. Switching to 'singlestep' mode helps to fix that, but execution overhead is huge.
> >
> > There is also blocks 'chaining' mechanism which is likely contributes to corrupted blocks vaddr inside of callbacks.
> > My guess is that 'pc' value for those chained blocks points to the first block of entire chain. Unfortunately, It is very
> > hard to debug, because I can only see block chains when I run whole Linux guest OS. Does Qemu has small test
> > application to trigger long enough chain of translation blocks?
> 
> No all registers should be resolved by the end of any block. There is
> currently no optimisation of register usage between TBs. If you are
> seeing PC corruption that would be a bug - but fundamentally things
> would break pretty quick if tb_lookup() and friends didn't have an
> accurate PC.

I managed to root cause source of corrupted addresses in plugin callbacks.
There were basically 2 problems:

1. Memory IO operations force TCG to create special translation blocks to
process that memory load/store operation. The plugin gets notification for
this translation block as well, but instrumentation callbacks other than
memory ones are silently ignored. To make it correct, the plugin has to match
instruction execution callback from previous TB to memory callback from that
special TB. The fix was to expose internal ‘memOnly’ TB flag to the plugin to
handle such TBs differently.

2. Another problem is related to interrupts handling. Since we can insert pre-
callback on instructions only, the plugin is not aware if instruction is
actually executed or interrupted by an interrupt or exception. In fact, it
mistakenly interprets all interrupted instructions as executed. Adding API
to receive interrupt notification and appropriate handling of it fixes
the problem.

I will send those patches for review shortly and thank you for dissuading me
from going to wrong direction!

  reply	other threads:[~2023-11-21 16:40 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-13 18:33 Instruction virtual address in TCG Plugins Mikhail Tyutin
2023-11-13 20:58 ` Alex Bennée
2023-11-14  9:14   ` Mikhail Tyutin
2023-11-14 10:57     ` Alex Bennée
2023-11-21 16:39       ` Mikhail Tyutin [this message]
2023-11-21 17:24         ` Alex Bennée
2023-11-22 12:28           ` Mikhail Tyutin

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=f79fe2c0d7e5457ca7172862e96fd886@yadro.com \
    --to=m.tyutin@yadro.com \
    --cc=alex.bennee@linaro.org \
    --cc=erdnaxe@crans.org \
    --cc=ma.mandourr@gmail.com \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.org \
    /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.