Re: [Xen-devel] [PATCH] x86: Use deep C states for off-lined CPUs

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 28.02.12 at 23:08, Boris Ostrovsky <boris.ostrovsky@xxxxxxx> wrote:
> --- a/xen/arch/x86/acpi/cpu_idle.c    Mon Feb 27 17:05:18 2012 +0000
> +++ b/xen/arch/x86/acpi/cpu_idle.c    Tue Feb 28 23:02:53 2012 +0100
> @@ -573,10 +573,10 @@ static void acpi_dead_idle(void)
>      if ( (cx = &power->states[power->count-1]) == NULL )
>          goto default_halt;
>  
> -    mwait_ptr = (void *)&mwait_wakeup(smp_processor_id());
> -
>      if ( cx->entry_method == ACPI_CSTATE_EM_FFH )
>      {
> +        mwait_ptr = (void *)&mwait_wakeup(smp_processor_id());
> +

If you're concerned about the placement of this (the change being
unrelated to what your patch is aiming it anyway), then you should
- explain why
- move the declaration of mwait_ptr also into the if() scope

>          /*
>           * Cache must be flushed as the last operation before sleeping.
>           * Otherwise, CPU may still hold dirty data, breaking cache 
> coherency,
> @@ -601,6 +601,20 @@ static void acpi_dead_idle(void)
>              mb();
>              __mwait(cx->address, 0);
>          }
> +    } 
> +    else if ( cx->entry_method == ACPI_CSTATE_EM_SYSIO )
> +    {
> +        /* Avoid references to shared data after the cache flush */
> +        u32 address = cx->address;
> +        u32 pmtmr_ioport_local = pmtmr_ioport;
> +
> +        wbinvd();    
> +
> +        while ( 1 )
> +        {
> +            inb(address);
> +            inl(pmtmr_ioport_local);
> +        }

You will need to eliminate the reservations of the Intel folks for this
to be accepted, I'm afraid, or make this AMD specific (provided the
issues pointed out by them don't affect AMD systems).

Jan

>      }
>  
>  default_halt:



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel