From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.126; helo=mga18.intel.com; envelope-from=eric.dong@intel.com; receiver=edk2-devel@lists.01.org Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) (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 08FBF21124FF3 for ; Mon, 18 Jun 2018 22:20:45 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga106.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 18 Jun 2018 22:20:45 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,242,1526367600"; d="scan'208";a="64368615" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by fmsmga004.fm.intel.com with ESMTP; 18 Jun 2018 22:20:45 -0700 Received: from fmsmsx154.amr.corp.intel.com (10.18.116.70) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.319.2; Mon, 18 Jun 2018 22:20:43 -0700 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by FMSMSX154.amr.corp.intel.com (10.18.116.70) with Microsoft SMTP Server (TLS) id 14.3.319.2; Mon, 18 Jun 2018 22:20:42 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.223]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.82]) with mapi id 14.03.0319.002; Tue, 19 Jun 2018 13:20:12 +0800 From: "Dong, Eric" To: Laszlo Ersek , Leo Duran , "edk2-devel@lists.01.org" CC: "Justen, Jordan L" , Jeff Fan , "Gao, Liming" Thread-Topic: [edk2] [PATCH] UefiCpuPkg/LocalApicLib: Exclude second SendIpi sequence on AMD processors. Thread-Index: AQHUA1LPa/FZSl4k8UmgXPv7vXfdFaRfbugAgAekSrA= Date: Tue, 19 Jun 2018 05:20:11 +0000 Message-ID: References: <1528920674-24912-1-git-send-email-leo.duran@amd.com> <1528920674-24912-2-git-send-email-leo.duran@amd.com> <5337b182-f3f5-5f30-7901-9e431eccb506@redhat.com> In-Reply-To: <5337b182-f3f5-5f30-7901-9e431eccb506@redhat.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 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: Tue, 19 Jun 2018 05:20:46 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Reviewed-by: Eric Dong and pushed the patch with the= change suggested by Laszlo. Thanks, Eric -----Original Message----- From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Lasz= lo Ersek Sent: Friday, June 15, 2018 12:36 AM To: Leo Duran ; edk2-devel@lists.01.org Cc: Justen, Jordan L ; Jeff Fan ; Gao, Liming Subject: Re: [edk2] [PATCH] UefiCpuPkg/LocalApicLib: Exclude second SendIpi= sequence on AMD processors. On 06/13/18 22:11, Leo Duran wrote: > On AMD processors the second SendIpi in the SendInitSipiSipi and=20 > SendInitSipiSipiAllExcludingSelf routines is not required, and may=20 > cause undesired side-effects during MP initialization. >=20 > This patch leverages the StandardSignatureIsAuthenticAMD check to=20 > exclude the second SendIpi and its associated MicroSecondDelay (200). >=20 > 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=20 > ++++++++---- > 2 files changed, 16 insertions(+), 8 deletions(-) >=20 > diff --git a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c=20 > 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 =3D LOCAL_APIC_DELIVERY_MODE_STARTUP; > IcrLow.Bits.Level =3D 1; > SendIpi (IcrLow.Uint32, ApicId); > - MicroSecondDelay (200); > - SendIpi (IcrLow.Uint32, ApicId); > + if (!StandardSignatureIsAuthenticAMD()) { > + MicroSecondDelay (200); > + SendIpi (IcrLow.Uint32, ApicId); > + } > } > =20 > /** > @@ -588,8 +590,10 @@ SendInitSipiSipiAllExcludingSelf ( > IcrLow.Bits.Level =3D 1; > IcrLow.Bits.DestinationShorthand =3D 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); > + } > } > =20 > /** > diff --git=20 > a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c=20 > 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 =3D LOCAL_APIC_DELIVERY_MODE_STARTUP; > IcrLow.Bits.Level =3D 1; > SendIpi (IcrLow.Uint32, ApicId); > - MicroSecondDelay (200); > - SendIpi (IcrLow.Uint32, ApicId); > + if (!StandardSignatureIsAuthenticAMD()) { > + MicroSecondDelay (200); > + SendIpi (IcrLow.Uint32, ApicId); > + } > } > =20 > /** > @@ -683,8 +685,10 @@ SendInitSipiSipiAllExcludingSelf ( > IcrLow.Bits.Level =3D 1; > IcrLow.Bits.DestinationShorthand =3D 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); > + } > } > =20 > /** >=20 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 a= nd commit the patch just yet. Anyway, I do suggest a small coding style imp= rovement (which we can do ourselves before we push the patch): there should be a space character between "StandardSignatureIsAuthenticAMD"= and "()". Thanks Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel