This doesn't save much as cpu-exec-common still needs to be built
per-target for its knowledge of CPUState but this helps with keeping
things organised.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 accel/tcg/cpu-exec-common.c  | 30 ----------------------
 accel/tcg/cpu-exec-softmmu.c | 50 ++++++++++++++++++++++++++++++++++++
 accel/tcg/meson.build        | 10 ++++++++
 3 files changed, 60 insertions(+), 30 deletions(-)
 create mode 100644 accel/tcg/cpu-exec-softmmu.c

diff --git a/accel/tcg/cpu-exec-common.c b/accel/tcg/cpu-exec-common.c
index e7962c9348..c6b0ad303e 100644
--- a/accel/tcg/cpu-exec-common.c
+++ b/accel/tcg/cpu-exec-common.c
@@ -32,36 +32,6 @@ void cpu_loop_exit_noexc(CPUState *cpu)
     cpu_loop_exit(cpu);
 }
 
-#if defined(CONFIG_SOFTMMU)
-void cpu_reloading_memory_map(void)
-{
-    if (qemu_in_vcpu_thread() && current_cpu->running) {
-        /* The guest can in theory prolong the RCU critical section as long
-         * as it feels like. The major problem with this is that because it
-         * can do multiple reconfigurations of the memory map within the
-         * critical section, we could potentially accumulate an unbounded
-         * collection of memory data structures awaiting reclamation.
-         *
-         * Because the only thing we're currently protecting with RCU is the
-         * memory data structures, it's sufficient to break the critical section
-         * in this callback, which we know will get called every time the
-         * memory map is rearranged.
-         *
-         * (If we add anything else in the system that uses RCU to protect
-         * its data structures, we will need to implement some other mechanism
-         * to force TCG CPUs to exit the critical section, at which point this
-         * part of this callback might become unnecessary.)
-         *
-         * This pair matches cpu_exec's rcu_read_lock()/rcu_read_unlock(), which
-         * only protects cpu->as->dispatch. Since we know our caller is about
-         * to reload it, it's safe to split the critical section.
-         */
-        rcu_read_unlock();
-        rcu_read_lock();
-    }
-}
-#endif
-
 void cpu_loop_exit(CPUState *cpu)
 {
     /* Undo the setting in cpu_tb_exec.  */
diff --git a/accel/tcg/cpu-exec-softmmu.c b/accel/tcg/cpu-exec-softmmu.c
new file mode 100644
index 0000000000..2318dd8c7d
--- /dev/null
+++ b/accel/tcg/cpu-exec-softmmu.c
@@ -0,0 +1,50 @@
+/*
+ *  Emulator main CPU execution loop, softmmu bits
+ *
+ *  Copyright (c) 2003-2005 Fabrice Bellard
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/core/cpu.h"
+#include "sysemu/cpus.h"
+
+void cpu_reloading_memory_map(void)
+{
+    if (qemu_in_vcpu_thread() && current_cpu->running) {
+        /* The guest can in theory prolong the RCU critical section as long
+         * as it feels like. The major problem with this is that because it
+         * can do multiple reconfigurations of the memory map within the
+         * critical section, we could potentially accumulate an unbounded
+         * collection of memory data structures awaiting reclamation.
+         *
+         * Because the only thing we're currently protecting with RCU is the
+         * memory data structures, it's sufficient to break the critical section
+         * in this callback, which we know will get called every time the
+         * memory map is rearranged.
+         *
+         * (If we add anything else in the system that uses RCU to protect
+         * its data structures, we will need to implement some other mechanism
+         * to force TCG CPUs to exit the critical section, at which point this
+         * part of this callback might become unnecessary.)
+         *
+         * This pair matches cpu_exec's rcu_read_lock()/rcu_read_unlock(), which
+         * only protects cpu->as->dispatch. Since we know our caller is about
+         * to reload it, it's safe to split the critical section.
+         */
+        rcu_read_unlock();
+        rcu_read_lock();
+    }
+}
diff --git a/accel/tcg/meson.build b/accel/tcg/meson.build
index aeb20a6ef0..bdc086b90d 100644
--- a/accel/tcg/meson.build
+++ b/accel/tcg/meson.build
@@ -1,3 +1,9 @@
+#
+# Currently most things here end up in specific_ss eventually because
+# they need knowledge of CPUState. Stuff that that doesn't can live in
+# common user, softmmu or overall code
+#
+
 tcg_ss = ss.source_set()
 tcg_ss.add(files(
   'tcg-all.c',
@@ -9,6 +15,7 @@ tcg_ss.add(files(
   'translate-all.c',
   'translator.c',
 ))
+
 tcg_ss.add(when: 'CONFIG_USER_ONLY', if_true: files('user-exec.c'))
 tcg_ss.add(when: 'CONFIG_SOFTMMU', if_false: files('user-exec-stub.c'))
 tcg_ss.add(when: 'CONFIG_PLUGIN', if_true: [files('plugin-gen.c')])
@@ -27,3 +34,6 @@ tcg_module_ss.add(when: ['CONFIG_SOFTMMU', 'CONFIG_TCG'], if_true: files(
   'tcg-accel-ops-icount.c',
   'tcg-accel-ops-rr.c',
 ))
+
+# Common softmmu code
+softmmu_ss.add(files('cpu-exec-softmmu.c'))
-- 
2.39.2

On Mon, 20 Mar 2023 10:10:27 +0000
Alex Bennée <alex.bennee@linaro.org> wrote:

> This doesn't save much as cpu-exec-common still needs to be built
> per-target for its knowledge of CPUState but this helps with keeping
> things organised.

> --- /dev/null
> +++ b/accel/tcg/cpu-exec-softmmu.c

Could `cpu_reloading_memory_map` be pushed closer to its only user
(softmmu/physmem.c) instead of creating a new file in accel/tcg?

Maybe I'm missing something, but I see other usages of current_cpu in
softmmu:

    $ git grep 'current_cpu->' softmmu/|cat
    softmmu/cpus.c:        current_cpu->stop = true;
    softmmu/memory.c:        return current_cpu->cpu_index;
    softmmu/runstate.c:        current_cpu->crash_occurred = true;

Maybe you envision more stuff in cpu-exec-softmmu.c.

Reviewed-by: Alessandro Di Federico <ale@rev.ng>

-- 
Alessandro Di Federico
rev.ng Labs

How is this conditional on CONFIG_TCG? To me it looks like this breaks !CONFIG_TCG.
Careful, the meson.build in accel/tcg/meson.build is always recursed.

This code was in tcg_ss before, why not simply add it to tcg_ss and then to specific_ss along with the other tcg pieces?

Ciao,

C


On 3/20/23 11:10, Alex Bennée wrote:
> This doesn't save much as cpu-exec-common still needs to be built
> per-target for its knowledge of CPUState but this helps with keeping
> things organised.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  accel/tcg/cpu-exec-common.c  | 30 ----------------------
>  accel/tcg/cpu-exec-softmmu.c | 50 ++++++++++++++++++++++++++++++++++++
>  accel/tcg/meson.build        | 10 ++++++++
>  3 files changed, 60 insertions(+), 30 deletions(-)
>  create mode 100644 accel/tcg/cpu-exec-softmmu.c
> 
> diff --git a/accel/tcg/cpu-exec-common.c b/accel/tcg/cpu-exec-common.c
> index e7962c9348..c6b0ad303e 100644
> --- a/accel/tcg/cpu-exec-common.c
> +++ b/accel/tcg/cpu-exec-common.c
> @@ -32,36 +32,6 @@ void cpu_loop_exit_noexc(CPUState *cpu)
>      cpu_loop_exit(cpu);
>  }
>  
> -#if defined(CONFIG_SOFTMMU)
> -void cpu_reloading_memory_map(void)
> -{
> -    if (qemu_in_vcpu_thread() && current_cpu->running) {
> -        /* The guest can in theory prolong the RCU critical section as long
> -         * as it feels like. The major problem with this is that because it
> -         * can do multiple reconfigurations of the memory map within the
> -         * critical section, we could potentially accumulate an unbounded
> -         * collection of memory data structures awaiting reclamation.
> -         *
> -         * Because the only thing we're currently protecting with RCU is the
> -         * memory data structures, it's sufficient to break the critical section
> -         * in this callback, which we know will get called every time the
> -         * memory map is rearranged.
> -         *
> -         * (If we add anything else in the system that uses RCU to protect
> -         * its data structures, we will need to implement some other mechanism
> -         * to force TCG CPUs to exit the critical section, at which point this
> -         * part of this callback might become unnecessary.)
> -         *
> -         * This pair matches cpu_exec's rcu_read_lock()/rcu_read_unlock(), which
> -         * only protects cpu->as->dispatch. Since we know our caller is about
> -         * to reload it, it's safe to split the critical section.
> -         */
> -        rcu_read_unlock();
> -        rcu_read_lock();
> -    }
> -}
> -#endif
> -
>  void cpu_loop_exit(CPUState *cpu)
>  {
>      /* Undo the setting in cpu_tb_exec.  */
> diff --git a/accel/tcg/cpu-exec-softmmu.c b/accel/tcg/cpu-exec-softmmu.c
> new file mode 100644
> index 0000000000..2318dd8c7d
> --- /dev/null
> +++ b/accel/tcg/cpu-exec-softmmu.c
> @@ -0,0 +1,50 @@
> +/*
> + *  Emulator main CPU execution loop, softmmu bits
> + *
> + *  Copyright (c) 2003-2005 Fabrice Bellard
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/core/cpu.h"
> +#include "sysemu/cpus.h"
> +
> +void cpu_reloading_memory_map(void)
> +{
> +    if (qemu_in_vcpu_thread() && current_cpu->running) {
> +        /* The guest can in theory prolong the RCU critical section as long
> +         * as it feels like. The major problem with this is that because it
> +         * can do multiple reconfigurations of the memory map within the
> +         * critical section, we could potentially accumulate an unbounded
> +         * collection of memory data structures awaiting reclamation.
> +         *
> +         * Because the only thing we're currently protecting with RCU is the
> +         * memory data structures, it's sufficient to break the critical section
> +         * in this callback, which we know will get called every time the
> +         * memory map is rearranged.
> +         *
> +         * (If we add anything else in the system that uses RCU to protect
> +         * its data structures, we will need to implement some other mechanism
> +         * to force TCG CPUs to exit the critical section, at which point this
> +         * part of this callback might become unnecessary.)
> +         *
> +         * This pair matches cpu_exec's rcu_read_lock()/rcu_read_unlock(), which
> +         * only protects cpu->as->dispatch. Since we know our caller is about
> +         * to reload it, it's safe to split the critical section.
> +         */
> +        rcu_read_unlock();
> +        rcu_read_lock();
> +    }
> +}
> diff --git a/accel/tcg/meson.build b/accel/tcg/meson.build
> index aeb20a6ef0..bdc086b90d 100644
> --- a/accel/tcg/meson.build
> +++ b/accel/tcg/meson.build
> @@ -1,3 +1,9 @@
> +#
> +# Currently most things here end up in specific_ss eventually because
> +# they need knowledge of CPUState. Stuff that that doesn't can live in
> +# common user, softmmu or overall code
> +#
> +
>  tcg_ss = ss.source_set()
>  tcg_ss.add(files(
>    'tcg-all.c',
> @@ -9,6 +15,7 @@ tcg_ss.add(files(
>    'translate-all.c',
>    'translator.c',
>  ))
> +
>  tcg_ss.add(when: 'CONFIG_USER_ONLY', if_true: files('user-exec.c'))
>  tcg_ss.add(when: 'CONFIG_SOFTMMU', if_false: files('user-exec-stub.c'))
>  tcg_ss.add(when: 'CONFIG_PLUGIN', if_true: [files('plugin-gen.c')])
> @@ -27,3 +34,6 @@ tcg_module_ss.add(when: ['CONFIG_SOFTMMU', 'CONFIG_TCG'], if_true: files(
>    'tcg-accel-ops-icount.c',
>    'tcg-accel-ops-rr.c',
>  ))
> +
> +# Common softmmu code
> +softmmu_ss.add(files('cpu-exec-softmmu.c'))

Claudio Fontana <cfontana@suse.de> writes:

> How is this conditional on CONFIG_TCG? To me it looks like this breaks !CONFIG_TCG.
> Careful, the meson.build in accel/tcg/meson.build is always recursed.

Surely it shouldn't be in accel/tcg then?

> This code was in tcg_ss before, why not simply add it to tcg_ss and
> then to specific_ss along with the other tcg pieces?

tcg_ss is rebuilt for every target. So far the code in cpu-exec-softmmu
should only need to for softmmu targets and hopefully share the same
binary for all variants.

I guess I'll have to do something more in line with the recent
re-factoring of gdbstub:

  # We build two versions of gdbstub, one for each mode
  gdb_user_ss.add(files('gdbstub.c', 'user.c'))
  gdb_softmmu_ss.add(files('gdbstub.c', 'softmmu.c'))

  gdb_user_ss = gdb_user_ss.apply(config_host, strict: false)
  gdb_softmmu_ss = gdb_softmmu_ss.apply(config_host, strict: false)

  libgdb_user = static_library('gdb_user',
                               gdb_user_ss.sources() + genh,
                               name_suffix: 'fa',
                               c_args: '-DCONFIG_USER_ONLY')

  libgdb_softmmu = static_library('gdb_softmmu',
                                  gdb_softmmu_ss.sources() + genh,
                                  name_suffix: 'fa')

  gdb_user = declare_dependency(link_whole: libgdb_user)
  user_ss.add(gdb_user)
  gdb_softmmu = declare_dependency(link_whole: libgdb_softmmu)
  softmmu_ss.add(gdb_softmmu)


>
> Ciao,
>
> C
>
>
> On 3/20/23 11:10, Alex Bennée wrote:
>> This doesn't save much as cpu-exec-common still needs to be built
>> per-target for its knowledge of CPUState but this helps with keeping
>> things organised.
>> 
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>  accel/tcg/cpu-exec-common.c  | 30 ----------------------
>>  accel/tcg/cpu-exec-softmmu.c | 50 ++++++++++++++++++++++++++++++++++++
>>  accel/tcg/meson.build        | 10 ++++++++
>>  3 files changed, 60 insertions(+), 30 deletions(-)
>>  create mode 100644 accel/tcg/cpu-exec-softmmu.c
>> 
>> diff --git a/accel/tcg/cpu-exec-common.c b/accel/tcg/cpu-exec-common.c
>> index e7962c9348..c6b0ad303e 100644
>> --- a/accel/tcg/cpu-exec-common.c
>> +++ b/accel/tcg/cpu-exec-common.c
>> @@ -32,36 +32,6 @@ void cpu_loop_exit_noexc(CPUState *cpu)
>>      cpu_loop_exit(cpu);
>>  }
>>  
>> -#if defined(CONFIG_SOFTMMU)
>> -void cpu_reloading_memory_map(void)
>> -{
>> -    if (qemu_in_vcpu_thread() && current_cpu->running) {
>> -        /* The guest can in theory prolong the RCU critical section as long
>> -         * as it feels like. The major problem with this is that because it
>> -         * can do multiple reconfigurations of the memory map within the
>> -         * critical section, we could potentially accumulate an unbounded
>> -         * collection of memory data structures awaiting reclamation.
>> -         *
>> -         * Because the only thing we're currently protecting with RCU is the
>> -         * memory data structures, it's sufficient to break the critical section
>> -         * in this callback, which we know will get called every time the
>> -         * memory map is rearranged.
>> -         *
>> -         * (If we add anything else in the system that uses RCU to protect
>> -         * its data structures, we will need to implement some other mechanism
>> -         * to force TCG CPUs to exit the critical section, at which point this
>> -         * part of this callback might become unnecessary.)
>> -         *
>> -         * This pair matches cpu_exec's rcu_read_lock()/rcu_read_unlock(), which
>> -         * only protects cpu->as->dispatch. Since we know our caller is about
>> -         * to reload it, it's safe to split the critical section.
>> -         */
>> -        rcu_read_unlock();
>> -        rcu_read_lock();
>> -    }
>> -}
>> -#endif
>> -
>>  void cpu_loop_exit(CPUState *cpu)
>>  {
>>      /* Undo the setting in cpu_tb_exec.  */
>> diff --git a/accel/tcg/cpu-exec-softmmu.c b/accel/tcg/cpu-exec-softmmu.c
>> new file mode 100644
>> index 0000000000..2318dd8c7d
>> --- /dev/null
>> +++ b/accel/tcg/cpu-exec-softmmu.c
>> @@ -0,0 +1,50 @@
>> +/*
>> + *  Emulator main CPU execution loop, softmmu bits
>> + *
>> + *  Copyright (c) 2003-2005 Fabrice Bellard
>> + *
>> + * This library is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2.1 of the License, or (at your option) any later version.
>> + *
>> + * This library is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "hw/core/cpu.h"
>> +#include "sysemu/cpus.h"
>> +
>> +void cpu_reloading_memory_map(void)
>> +{
>> +    if (qemu_in_vcpu_thread() && current_cpu->running) {
>> +        /* The guest can in theory prolong the RCU critical section as long
>> +         * as it feels like. The major problem with this is that because it
>> +         * can do multiple reconfigurations of the memory map within the
>> +         * critical section, we could potentially accumulate an unbounded
>> +         * collection of memory data structures awaiting reclamation.
>> +         *
>> +         * Because the only thing we're currently protecting with RCU is the
>> +         * memory data structures, it's sufficient to break the critical section
>> +         * in this callback, which we know will get called every time the
>> +         * memory map is rearranged.
>> +         *
>> +         * (If we add anything else in the system that uses RCU to protect
>> +         * its data structures, we will need to implement some other mechanism
>> +         * to force TCG CPUs to exit the critical section, at which point this
>> +         * part of this callback might become unnecessary.)
>> +         *
>> +         * This pair matches cpu_exec's rcu_read_lock()/rcu_read_unlock(), which
>> +         * only protects cpu->as->dispatch. Since we know our caller is about
>> +         * to reload it, it's safe to split the critical section.
>> +         */
>> +        rcu_read_unlock();
>> +        rcu_read_lock();
>> +    }
>> +}
>> diff --git a/accel/tcg/meson.build b/accel/tcg/meson.build
>> index aeb20a6ef0..bdc086b90d 100644
>> --- a/accel/tcg/meson.build
>> +++ b/accel/tcg/meson.build
>> @@ -1,3 +1,9 @@
>> +#
>> +# Currently most things here end up in specific_ss eventually because
>> +# they need knowledge of CPUState. Stuff that that doesn't can live in
>> +# common user, softmmu or overall code
>> +#
>> +
>>  tcg_ss = ss.source_set()
>>  tcg_ss.add(files(
>>    'tcg-all.c',
>> @@ -9,6 +15,7 @@ tcg_ss.add(files(
>>    'translate-all.c',
>>    'translator.c',
>>  ))
>> +
>>  tcg_ss.add(when: 'CONFIG_USER_ONLY', if_true: files('user-exec.c'))
>>  tcg_ss.add(when: 'CONFIG_SOFTMMU', if_false: files('user-exec-stub.c'))
>>  tcg_ss.add(when: 'CONFIG_PLUGIN', if_true: [files('plugin-gen.c')])
>> @@ -27,3 +34,6 @@ tcg_module_ss.add(when: ['CONFIG_SOFTMMU', 'CONFIG_TCG'], if_true: files(
>>    'tcg-accel-ops-icount.c',
>>    'tcg-accel-ops-rr.c',
>>  ))
>> +
>> +# Common softmmu code
>> +softmmu_ss.add(files('cpu-exec-softmmu.c'))


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

On 3/20/23 14:32, Alex Bennée wrote:
> 
> Claudio Fontana <cfontana@suse.de> writes:
> 
>> How is this conditional on CONFIG_TCG? To me it looks like this breaks !CONFIG_TCG.
>> Careful, the meson.build in accel/tcg/meson.build is always recursed.
> 
> Surely it shouldn't be in accel/tcg then?


Hi Alex,

maybe we did not understand each other.

What I mean is that accel/tcg/meson.build is not conditionally read, it is _always_ read.

Therefore TCG-specific code needs to be conditionally included using the CONFIG_TCG.

Ciao,

Claudio

> 
>> This code was in tcg_ss before, why not simply add it to tcg_ss and
>> then to specific_ss along with the other tcg pieces?
> 
> tcg_ss is rebuilt for every target. So far the code in cpu-exec-softmmu
> should only need to for softmmu targets and hopefully share the same
> binary for all variants.
> 
> I guess I'll have to do something more in line with the recent
> re-factoring of gdbstub:
> 
>   # We build two versions of gdbstub, one for each mode
>   gdb_user_ss.add(files('gdbstub.c', 'user.c'))
>   gdb_softmmu_ss.add(files('gdbstub.c', 'softmmu.c'))
> 
>   gdb_user_ss = gdb_user_ss.apply(config_host, strict: false)
>   gdb_softmmu_ss = gdb_softmmu_ss.apply(config_host, strict: false)
> 
>   libgdb_user = static_library('gdb_user',
>                                gdb_user_ss.sources() + genh,
>                                name_suffix: 'fa',
>                                c_args: '-DCONFIG_USER_ONLY')
> 
>   libgdb_softmmu = static_library('gdb_softmmu',
>                                   gdb_softmmu_ss.sources() + genh,
>                                   name_suffix: 'fa')
> 
>   gdb_user = declare_dependency(link_whole: libgdb_user)
>   user_ss.add(gdb_user)
>   gdb_softmmu = declare_dependency(link_whole: libgdb_softmmu)
>   softmmu_ss.add(gdb_softmmu)
> 
> 
>>
>> Ciao,
>>
>> C
>>
>>
>> On 3/20/23 11:10, Alex Bennée wrote:
>>> This doesn't save much as cpu-exec-common still needs to be built
>>> per-target for its knowledge of CPUState but this helps with keeping
>>> things organised.
>>>
>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>> ---
>>>  accel/tcg/cpu-exec-common.c  | 30 ----------------------
>>>  accel/tcg/cpu-exec-softmmu.c | 50 ++++++++++++++++++++++++++++++++++++
>>>  accel/tcg/meson.build        | 10 ++++++++
>>>  3 files changed, 60 insertions(+), 30 deletions(-)
>>>  create mode 100644 accel/tcg/cpu-exec-softmmu.c
>>>
>>> diff --git a/accel/tcg/cpu-exec-common.c b/accel/tcg/cpu-exec-common.c
>>> index e7962c9348..c6b0ad303e 100644
>>> --- a/accel/tcg/cpu-exec-common.c
>>> +++ b/accel/tcg/cpu-exec-common.c
>>> @@ -32,36 +32,6 @@ void cpu_loop_exit_noexc(CPUState *cpu)
>>>      cpu_loop_exit(cpu);
>>>  }
>>>  
>>> -#if defined(CONFIG_SOFTMMU)
>>> -void cpu_reloading_memory_map(void)
>>> -{
>>> -    if (qemu_in_vcpu_thread() && current_cpu->running) {
>>> -        /* The guest can in theory prolong the RCU critical section as long
>>> -         * as it feels like. The major problem with this is that because it
>>> -         * can do multiple reconfigurations of the memory map within the
>>> -         * critical section, we could potentially accumulate an unbounded
>>> -         * collection of memory data structures awaiting reclamation.
>>> -         *
>>> -         * Because the only thing we're currently protecting with RCU is the
>>> -         * memory data structures, it's sufficient to break the critical section
>>> -         * in this callback, which we know will get called every time the
>>> -         * memory map is rearranged.
>>> -         *
>>> -         * (If we add anything else in the system that uses RCU to protect
>>> -         * its data structures, we will need to implement some other mechanism
>>> -         * to force TCG CPUs to exit the critical section, at which point this
>>> -         * part of this callback might become unnecessary.)
>>> -         *
>>> -         * This pair matches cpu_exec's rcu_read_lock()/rcu_read_unlock(), which
>>> -         * only protects cpu->as->dispatch. Since we know our caller is about
>>> -         * to reload it, it's safe to split the critical section.
>>> -         */
>>> -        rcu_read_unlock();
>>> -        rcu_read_lock();
>>> -    }
>>> -}
>>> -#endif
>>> -
>>>  void cpu_loop_exit(CPUState *cpu)
>>>  {
>>>      /* Undo the setting in cpu_tb_exec.  */
>>> diff --git a/accel/tcg/cpu-exec-softmmu.c b/accel/tcg/cpu-exec-softmmu.c
>>> new file mode 100644
>>> index 0000000000..2318dd8c7d
>>> --- /dev/null
>>> +++ b/accel/tcg/cpu-exec-softmmu.c
>>> @@ -0,0 +1,50 @@
>>> +/*
>>> + *  Emulator main CPU execution loop, softmmu bits
>>> + *
>>> + *  Copyright (c) 2003-2005 Fabrice Bellard
>>> + *
>>> + * This library is free software; you can redistribute it and/or
>>> + * modify it under the terms of the GNU Lesser General Public
>>> + * License as published by the Free Software Foundation; either
>>> + * version 2.1 of the License, or (at your option) any later version.
>>> + *
>>> + * This library is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>> + * Lesser General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU Lesser General Public
>>> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
>>> + */
>>> +
>>> +#include "qemu/osdep.h"
>>> +#include "hw/core/cpu.h"
>>> +#include "sysemu/cpus.h"
>>> +
>>> +void cpu_reloading_memory_map(void)
>>> +{
>>> +    if (qemu_in_vcpu_thread() && current_cpu->running) {
>>> +        /* The guest can in theory prolong the RCU critical section as long
>>> +         * as it feels like. The major problem with this is that because it
>>> +         * can do multiple reconfigurations of the memory map within the
>>> +         * critical section, we could potentially accumulate an unbounded
>>> +         * collection of memory data structures awaiting reclamation.
>>> +         *
>>> +         * Because the only thing we're currently protecting with RCU is the
>>> +         * memory data structures, it's sufficient to break the critical section
>>> +         * in this callback, which we know will get called every time the
>>> +         * memory map is rearranged.
>>> +         *
>>> +         * (If we add anything else in the system that uses RCU to protect
>>> +         * its data structures, we will need to implement some other mechanism
>>> +         * to force TCG CPUs to exit the critical section, at which point this
>>> +         * part of this callback might become unnecessary.)
>>> +         *
>>> +         * This pair matches cpu_exec's rcu_read_lock()/rcu_read_unlock(), which
>>> +         * only protects cpu->as->dispatch. Since we know our caller is about
>>> +         * to reload it, it's safe to split the critical section.
>>> +         */
>>> +        rcu_read_unlock();
>>> +        rcu_read_lock();
>>> +    }
>>> +}
>>> diff --git a/accel/tcg/meson.build b/accel/tcg/meson.build
>>> index aeb20a6ef0..bdc086b90d 100644
>>> --- a/accel/tcg/meson.build
>>> +++ b/accel/tcg/meson.build
>>> @@ -1,3 +1,9 @@
>>> +#
>>> +# Currently most things here end up in specific_ss eventually because
>>> +# they need knowledge of CPUState. Stuff that that doesn't can live in
>>> +# common user, softmmu or overall code
>>> +#
>>> +
>>>  tcg_ss = ss.source_set()
>>>  tcg_ss.add(files(
>>>    'tcg-all.c',
>>> @@ -9,6 +15,7 @@ tcg_ss.add(files(
>>>    'translate-all.c',
>>>    'translator.c',
>>>  ))
>>> +
>>>  tcg_ss.add(when: 'CONFIG_USER_ONLY', if_true: files('user-exec.c'))
>>>  tcg_ss.add(when: 'CONFIG_SOFTMMU', if_false: files('user-exec-stub.c'))
>>>  tcg_ss.add(when: 'CONFIG_PLUGIN', if_true: [files('plugin-gen.c')])
>>> @@ -27,3 +34,6 @@ tcg_module_ss.add(when: ['CONFIG_SOFTMMU', 'CONFIG_TCG'], if_true: files(
>>>    'tcg-accel-ops-icount.c',
>>>    'tcg-accel-ops-rr.c',
>>>  ))
>>> +
>>> +# Common softmmu code
>>> +softmmu_ss.add(files('cpu-exec-softmmu.c'))
> 
> 

Claudio Fontana <cfontana@suse.de> writes:

> On 3/20/23 14:32, Alex Bennée wrote:
>> 
>> Claudio Fontana <cfontana@suse.de> writes:
>> 
>>> How is this conditional on CONFIG_TCG? To me it looks like this breaks !CONFIG_TCG.
>>> Careful, the meson.build in accel/tcg/meson.build is always recursed.
>> 
>> Surely it shouldn't be in accel/tcg then?
>
>
> Hi Alex,
>
> maybe we did not understand each other.
>
> What I mean is that accel/tcg/meson.build is not conditionally read, it is _always_ read.
>
> Therefore TCG-specific code needs to be conditionally included using
> the CONFIG_TCG.

Ahh I see now, right I can fix that up next revision if there is
interest in this approach.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

On 3/20/23 07:33, Alex Bennée wrote:
> 
> Claudio Fontana <cfontana@suse.de> writes:
> 
>> On 3/20/23 14:32, Alex Bennée wrote:
>>>
>>> Claudio Fontana <cfontana@suse.de> writes:
>>>
>>>> How is this conditional on CONFIG_TCG? To me it looks like this breaks !CONFIG_TCG.
>>>> Careful, the meson.build in accel/tcg/meson.build is always recursed.
>>>
>>> Surely it shouldn't be in accel/tcg then?
>>
>>
>> Hi Alex,
>>
>> maybe we did not understand each other.
>>
>> What I mean is that accel/tcg/meson.build is not conditionally read, it is _always_ read.
>>
>> Therefore TCG-specific code needs to be conditionally included using
>> the CONFIG_TCG.
> 
> Ahh I see now, right I can fix that up next revision if there is
> interest in this approach.

Yes, that seems fine.  With either when: or subdir_done(),

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


r~