Re: [Xen-devel] [patch] x86: Add a delay between INIT & SIPIs for AP bring-up in X2APIC case

[Tim Deegan <tim@xxxxxxx>]

At 01:32 +0000 on 22 Dec (1324517520), Wei, Gang wrote:
> Tim Deegan wrote on 2011-12-22:
> > At 12:29 +0000 on 21 Dec (1324470582), Wei, Gang wrote:
> >> Without this delay, Xen could not bring APs up while working with
> >> TXT/tboot, because tboot need some time in APs to handle INIT before
> >> becoming ready for receiving SIPIs. (this delay was removed as part
> >> of c/s 23724 by Tim Deegan)
> > 
> > It was removed because I was seeing the opposite problem -- if there
> > was a delay, the AP did not come up.  Unfortunately I don;'t have
> > sucah a machine available any more, so I can't check whether this breaks 
> > boot there.
> > 
> > Is this something that can be fixed in tboot?  If not, than this patch
> > is OK, provided it gets a code comment explaining _why_ tboot needs the 
> > delay.
> 
> First, Linux kernel always place a delay between INIT & SIPIs.

Linux does a lot of things. :)  That delay is suggested in the manuals
too, but AIUI it's for older hardware that doesn't synchronously deliver
the INIT.  And there are machines where the delay causes the AP not to
come up.  But I suppose on such a machine tboot will already have
brought up the APs (or failed to) so there's no harm in the delay.

> Second, with tboot AP is actually spinning in a mini-guest before
> receiving INIT, upon receiving INIT ipi, AP need time to VMExit,
> update VMCS to tracking SIPIs and VMResume. After that SIPI is able to
> be trapped by APs. This is MUST if giving no change to current AP
> bring-up method(INIT-SIPI-SIPI) for tboot.

Oh I see - and the CPU blocks SIPIs in root mode; how inconvenient. 
Does tboot also need a delay between the two SIPIs then?

> I have provided a resent patch with code comment in a previous reply.

Your comment should say _why_ -- in particular that while tboot is in
root mode handling the INIT the CPU will drop any SIPIs. 

With that change, Ack. 

Tim.

> Jimmy
> 
> > 
> > Cheers,
> > 
> > Tim.
> > 
> >> diff -r d1aefee43af1 xen/arch/x86/smpboot.c
> >> --- a/xen/arch/x86/smpboot.c    Wed Dec 21 18:51:31 2011 +0800
> >> +++ b/xen/arch/x86/smpboot.c    Wed Dec 21 20:26:39 2011 +0800
> >> @@ -42,6 +42,7 @@
> >>  #include <asm/msr.h> #include <asm/mtrr.h> #include <asm/time.h>
> >>  +#include <asm/tboot.h> #include <mach_apic.h> #include
> >>  <mach_wakecpu.h> #include <smpboot_hooks.h>
> >> @@ -463,6 +464,10 @@ static int wakeup_secondary_cpu(int phys
> >>              send_status = apic_read(APIC_ICR) & APIC_ICR_BUSY;
> >>          } while ( send_status && (timeout++ < 1000) );
> >>      }
> >> +    else if ( tboot_in_measured_env() )
> >> +    {
> >> +        udelay(10);
> >> +    }
> >> 
> >>      /*
> >>       * Should we send STARTUP IPIs ?

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel