Jan Beulich wrote on 2012-01-05:
>>>> On 05.01.12 at 15:53, "Wei, Gang" <gang....@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.cih...@intel.com>
>> Signed-off-by: Shane Wang <shane.w...@intel.com>
>> Signed-off-by: Gang Wei <gang....@intel.com>
>>
>> diff -r cbf1ce3afd74 xen/arch/x86/smpboot.c
>> --- a/xen/arch/x86/smpboot.c Sat Dec 31 16:18:55 2011 +0800
>> +++ b/xen/arch/x86/smpboot.c Sat Dec 31 18:50:14 2011 +0800
>> @@ -586,7 +586,9 @@
>> smpboot_setup_warm_reset_vector(start_eip);
>>
>> /* Starting actual IPI sequence... */
>> - boot_error = wakeup_secondary_cpu(apicid, start_eip);
>> + boot_error = tboot_wake_ap(apicid, start_eip);
>
> As tboot.h is being included here anyway, it would seem more clean to
> me to guard the call with tboot_in_measured_env() here rather than in
> the called function.
Ok, to be updated in next version.
>> + if ( boot_error )
>> + boot_error = wakeup_secondary_cpu(apicid, start_eip);
>>
>> if ( !boot_error )
>> {
>> diff -r cbf1ce3afd74 xen/arch/x86/tboot.c
>> --- a/xen/arch/x86/tboot.c Sat Dec 31 16:18:55 2011 +0800
>> +++ b/xen/arch/x86/tboot.c Sat Dec 31 18:50:14 2011 +0800
>> @@ -123,6 +123,10 @@
>> 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 >= 5 )
>> + printk(" num_in_wfs: %u\n", tboot_shared->num_in_wfs);
>
> You're printing this field, but aren't making any other use of it?
This field is just used by Linux kernel. I will remove this printk.
>> + 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
> +533,19
>> @@
>> panic("Memory integrity was lost on resume (%d)\n", error); }
>> +int tboot_wake_ap(int apicid, unsigned long sipi_vec) {
>> + if ( tboot_in_measured_env() && g_tboot_shared->version >= 6 &&
>> + (g_tboot_shared->flags & TB_FLAG_AP_WAKE_SUPPORT) )
>> + {
>> + printk("waking AP %d w/ monitor write\n", apicid);
>
> Please, no once-per-CPU printk()-s, especially if they don't depend on
> per-CPU properties.
Ok, remove it in next version.
>> + g_tboot_shared->ap_wake_addr = sipi_vec;
>> + g_tboot_shared->ap_wake_trigger = apicid;
>> + return 0;
>> + }
>> + return -1;
>> +}
>> +
>> /*
>> * Local variables:
>> * mode: C
Thanks for comments.
Jimmy
------------------------------------------------------------------------------
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
_______________________________________________
tboot-devel mailing list
tboot-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tboot-devel
