From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-x244.google.com (mail-wm0-x244.google.com [IPv6:2a00:1450:400c:c09::244]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id D954981E49 for ; Fri, 11 Nov 2016 02:16:20 -0800 (PST) Received: by mail-wm0-x244.google.com with SMTP id a20so8376634wme.2 for ; Fri, 11 Nov 2016 02:16:24 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:subject:to:references:cc:from:message-id:date:user-agent :mime-version:in-reply-to:content-transfer-encoding; bh=2WBwAU6EeZvHmepMhvBQuYC2rG+kLvy1Qb8iydKCLUI=; b=kHoYv/ltIMcGKaeBL/11l86u1F4DsmJbbPHhidw6X+BRNmLv6czm31Thpzb3aVfPdc GuYOS4SS1KoStu3G0sgr1gGYB6Fx/r1zTzYktGnADJ27Y2y7flMI90PXxM2M9ZRcCXSW O/hWNYRkQS55aAVfYSCsztEHmN59XK+4J8XEzlk27cvocNhsMJWv9JX+mZ0FX5KXIaj0 Vdn2SO6IEcpTdk3LVfkDjoOSXAfFxgnykv0KoyAk5OqWWhegIss4FCGHJvuhnCLYUXJD TT1nGYJ0XU6WJQa/GSbGkoB3CvGboPB35tZJFJuSt8hAnKSM2zRzTqakkVFRqN6l1dCR Yv5A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:subject:to:references:cc:from:message-id :date:user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=2WBwAU6EeZvHmepMhvBQuYC2rG+kLvy1Qb8iydKCLUI=; b=Tt4hSQBf/YUDWnxYXCu7ZTc2rrdkk0/u0Y3QRScmN5C5yKBsx4SnKQJIy66e4MjcMl 0N8BKDEyz4045kSTThYzGcUtecj8X6RlXmvUaE5lRFl3Z2sEbtXb8M0vsxbImzuSiy1d yrNP/14JbXD/8/NhmBgt1mpKyW5yZKmtvelxFexmeTlZgwJDkQENwHGHhZfO266fiT9J V1bjG9ep7vyU8WR+Mr0k0NWvndsYGD5aua3HSrtdX8bn58nLoIMtjc8RCfIQ1BuYjTNC 8TM1CgYcPIj8XA7k5JgGTisquMvLXvPgKkOWFBVNs7yHkHkP4UE4o5Hq7M/4k8Qrh3GO u+ug== X-Gm-Message-State: ABUngvfJVXPo/kjT2rHHMPZeTJL6YpQhcuFS8WqOpWta52BnKsyrQ0jt/6YcWJ+Q21U4Bg== X-Received: by 10.28.8.202 with SMTP id 193mr10212799wmi.101.1478859383232; Fri, 11 Nov 2016 02:16:23 -0800 (PST) Received: from [192.168.10.150] (94-39-185-129.adsl-ull.clienti.tiscali.it. [94.39.185.129]) by smtp.googlemail.com with ESMTPSA id g184sm18093247wme.23.2016.11.11.02.16.22 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 11 Nov 2016 02:16:22 -0800 (PST) Sender: Paolo Bonzini To: Jeff Fan , edk2-devel@lists.01.org References: <20161111054545.19616-1-jeff.fan@intel.com> <20161111054545.19616-4-jeff.fan@intel.com> Cc: Michael D Kinney , Laszlo Ersek , Jiewen Yao From: Paolo Bonzini Message-ID: Date: Fri, 11 Nov 2016 11:16: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: <20161111054545.19616-4-jeff.fan@intel.com> Subject: Re: [PATCH v2 3/3] UefiCpuPkg/PiSmmCpuDxeSmm: Decrease mNumberToFinish in AP safe code 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: Fri, 11 Nov 2016 10:16:21 -0000 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit On 11/11/2016 06:45, Jeff Fan wrote: > We will put APs into hlt-loop in safe code. But we decrease mNumberToFinish > before APs enter into the safe code. Paolo pointed out this gap. > > This patch is to move mNumberToFinish decreasing to the safe code. It could > make sure BSP could wait for all APs are running in safe code. > > https://bugzilla.tianocore.org/show_bug.cgi?id=216 > > Reported-by: Paolo Bonzini > Cc: Laszlo Ersek > Cc: Paolo Bonzini > Cc: Jiewen Yao > Cc: Michael D Kinney > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Jeff Fan > --- > UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 17 +++++++---------- > UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c | 6 ++++-- > UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 4 +++- > UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c | 6 ++++-- > 4 files changed, 18 insertions(+), 15 deletions(-) > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c > index e53e096..34547e0 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c > @@ -79,9 +79,11 @@ BOOLEAN mAcpiS3Enable = TRUE; > > UINT8 *mApHltLoopCode = NULL; > UINT8 mApHltLoopCodeTemplate[] = { > - 0xFA, // cli > - 0xF4, // hlt > - 0xEB, 0xFC // jmp $-2 > + 0x8B, 0x44, 0x24, 0x04, // mov eax, dword ptr [esp+4] > + 0xF0, 0xFF, 0x08, // lock dec dword ptr [eax] > + 0xFA, // cli > + 0xF4, // hlt > + 0xEB, 0xFC // jmp $-2 > }; > > /** > @@ -399,17 +401,12 @@ MPRendezvousProcedure ( > } > > // > - // Count down the number with lock mechanism. > - // > - InterlockedDecrement (&mNumberToFinish); > - > - // > - // Place AP into the safe code > + // Place AP into the safe code, count down the number with lock mechanism in the safe code. > // > TopOfStack = (UINT32) (UINTN) Stack + sizeof (Stack); > TopOfStack &= ~(UINT32) (CPU_STACK_ALIGNMENT - 1); > CopyMem ((VOID *) (UINTN) mApHltLoopCode, mApHltLoopCodeTemplate, sizeof (mApHltLoopCodeTemplate)); > - TransferApToSafeState ((UINT32) (UINTN) mApHltLoopCode, TopOfStack); > + TransferApToSafeState ((UINT32) (UINTN) mApHltLoopCode, TopOfStack, &mNumberToFinish); > } > > /** > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c > index 8b880d6..9760373 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c > @@ -100,17 +100,19 @@ InitGdt ( > > @param[in] ApHltLoopCode The 32-bit address of the safe hlt-loop function. > @param[in] TopOfStack A pointer to the new stack to use for the ApHltLoopCode. > + @param[in] NumberToFinish Semaphore of APs finish count. > > **/ > VOID > TransferApToSafeState ( > IN UINT32 ApHltLoopCode, > - IN UINT32 TopOfStack > + IN UINT32 TopOfStack, > + IN UINT32 *NumberToFinish > ) > { > SwitchStack ( > (SWITCH_STACK_ENTRY_POINT) (UINTN) ApHltLoopCode, > - NULL, > + NumberToFinish, > NULL, > (VOID *) (UINTN) TopOfStack > ); > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h > index 6c98659..88d9c85 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h > @@ -830,12 +830,14 @@ GetAcpiS3EnableFlag ( > > @param[in] ApHltLoopCode The 32-bit address of the safe hlt-loop function. > @param[in] TopOfStack A pointer to the new stack to use for the ApHltLoopCode. > + @param[in] NumberToFinish Semaphore of APs finish count. > > **/ > VOID > TransferApToSafeState ( > IN UINT32 ApHltLoopCode, > - IN UINT32 TopOfStack > + IN UINT32 TopOfStack, > + IN UINT32 *NumberToFinish > ); > > #endif > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c > index 62338b7..6844c3f 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c > @@ -105,18 +105,20 @@ GetProtectedModeCS ( > > @param[in] ApHltLoopCode The 32-bit address of the safe hlt-loop function. > @param[in] TopOfStack A pointer to the new stack to use for the ApHltLoopCode. > + @param[in] NumberToFinish Semaphore of APs finish count. > > **/ > VOID > TransferApToSafeState ( > IN UINT32 ApHltLoopCode, > - IN UINT32 TopOfStack > + IN UINT32 TopOfStack, > + IN UINT32 *NumberToFinish > ) > { > AsmDisablePaging64 ( > GetProtectedModeCS (), > (UINT32) (UINTN) ApHltLoopCode, > - 0, > + (UINT32) (UINTN) NumberToFinish, > 0, > TopOfStack > ); > Reviewed-by: Paolo Bonzini