[PATCH-for-5.1 v3 0/2] stubs: Fix notify-event stub linkage error on MinGW

[PATCH-for-5.1 v3 0/2] stubs: Fix notify-event stub linkage error on MinGW

[Philippe Mathieu-Daudé]

2 trivial patches to fix the link error reported by Thomas:

  LINK    tests/test-timed-average.exe
 libqemuutil.a(main-loop.o): In function `qemu_notify_event':
 util/main-loop.c:139: multiple definition of `qemu_notify_event'
 tests/test-timed-average.o:/builds/huth/qemu/tests/../stubs/notify-event.c:5: first defined here
 collect2: error: ld returned 1 exit status
 rules.mak:124: recipe for target 'tests/test-timed-average.exe' failed

Since v2:
- Remove incorrectly added abort() in qemu_timer_notify_cb()

Philippe Mathieu-Daudé (2):
  exec: Restrict icount to softmmu
  stubs: Remove qemu_notify_event()

 include/sysemu/cpus.h  | 4 ++++
 exec.c                 | 4 ----
 softmmu/cpus.c         | 7 +++++++
 stubs/cpu-get-icount.c | 2 +-
 stubs/notify-event.c   | 6 ------
 stubs/Makefile.objs    | 1 -
 6 files changed, 12 insertions(+), 12 deletions(-)
 delete mode 100644 stubs/notify-event.c

-- 
2.21.3

[PATCH-for-5.1 v3 1/2] exec: Restrict icount to softmmu

[Philippe Mathieu-Daudé]

'icount' feature is only meaningful when using softmmu.
Move it out of the globally used exec.c, and define it as
'false' in user-mode emulation.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/sysemu/cpus.h | 4 ++++
 exec.c                | 4 ----
 softmmu/cpus.c        | 7 +++++++
 3 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/include/sysemu/cpus.h b/include/sysemu/cpus.h
index 3c1da6a018..d8442aa9f0 100644
--- a/include/sysemu/cpus.h
+++ b/include/sysemu/cpus.h
@@ -11,9 +11,13 @@ void pause_all_vcpus(void);
 void cpu_stop_current(void);
 void cpu_ticks_init(void);
 
+#if !defined(CONFIG_USER_ONLY)
 void configure_icount(QemuOpts *opts, Error **errp);
 extern int use_icount;
 extern int icount_align_option;
+#else
+#define use_icount false
+#endif
 
 /* drift information for info jit command */
 extern int64_t max_delay;
diff --git a/exec.c b/exec.c
index 6f381f98e2..a89ffa93c1 100644
--- a/exec.c
+++ b/exec.c
@@ -102,10 +102,6 @@ uintptr_t qemu_host_page_size;
 intptr_t qemu_host_page_mask;
 
 #if !defined(CONFIG_USER_ONLY)
-/* 0 = Do not count executed instructions.
-   1 = Precise instruction counting.
-   2 = Adaptive rate instruction counting.  */
-int use_icount;
 
 typedef struct PhysPageEntry PhysPageEntry;
 
diff --git a/softmmu/cpus.c b/softmmu/cpus.c
index a802e899ab..a4772034c0 100644
--- a/softmmu/cpus.c
+++ b/softmmu/cpus.c
@@ -81,6 +81,13 @@
 
 #endif /* CONFIG_LINUX */
 
+/*
+ * 0 = Do not count executed instructions.
+ * 1 = Precise instruction counting.
+ * 2 = Adaptive rate instruction counting.
+ */
+int use_icount;
+
 static QemuMutex qemu_global_mutex;
 
 int64_t max_delay;
-- 
2.21.3

Re: [PATCH-for-5.1 v3 1/2] exec: Restrict icount to softmmu

[Claudio Fontana]

Hi Philippe,

could you take a look if this series already addresses the issue?

https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00067.html

Everything icount related is already moved to softmmu and made TCG only.

I will post a new version of the series today with a couple changes;

the series could then be ready if HVF is already ready with its synchronize_state implementation? Otherwise we'd have to hold back the HVF patch.

Thanks!

Claudio


