Re: [Qemu-devel] [Qemu-ppc] [PATCH v3] spapr: Support ibm, dynamic-memory-v2 property

[David Gibson]

On Thu, May 03, 2018 at 02:34:21PM +0200, Greg Kurz wrote:
> On Wed, 18 Apr 2018 13:33:44 +1000
> David Gibson <address@hidden> wrote:
> 
> > On Tue, Apr 17, 2018 at 02:39:09PM +0530, Bharata B Rao wrote:
> > > On Tue, Apr 17, 2018 at 11:14:27AM +1000, David Gibson wrote:  
> > > > >  static void spapr_machine_2_12_class_options(MachineClass *mc)
> > > > > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> > > > > index d60b7c6d7a..5e044c44af 100644
> > > > > --- a/include/hw/ppc/spapr.h
> > > > > +++ b/include/hw/ppc/spapr.h
> > > > > @@ -149,6 +149,7 @@ struct sPAPRMachineState {
> > > > >      sPAPROptionVector *ov5;         /* QEMU-supported option vectors 
> > > > > */
> > > > >      sPAPROptionVector *ov5_cas;     /* negotiated (via CAS) option 
> > > > > vectors */
> > > > >      uint32_t max_compat_pvr;
> > > > > +    bool use_ibm_dynamic_memory_v2;  
> > > > 
> > > > TBH, I'm not really sure we even need to adjust this by machine type.  
> > > 
> > > There are other similar features controlled by ov5 bits that
> > > are also determined by machine type version:
> > > 
> > > Memory hotplug support -- sPAPRMachineClass.dr_lmb_enabled
> > > Dedicated HP event support -- sPAPRMachineState.use_hotplug_event_source  
> > 
> > As for user settability the issue isn't that it's set by ov5, but what
> > the effect of the feature is.  Those other features alter runtime
> > hypervisor behaviour and that behaviour has to remain the same across
> > a migration.  Therefore we have to keep the behaviour consistent for
> > old machine types.
> > 
> > This feature affects only boot time behaviour.  It has a similar
> > effect to what a firmware update might, on real hardware.  Furthermore
> > the way CAS and the device tree work, this is vanishingly unlikely to
> > break existing guests.
> > 
> 
> The logic in spapr_ov5_cas_needed() assumes that pre 2.8 machine types only
> expose OV5_FORM1_AFFINITY and OV5_DRCONF_MEMORY to guests. Adding OV5_DRMEM_V2
> unconditionally breaks this assumption and backward migration to pre 2.8 QEMU
> versions because they don't expect the "spapr_option_vector_ov5_cas" 
> subsection.
> 
> This can cause problems in cloud environments that still have systems with
> older QEMU versions, eg, hosts running ubuntu LTS 16.04.4 (QEMU 2.5) which are
> likely to stay around until admins could transition to some newer OS.

Ah, good point.

> > > Are you saying that presence of ibm,dynamic-memory-v2 probably shouldn't
> > > be dependent on machine type ?  
> > 
> > Yes, I am.
> > 
> 
> I agree but we should also not put it in the migration stream then, like we
> already do for OV5_FORM1_AFFINITY and OV5_DRCONF_MEMORY.
> 
> I've spotted another backward migration breakage wrt old, but still
> in use, QEMU versions. I'll send a series for both issues ASAP, so
> that it has a chance to land in QEMU 2.11.2.

Thanks.  I've merged your patch for 2.13 now.

-- 
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

signature.asc
Description: PGP signature