Re: [Qemu-devel] [PATCH 2/2] vl: Print CPU help after we've registered the CPU accelerators

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 4017 bytes --]

On Wed, Mar 08, 2017 at 09:08:50AM -0300, Eduardo Habkost wrote:
> On Wed, Mar 08, 2017 at 01:33:45PM +1100, David Gibson wrote:
> > On Tue, Mar 07, 2017 at 09:02:37AM -0300, Eduardo Habkost wrote:
> > > On Tue, Mar 07, 2017 at 02:31:05PM +1100, David Gibson wrote:
> > > > On Mon, Mar 06, 2017 at 08:47:52AM -0300, Eduardo Habkost wrote:
> > > > > On Mon, Mar 06, 2017 at 10:06:27AM +1100, David Gibson wrote:
> > > > > > On Fri, Mar 03, 2017 at 11:58:07AM -0300, Eduardo Habkost wrote:
> > > > > > > On Tue, Jan 31, 2017 at 02:11:59PM +0100, Thomas Huth wrote:
> > > > > > > > When running with KVM on POWER, we register some CPU types during
> > > > > > > > the initialization function of the ppc64 KVM code (which unfortunately
> > > > > > > > also can not be done via a type_init() like it is done on x86).
> > > > > > > 
> > > > > > > Can you elaborate why it can't be done via type_init()? If the
> > > > > > > QOM type hierarchy depends on any runtime data unavailable at
> > > > > > > type_init(), we should fix that.
> > > > > > 
> > > > > > Hmm.. how?  This is specifically for the special 'host' cpu in the
> > > > > > case of KVM.  We can't use a static configuration here, because there
> > > > > > are things on the host that could limit what features of the CPU are
> > > > > > available for guest use.  So, we need KVM to be initialized in order
> > > > > > to query that information.
> > > > > 
> > > > > There's nothing saying you need to query that information before
> > > > > type_register() or during class_init, does it? The behavior of
> > > > > TYPE_HOST_POWERPC_CPU after object_new() is called can be as
> > > > > dynamic as you want it to, but the QOM type hierarchy is supposed
> > > > > to be static.
> > > > 
> > > > So, the thing is we have some properties that logically belong in the
> > > > CPU class, rather than instance, and that's correct for all TCG CPUs,
> > > > but not for the KVM host CPU.  It seems nasty to have to force all
> > > > those things into the instance just because of KVM.
> > > 
> > > You can still register any class properties you want, without
> > > querying KVM first. Are there properties that you are able to
> > > register only after querying KVM? Is the set of class properties
> > > on TYPE_HOST_POWERPC_CPU different depending on the host
> > > capabilities?
> > 
> > Sorry, I used "properties" sloppily, not meaning QOM properties.
> > 
> > There are several fields in the CPU class which describe CPU
> > capabilities - bitmasks indicating which instructions are available,
> > and L1 cacheline sizes.  There are some other things that are in the
> > CPU instance at the moment, but arguably would belong better in the
> > class: available page sizes and PTE encodings, for example.
> > 
> > At the real hardware level these things are dependent only the model
> > of CPU - hence belonging in cpu class, not instance.  But, because of
> > the way virtualization works on POWER, some of the features may not be
> > available to guests, due to configuration of the hypervisor.  So for
> > the "host" cpu we need to query KVM to see which CPU features are
> > actually available.
> > 
> 
> I see. If there is data that is available only at PowerPCCPUClass
> and you don't want to duplicate it at PowerPCCPU, we can have a
> solution for that: instead of late-registration of the class, we
> could leave those fields to be populated after KVM is
> initialized.

Ok, that sounds workable.

> Anyway, I don't think this is urgent: the code already treats
> "host" as an exception in ppc_cpu_list(), and (AFAICS) the
> original problem this patch addresses is related to the
> inaccurate alias information only (which is not a consequence of
> the late type_register() call).

I agree.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]