[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 02/10] accel/tcg: move cpu_reloading_memory_map into cpu-exec
From: |
Alex Bennée |
Subject: |
Re: [PATCH 02/10] accel/tcg: move cpu_reloading_memory_map into cpu-exec-softmmu |
Date: |
Mon, 20 Mar 2023 13:32:37 +0000 |
User-agent: |
mu4e 1.9.22; emacs 29.0.60 |
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
- [PATCH 06/10] includes: move irq definitions out of cpu-all.h, (continued)
Re: [PATCH 02/10] accel/tcg: move cpu_reloading_memory_map into cpu-exec-softmmu, Alessandro Di Federico, 2023/03/21
[PATCH 07/10] accel/tcg: use QEMU_IOTHREAD_LOCK_GUARD to cover the exit, Alex Bennée, 2023/03/20