[PATCH v2] x86: add a new SMP bring up way for tboot case

[PATCH v2] x86: add a new SMP bring up way for tboot case

[Qiaowei Ren]

tboot provides a better AP wakeup mechanism based on cpu MWAIT feature for
OS/VMM. With this mechanism, system will boot faster and will NOT require
VT to be enabled. But it requires that OS/VMM must have support it, otherwise
system can never boot up.

Once this mechanism is enabled, tboot will put APs waiting in MWAIT loops
before launching kernel. kernel can check the new flag field in v6 tboot
shared page for the hint. If the bit TB_FLAG_AP_WAKE_SUPPORT in flag field
is set, kernel BSP has to write the monitored memory (tboot->ap_wake_trigger)
to bring APs out of MWAIT loops. The sipi vector should be written in
tboot->ap_wake_addr before waking up APs.

Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com>
Signed-off-by: Xiaoyan Zhang <xiaoyan.zhang@intel.com>
Signed-off-by: Gang Wei <gang.wei@intel.com>
---
 arch/x86/kernel/smpboot.c |   21 +++++++++++----------
 arch/x86/kernel/tboot.c   |   17 +++++++++++++++++
 include/linux/tboot.h     |   17 +++++++++++++++++
 3 files changed, 45 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index c80a33b..884c3e7 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -662,8 +662,7 @@ static int __cpuinit do_boot_cpu(int apicid, int cpu, struct task_struct *idle)
 	/* start_ip had better be page-aligned! */
 	unsigned long start_ip = real_mode_header->trampoline_start;
 
-	unsigned long boot_error = 0;
-	int timeout;
+	int timeout, boot_error = 0;
 
 	/* Just in case we booted with a single CPU. */
 	alternatives_enable_smp();
@@ -710,14 +709,16 @@ static int __cpuinit do_boot_cpu(int apicid, int cpu, struct task_struct *idle)
 		}
 	}
 
-	/*
-	 * Kick the secondary CPU. Use the method in the APIC driver
-	 * if it's defined - or use an INIT boot APIC message otherwise:
-	 */
-	if (apic->wakeup_secondary_cpu)
-		boot_error = apic->wakeup_secondary_cpu(apicid, start_ip);
-	else
-		boot_error = wakeup_secondary_cpu_via_init(apicid, start_ip);
+	if (!tboot_wake_up(apicid, start_ip)) {
+		/*
+		 * Kick the secondary CPU. Use the method in the APIC driver
+		 * if it's defined - or use an INIT boot APIC message otherwise:
+		 */
+		if (apic->wakeup_secondary_cpu)
+			boot_error = apic->wakeup_secondary_cpu(apicid, start_ip);
+		else
+			boot_error = wakeup_secondary_cpu_via_init(apicid, start_ip);
+	}
 
 	if (!boot_error) {
 		/*
diff --git a/arch/x86/kernel/tboot.c b/arch/x86/kernel/tboot.c
index f84fe00..e5e50b8 100644
--- a/arch/x86/kernel/tboot.c
+++ b/arch/x86/kernel/tboot.c
@@ -101,6 +101,8 @@ void __init tboot_probe(void)
 	pr_debug("shutdown_entry: 0x%x\n", tboot->shutdown_entry);
 	pr_debug("tboot_base: 0x%08x\n", tboot->tboot_base);
 	pr_debug("tboot_size: 0x%x\n", tboot->tboot_size);
+	if (tboot->version >= 6)
+		pr_info("flags: 0x%08x\n", tboot->flags);
 }
 
 static pgd_t *tboot_pg_dir;
@@ -453,3 +455,18 @@ int tboot_force_iommu(void)
 
 	return 1;
 }
+
+int tboot_wake_up(int apicid, unsigned long sipi_vec)
+{
+	if (!tboot_enabled())
+		return 0;
+
+	if ((tboot->version < 6) ||
+	    !(tboot->flags & TB_FLAG_AP_WAKE_SUPPORT))
+		return 0;
+
+	tboot->ap_wake_addr = sipi_vec;
+	tboot->ap_wake_trigger = apicid;
+
+	return 1;
+}
diff --git a/include/linux/tboot.h b/include/linux/tboot.h
index c75128b..d65f7e9 100644
--- a/include/linux/tboot.h
+++ b/include/linux/tboot.h
@@ -124,8 +124,23 @@ struct tboot {
 
 	/* number of processors in wait-for-SIPI */
 	u32 num_in_wfs;
+
+	/*
+	 * version 6+ fields:
+	 */
+
+	u32 flags;
+
+	/* phys addr of kernel/VMM SIPI vector */
+	u64 ap_wake_addr;
+
+	/* kernel/VMM writes APIC ID to wake AP */
+	u32 ap_wake_trigger;
 } __packed;
 
