From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=66.187.233.73; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) (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 3633621107463 for ; Thu, 14 Jun 2018 09:36:27 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 0363A4068042; Thu, 14 Jun 2018 16:36:27 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-139.rdu2.redhat.com [10.10.120.139]) by smtp.corp.redhat.com (Postfix) with ESMTP id D0DF9201E8ED; Thu, 14 Jun 2018 16:36:25 +0000 (UTC) To: Leo Duran , edk2-devel@lists.01.org Cc: Jordan Justen , Jeff Fan , Liming Gao References: <1528920674-24912-1-git-send-email-leo.duran@amd.com> <1528920674-24912-2-git-send-email-leo.duran@amd.com> From: Laszlo Ersek Message-ID: <5337b182-f3f5-5f30-7901-9e431eccb506@redhat.com> Date: Thu, 14 Jun 2018 18:36:24 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: <1528920674-24912-2-git-send-email-leo.duran@amd.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.6 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.5]); Thu, 14 Jun 2018 16:36:27 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.5]); Thu, 14 Jun 2018 16:36:27 +0000 (UTC) for IP:'10.11.54.6' DOMAIN:'int-mx06.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'lersek@redhat.com' RCPT:'' Subject: Re: [PATCH] UefiCpuPkg/LocalApicLib: Exclude second SendIpi sequence on AMD processors. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 14 Jun 2018 16:36:28 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 06/13/18 22:11, Leo Duran wrote: > On AMD processors the second SendIpi in the SendInitSipiSipi and > SendInitSipiSipiAllExcludingSelf routines is not required, and may cause > undesired side-effects during MP initialization. > > This patch leverages the StandardSignatureIsAuthenticAMD check to exclude > the second SendIpi and its associated MicroSecondDelay (200). > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Leo Duran > Cc: Jordan Justen > Cc: Jeff Fan > Cc: Liming Gao > --- > UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c | 12 ++++++++---- > UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c | 12 ++++++++---- > 2 files changed, 16 insertions(+), 8 deletions(-) > > diff --git a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c > index b0b7e32..6e80536 100644 > --- a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c > +++ b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c > @@ -554,8 +554,10 @@ SendInitSipiSipi ( > IcrLow.Bits.DeliveryMode = LOCAL_APIC_DELIVERY_MODE_STARTUP; > IcrLow.Bits.Level = 1; > SendIpi (IcrLow.Uint32, ApicId); > - MicroSecondDelay (200); > - SendIpi (IcrLow.Uint32, ApicId); > + if (!StandardSignatureIsAuthenticAMD()) { > + MicroSecondDelay (200); > + SendIpi (IcrLow.Uint32, ApicId); > + } > } > > /** > @@ -588,8 +590,10 @@ SendInitSipiSipiAllExcludingSelf ( > IcrLow.Bits.Level = 1; > IcrLow.Bits.DestinationShorthand = LOCAL_APIC_DESTINATION_SHORTHAND_ALL_EXCLUDING_SELF; > SendIpi (IcrLow.Uint32, 0); > - MicroSecondDelay (200); > - SendIpi (IcrLow.Uint32, 0); > + if (!StandardSignatureIsAuthenticAMD()) { > + MicroSecondDelay (200); > + SendIpi (IcrLow.Uint32, 0); > + } > } > > /** > diff --git a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c > index 1f4dcf7..5d82836 100644 > --- a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c > +++ b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c > @@ -649,8 +649,10 @@ SendInitSipiSipi ( > IcrLow.Bits.DeliveryMode = LOCAL_APIC_DELIVERY_MODE_STARTUP; > IcrLow.Bits.Level = 1; > SendIpi (IcrLow.Uint32, ApicId); > - MicroSecondDelay (200); > - SendIpi (IcrLow.Uint32, ApicId); > + if (!StandardSignatureIsAuthenticAMD()) { > + MicroSecondDelay (200); > + SendIpi (IcrLow.Uint32, ApicId); > + } > } > > /** > @@ -683,8 +685,10 @@ SendInitSipiSipiAllExcludingSelf ( > IcrLow.Bits.Level = 1; > IcrLow.Bits.DestinationShorthand = LOCAL_APIC_DESTINATION_SHORTHAND_ALL_EXCLUDING_SELF; > SendIpi (IcrLow.Uint32, 0); > - MicroSecondDelay (200); > - SendIpi (IcrLow.Uint32, 0); > + if (!StandardSignatureIsAuthenticAMD()) { > + MicroSecondDelay (200); > + SendIpi (IcrLow.Uint32, 0); > + } > } > > /** > Given all the feedback (thanks all for that!), I'm fine with the patch. Reviewed-by: Laszlo Ersek I'm only a reviewer for UefiCpuPkg, not a maintainer, so I can't go ahead and commit the patch just yet. Anyway, I do suggest a small coding style improvement (which we can do ourselves before we push the patch): there should be a space character between "StandardSignatureIsAuthenticAMD" and "()". Thanks Laszlo