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

[Boris Ostrovsky <boris.ostrovsky@xxxxxxx>]

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@xxxxxxx>
> > # 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@xxxxxxx>
> >
> > 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@xxxxxxxxxxxxx
> > http://lists.xen.org/xen-devel


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