Re: [Qemu-devel] [PATCH 17/25] spapr: add a sPAPRXive object to the machine

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

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

On Mon, Dec 04, 2017 at 09:32:00AM +0100, Cédric Le Goater wrote:
> On 12/04/2017 02:59 AM, David Gibson wrote:
> > On Fri, Dec 01, 2017 at 09:10:24AM +0100, Cédric Le Goater wrote:
> >> On 12/01/2017 05:14 AM, David Gibson wrote:
> >>> On Thu, Nov 30, 2017 at 03:15:09PM +0000, Cédric Le Goater wrote:
> >>>> On 11/30/2017 05:55 AM, David Gibson wrote:
> >>>>> On Thu, Nov 23, 2017 at 02:29:47PM +0100, Cédric Le Goater wrote:
> >>>>>> The XIVE object is designed to be always available, so it is created
> >>>>>> unconditionally on newer machines.
> >>>>>
> >>>>> There doesn't actually seem to be anything dependent on machine
> >>>>> version here.
> >>>>
> >>>> No. I thought that was too early in the patchset. This is handled 
> >>>> in the last patch with a 'xive_exploitation' bool which is set to 
> >>>> false on older machines. 
> >>>>
> >>>> But, nevertheless, the XIVE objects are always created even if not
> >>>> used. Something to discuss.
> >>>
> >>> That'll definitely break backwards migration, since the destination
> >>> won't understand the (unused but still present) xive state it
> >>> receives. 
> >>
> >> no because it's not sent. the vmstate 'needed' op of the sPAPRXive
> >> object discards it :
> >>
> >>     static bool vmstate_spapr_xive_needed(void *opaque)
> >>     {
> >>         sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> >>  
> >>         return spapr->xive_exploitation;
> >>     }
> > 
> > Ah, sorry, missed that.  Once we have negotiation we'll need to make
> > sure the xive_exploitation bit is sent first, of course, but I'm
> > pretty sure the machine state is already sent first.
> > 
> >>> So xives can only be created on new machine types. 
> >>
> >> That would be better I agree. I can probably use the 'xive_exploitation'
> >> bool to condition its creation.
> > 
> > Hrm.  I'm less sure about that - I'm not sure the lifetimes line up.
> > But I'd like to avoid creating them on earlier machine types, even if
> > xive_exploitation can't do the trick.
> 
> Yes. I agree. I think we can work something out without introducing
> too much complexity. The XIVE object is directly used by the
> machine only to set/unset IRQ numbers. Otherwise, it is always 
> conditioned by :
> 
>     spapr_ovec_test(spapr->ov5_cas, OV5_XIVE_EXPLOIT)
> 
> I think adding a couple of more tests on the 'xive_exploitation'
> bool should work out for older machines.

Ok.  If not you can always add a "xive_possible" flag to the MachineClass.

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