+/* kernel/VMM use INIT-SIPI-SIPI if clear, ap_wake_* if set */
+#define TB_FLAG_AP_WAKE_SUPPORT 0X00000001
+
 /*
  * UUID for tboot data struct to facilitate matching
  * defined as {663C8DFF-E8B3-4b82-AABF-19EA4D057A08} by tboot, which is
@@ -146,6 +161,7 @@ extern void tboot_shutdown(u32 shutdown_type);
 extern struct acpi_table_header *tboot_get_dmar_table(
 				      struct acpi_table_header *dmar_tbl);
 extern int tboot_force_iommu(void);
+extern int tboot_wake_up(int apicid, unsigned long sipi_vec);
 
 #else
 
@@ -156,6 +172,7 @@ extern int tboot_force_iommu(void);
 					do { } while (0)
 #define tboot_get_dmar_table(dmar_tbl)	(dmar_tbl)
 #define tboot_force_iommu()		0
+#define tboot_wake_up(apicid, sipi_vec) 0
 
 #endif /* !CONFIG_INTEL_TXT */
 
-- 
1.7.9.5

RE: [PATCH v2] x86: add a new SMP bring up way for tboot case

[Wei, Gang]

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

Any comments on this patch?

Jimmy

Ren, Qiaowei wrote on 2012-10-11:
> tboot provides a better AP wakeup mechanism based on cpu MWAIT feature
> for OS/VMM. With this mechanism, system will boot faster and will NOT
> require VT to be enabled. But it requires that OS/VMM must have support
> it, otherwise system can never boot up.
> 
> Once this mechanism is enabled, tboot will put APs waiting in MWAIT
> loops before launching kernel. kernel can check the new flag field in v6
> tboot shared page for the hint. If the bit TB_FLAG_AP_WAKE_SUPPORT in
> flag field is set, kernel BSP has to write the monitored memory
> (tboot->ap_wake_trigger) to bring APs out of MWAIT loops. The sipi
> vector should be written in tboot->ap_wake_addr before waking up APs.
> 
> Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com>
> Signed-off-by: Xiaoyan Zhang <xiaoyan.zhang@intel.com>
> Signed-off-by: Gang Wei <gang.wei@intel.com>
> ---
>  arch/x86/kernel/smpboot.c |   21 +++++++++++----------
>  arch/x86/kernel/tboot.c   |   17 +++++++++++++++++
>  include/linux/tboot.h     |   17 +++++++++++++++++
>  3 files changed, 45 insertions(+), 10 deletions(-)
> diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
> index c80a33b..884c3e7 100644
> --- a/arch/x86/kernel/smpboot.c
> +++ b/arch/x86/kernel/smpboot.c
> @@ -662,8 +662,7 @@ static int __cpuinit do_boot_cpu(int apicid, int cpu,
> struct task_struct *idle)
>  	/* start_ip had better be page-aligned! */
>  	unsigned long start_ip = real_mode_header->trampoline_start;
> -	unsigned long boot_error = 0;
> -	int timeout;
> +	int timeout, boot_error = 0;
> 
>  	/* Just in case we booted with a single CPU. */
>  	alternatives_enable_smp();
> @@ -710,14 +709,16 @@ static int __cpuinit do_boot_cpu(int apicid, int
cpu,
> struct task_struct *idle)
>  		}
>  	}
> -	/* -	 * Kick the secondary CPU. Use the method in the APIC driver
-	 *
> if it's defined - or use an INIT boot APIC message otherwise: -	 */
-	if
> (apic->wakeup_secondary_cpu) -		boot_error =
> apic->wakeup_secondary_cpu(apicid, start_ip); -	else -
boot_error =
> wakeup_secondary_cpu_via_init(apicid, start_ip); +	if
> (!tboot_wake_up(apicid, start_ip)) { +		/* +		 *
Kick the secondary CPU.
> Use the method in the APIC driver +		 * if it's defined - or use
an INIT
> boot APIC message otherwise: +		 */ +		if
(apic->wakeup_secondary_cpu)
> +			boot_error = apic->wakeup_secondary_cpu(apicid,
start_ip); +		else
> +			boot_error = wakeup_secondary_cpu_via_init(apicid,
start_ip); +	}
> 
>  	if (!boot_error) {
>  		/*
> diff --git a/arch/x86/kernel/tboot.c b/arch/x86/kernel/tboot.c
> index f84fe00..e5e50b8 100644
> --- a/arch/x86/kernel/tboot.c
> +++ b/arch/x86/kernel/tboot.c
> @@ -101,6 +101,8 @@ void __init tboot_probe(void)
>  	pr_debug("shutdown_entry: 0x%x\n", tboot->shutdown_entry);
>  	pr_debug("tboot_base: 0x%08x\n", tboot->tboot_base);
>  	pr_debug("tboot_size: 0x%x\n", tboot->tboot_size);
> +	if (tboot->version >= 6)
> +		pr_info("flags: 0x%08x\n", tboot->flags);
>  }
>  
>  static pgd_t *tboot_pg_dir;
> @@ -453,3 +455,18 @@ int tboot_force_iommu(void)
> 
>  	return 1;
>  }
> +
> +int tboot_wake_up(int apicid, unsigned long sipi_vec)
> +{
> +	if (!tboot_enabled())
> +		return 0;
> +
> +	if ((tboot->version < 6) ||
> +	    !(tboot->flags & TB_FLAG_AP_WAKE_SUPPORT))
> +		return 0;
> +
> +	tboot->ap_wake_addr = sipi_vec;
> +	tboot->ap_wake_trigger = apicid;
> +
> +	return 1;
> +}
> diff --git a/include/linux/tboot.h b/include/linux/tboot.h
> index c75128b..d65f7e9 100644
> --- a/include/linux/tboot.h
> +++ b/include/linux/tboot.h
> @@ -124,8 +124,23 @@ struct tboot {
> 
>  	/* number of processors in wait-for-SIPI */
>  	u32 num_in_wfs;
> +
> +	/*
> +	 * version 6+ fields:
> +	 */
> +
> +	u32 flags;
> +
> +	/* phys addr of kernel/VMM SIPI vector */
> +	u64 ap_wake_addr;
> +
> +	/* kernel/VMM writes APIC ID to wake AP */
> +	u32 ap_wake_trigger;
>  } __packed;
> +/* kernel/VMM use INIT-SIPI-SIPI if clear, ap_wake_* if set */
> +#define TB_FLAG_AP_WAKE_SUPPORT 0X00000001
> +
>  /*
>   * UUID for tboot data struct to facilitate matching
>   * defined as {663C8DFF-E8B3-4b82-AABF-19EA4D057A08} by tboot, which is
> @@ -146,6 +161,7 @@ extern void tboot_shutdown(u32 shutdown_type);
>  extern struct acpi_table_header *tboot_get_dmar_table(
>  				      struct acpi_table_header *dmar_tbl);
>  extern int tboot_force_iommu(void);
> +extern int tboot_wake_up(int apicid, unsigned long sipi_vec);
> 
>  #else
> @@ -156,6 +172,7 @@ extern int tboot_force_iommu(void);
>  					do { } while (0)
>  #define tboot_get_dmar_table(dmar_tbl)	(dmar_tbl)
>  #define tboot_force_iommu()		0
> +#define tboot_wake_up(apicid, sipi_vec) 0
> 
>  #endif /* !CONFIG_INTEL_TXT */

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 8586 bytes --]

