Re: [Qemu-devel] [PULL, 14/18] translate-all: discard TB when tb_link_page returns an existing matching TB

[Emilio G. Cota]

On Tue, Jul 03, 2018 at 08:38:52 +0300, Pavel Dovgalyuk wrote:
> > From: Emilio G. Cota [mailto:address@hidden
> > On Mon, Jul 02, 2018 at 08:52:14 +0300, Pavel Dovgalyuk wrote:
> > > The same failure can be reproduced with linux-0.2.img, which was
> > > downloaded from QEMU site.
> > > I can't find it now, but I can upload this file if needed.
> > 
> > Please upload it somewhere and share the full QEMU invocation
> > needed to replicate.
> 
> https://github.com/Dovgalyuk/qemu-images/blob/master/linux-0.2.img
> 
> qemu-system-i386 -drive file=images/linux-0.2.img,if=none,snapshot,id=img 
> -drive
> driver=blkreplay,if=none,id=rr,image=img -device ide-hd,drive=rr -net none 
> -icount
> shift=5,rr=record,rrfile=linux02.rr

The appended patch fixes it for me. Can you please test on your
windows image?

The rationale is to honour CF_NOCACHE, so that we always return
a new TB from tb_gen_code.

Thanks,

                Emilio

---
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 170b957..49d77fa 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -1446,7 +1446,8 @@ static void do_tb_phys_invalidate(TranslationBlock *tb, 
bool rm_from_page_list)
     phys_pc = tb->page_addr[0] + (tb->pc & ~TARGET_PAGE_MASK);
     h = tb_hash_func(phys_pc, tb->pc, tb->flags, tb_cflags(tb) & CF_HASH_MASK,
                      tb->trace_vcpu_dstate);
-    if (!qht_remove(&tb_ctx.htable, tb, h)) {
+    if (!(tb->cflags & CF_NOCACHE) &&
+        !qht_remove(&tb_ctx.htable, tb, h)) {
         return;
     }
 
@@ -1604,8 +1605,6 @@ tb_link_page(TranslationBlock *tb, tb_page_addr_t phys_pc,
 {
     PageDesc *p;
     PageDesc *p2 = NULL;
-    void *existing_tb = NULL;
-    uint32_t h;
 
     assert_memory_lock();
 
@@ -1625,20 +1624,25 @@ tb_link_page(TranslationBlock *tb, tb_page_addr_t 
phys_pc,
         tb->page_addr[1] = -1;
     }
 
-    /* add in the hash table */
-    h = tb_hash_func(phys_pc, tb->pc, tb->flags, tb->cflags & CF_HASH_MASK,
-                     tb->trace_vcpu_dstate);
-    qht_insert(&tb_ctx.htable, tb, h, &existing_tb);
+    if (!(tb->cflags & CF_NOCACHE)) {
+        void *existing_tb = NULL;
+        uint32_t h;
 
-    /* remove TB from the page(s) if we couldn't insert it */
-    if (unlikely(existing_tb)) {
-        tb_page_remove(p, tb);
-        invalidate_page_bitmap(p);
-        if (p2) {
-            tb_page_remove(p2, tb);
-            invalidate_page_bitmap(p2);
+        /* add in the hash table */
+        h = tb_hash_func(phys_pc, tb->pc, tb->flags, tb->cflags & CF_HASH_MASK,
+                         tb->trace_vcpu_dstate);
+        qht_insert(&tb_ctx.htable, tb, h, &existing_tb);
+
+        /* remove TB from the page(s) if we couldn't insert it */
+        if (unlikely(existing_tb)) {
+            tb_page_remove(p, tb);
+            invalidate_page_bitmap(p);
+            if (p2) {
+                tb_page_remove(p2, tb);
+                invalidate_page_bitmap(p2);
+            }
+            tb = existing_tb;
         }
-        tb = existing_tb;
     }
 
     if (p2 && p2 != p) {