[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] hw: add .min_cpus and .default_cpus fields to m
From: |
Eduardo Habkost |
Subject: |
Re: [Qemu-devel] [PATCH] hw: add .min_cpus and .default_cpus fields to machine_class |
Date: |
Tue, 7 Nov 2017 10:31:49 -0200 |
User-agent: |
Mutt/1.9.1 (2017-09-22) |
On Mon, Nov 06, 2017 at 04:43:34PM -0800, Alistair Francis wrote:
> On Mon, Nov 6, 2017 at 12:13 PM, Emilio G. Cota <address@hidden> 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.
I don't believe a TYPE_ACCEL object is created in *-user. We can
consider changing that, but not during 2.11 freeze.
> >
> >
> > ---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 <address@hidden>
>
> This works great for me, is this ok to be applied?
It depends if we are already relying on initialization of CPUs of
different types to be calling two different ->tcg_initialize()
functions. I think that's the case today, so this looks
reasonable as a quick fix for 2.11.
I would wait for an Acked-by from Richard Henderson before
applying it, though. Richard, what do you think?
>
> 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
> >
> >
--
Eduardo
- [Qemu-devel] [PATCH] hw: add .min_cpus and .default_cpus fields to machine_class, Emilio G. Cota, 2017/11/03
- Re: [Qemu-devel] [PATCH] hw: add .min_cpus and .default_cpus fields to machine_class, Emilio G. Cota, 2017/11/06
- Re: [Qemu-devel] [PATCH] hw: add .min_cpus and .default_cpus fields to machine_class, Alistair Francis, 2017/11/06
- Re: [Qemu-devel] [PATCH] hw: add .min_cpus and .default_cpus fields to machine_class, Emilio G. Cota, 2017/11/06
- Re: [Qemu-devel] [PATCH] hw: add .min_cpus and .default_cpus fields to machine_class, Alistair Francis, 2017/11/06
- Re: [Qemu-devel] [Qemu-arm] [PATCH] hw: add .min_cpus and .default_cpus fields to machine_class, Philippe Mathieu-Daudé, 2017/11/06
- Re: [Qemu-devel] [Qemu-arm] [PATCH] hw: add .min_cpus and .default_cpus fields to machine_class, Alistair Francis, 2017/11/06
- Re: [Qemu-devel] [Qemu-arm] [PATCH] hw: add .min_cpus and .default_cpus fields to machine_class, Philippe Mathieu-Daudé, 2017/11/07