On 11/03/2011 17:53, "Liu, Jinsong" <jinsong.liu@xxxxxxxxx> wrote:
> Keir Fraser wrote:
>> On 11/03/2011 16:50, "Liu, Jinsong" <jinsong.liu@xxxxxxxxx> wrote:
>>
>>> we did experiment, if did wbinvd at current position (at play_dead),
>>> sometimes it bring strange issue when repeatly cpu offline/online.
>>> so for cpu dead, the near wbinvd to last step, the safer.
>>> wbinvd would better be the last ops before cpu dead, to avoid
>>> potential cache coherency break.
>>
>> Okay, I applied your patches. However in a follow-up patch (c/s
>> 23025) I have removed the WBINVD instructions from the default paths
>> (i.e., the HLT loops) as the CPU still does cache coherency while in
>> HLT/C1 state.
>>
>> Does that look okay to you?
>>
>> -- Keir
>
> It's right that cpu continue snoop bus transaction to keep cache coherency
> when HLT/C1.
>
> However, I think it better not remove wbinvd from HLT loops.
> I'm not quite sure if 'enter Cx' == 'cpu play dead'. After all, resume from Cx
> different from cpu booting: when cpu online, it start from SIPI-SIPI.
> I worried some unknown hardware behavior will bring issue if we treat 'cpu
> play dead' totally same as 'enter Cx'.
> Maybe that's the reason why kernel insist on reserveing wbinvd before HLT when
> play dead.
An APIC INIT doesn't touch the memory hierarchy, cache controls, nor CR0.CD
and CR0.NW. The care over cache invalidation I believe solely comes down to
the deep sleep that the CPU can be placed into via MWAIT, in which the
cache-control logic gets switched off.
-- Keir
> Thanks,
> Jinsong
>
>>
>>> In fact, it can do wbinvd inside loop, but as cpu_offline_3.patch
>>> said,
>>> at Xen 7400 when hyperthreading, the offlined thread may be
>>> spuriously waken up by its brother, and frequently waken inside the
>>> dead loop.
>>> In such case, considering heavy workload of wbinvd, we add a
>>> light-weight clflush instruction inside loop.
>>>
>>> Thanks,
>>> Jinsong
>>>
>>>
>>>>
>>>>> Signed-off-by: Liu, Jinsong <jinsong.liu@xxxxxxxxx>
>>>>>
>>>>> diff -r 2dc3c1cc1bba xen/arch/x86/acpi/cpu_idle.c
>>>>> --- a/xen/arch/x86/acpi/cpu_idle.c Mon Mar 07 05:31:46 2022 +0800
>>>>> +++ b/xen/arch/x86/acpi/cpu_idle.c Thu Mar 10 23:40:51 2022 +0800
>>>>> @@ -562,11 +562,14 @@ static void acpi_dead_idle(void)
>>>>> if ( (cx = &power->states[power->count-1]) == NULL )
>>>>> goto default_halt;
>>>>>
>>>>> + /*
>>>>> + * cache must be flashed as the last ops before cpu going into
>>>>> dead, + * otherwise, cpu may dead with dirty data breaking
>>>>> cache coherency, + * leading to strange errors.
>>>>> + */
>>>>> + wbinvd();
>>>>> for ( ; ; )
>>>>> {
>>>>> - if ( !power->flags.bm_check && cx->type == ACPI_STATE_C3 )
>>>>> - ACPI_FLUSH_CPU_CACHE();
>>>>> -
>>>>> switch ( cx->entry_method )
>>>>> {
>>>>> case ACPI_CSTATE_EM_FFH:
>>>>> @@ -584,6 +587,7 @@ static void acpi_dead_idle(void) }
>>>>>
>>>>> default_halt:
>>>>> + wbinvd();
>>>>> for ( ; ; )
>>>>> halt();
>>>>> }
>>>>> diff -r 2dc3c1cc1bba xen/arch/x86/domain.c
>>>>> --- a/xen/arch/x86/domain.c Mon Mar 07 05:31:46 2022 +0800
>>>>> +++ b/xen/arch/x86/domain.c Thu Mar 10 23:40:51 2022 +0800
>>>>> @@ -93,6 +93,12 @@ static void default_idle(void)
>>>>>
>>>>> static void default_dead_idle(void)
>>>>> {
>>>>> + /*
>>>>> + * cache must be flashed as the last ops before cpu going into
>>>>> dead, + * otherwise, cpu may dead with dirty data breaking
>>>>> cache coherency, + * leading to strange errors.
>>>>> + */
>>>>> + wbinvd();
>>>>> for ( ; ; )
>>>>> halt();
>>>>> }
>>>>> @@ -100,7 +106,6 @@ static void play_dead(void)
>>>>> static void play_dead(void)
>>>>> {
>>>>> local_irq_disable();
>>>>> - wbinvd();
>>>>>
>>>>> /*
>>>>> * NOTE: After cpu_exit_clear, per-cpu variables are no longer
>>>>> accessible,
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home