RE: [PATCH v2] x86: add a new SMP bring up way for tboot case

[Ren, Qiaowei]

Any comments on this patch?

Thanks,
Qiaowei

-----Original Message-----
From: Ren, Qiaowei 
Sent: Thursday, October 11, 2012 6:11 PM
To: Thomas Gleixner; Ingo Molnar; H. Peter Anvin; x86@kernel.org
Cc: Maliszewski, Richard L; linux-kernel@vger.kernel.org; tboot-devel@lists.sourceforge.net; Ren, Qiaowei; Xiaoyan Zhang; Wei, Gang
Subject: [PATCH v2] x86: add a new SMP bring up way for tboot case

tboot provides a better AP wakeup mechanism based on cpu MWAIT feature for OS/VMM. With this mechanism, system will boot faster and will NOT require VT to be enabled. But it requires that OS/VMM must have support it, otherwise system can never boot up.

Once this mechanism is enabled, tboot will put APs waiting in MWAIT loops before launching kernel. kernel can check the new flag field in v6 tboot shared page for the hint. If the bit TB_FLAG_AP_WAKE_SUPPORT in flag field is set, kernel BSP has to write the monitored memory (tboot->ap_wake_trigger) to bring APs out of MWAIT loops. The sipi vector should be written in
tboot->ap_wake_addr before waking up APs.

Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com>
Signed-off-by: Xiaoyan Zhang <xiaoyan.zhang@intel.com>
Signed-off-by: Gang Wei <gang.wei@intel.com>
---
 arch/x86/kernel/smpboot.c |   21 +++++++++++----------
 arch/x86/kernel/tboot.c   |   17 +++++++++++++++++
 include/linux/tboot.h     |   17 +++++++++++++++++
 3 files changed, 45 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c index c80a33b..884c3e7 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -662,8 +662,7 @@ static int __cpuinit do_boot_cpu(int apicid, int cpu, struct task_struct *idle)
 	/* start_ip had better be page-aligned! */
 	unsigned long start_ip = real_mode_header->trampoline_start;
 
-	unsigned long boot_error = 0;
-	int timeout;
+	int timeout, boot_error = 0;
 
 	/* Just in case we booted with a single CPU. */
 	alternatives_enable_smp();
@@ -710,14 +709,16 @@ static int __cpuinit do_boot_cpu(int apicid, int cpu, struct task_struct *idle)
 		}
 	}
 
