Re: [PATCH] x86: extend runstate area updates

[Ian Campbell <Ian.Campbell@citrix.com>]

On Sat, 2009-11-21 at 00:48 +0000, Jeremy Fitzhardinge wrote: 
> On 11/21/09 02:57, Ian Campbell wrote:
> > On Fri, 2009-11-20 at 18:09 +0000, Ian Campbell wrote:
> >   
> >>
> >> I can't see where the guest runstate pointer is supposed to be either
> >> restored or re-setup on resume. I tried adding a setup_runstate_info
> >> to xen_timer_resume (to match the call in xen_timer_setup) but that
> >> seems like it is already too late -- I still see the warnings trigger.
> >> I'm not sure how this is possible since I thought we were in a
> >> stop_machine section at this point.
> >>     
> > The xen_sched_clock calls are as a result of the various printks, e.g.
> > in xen_vcpu_setup, in order to add the timestamp to the output.
> > Therefore we need to ensure we reset the runstate info before any
> > printks.
> >   
> 
> Thanks for investigating this.  Does this mean there was a
> longer-standing bug where the runstate was just completely invalid after
> a save/restore?

I think so, yes.

> > diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> > index f09e8c3..219fb3f 100644
> > --- a/arch/x86/xen/enlighten.c
> > +++ b/arch/x86/xen/enlighten.c
> > @@ -145,6 +158,8 @@ void xen_vcpu_restore(void)
> >  			    HYPERVISOR_vcpu_op(VCPUOP_down, cpu, NULL))
> >  				BUG();
> >  
> > +			xen_setup_runstate_info(cpu);
> > +
> >  			xen_vcpu_setup(cpu);
> >   
> 
> This is only run when have_vcpu_info_placement is set.  I checked in a
> followup patch to rearrange the loop so that it runs unconditionally,
> but then only does xen_vcpu_setup() when have_vcpu_info_placement.

Make sense, thanks.

I think this is stable material back as far as they are still
maintaining them. The patch which exposed the issue was backported to
3.4.2.

Ian.