Re: arm_cpu_post_init (Was: Re: arm: "max" CPU class hierarchy changes possible?)

From: Andrew Jones <drjones@redhat.com>
To: Claudio Fontana <cfontana@suse.de>
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
	"Eduardo Habkost" <ehabkost@redhat.com>,
	"Richard Henderson" <richard.henderson@linaro.org>,
	"Philippe Mathieu-Daudé" <f4bug@amsat.org>,
	qemu-devel <qemu-devel@nongnu.org>,
	qemu-arm <qemu-arm@nongnu.org>,
	"Paolo Bonzini" <pbonzini@redhat.com>
Subject: Re: arm_cpu_post_init (Was: Re: arm: "max" CPU class hierarchy changes possible?)
Date: Thu, 18 Mar 2021 13:59:08 +0100	[thread overview]
Message-ID: <20210318125908.zwpm47ftlsuen3zo@kamzik.brq.redhat.com> (raw)
In-Reply-To: <fc769a96-a304-7429-5dee-a65b52179b1c@suse.de>

On Thu, Mar 18, 2021 at 01:42:36PM +0100, Claudio Fontana wrote:
> On 3/18/21 1:08 PM, Andrew Jones wrote:
> > On Thu, Mar 18, 2021 at 12:32:30PM +0100, Claudio Fontana wrote:
> >> And why do we have a separate arm_cpu_finalize_features()?
> > 
> > Separate, because it's not just called from arm_cpu_realizefn().
> 
> In particular it is also called by the monitor.c in qmp_query_cpu_model_expansion(),
> 
> which basically creates an object of the cpu subclass,
> and then calls arm_cpu_finalize_[features]() explicitly on the object.
> 
> Is the qdev realize() method not called in this case? Should instead it be triggered, rather than initializing/realizing an incomplete object?

Can you elaborate on what you mean by "triggered"? The QMP query does the
least that it can get away with while still reusing the CPU model's
feature initialization code. Any suggestions for improving that,
preferably in the form of a patch, would be welcome. If it works well for
Arm, then it could probably be applied to other architectures. The Arm QMP
query is modeled off the others.

> 
> > 
> >>
> >> Nothing in the ARM cpu classes initializations ever seems to be "final" to me.
> > 
> > Some CPU features cannot be simply switched on/off at the property
> > parse time. For example, there could be dependencies on multiple
> > properties, the mutual exclusion of properties, or other aspects
> > that can only be known later than property parse time. That stuff
> > goes in arm_cpu_finalize_features().
> 
> Seems like _part_ of that is in arm_cpu_finalize_[features]() (in practice, this ends up being AARCH64-only stuff,
> ie SVE, PAUTH and KVM).
> 
> After calling that, the arm realizefn() also does further setting and unsetting of features, checking previous feature states.
> 
> There is a whole lot following the arm_cpu_finalize_[features]() call,
> there are ~300 lines of features initializations happening _after_ the call to arm_cpu_finalize_[features]().
>

Any feature that needs a finalizer (i.e. it can't be managed at property
parsing time) and wants to be probable by the QMP query will need to have
its finalizing moved to arm_cpu_finalize_features(). Some of those ~300
lines might be better pushed into property set/get handlers, others will
never want to be advertised in the QMP query, but the rest will get moved
when the need is great enough.

Thanks,
drew