-	/*
-	 * Kick the secondary CPU. Use the method in the APIC driver
-	 * if it's defined - or use an INIT boot APIC message otherwise:
-	 */
-	if (apic->wakeup_secondary_cpu)
-		boot_error = apic->wakeup_secondary_cpu(apicid, start_ip);
-	else
-		boot_error = wakeup_secondary_cpu_via_init(apicid, start_ip);
+	if (!tboot_wake_up(apicid, start_ip)) {
+		/*
+		 * Kick the secondary CPU. Use the method in the APIC driver
+		 * if it's defined - or use an INIT boot APIC message otherwise:
+		 */
+		if (apic->wakeup_secondary_cpu)
+			boot_error = apic->wakeup_secondary_cpu(apicid, start_ip);
+		else
+			boot_error = wakeup_secondary_cpu_via_init(apicid, start_ip);
+	}
 
 	if (!boot_error) {
 		/*
diff --git a/arch/x86/kernel/tboot.c b/arch/x86/kernel/tboot.c index f84fe00..e5e50b8 100644
--- a/arch/x86/kernel/tboot.c
+++ b/arch/x86/kernel/tboot.c
@@ -101,6 +101,8 @@ void __init tboot_probe(void)
 	pr_debug("shutdown_entry: 0x%x\n", tboot->shutdown_entry);
 	pr_debug("tboot_base: 0x%08x\n", tboot->tboot_base);
 	pr_debug("tboot_size: 0x%x\n", tboot->tboot_size);
+	if (tboot->version >= 6)
+		pr_info("flags: 0x%08x\n", tboot->flags);
 }
 
 static pgd_t *tboot_pg_dir;
@@ -453,3 +455,18 @@ int tboot_force_iommu(void)
 
 	return 1;
 }
+
+int tboot_wake_up(int apicid, unsigned long sipi_vec) {
+	if (!tboot_enabled())
+		return 0;
+
+	if ((tboot->version < 6) ||
+	    !(tboot->flags & TB_FLAG_AP_WAKE_SUPPORT))
+		return 0;
+
+	tboot->ap_wake_addr = sipi_vec;
+	tboot->ap_wake_trigger = apicid;
+
+	return 1;
+}
diff --git a/include/linux/tboot.h b/include/linux/tboot.h index c75128b..d65f7e9 100644
--- a/include/linux/tboot.h
+++ b/include/linux/tboot.h
@@ -124,8 +124,23 @@ struct tboot {
 
 	/* number of processors in wait-for-SIPI */
 	u32 num_in_wfs;
+
+	/*
+	 * version 6+ fields:
+	 */
+
+	u32 flags;
+
+	/* phys addr of kernel/VMM SIPI vector */
+	u64 ap_wake_addr;
+
+	/* kernel/VMM writes APIC ID to wake AP */
+	u32 ap_wake_trigger;
 } __packed;
 
+/* kernel/VMM use INIT-SIPI-SIPI if clear, ap_wake_* if set */ #define 
+TB_FLAG_AP_WAKE_SUPPORT 0X00000001
+
 /*
  * UUID for tboot data struct to facilitate matching
  * defined as {663C8DFF-E8B3-4b82-AABF-19EA4D057A08} by tboot, which is @@ -146,6 +161,7 @@ extern void tboot_shutdown(u32 shutdown_type);  extern struct acpi_table_header *tboot_get_dmar_table(
 				      struct acpi_table_header *dmar_tbl);  extern int tboot_force_iommu(void);
+extern int tboot_wake_up(int apicid, unsigned long sipi_vec);
 
 #else
 
@@ -156,6 +172,7 @@ extern int tboot_force_iommu(void);
 					do { } while (0)
 #define tboot_get_dmar_table(dmar_tbl)	(dmar_tbl)
 #define tboot_force_iommu()		0
+#define tboot_wake_up(apicid, sipi_vec) 0
 
 #endif /* !CONFIG_INTEL_TXT */
 
--
1.7.9.5

Re: [PATCH v2] x86: add a new SMP bring up way for tboot case

[H. Peter Anvin]

On 03/19/2013 07:14 PM, Ren, Qiaowei wrote:
> Any comments on this patch?
> 
> Thanks,
> Qiaowei

The biggest question is probably if we can use an existing hook of some
sort.

Overriding the apic method is probably not the right way to go, though.
 tglx, do you have any opinions here?

Furthermore, this really is not nice:

> +int tboot_wake_up(int apicid, unsigned long sipi_vec) {
> +	if (!tboot_enabled())
> +		return 0;
> +
> +	if ((tboot->version < 6) ||
> +	    !(tboot->flags & TB_FLAG_AP_WAKE_SUPPORT))
> +		return 0;
> +
> +	tboot->ap_wake_addr = sipi_vec;
> +	tboot->ap_wake_trigger = apicid;
> +
> +	return 1;
> +}

Not only don't you set boot_error for the code above it, but there is
absolutely no indication how that does its job (are those active
operations?  If so they should use writel()), nor does it include any
kind of synchronization.

	-hpa

RE: [PATCH v2] x86: add a new SMP bring up way for tboot case

[Ren, Qiaowei]

> -----Original Message-----
> From: H. Peter Anvin [mailto:hpa@zytor.com]
> Sent: Wednesday, March 20, 2013 11:02 AM
> To: Ren, Qiaowei
> Cc: Thomas Gleixner; Ingo Molnar; x86@kernel.org; Maliszewski, Richard L;
> linux-kernel@vger.kernel.org; tboot-devel@lists.sourceforge.net; Xiaoyan
> Zhang; Wei, Gang
> Subject: Re: [PATCH v2] x86: add a new SMP bring up way for tboot case
> 
> On 03/19/2013 07:14 PM, Ren, Qiaowei wrote:
> > Any comments on this patch?
> >
> > Thanks,
> > Qiaowei
> 
> The biggest question is probably if we can use an existing hook of some sort.
> 
> Overriding the apic method is probably not the right way to go, though.
>  tglx, do you have any opinions here?
> 

If we can't use an existing hook, could you please give me any suggestion about other opinions here?

