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-de...@lists.xensource.com > > Cc: tboot-devel@lists.sourceforge.net; 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....@intel.com> 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 ------------------------------------------------------------------------------ Write once. Port to many. Get the SDK and tools to simplify cross-platform app development. Create new or port existing apps to sell to consumers worldwide. Explore the Intel AppUpSM program developer opportunity. appdeveloper.intel.com/join http://p.sf.net/sfu/intel-appdev _______________________________________________ tboot-devel mailing list tboot-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tboot-devel