On 3/20/23 14:32, Alex Bennée wrote:
>
> Claudio Fontana <cfont...@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.ben...@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'))
>
>