> Furthermore, this really is not nice:
>
> > +int tboot_wake_up(int apicid, unsigned long sipi_vec) {
> > +	if (!tboot_enabled())
> > +		return 0;
> > +
> > +	if ((tboot->version < 6) ||
> > +	    !(tboot->flags & TB_FLAG_AP_WAKE_SUPPORT))
> > +		return 0;
> > +
> > +	tboot->ap_wake_addr = sipi_vec;
> > +	tboot->ap_wake_trigger = apicid;
> > +
> > +	return 1;
> > +}
> 
> Not only don't you set boot_error for the code above it, but there is absolutely
> no indication how that does its job (are those active operations?  If so they
> should use writel()), nor does it include any kind of synchronization.
> 

Ok. I should add some comments to explain how that does its job. 
But I guess boot_error don't have to be set inside this function.

Thanks,
Qiaowei

Re: [PATCH v2] x86: add a new SMP bring up way for tboot case

[Wei, Gang]

Jan Beulich wrote on 2012-01-10:
>>>> On 09.01.12 at 17:01, "Wei, Gang" <gang.wei@intel.com> wrote:
>> tboot may be trying to put APs waiting in MWAIT loops before launching Xen.
>> Xen could check the new flag field in v6 tboot shared page for the
>> hint. If TB_FLAG_AP_WAKE_SUPPORT bit in flag field is set, Xen BSP
>> have to write the monitored memory(g_tboot_shared->ap_wake_trigger)
>> to bring APs out of MWAIT loops. The sipi vector should be written
>> in g_tboot_shared->ap_wake_addr before waking up APs.
>> 
>> Signed-off-by: Joseph Cihula <joseph.cihula@intel.com>
>> Signed-off-by: Shane Wang <shane.wang@intel.com>
>> Signed-off-by: Gang Wei <gang.wei@intel.com>
>> 
>> diff -r 8087674cceb9 xen/arch/x86/smpboot.c
>> --- a/xen/arch/x86/smpboot.c	Thu Jan 05 22:42:02 2012 +0800
>> +++ b/xen/arch/x86/smpboot.c	Mon Jan 09 23:55:41 2012 +0800
>> @@ -586,7 +586,10 @@
>>      smpboot_setup_warm_reset_vector(start_eip);
>>      
>>      /* Starting actual IPI sequence... */
>> -    boot_error = wakeup_secondary_cpu(apicid, start_eip);
>> +    if ( tboot_in_measured_env() )
>> +        boot_error = tboot_wake_ap(apicid, start_eip);
>> +    else
>> +        boot_error = wakeup_secondary_cpu(apicid, start_eip);
> 
> I'm afraid this is broken now for older tboot - you want to call
> wakeup_secondary_cpu() if tboot_wake_ap() failed. All I had asked in
> the first review round was that you don't call
> tboot_wake_ap() without tboot_in_measured_env() returning true.

Oh, yes. I should not do this in mid-night. V3 patch will fix it.

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

Re: [PATCH v2] x86: add a new SMP bring up way for tboot case

[Jan Beulich]

>>> On 09.01.12 at 17:01, "Wei, Gang" <gang.wei@intel.com> wrote:
> tboot may be trying to put APs waiting in MWAIT loops before launching Xen. 
> Xen could check the new flag field in v6 tboot shared page for the hint. If 
> TB_FLAG_AP_WAKE_SUPPORT bit in flag field is set, Xen BSP have to write the 
> monitored memory(g_tboot_shared->ap_wake_trigger) to bring APs out of MWAIT 
> loops. The sipi vector should be written in g_tboot_shared->ap_wake_addr before 
> waking up APs.
> 
> Signed-off-by: Joseph Cihula <joseph.cihula@intel.com>
> Signed-off-by: Shane Wang <shane.wang@intel.com>
> Signed-off-by: Gang Wei <gang.wei@intel.com>
> 
> diff -r 8087674cceb9 xen/arch/x86/smpboot.c
> --- a/xen/arch/x86/smpboot.c	Thu Jan 05 22:42:02 2012 +0800
> +++ b/xen/arch/x86/smpboot.c	Mon Jan 09 23:55:41 2012 +0800
> @@ -586,7 +586,10 @@
>      smpboot_setup_warm_reset_vector(start_eip);
>  
>      /* Starting actual IPI sequence... */
> -    boot_error = wakeup_secondary_cpu(apicid, start_eip);
> +    if ( tboot_in_measured_env() )
> +        boot_error = tboot_wake_ap(apicid, start_eip);
> +    else
> +        boot_error = wakeup_secondary_cpu(apicid, start_eip);

I'm afraid this is broken now for older tboot - you want to call
wakeup_secondary_cpu() if tboot_wake_ap() failed. All I had
asked in the first review round was that you don't call
tboot_wake_ap() without tboot_in_measured_env() returning
true.

Jan

