From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id C073481CC1 for ; Wed, 9 Nov 2016 07:54:23 -0800 (PST) Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 78B168B120; Wed, 9 Nov 2016 15:54:26 +0000 (UTC) Received: from [10.36.112.22] (ovpn-112-22.ams2.redhat.com [10.36.112.22]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id uA9FsM3Q014397 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 9 Nov 2016 10:54:24 -0500 To: "Yao, Jiewen" , Laszlo Ersek References: <1478251854-14660-1-git-send-email-jiewen.yao@intel.com> <08406bf5-4377-63a1-8dd9-34479c015d4b@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C50386C0CB8@shsmsx102.ccr.corp.intel.com> <74D8A39837DF1E4DA445A8C0B3885C50386C10BD@shsmsx102.ccr.corp.intel.com> Cc: "Tian, Feng" , "edk2-devel@ml01.01.org" , "Kinney, Michael D" , "Fan, Jeff" , "Zeng, Star" From: Paolo Bonzini Message-ID: <3be2f1bf-8c0a-e470-a5c0-a6130b159da5@redhat.com> Date: Wed, 9 Nov 2016 16:54:21 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C50386C10BD@shsmsx102.ccr.corp.intel.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Wed, 09 Nov 2016 15:54:26 +0000 (UTC) Subject: Re: [PATCH V2 0/6] Enable SMM page level protection. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 09 Nov 2016 15:54:23 -0000 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit On 09/11/2016 16:01, Yao, Jiewen wrote: > 1) CpuS3.c – EarlyInitializeCpu() > 2) CpuS3.c – SmmRelocateBases() > 3) CpuS3.c – InitializeCpu() > 4) S3Resume.c – SendSmiIpiAllExcludingSelf() > > I believe we can guarantee 1/2/3 is good, because I found we check BSP > check mNumberToFinish. > > 4 is a risk, because there is no AP finish check. If the AP is in below > 1M with CR3 in SMRAM, it will be a trouble. > > Once the AP executes RSM and return to non-SMM, the CR3 is no longer > valid and AP must be crashed immediately. WoW! > > The fix, I believe, is same. > > We should make 1) AP is in above 1M reserved memory, Is this because of the NMI case? > and 2) AP is in protected mode with paging disabled. It is not clear to me what the (4) SIPI done is there for, and why it is triggered in S3Resume.c rather than CpuS3.c. And why does it take so much for APs to complete it? That said, by the time you close and lock SMRAM, you aren't even sure that you have reached the cli;hlt loop in the rendezvous funnel. In practice you will be there, but there is still a theoretical race. InterlockedDecrement (&mNumberToFinish) should be moved from EarlyMPRendezvousProcedure/MPRendezvousProcedure to GoToSleep, and GoToSleep should leave 64-bit mode before doing it. This will fix the S3 bug as well. It's only needed for 64-bit mode, but it is doable for the Ia32 version as well. Perhaps EarlyMPRendezvousProcedure and MPRendezvousProcedure can return &mNumberToFinish; what do you think? Paolo