From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752965AbcGMKXL (ORCPT ); Wed, 13 Jul 2016 06:23:11 -0400 Received: from mga01.intel.com ([192.55.52.88]:9046 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751188AbcGMKXH (ORCPT ); Wed, 13 Jul 2016 06:23:07 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.28,356,1464678000"; d="scan'208";a="994297849" From: Chen Yu To: Pavel Machek Cc: linux-pm@vger.kernel.org, x86@kernel.org, Thomas Gleixner , "H. Peter Anvin" , Borislav Petkov , Peter Zijlstra , Ingo Molnar , Len Brown , linux-kernel@vger.kernel.org, James Morse , "Rafael J. Wysocki" , varun koyyalagunta Subject: Re: [PATCH] x86 / hibernate: Use hlt_play_dead() when resuming from hibernation References: <1467105403-5085-1-git-send-email-yu.c.chen@intel.com> <12570565.xIMhLmhDgj@vostro.rjw.lan> <20160713095617.GB27870@amd> Message-ID: <57861825.90805@intel.com> Date: Wed, 13 Jul 2016 18:29:57 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 MIME-Version: 1.0 In-Reply-To: <20160713095617.GB27870@amd> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Pavel, On 2016年07月13日 17:56, Pavel Machek wrote: > On Sun 2016-07-10 03:49:25, Rafael J. Wysocki wrote: >> From: Rafael J. Wysocki >> >> On Intel hardware, native_play_dead() uses mwait_play_dead() by >> default and only falls back to the other methods if that fails. >> That also happens during resume from hibernation, when the restore >> (boot) kernel runs disable_nonboot_cpus() to take all of the CPUs >> except for the boot one offline. >> >> However, that is problematic, because the address passed to >> __monitor() in mwait_play_dead() is likely to be written to in the >> last phase of hibernate image restoration and that causes the "dead" >> CPU to start executing instructions again. Unfortunately, the page >> containing the address in that CPU's instruction pointer may not be >> valid any more at that point. >> >> First, that page may have been overwritten with image kernel memory >> contents already, so the instructions the CPU attempts to execute may >> simply be invalid. Second, the page tables previously used by that >> CPU may have been overwritten by image kernel memory contents, so the >> address in its instruction pointer is impossible to resolve then. >> >> A report from Varun Koyyalagunta and investigation carried out by >> Chen Yu show that the latter sometimes happens in practice. >> >> To prevent it from happening, modify native_play_dead() to make >> it use hlt_play_dead() instead of mwait_play_dead() during resume >> from hibernation which avoids the inadvertent "revivals" of "dead" >> CPUs. >> >> A slightly unpleasant consequence of this change is that if the >> system is hibernated with one or more CPUs offline, it will generally >> draw more power after resume than it did before hibernation, because >> the physical state entered by CPUs via hlt_play_dead() is higher-power >> than the mwait_play_dead() one in the majority of cases. It is >> possible to work around this, but it is unclear how much of a problem >> that's going to be in practice, so the workaround will be implemented >> later if it turns out to be necessary. >> >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=106371 >> Reported-by: Varun Koyyalagunta >> Original-by: Chen Yu >> Signed-off-by: Rafael J. Wysocki > I notice that it changes even i386, where it should not be > neccessary. But we probably should switch i386 to support similar to > x86-64 one day (and I have patches) so no problem there. > > But I wonder if simpler solution is to place the mwait semaphore into > known address? (Nosave region comes to mind?) Previously we tried to change the monitor address from task.flag to the zero page, because no one would write data to zero page. But there is still problem because of a possible ping-pong wake up scenario in mwait_play_dead: As Varun Koyyalagunta said(on his x86 platform) one possible implementation of a clflush is a read-invalidate snoop, which is what a store might look like, so cflush might wake up the cpu from mwait. 1. CPU1 waits at zero page 2. CPU2 cflush zero page, wake CPU1 up, then CPU2 waits at zero page 3. CPU1 is woken up, and invoke cflush zero page, thus wake up CPU2 again. then the nonboot CPUs never sleep for long. So it's better to monitor different address for each nonboot CPUs, however since there is only one zero page, at most: PAGE_SIZE/L1_CACHE_LINE CPUs are satisfied, which is usually 64 on a x86_64, apparently it's not enough for servers, maybe more zero pages are required. So we tried to use hlt, which looks simpler. Using Nosave region might also have this problem IMO. thanks, Yu