On 8/5/20 12:01 PM, Philippe Mathieu-Daudé wrote:
> 'icount' feature is only meaningful when using softmmu.
> Move it out of the globally used exec.c, and define it as
> 'false' in user-mode emulation.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  include/sysemu/cpus.h | 4 ++++
>  exec.c                | 4 ----
>  softmmu/cpus.c        | 7 +++++++
>  3 files changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/include/sysemu/cpus.h b/include/sysemu/cpus.h
> index 3c1da6a018..d8442aa9f0 100644
> --- a/include/sysemu/cpus.h
> +++ b/include/sysemu/cpus.h
> @@ -11,9 +11,13 @@ void pause_all_vcpus(void);
>  void cpu_stop_current(void);
>  void cpu_ticks_init(void);
>  
> +#if !defined(CONFIG_USER_ONLY)
>  void configure_icount(QemuOpts *opts, Error **errp);
>  extern int use_icount;
>  extern int icount_align_option;
> +#else
> +#define use_icount false
> +#endif
>  
>  /* drift information for info jit command */
>  extern int64_t max_delay;
> diff --git a/exec.c b/exec.c
> index 6f381f98e2..a89ffa93c1 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -102,10 +102,6 @@ uintptr_t qemu_host_page_size;
>  intptr_t qemu_host_page_mask;
>  
>  #if !defined(CONFIG_USER_ONLY)
> -/* 0 = Do not count executed instructions.
> -   1 = Precise instruction counting.
> -   2 = Adaptive rate instruction counting.  */
> -int use_icount;
>  
>  typedef struct PhysPageEntry PhysPageEntry;
>  
> diff --git a/softmmu/cpus.c b/softmmu/cpus.c
> index a802e899ab..a4772034c0 100644
> --- a/softmmu/cpus.c
> +++ b/softmmu/cpus.c
> @@ -81,6 +81,13 @@
>  
>  #endif /* CONFIG_LINUX */
>  
> +/*
> + * 0 = Do not count executed instructions.
> + * 1 = Precise instruction counting.
> + * 2 = Adaptive rate instruction counting.
> + */
> +int use_icount;
> +
>  static QemuMutex qemu_global_mutex;
>  
>  int64_t max_delay;
> 

Re: [PATCH-for-5.1 v3 1/2] exec: Restrict icount to softmmu

[Philippe Mathieu-Daudé]

On 8/5/20 12:12 PM, Claudio Fontana wrote:
> Hi Philippe,
> 
> could you take a look if this series already addresses the issue?
> 
> https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg00067.html

I had tagged this series as "after-release" review and hadn't looked
at it yet :)

> 
> Everything icount related is already moved to softmmu and made TCG only.

Good. I was looking for a surgical change for 5.1, but I think it is too
late to have the WHPX CI running now (Thomas?). So we'll probably ignore
my surgical series and proceed with yours when 5.2 development window
opens.

> 
> I will post a new version of the series today with a couple changes;
> 
> the series could then be ready if HVF is already ready with its synchronize_state implementation? Otherwise we'd have to hold back the HVF patch.
> 
> Thanks!
> 
> Claudio
> 
> 
> On 8/5/20 12:01 PM, Philippe Mathieu-Daudé wrote:
>> 'icount' feature is only meaningful when using softmmu.
>> Move it out of the globally used exec.c, and define it as
>> 'false' in user-mode emulation.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>  include/sysemu/cpus.h | 4 ++++
>>  exec.c                | 4 ----
>>  softmmu/cpus.c        | 7 +++++++
>>  3 files changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/sysemu/cpus.h b/include/sysemu/cpus.h
>> index 3c1da6a018..d8442aa9f0 100644
>> --- a/include/sysemu/cpus.h
>> +++ b/include/sysemu/cpus.h
>> @@ -11,9 +11,13 @@ void pause_all_vcpus(void);
>>  void cpu_stop_current(void);
>>  void cpu_ticks_init(void);
>>  
>> +#if !defined(CONFIG_USER_ONLY)
>>  void configure_icount(QemuOpts *opts, Error **errp);
>>  extern int use_icount;
>>  extern int icount_align_option;
>> +#else
>> +#define use_icount false
>> +#endif
>>  
>>  /* drift information for info jit command */
>>  extern int64_t max_delay;
>> diff --git a/exec.c b/exec.c
>> index 6f381f98e2..a89ffa93c1 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -102,10 +102,6 @@ uintptr_t qemu_host_page_size;
>>  intptr_t qemu_host_page_mask;
>>  
>>  #if !defined(CONFIG_USER_ONLY)
>> -/* 0 = Do not count executed instructions.
>> -   1 = Precise instruction counting.
>> -   2 = Adaptive rate instruction counting.  */
>> -int use_icount;
>>  
>>  typedef struct PhysPageEntry PhysPageEntry;
>>  
>> diff --git a/softmmu/cpus.c b/softmmu/cpus.c
>> index a802e899ab..a4772034c0 100644
>> --- a/softmmu/cpus.c
>> +++ b/softmmu/cpus.c
>> @@ -81,6 +81,13 @@
>>  
>>  #endif /* CONFIG_LINUX */
>>  
>> +/*
>> + * 0 = Do not count executed instructions.
>> + * 1 = Precise instruction counting.
>> + * 2 = Adaptive rate instruction counting.
>> + */
>> +int use_icount;
>> +
>>  static QemuMutex qemu_global_mutex;
>>  
>>  int64_t max_delay;
>>
> 

