Re: [Qemu-devel] [for-4.0 PATCH v2 8/9] q35/440fx/arm/spapr: Add QEMU 4.0 machine type

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

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

On Tue, Dec 04, 2018 at 04:27:48PM -0200, Eduardo Habkost wrote:
> On Mon, Dec 03, 2018 at 09:28:36AM -0700, Alex Williamson wrote:
> > Including all machine types that might have a pcie-root-port.
> > 
> > Cc: Peter Maydell <peter.maydell@linaro.org>
> > Cc: Michael S. Tsirkin <mst@redhat.com>
> > Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Richard Henderson <rth@twiddle.net>
> > Cc: Eduardo Habkost <ehabkost@redhat.com>
> > Cc: David Gibson <david@gibson.dropbear.id.au>
> > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> > ---
> [...]
> > +static void virt_3_1_instance_init(Object *obj)
> > +{
> > +    virt_4_0_instance_init(obj);
> > +}
> [...]
> > +/*
> > + * pseries-4.0
> > + */
> > +static void spapr_machine_4_0_instance_options(MachineState *machine)
> > +{
> > +}
> 
> Not something specific about patch, but:
> 
> Why exactly virt and pseries machines need separate instance
> initialization functions?

So looking through, it appears to have been used exactly twice.

1) For pseries-2.7 and earlier we turn off
   spapr->use_hotplug_event_source

That's setting the initial value of a feature flag that can later be
overridden by a property.  I can't actually see any good reason we
want the property, but I guess it's there now.  Regardless it should
be pretty easy to fix - just initialize the state value from one in
the class.

2) For pseries-2.2 and earlier we turn on machine->suppress_vmdesc.

I don't really know what suppress_vmdesc is about.  Again it appears
to be adjustable with a property, which is why it's in MachineState
rather than MachineClass.  It could be fixed as (1), but that's
generic code rather than in my bailiwick.

Note that x86 also appears to set this on the instance for pc-2.2 and
earlier.

> I'd like to eventually make all machine-type-version-specific
> behavior instrospectable, so it needs to be available at
> class_init time.

-- 
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: 833 bytes --]