>      if ( !boot_error )
>      {
> diff -r 8087674cceb9 xen/arch/x86/tboot.c
> --- a/xen/arch/x86/tboot.c	Thu Jan 05 22:42:02 2012 +0800
> +++ b/xen/arch/x86/tboot.c	Mon Jan 09 23:55:41 2012 +0800
> @@ -123,6 +123,8 @@
>      printk("  shutdown_entry: 0x%08x\n", tboot_shared->shutdown_entry);
>      printk("  tboot_base: 0x%08x\n", tboot_shared->tboot_base);
>      printk("  tboot_size: 0x%x\n", tboot_shared->tboot_size);
> +    if ( tboot_shared->version >= 6 )
> +        printk("  flags: 0x%08x\n", tboot_shared->flags);
>  
>      /* these will be needed by tboot_protect_mem_regions() and/or
>         tboot_parse_dmar_table(), so get them now */
> @@ -529,6 +531,18 @@
>      panic("Memory integrity was lost on resume (%d)\n", error);
>  }
>  
> +int tboot_wake_ap(int apicid, unsigned long sipi_vec)
> +{
> +    if ( g_tboot_shared->version >= 6 &&
> +         (g_tboot_shared->flags & TB_FLAG_AP_WAKE_SUPPORT) )
> +    {
> +        g_tboot_shared->ap_wake_addr = sipi_vec;
> +        g_tboot_shared->ap_wake_trigger = apicid;
> +        return 0;
> +    }
> +    return -1;
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff -r 8087674cceb9 xen/include/asm-x86/tboot.h
> --- a/xen/include/asm-x86/tboot.h	Thu Jan 05 22:42:02 2012 +0800
> +++ b/xen/include/asm-x86/tboot.h	Mon Jan 09 23:55:41 2012 +0800
> @@ -85,7 +85,7 @@
>  typedef struct __packed {
>      /* version 3+ fields: */
>      uuid_t    uuid;              /* {663C8DFF-E8B3-4b82-AABF-19EA4D057A08} */
> -    uint32_t  version;           /* Version number; currently supports 0.4 
> */
> +    uint32_t  version;           /* Version number; currently supports 0.6 
> */
>      uint32_t  log_addr;          /* physical addr of tb_log_t log */
>      uint32_t  shutdown_entry;    /* entry point for tboot shutdown */
>      uint32_t  shutdown_type;     /* type of shutdown (TB_SHUTDOWN_*) */
> @@ -99,6 +99,13 @@
>      /* version 4+ fields: */
>                                   /* populated by tboot; will be encrypted 
> */
>      uint8_t   s3_key[TB_KEY_SIZE];
> +    /* version 5+ fields: */
> +    uint8_t   reserved_align[3]; /* used to 4byte-align num_in_wfs */
> +    uint32_t  num_in_wfs;        /* number of processors in wait-for-SIPI */
> +    /* version 6+ fields: */
> +    uint32_t  flags;
> +    uint64_t  ap_wake_addr;      /* phys addr of kernel/VMM SIPI vector */
> +    uint32_t  ap_wake_trigger;   /* kernel/VMM writes APIC ID to wake AP */
>  } tboot_shared_t;
>  
>  #define TB_SHUTDOWN_REBOOT      0
> @@ -107,6 +114,9 @@
>  #define TB_SHUTDOWN_S3          3
>  #define TB_SHUTDOWN_HALT        4
>  
> +#define TB_FLAG_AP_WAKE_SUPPORT   0x00000001  /* kernel/VMM use 
> INIT-SIPI-SIPI
> +                                                 if clear, ap_wake_* if set 
> */
> +
>  /* {663C8DFF-E8B3-4b82-AABF-19EA4D057A08} */
>  #define TBOOT_SHARED_UUID    { 0x663c8dff, 0xe8b3, 0x4b82, 0xaabf, \
>                                 { 0x19, 0xea, 0x4d, 0x5, 0x7a, 0x8 } };
> @@ -120,6 +130,7 @@
>  int tboot_parse_dmar_table(acpi_table_handler dmar_handler);
>  int tboot_s3_resume(void);
>  void tboot_s3_error(int error);
> +int tboot_wake_ap(int apicid, unsigned long sipi_vec);
>  
>  #endif /* __TBOOT_H__ */



------------------------------------------------------------------------------
Ridiculously easy VDI. With Citrix VDI-in-a-Box, you don't need a complex
infrastructure or vast IT resources to deliver seamless, secure access to
virtual desktops. With this all-in-one solution, easily deploy virtual 
desktops for less than the cost of PCs and save 60% on VDI infrastructure 
costs. Try it free! http://p.sf.net/sfu/Citrix-VDIinabox

[PATCH v2] x86: add a new SMP bring up way for tboot case

[Wei, Gang]

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

tboot may be trying to put APs waiting in MWAIT loops before launching Xen. Xen could check the new flag field in v6 tboot shared page for the hint. If TB_FLAG_AP_WAKE_SUPPORT bit in flag field is set, Xen BSP have to write the monitored memory(g_tboot_shared->ap_wake_trigger) to bring APs out of MWAIT loops. The sipi vector should be written in g_tboot_shared->ap_wake_addr before waking up APs.

Signed-off-by: Joseph Cihula <joseph.cihula@intel.com>
Signed-off-by: Shane Wang <shane.wang@intel.com>
Signed-off-by: Gang Wei <gang.wei@intel.com>

diff -r 8087674cceb9 xen/arch/x86/smpboot.c
--- a/xen/arch/x86/smpboot.c	Thu Jan 05 22:42:02 2012 +0800
+++ b/xen/arch/x86/smpboot.c	Mon Jan 09 23:55:41 2012 +0800
@@ -586,7 +586,10 @@
     smpboot_setup_warm_reset_vector(start_eip);
 
     /* Starting actual IPI sequence... */