Re: [PATCH-for-5.1 v3 1/2] exec: Restrict icount to softmmu

[Richard Henderson]

On 8/5/20 3:01 AM, Philippe Mathieu-Daudé wrote:
> 'icount' feature is only meaningful when using softmmu.
> Move it out of the globally used exec.c, and define it as
> 'false' in user-mode emulation.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  include/sysemu/cpus.h | 4 ++++
>  exec.c                | 4 ----
>  softmmu/cpus.c        | 7 +++++++
>  3 files changed, 11 insertions(+), 4 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH-for-5.1 v3 2/2] stubs: Remove qemu_notify_event()

[Philippe Mathieu-Daudé]

We don't need the qemu_notify_event() stub anymore.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 stubs/cpu-get-icount.c | 2 +-
 stubs/notify-event.c   | 6 ------
 stubs/Makefile.objs    | 1 -
 3 files changed, 1 insertion(+), 8 deletions(-)
 delete mode 100644 stubs/notify-event.c

diff --git a/stubs/cpu-get-icount.c b/stubs/cpu-get-icount.c
index b35f844638..4578217fe2 100644
--- a/stubs/cpu-get-icount.c
+++ b/stubs/cpu-get-icount.c
@@ -17,5 +17,5 @@ int64_t cpu_get_icount_raw(void)
 
 void qemu_timer_notify_cb(void *opaque, QEMUClockType type)
 {
-    qemu_notify_event();
+    /* Nothing to do. */
 }
diff --git a/stubs/notify-event.c b/stubs/notify-event.c
deleted file mode 100644
index 827bb52d1a..0000000000
--- a/stubs/notify-event.c
+++ /dev/null
@@ -1,6 +0,0 @@
-#include "qemu/osdep.h"
-#include "qemu/main-loop.h"
-
-void qemu_notify_event(void)
-{
-}
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index d42046afe4..cb374c96db 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -12,7 +12,6 @@ stub-obj-y += isa-bus.o
 stub-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
 stub-obj-$(CONFIG_LINUX_IO_URING) += io_uring.o
 stub-obj-y += monitor-core.o
-stub-obj-y += notify-event.o
 stub-obj-y += pci-bus.o
 stub-obj-y += qmp_memory_device.o
 stub-obj-y += qtest.o
-- 
2.21.3

Re: [PATCH-for-5.1 v3 0/2] stubs: Fix notify-event stub linkage error on MinGW

[Thomas Huth]

On 05/08/2020 12.01, Philippe Mathieu-Daudé wrote:
> 2 trivial patches to fix the link error reported by Thomas:
> 
>   LINK    tests/test-timed-average.exe
>  libqemuutil.a(main-loop.o): In function `qemu_notify_event':
>  util/main-loop.c:139: multiple definition of `qemu_notify_event'
>  tests/test-timed-average.o:/builds/huth/qemu/tests/../stubs/notify-event.c:5: first defined here
>  collect2: error: ld returned 1 exit status
>  rules.mak:124: recipe for target 'tests/test-timed-average.exe' failed
> 
> Since v2:
> - Remove incorrectly added abort() in qemu_timer_notify_cb()

Works for me (tested here:
https://gitlab.com/huth/qemu/-/pipelines/174503835 ) ... but I wonder
whether there is a code path that uses the stub from
stubs/cpu-get-icount.c but the qemu_notify_event() from util/main-loop.c ?

 Thomas