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

[Boris Ostrovsky <boris.ostrovsky@amd.com>]

As far as I can tell the most relevant change in Linux was this: 
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=ea53069231f9317062910d6e772cca4ce93de8c8
and it sounds that it was made mostly because MWAIT-based idle is more 
efficient on Intel processors. That's not the case on AMD where IO-based 
idle is preferred (and I am not aware of any issues, at least so far).

I can make the patch to be AMD_specific but since for the most parts the 
logic is the same as in acpi_idle_do_entry() won't we have to modify 
that function as well?

-boris


On 02/28/12 23:58, Liu, Jinsong wrote:
> I don't think we should go back to old SYSIO method, the history here is:
>
> Xen originally has SYSIO method when offline cpu, but at c/s 23022 we cancel it as reason below
> ======================
> x86: Fix cpu offline bug: cancel SYSIO method when play dead
>
> Play dead is a fragile and tricky point of cpu offline logic.  For how
> to play cpu dead, linux kernel changed several times: Very old kernel
> support 3 ways to play cpu dead: mwait, SYSIO, and halt, just like
> what cpuidle did when enter C3; Later, it cancel mwait and SYSIO
> support, only use halt to play dead; Latest linux 2.6.38 add mwait
> support when cpu dead.
>
> This patch cancel SYSIO method when cpu dead, keep same with latest
> kernel.
>
> SYSIO is an obsoleted method to enter deep C, with some tricky
> hardware behavior, and seldom supported in new platform.  Xen
> experiment indicate that when cpu dead, SYSIO method would trigger
> unknown issue which would bring strange error.  We now cancel SYSIO
> method when cpu dead, after all, correctness is more important than
> power save, and btw new platform use mwait.
> ======================
>
> Thanks,
> Jinsong
>
> Boris Ostrovsky wrote:
>> # HG changeset patch
>> # User Boris Ostrovsky<boris.ostrovsky@amd.com>
>> # Date 1330466573 -3600
>> # Node ID 9e5991ad9c85b5176ce269001e7957e8805dd93c
>> # Parent  a7bacdc5449a2f7bb9c35b2a1334b463fe9f29a9
>> x86: Use deep C states for off-lined CPUs
>>
>> Currently when a core is taken off-line it is placed in C1 state
>> (unless MONITOR/MWAIT is used). This patch allows a core to go to
>> deeper C states resulting in significantly higher power savings.
>>
>> Signed-off-by: Boris Ostrovsky<boris.ostrovsky@amd.com>
>>
>> diff -r a7bacdc5449a -r 9e5991ad9c85 xen/arch/x86/acpi/cpu_idle.c
>> --- 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());
>> +
>>           /*
>>            * 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);
>> +        }
>>       }
>>
>>   default_halt:
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel
>
>