[Qemu-devel] [PATCH] tcg: fix --disable-tcg build breakage introduced by tb_lock removal

Emilio G. Cota posted 1 patch 5 years, 9 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1529684183-17385-1-git-send-email-cota@braap.org
Test checkpatch passed
Test docker-mingw@fedora passed
Test docker-quick@centos7 passed
Test s390x passed
accel/stubs/tcg-stub.c | 4 ----
exec.c                 | 4 ++++
2 files changed, 4 insertions(+), 4 deletions(-)
[Qemu-devel] [PATCH] tcg: fix --disable-tcg build breakage introduced by tb_lock removal
Posted by Emilio G. Cota 5 years, 9 months ago
Tested to build x86_64-softmmu and i386-softmmu targets.

Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Emilio G. Cota <cota@braap.org>
---
 accel/stubs/tcg-stub.c | 4 ----
 exec.c                 | 4 ++++
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/accel/stubs/tcg-stub.c b/accel/stubs/tcg-stub.c
index ee575a8..76ae461 100644
--- a/accel/stubs/tcg-stub.c
+++ b/accel/stubs/tcg-stub.c
@@ -21,10 +21,6 @@ void tb_flush(CPUState *cpu)
 {
 }
 
-void tb_unlock(void)
-{
-}
-
 void tlb_set_dirty(CPUState *cpu, target_ulong vaddr)
 {
 }
diff --git a/exec.c b/exec.c
index 28f9bdc..3baa3dc 100644
--- a/exec.c
+++ b/exec.c
@@ -2645,18 +2645,22 @@ void memory_notdirty_write_prepare(NotDirtyInfo *ndi,
     ndi->pages = NULL;
 
     assert(tcg_enabled());
+#ifdef CONFIG_TCG
     if (!cpu_physical_memory_get_dirty_flag(ram_addr, DIRTY_MEMORY_CODE)) {
         ndi->pages = page_collection_lock(ram_addr, ram_addr + size);
         tb_invalidate_phys_page_fast(ndi->pages, ram_addr, size);
     }
+#endif
 }
 
 /* Called within RCU critical section. */
 void memory_notdirty_write_complete(NotDirtyInfo *ndi)
 {
     if (ndi->pages) {
+#ifdef CONFIG_TCG
         page_collection_unlock(ndi->pages);
         ndi->pages = NULL;
+#endif
     }
 
     /* Set both VGA and migration bits for simplicity and to remove
-- 
2.7.4


Re: [Qemu-devel] [PATCH] tcg: fix --disable-tcg build breakage introduced by tb_lock removal
Posted by Peter Maydell 5 years, 9 months ago
On 22 June 2018 at 17:16, Emilio G. Cota <cota@braap.org> wrote:
> Tested to build x86_64-softmmu and i386-softmmu targets.
>
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Emilio G. Cota <cota@braap.org>
> ---
>  accel/stubs/tcg-stub.c | 4 ----
>  exec.c                 | 4 ++++
>  2 files changed, 4 insertions(+), 4 deletions(-)

This still doesn't link for me:

  LINK    x86_64-softmmu/qemu-system-x86_64
exec.o: In function `tlb_reset_dirty_range_all':
/home/petmay01/linaro/qemu-from-laptop/qemu/exec.c:1334: undefined
reference to `tlb_reset_dirty'
exec.o: In function `tcg_commit':
/home/petmay01/linaro/qemu-from-laptop/qemu/exec.c:3056: undefined
reference to `cpu_reloading_memory_map'
cpus.o: In function `tcg_cpu_exec':
/home/petmay01/linaro/qemu-from-laptop/qemu/cpus.c:1362: undefined
reference to `cpu_exec'
cpus.o: In function `qemu_tcg_rr_cpu_thread_fn':
/home/petmay01/linaro/qemu-from-laptop/qemu/cpus.c:1401: undefined
reference to `tcg_register_thread'
/home/petmay01/linaro/qemu-from-laptop/qemu/cpus.c:1471: undefined
reference to `cpu_exec_step_atomic'
cpus.o: In function `qemu_tcg_cpu_thread_fn':
/home/petmay01/linaro/qemu-from-laptop/qemu/cpus.c:1637: undefined
reference to `tcg_register_thread'
/home/petmay01/linaro/qemu-from-laptop/qemu/cpus.c:1673: undefined
reference to `cpu_exec_step_atomic'
cpus.o: In function `qemu_tcg_init_vcpu':
/home/petmay01/linaro/qemu-from-laptop/qemu/cpus.c:1865: undefined
reference to `tcg_region_init'
/home/petmay01/linaro/qemu-from-laptop/qemu/cpus.c:1875: undefined
reference to `parallel_cpus'
collect2: error: ld returned 1 exit status

Configured with --target-list=x86_64-softmmu --enable-debug --disable-tcg

It's probably the --enable-debug that makes the difference:
for instance cpu_reloading_memory_map() is referenced from
tcg_commit(), which is only called from within an "if (tcg_enabled())"
guard; with debug disabled the compiler probably figures out that
tcg_commit() is unreachable and doesn't put it in the .o file.

Possibly this has always been broken and wasn't a regression?
If so I guess we should apply your patch and then fix this
separately...

thanks
-- PMM

Re: [Qemu-devel] [PATCH] tcg: fix --disable-tcg build breakage introduced by tb_lock removal
Posted by Peter Maydell 5 years, 9 months ago
On 22 June 2018 at 17:24, Peter Maydell <peter.maydell@linaro.org> wrote:
> Possibly this has always been broken and wasn't a regression?
> If so I guess we should apply your patch and then fix this
> separately...

Yep, before your tb_lock changes on my system I can build
--disable-tcg, but not --disable-tcg --enable-debug. After
your tb_lock changes, both fail, and this patch fixes the
--disable-tcg case again. So I think we could reasonably
apply this (to fix the travis builds) and then look at
the longer-standing "can't build --disable-tcg --enable-debug"
issue separately.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] tcg: fix --disable-tcg build breakage introduced by tb_lock removal
Posted by Emilio G. Cota 5 years, 9 months ago
On Fri, Jun 22, 2018 at 17:24:33 +0100, Peter Maydell wrote:
> On 22 June 2018 at 17:16, Emilio G. Cota <cota@braap.org> wrote:
> It's probably the --enable-debug that makes the difference:
> for instance cpu_reloading_memory_map() is referenced from
> tcg_commit(), which is only called from within an "if (tcg_enabled())"
> guard; with debug disabled the compiler probably figures out that
> tcg_commit() is unreachable and doesn't put it in the .o file.

Yes, I get the same errors with both --enable-debug and --disable-tcg.

> Possibly this has always been broken and wasn't a regression?
> If so I guess we should apply your patch and then fix this
> separately...

v2.12.0 doesn't build either with the options above, so the
regression is fixed with the patch.

I'll send a v2 patch as a separate thread with an updated commit
message to reflect the above.

Thanks,

		Emilio