Re: [RFC PATCH] C2 could be mapped to C3 so need a flush cache

From: Luming Yu
Date: Tue Sep 08 2009 - 04:28:58 EST


On Tue, Sep 8, 2009 at 3:52 PM, Arjan van de Ven<arjan@xxxxxxxxxxxxx> wrote:
> On Tue, 8 Sep 2009 13:09:44 +0800
> Luming Yu <luming.yu@xxxxxxxxx> wrote:
>
>> On Tue, Sep 8, 2009 at 11:49 AM, Arjan van de
>> Ven<arjan@xxxxxxxxxxxxx> wrote:
>> > On Tue, 8 Sep 2009 10:26:06 +0800
>> > Luming Yu <luming.yu@xxxxxxxxx> wrote:
>> >
>> >> Hi there,
>> >>
>> >> I came across acpi_idle_enter_simple, noticed it looks like a bug
>> >> if we don't flush cache for C2.
>> >> Because some platforms just map C2 to C3.
>> >
>> > I think you are confusing ACPI C3 with HW C3.
>> >
>> > Only for ACPI C3 class do you need to flush the cache for this case.
>> > For HW C3, if you would need to flush the cache, the BIOS would
>> > assign it ACPI C3 class.
>> >
>>
>> There is no confusion,I just extend the existing kernel logic as below
>> to cover cache flush..
>>
>> "
>> "/*
>> Â* Some BIOS implementations switch to C3 in the published C2 state.
>> Â* This seems to be a common problem on AMD boxen, but other vendors
>> Â* are affected too. We pick the most conservative approach: we assume
>> Â* that the local APIC stops in both C2 and C3.
>> Â*/
>> static void lapic_timer_check_state
>>
>> "
>
> unlike lapic behavior, the cache flush behavior is very explicit in the
> ACPI spec. In fact, it is the DEFINITION of ACPI C3. Local apic
> behavior otoh isn't anywhere near the acpi spec in this regard.
> (yes I know of the value of specs, but this one is rather clear).

The patch is not going to break Any SPECs. It is just trying to save
those box that doing ACPI C3 under the name of ACPI C2.
However, for correctly implemented system, the redundant flush
is a problem, although is at minimal level. if the redundant flush were
at entry point to C3 or C1, I would reject this kind patch by my self.

>
> Do you have an example of a specific machine where this is fscked up?
> (if so, it's blacklist worthy.. probably worth blacklisting just
> outright doing C states on it)

Those boxes that have c2 lapic timer stop issue is supposed to need this patch.
But I don't have such kind of system. So test and comments are needed
by those who actually have such kind of box.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/