[Xen-devel] RE: [PATCH 3/12] Provide basic Xen PM infrastructure

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

>From: Keir Fraser [mailto:keir@xxxxxxxxxxxxx]
>Sent: 2007年6月5日 17:49
>
>A few comments are included below, but I should also add that I suspect
>the
>sleep/wakeup code is broken by the fact that x86/64 Xen is relocated in
>physical memory. The sleep/wakeup code should be potentially added to
>the
>trampoline at 0x90000?

Thanks for your comments, and yes the relocation may break this logic. 
But it should be easy to fix that, since only wakeup code is affected. So 
do you prefer to checking the basic code first, and then let me to catch 
up some fix to make it re-work, or asking me to resend all patches after 
a local rebase?

BTW, you should look at the latest version at:
http://lists.xensource.com/archives/html/xen-devel/2007-05/msg00536.html


>>
>> --- a/xen/arch/x86/boot/x86_32.S Wed Feb 14 11:13:40 2007 +0800
>> +++ b/xen/arch/x86/boot/x86_32.S Wed Feb 14 11:13:40 2007 +0800
>> @@ -146,6 +146,8 @@ start_paging:
>>          rdmsr
>>          bts     $_EFER_NX,%eax
>>          wrmsr
>> +        mov     $1,%eax
>> +        mov     %eax, nx_enabled-__PAGE_OFFSET
>>  no_execute_disable:
>>          pop     %ebx
>>  #endif
>
>The boot code has changed so this won't apply. In any case you can use
>cpu_has_nx, or create a macro called nx_enabled. There's no need for
>another
>global variable.

Sure.

>>      unsigned int nr_cpus = cpus_weight(selected);
>> -
>> -    ASSERT(local_irq_is_enabled());
>> +    unsigned int self = cpu_isset(smp_processor_id(), selected);
>> +
>> +    ASSERT(!self || local_irq_is_enabled());
>>
>>      if ( nr_cpus == 0 )
>>          return 0;
>
>That's rather bogus isn't it?
>

Yes bogus, and it was removed in latest version I sent out in May as 
link above.:-)

Thanks,
Kevin

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