Re: [PATCH 3/12] Provide basic Xen PM infrastructure

[Keir Fraser <keir@xensource.com>]

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?

On 14/2/07 09:13, "Tian, Kevin" <kevin.tian@intel.com> wrote:

> Add basic infrastructure for xen power management. Now
> only S3 (suspend to ram) is supported.
> 
> Signed-off-by Ke Yu <ke.yu@intel.com>
> Signed-off-by Kevin Tian <kevin.tian@intel.com>
> 
> --- 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.

> diff -r 13e258a58044 xen/arch/x86/smp.c
> --- a/xen/arch/x86/smp.c Wed Feb 14 11:13:40 2007 +0800
> +++ b/xen/arch/x86/smp.c Wed Feb 14 14:59:49 2007 +0800
> @@ -276,8 +276,9 @@ int on_selected_cpus(
>  {
>      struct call_data_struct data;
>      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?

 -- Keir