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

[Tim Deegan <tim@xxxxxxx>]

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. 

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 ?
> 
> Jimmy
> 
> 
> > -----Original Message-----
> > From: Wei, Gang
> > Sent: Wednesday, December 21, 2011 8:18 PM
> > To: Keir Fraser; xen-devel@xxxxxxxxxxxxxxxxxxx
> > Cc: tboot-devel@xxxxxxxxxxxxxxxxxxxxx; Jan Beulich; Tim Deegan; Cihula,
> > Joseph; Wei, Gang
> > Subject: RE: [patch] x86: Add a delay between INIT & SIPIs for AP bring-up 
> > in
> > X2APIC case
> > 
> > Keir Fraser wrote on 2011-12-21:
> > > On 21/12/2011 11:22, "Wei, Gang" <gang.wei@xxxxxxxxx> 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)
> > >
> > > Of course Tim will need to review this himself, but a mdelay() right
> > > here, only on the x2apic path just looks bizarre and fragile.
> > >
> > > Could we make the !x2apic_enabled conditionals that Tim added be
> > > !(x2apic_enabled || tboot_in_measured_env()) instead? At least that is
> > > somewhat self-documenting and clearly only affects tboot!
> > 
> > Does below patch make more sense?
> > 
> > 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 19:08:57 2011 +0800
> > @@ -463,6 +463,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 ?
> > 
> > Jimmy

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