qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Max Filippov <jcmvbkbc@gmail.com>
To: Avi Kivity <avi@redhat.com>
Cc: Jan Kiszka <jan.kiszka@siemens.com>, qemu-devel <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] Xtensa misuse of tb_invalidate_phys_page_range()?
Date: Sun, 25 Mar 2012 06:00:46 +0400	[thread overview]
Message-ID: <4F6E7C4E.9090409@gmail.com> (raw)
In-Reply-To: <4F673DEE.3040508@redhat.com>

>>> void HELPER(wsr_ibreaka)(uint32_t i, uint32_t v)
>>> {
>>>     if (env->sregs[IBREAKENABLE]&  (1<<  i)&&  env->sregs[IBREAKA + i]
>>> != v) {
>>>         tb_invalidate_phys_page_range(
>>>                 env->sregs[IBREAKA + i], env->sregs[IBREAKA + i] + 1, 0);
>>>         tb_invalidate_phys_page_range(v, v + 1, 0);
>>>     }
>>>     env->sregs[IBREAKA + i] = v;
>>> }
>>>
>>> tb_invalidate_phys_page_range() expects a virtual address in user mode,
>>> and a ram_addr_t in system mode.  I'm guessing that v is a virtual address?
>>
>> Yes, it's a virtual address here, as well as in wsr_lbeg/wsr_lend helpers.
>> I made a test for it and it actually fails. I wonder how could it stay unnoticed
>> that long :()
>
> There are many silent breakages like that, don't worry.
>
>>> This needs to be fixed for system mode if so (and in any case - even if
>>> it's a physical address, it needs to be translated to a ram_addr_t).
>>
>> Sure. Will try to fix it, though it's completely unclear to me now
>> how to do it efficiently.
>
> Since I'm rewriting this area, don't worry about efficiency.  Let's get
> it correct and after the rewrite we can reexamine efficiency.
>
> I imagine you'll need something like breakpoint_invalidate().

The following RFC patch takes the obvious approach of sharing the breakpoint_invalidate
implementation to address this issue. But if we're talking about correctness it's not
100% correct, because guest TLB is independent of both instruction breakpoints and
zero overhead loops, and at the moment of TB invalidation relevant TLB mapping may not
exist.

Even if we managed to record physical addresses of TBs at the current IBREAKA/LEND, it's
not enough because we need to invalidate TBs both at the old and at the new IBREAKA/LEND
virtual addresses.

What do you think, do we need yet another address-to-TB map?

-- 
Thanks.
-- Max

  reply	other threads:[~2012-03-25  2:01 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-19 11:58 [Qemu-devel] Xtensa misuse of tb_invalidate_phys_page_range()? Avi Kivity
2012-03-19 13:51 ` Max Filippov
2012-03-19 14:08   ` Avi Kivity
2012-03-25  2:00     ` Max Filippov [this message]
2012-03-25  2:04       ` [Qemu-devel] [RFC] target-xtensa: fix tb invalidation for IBREAK and LOOP Max Filippov
2012-03-25  9:53       ` [Qemu-devel] Xtensa misuse of tb_invalidate_phys_page_range()? Avi Kivity

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=4F6E7C4E.9090409@gmail.com \
    --to=jcmvbkbc@gmail.com \
    --cc=avi@redhat.com \
    --cc=jan.kiszka@siemens.com \
    --cc=qemu-devel@nongnu.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).