From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by groups.io with SMTP; Tue, 02 Jul 2019 07:16:40 -0700 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 16028B2DDB; Tue, 2 Jul 2019 14:16:34 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-117-172.ams2.redhat.com [10.36.117.172]) by smtp.corp.redhat.com (Postfix) with ESMTP id 08EE43843; Tue, 2 Jul 2019 14:16:29 +0000 (UTC) Subject: Re: [edk2-devel] [Patch v2 2/2] UefiCpuPkg/PiSmmCpuDxeSmm: Enable MM MP Protocol. To: devel@edk2.groups.io, eric.dong@intel.com Cc: Ray Ni References: <20190702073714.32656-1-eric.dong@intel.com> <20190702073714.32656-3-eric.dong@intel.com> From: "Laszlo Ersek" Message-ID: Date: Tue, 2 Jul 2019 16:16:29 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20190702073714.32656-3-eric.dong@intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Tue, 02 Jul 2019 14:16:34 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Hi Eric, I cannot apply this patch for testing, because it is corrupt. Git complains about it (and it's justified): On 07/02/19 09:37, Dong, Eric wrote: > https://bugzilla.tianocore.org/show_bug.cgi?id=1937 > > v2 changes: > 1. Remove some duplicated global variables. > 2. Enhance token design to support multiple task trig for different APs > at the same time. > > V1 changes: > Add MM Mp Protocol in PiSmmCpuDxeSmm driver. > > Cc: Ray Ni > Cc: Laszlo Ersek > Signed-off-by: Eric Dong > --- > UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 555 ++++++++++++++++++- > UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 11 + > UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 160 +++++- > UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 3 + > UefiCpuPkg/PiSmmCpuDxeSmm/SmmMp.c | 372 +++++++++++++ > UefiCpuPkg/PiSmmCpuDxeSmm/SmmMp.h | 286 ++++++++++ > 6 files changed, 1370 insertions(+), 17 deletions(-) > create mode 100644 UefiCpuPkg/PiSmmCpuDxeSmm/SmmMp.c > create mode 100644 UefiCpuPkg/PiSmmCpuDxeSmm/SmmMp.h > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > index 64fb4d6344..76bcee7133 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > @@ -22,6 +22,7 @@ UINTN mSemaphoreSize; > SPIN_LOCK *mPFLock = NULL; > SMM_CPU_SYNC_MODE mCpuSmmSyncMode; > BOOLEAN mMachineCheckSupported = FALSE; > +SPIN_LOCK **mApTokenLock; > Please note the empty line above. In the patch, the line consists of a single space character (before the line terminator), expressing that the line is preseved from the context. So this is a *valid* patch line. Now compare: > /** > Performs an atomic compare exchange operation to get semaphore. > @@ -146,6 +147,45 @@ ReleaseAllAPs ( > } > } > > +/** > + Wheck whether task has been finished by all APs. > + > + @param BlockMode Whether did it in block mode or non-block mode. > + > + @retval TRUE Task has been finished by all APs. > + @retval FALSE Task not has been finished by all APs. > + > +**/ > +BOOLEAN > +IsTaskFinishInAllAPs ( > + IN BOOLEAN BlockMode > + ) > +{ > + UINTN Index; > + > + for (Index = mMaxNumberOfCpus; Index-- > 0;) { > + // > + // Ignore BSP and APs which not call in SMM. > + // > + if ((Index == gSmmCpuPrivate->SmmCoreEntryContext.CurrentlyExecutingCpu) || (!*(mSmmMpSyncData->CpuData[Index].Present))) { > + continue; > + } > + > + if (BlockMode) { > + AcquireSpinLock(mSmmMpSyncData->CpuData[Index].Busy); > The above line is malformed. In the patch, the line has *zero* characters before the line terminator. That's invalid: the first character should be a space (preserved context), plus sign (new line) or minus sign (line being removed). I don't know how this patch was generated, but it doesn't conform to the expected format. Please both repost the series and push it to a topic branch in your repo. I had no problems applying the first patch in the series; what's more, in patch #2, the changes for the other files apply just fine. Thanks Laszlo