On 10/26/22 01:58, Alex Bennée wrote:

Richard Henderson <richard.hender...@linaro.org> writes:

Begin weaning user-only away from PageDesc.

Since, for user-only, all TB (and page) manipulation is done with
a single mutex, and there is no virtual/physical discontinuity to
split a TB across discontinuous pages, place all of the TBs into
a single IntervalTree. This makes it trivial to find all of the
TBs intersecting a range.

Retain the existing PageDesc + linked list implementation for
system mode.  Move the portion of the implementation that overlaps
the new user-only code behind the common ifdef.

Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
---
  accel/tcg/internal.h      |  16 +-
  include/exec/exec-all.h   |  43 ++++-
  accel/tcg/tb-maint.c      | 388 ++++++++++++++++++++++----------------
  accel/tcg/translate-all.c |   4 +-
  4 files changed, 280 insertions(+), 171 deletions(-)

<snip>
diff --git a/accel/tcg/tb-maint.c b/accel/tcg/tb-maint.c
index c8e921089d..14e8e47a6a 100644
--- a/accel/tcg/tb-maint.c
+++ b/accel/tcg/tb-maint.c
@@ -18,6 +18,7 @@
   */
#include "qemu/osdep.h"
+#include "qemu/interval-tree.h"
  #include "exec/cputlb.h"
  #include "exec/log.h"
  #include "exec/exec-all.h"
@@ -50,6 +51,75 @@ void tb_htable_init(void)
      qht_init(&tb_ctx.htable, tb_cmp, CODE_GEN_HTABLE_SIZE, mode);
  }

I wonder for the sake of avoiding recompilation of units later on and
having a clean separation between user and system mode it would be worth
putting this stuff in a tb-maint-user.c?

Something like that might be possible toward the end of the series, but there's too much tangle in the middle.

+bool tb_invalidate_phys_page_unwind(tb_page_addr_t addr, uintptr_t pc)
+{
+    assert(pc != 0);
+#ifdef TARGET_HAS_PRECISE_SMC
+    assert_memory_lock();

Out of interest is this just because x86 has such a strong memory model
you can get away with this sort of patching without explicit flushes?

Yes.

I'm curious why this is the only arch we jump through these hoops for?

It's probably the only one for which we have extensive test cases for self-modifying code (aka msdos). I can imagine that other old targets might technically require it, e.g. m68k, or some of the micro-controllers.

I'm actually not sure why we don't enable it everywhere -- it doesn't seem like it's adding lots of overhead. But until someone reports a bug, or it gets in the way of multi-target, I'm happy to not enable it anywhere else.

      /*
       * We remove all the TBs in the range [start, end[.
       * XXX: see if in some cases it could be faster to invalidate all the code
       */

I'm guessing this comment is quite stale now given we try quite hard to
avoid doing lots of code gen over and over again. The only case I can
think of is memory clear routines after we've had code which there might
be some heuristics we could use to detect but don't currently.

I think there's still room for exploration here, especially detecting page clearing as you say.


r~

Reply via email to