-    boot_error = wakeup_secondary_cpu(apicid, start_eip);
+    if ( tboot_in_measured_env() )
+        boot_error = tboot_wake_ap(apicid, start_eip);
+    else
+        boot_error = wakeup_secondary_cpu(apicid, start_eip);
 
     if ( !boot_error )
     {
diff -r 8087674cceb9 xen/arch/x86/tboot.c
--- a/xen/arch/x86/tboot.c	Thu Jan 05 22:42:02 2012 +0800
+++ b/xen/arch/x86/tboot.c	Mon Jan 09 23:55:41 2012 +0800
@@ -123,6 +123,8 @@
     printk("  shutdown_entry: 0x%08x\n", tboot_shared->shutdown_entry);
     printk("  tboot_base: 0x%08x\n", tboot_shared->tboot_base);
     printk("  tboot_size: 0x%x\n", tboot_shared->tboot_size);
+    if ( tboot_shared->version >= 6 )
+        printk("  flags: 0x%08x\n", tboot_shared->flags);
 
     /* these will be needed by tboot_protect_mem_regions() and/or
        tboot_parse_dmar_table(), so get them now */
@@ -529,6 +531,18 @@
     panic("Memory integrity was lost on resume (%d)\n", error);
 }
 
+int tboot_wake_ap(int apicid, unsigned long sipi_vec)
+{
+    if ( g_tboot_shared->version >= 6 &&
+         (g_tboot_shared->flags & TB_FLAG_AP_WAKE_SUPPORT) )
+    {
+        g_tboot_shared->ap_wake_addr = sipi_vec;
+        g_tboot_shared->ap_wake_trigger = apicid;
+        return 0;
+    }
+    return -1;
+}
+
 /*
  * Local variables:
  * mode: C
diff -r 8087674cceb9 xen/include/asm-x86/tboot.h
--- a/xen/include/asm-x86/tboot.h	Thu Jan 05 22:42:02 2012 +0800
+++ b/xen/include/asm-x86/tboot.h	Mon Jan 09 23:55:41 2012 +0800
@@ -85,7 +85,7 @@
 typedef struct __packed {
     /* version 3+ fields: */
     uuid_t    uuid;              /* {663C8DFF-E8B3-4b82-AABF-19EA4D057A08} */
-    uint32_t  version;           /* Version number; currently supports 0.4 */
+    uint32_t  version;           /* Version number; currently supports 0.6 */
     uint32_t  log_addr;          /* physical addr of tb_log_t log */
     uint32_t  shutdown_entry;    /* entry point for tboot shutdown */
     uint32_t  shutdown_type;     /* type of shutdown (TB_SHUTDOWN_*) */
@@ -99,6 +99,13 @@
     /* version 4+ fields: */
                                  /* populated by tboot; will be encrypted */
     uint8_t   s3_key[TB_KEY_SIZE];
+    /* version 5+ fields: */
+    uint8_t   reserved_align[3]; /* used to 4byte-align num_in_wfs */
+    uint32_t  num_in_wfs;        /* number of processors in wait-for-SIPI */
+    /* version 6+ fields: */
+    uint32_t  flags;
+    uint64_t  ap_wake_addr;      /* phys addr of kernel/VMM SIPI vector */
+    uint32_t  ap_wake_trigger;   /* kernel/VMM writes APIC ID to wake AP */
 } tboot_shared_t;
 
 #define TB_SHUTDOWN_REBOOT      0
@@ -107,6 +114,9 @@
 #define TB_SHUTDOWN_S3          3
 #define TB_SHUTDOWN_HALT        4
 
+#define TB_FLAG_AP_WAKE_SUPPORT   0x00000001  /* kernel/VMM use INIT-SIPI-SIPI
+                                                 if clear, ap_wake_* if set */
+
 /* {663C8DFF-E8B3-4b82-AABF-19EA4D057A08} */
 #define TBOOT_SHARED_UUID    { 0x663c8dff, 0xe8b3, 0x4b82, 0xaabf, \
                                { 0x19, 0xea, 0x4d, 0x5, 0x7a, 0x8 } };
@@ -120,6 +130,7 @@
 int tboot_parse_dmar_table(acpi_table_handler dmar_handler);
 int tboot_s3_resume(void);
 void tboot_s3_error(int error);
+int tboot_wake_ap(int apicid, unsigned long sipi_vec);
 
 #endif /* __TBOOT_H__ */


