On Mon, Nov 6, 2017 at 12:13 PM, Emilio G. Cota <c...@braap.org> wrote:
> On Mon, Nov 06, 2017 at 12:10:22 -0200, Eduardo Habkost wrote:
>> IMO, initialization state doesn't belong to CPUClass. We already
>> have a single accelerator object in MachineState::accelerator,
>> and tcg_initialized could be moved to a AccelState::initialized
>> field.
>
> I don't know how to cleanly get AccelState from a CPUClass pointer
> (as I said I'm not familiar with object code / qom) -- suggestions
> welcome! The best I could come up in the limited time I have for
> this is to use a static bool, as shown below.
>
>
> ---8<---
>
> Subject: [PATCH] qom: move CPUClass.tcg_initialize to a global
>
> 55c3cee ("qom: Introduce CPUClass.tcg_initialize", 2017-10-24)
> introduces a per-CPUClass bool that we check so that the target CPU
> is initialized for TCG only once. This works well except when
> we end up creating more than one CPUClass, in which case we end
> up incorrectly initializing TCG more than once, i.e. once for
> each CPUClass.
>
> This can be replicated with:
> $ aarch64-softmmu/qemu-system-aarch64 -machine xlnx-zcu102 -smp 6 \
> -global driver=xlnx,,zynqmp,property=has_rpu,value=on
> In this case the class name of the "RPUs" is prefixed by "cortex-r5-",
> whereas the "regular" CPUs are prefixed by "cortex-a53-". This
> results in two CPUClass instances being created.
>
> Fix it by introducing a static variable, so that only the first
> target CPU being initialized will initialize the target-dependent
> part of TCG, regardless of CPUClass instances.
>
> Fixes: 55c3ceef61fcf06fc98ddc752b7cce788ce7680b
> Signed-off-by: Emilio G. Cota <c...@braap.org>
This works great for me, is this ok to be applied?
Thanks,
Alistair
> ---
> exec.c | 5 +++--
> include/qom/cpu.h | 1 -
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index 97a24a8..8b579c0 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -792,11 +792,12 @@ void cpu_exec_initfn(CPUState *cpu)
> void cpu_exec_realizefn(CPUState *cpu, Error **errp)
> {
> CPUClass *cc = CPU_GET_CLASS(cpu);
> + static bool tcg_target_initialized;
>
> cpu_list_add(cpu);
>
> - if (tcg_enabled() && !cc->tcg_initialized) {
> - cc->tcg_initialized = true;
> + if (tcg_enabled() && !tcg_target_initialized) {
> + tcg_target_initialized = true;
> cc->tcg_initialize();
> }
>
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index fa4b0c9..c2fa151 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -209,7 +209,6 @@ typedef struct CPUClass {
> /* Keep non-pointer data at the end to minimize holes. */
> int gdb_num_core_regs;
> bool gdb_stop_before_watchpoint;
> - bool tcg_initialized;
> } CPUClass;
>
> #ifdef HOST_WORDS_BIGENDIAN
> --
> 2.7.4
>
>