[-- Attachment #2: mwait-smp-up-for-tboot-v2.patch --]
[-- Type: application/octet-stream, Size: 4157 bytes --]

x86: add a new SMP bring up way for tboot case

tboot may be trying to put APs waiting in MWAIT loops before launching Xen. Xen could check the new flag field in v6 tboot shared page for the hint. If TB_FLAG_AP_WAKE_SUPPORT bit in flag field is set, Xen BSP have to write the monitored memory(g_tboot_shared->ap_wake_trigger) to bring APs out of MWAIT loops. The sipi vector should be written in g_tboot_shared->ap_wake_addr before waking up APs.

Signed-off-by: Joseph Cihula <joseph.cihula@intel.com>
Signed-off-by: Shane Wang <shane.wang@intel.com>
Signed-off-by: Gang Wei <gang.wei@intel.com>

diff -r 8087674cceb9 xen/arch/x86/smpboot.c
--- a/xen/arch/x86/smpboot.c	Thu Jan 05 22:42:02 2012 +0800
+++ b/xen/arch/x86/smpboot.c	Mon Jan 09 23:55:41 2012 +0800
@@ -586,7 +586,10 @@
     smpboot_setup_warm_reset_vector(start_eip);
 
     /* Starting actual IPI sequence... */
-    boot_error = wakeup_secondary_cpu(apicid, start_eip);
+    if ( tboot_in_measured_env() )
+        boot_error = tboot_wake_ap(apicid, start_eip);
+    else
+        boot_error = wakeup_secondary_cpu(apicid, start_eip);
 
     if ( !boot_error )
     {
diff -r 8087674cceb9 xen/arch/x86/tboot.c
--- a/xen/arch/x86/tboot.c	Thu Jan 05 22:42:02 2012 +0800
+++ b/xen/arch/x86/tboot.c	Mon Jan 09 23:55:41 2012 +0800
@@ -123,6 +123,8 @@
     printk("  shutdown_entry: 0x%08x\n", tboot_shared->shutdown_entry);
     printk("  tboot_base: 0x%08x\n", tboot_shared->tboot_base);
     printk("  tboot_size: 0x%x\n", tboot_shared->tboot_size);
+    if ( tboot_shared->version >= 6 )
+        printk("  flags: 0x%08x\n", tboot_shared->flags);
 
     /* these will be needed by tboot_protect_mem_regions() and/or
        tboot_parse_dmar_table(), so get them now */
@@ -529,6 +531,18 @@
     panic("Memory integrity was lost on resume (%d)\n", error);
 }
 
+int tboot_wake_ap(int apicid, unsigned long sipi_vec)
+{
+    if ( g_tboot_shared->version >= 6 &&
+         (g_tboot_shared->flags & TB_FLAG_AP_WAKE_SUPPORT) )
+    {
+        g_tboot_shared->ap_wake_addr = sipi_vec;
+        g_tboot_shared->ap_wake_trigger = apicid;
+        return 0;
+    }
+    return -1;
+}
+
 /*
  * Local variables:
  * mode: C
diff -r 8087674cceb9 xen/include/asm-x86/tboot.h
--- a/xen/include/asm-x86/tboot.h	Thu Jan 05 22:42:02 2012 +0800
+++ b/xen/include/asm-x86/tboot.h	Mon Jan 09 23:55:41 2012 +0800
@@ -85,7 +85,7 @@
 typedef struct __packed {
     /* version 3+ fields: */
     uuid_t    uuid;              /* {663C8DFF-E8B3-4b82-AABF-19EA4D057A08} */
-    uint32_t  version;           /* Version number; currently supports 0.4 */
+    uint32_t  version;           /* Version number; currently supports 0.6 */
     uint32_t  log_addr;          /* physical addr of tb_log_t log */
     uint32_t  shutdown_entry;    /* entry point for tboot shutdown */
     uint32_t  shutdown_type;     /* type of shutdown (TB_SHUTDOWN_*) */
@@ -99,6 +99,13 @@
     /* version 4+ fields: */
                                  /* populated by tboot; will be encrypted */
     uint8_t   s3_key[TB_KEY_SIZE];
+    /* version 5+ fields: */
+    uint8_t   reserved_align[3]; /* used to 4byte-align num_in_wfs */
+    uint32_t  num_in_wfs;        /* number of processors in wait-for-SIPI */
+    /* version 6+ fields: */
+    uint32_t  flags;
+    uint64_t  ap_wake_addr;      /* phys addr of kernel/VMM SIPI vector */
+    uint32_t  ap_wake_trigger;   /* kernel/VMM writes APIC ID to wake AP */
 } tboot_shared_t;
 
 #define TB_SHUTDOWN_REBOOT      0
@@ -107,6 +114,9 @@
 #define TB_SHUTDOWN_S3          3
 #define TB_SHUTDOWN_HALT        4
 
+#define TB_FLAG_AP_WAKE_SUPPORT   0x00000001  /* kernel/VMM use INIT-SIPI-SIPI
+                                                 if clear, ap_wake_* if set */
+
 /* {663C8DFF-E8B3-4b82-AABF-19EA4D057A08} */
 #define TBOOT_SHARED_UUID    { 0x663c8dff, 0xe8b3, 0x4b82, 0xaabf, \
                                { 0x19, 0xea, 0x4d, 0x5, 0x7a, 0x8 } };
@@ -120,6 +130,7 @@
 int tboot_parse_dmar_table(acpi_table_handler dmar_handler);
 int tboot_s3_resume(void);
 void tboot_s3_error(int error);
+int tboot_wake_ap(int apicid, unsigned long sipi_vec);
 
 #endif /* __TBOOT_H__ */
 

[-- Attachment #3: Type: text/plain, Size: 430 bytes --]

------------------------------------------------------------------------------
Ridiculously easy VDI. With Citrix VDI-in-a-Box, you don't need a complex
infrastructure or vast IT resources to deliver seamless, secure access to
virtual desktops. With this all-in-one solution, easily deploy virtual 
desktops for less than the cost of PCs and save 60% on VDI infrastructure 
costs. Try it free! http://p.sf.net/sfu/Citrix-VDIinabox

[-- Attachment #4: Type: text/plain, Size: 164 bytes --]

_______________________________________________
tboot-devel mailing list
tboot-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tboot-devel