From nobody Fri May 3 13:49:15 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of groups.io designates 66.175.222.12 as permitted sender) client-ip=66.175.222.12; envelope-from=bounce+27952+52440+1787277+3901457@groups.io; helo=web01.groups.io; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.12 as permitted sender) smtp.mailfrom=bounce+27952+52440+1787277+3901457@groups.io; dmarc=fail(p=none dis=none) header.from=intel.com ARC-Seal: i=1; a=rsa-sha256; t=1576820093; cv=none; d=zohomail.com; s=zohoarc; b=aWuwEeDucOA7UwdN+D42hbJYCri7zaLDBbGFcy/ydjY0YzHhYDlNk5RYo5YJOWGSsBwAarmLnwLznJzyE5CJ2n22iKNf+yBB1owoPLYvfLQrsBPgHCuPA590ow3YlECYgdxwJ2q9azQ+FR9wtH0Kvr6JIJAYfvokdJlrO4S3DEA= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1576820093; h=Content-Transfer-Encoding:Cc:Date:From:List-Id:List-Unsubscribe:MIME-Version:Message-ID:Reply-To:Sender:Subject:To; bh=7OJdOyCCCuTYniSO5izpbYoJ34aXPmevc8zzZnl0jLI=; b=aQ1XaHQztaaAUtc3+B3jLinnLbqppKyVI/0/M/Q8oQK+dhQjggrMoEFIE02pEJL73TUqCuRQdwSmCbxfDBey1BA5kKaPg1Rhm4rzl+tw8leT8uB3oLFWBLfRwElgM/iNN7kISuHpIhYiWHBLbmXad6HZspW25pfVSQWapp+8430= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.12 as permitted sender) smtp.mailfrom=bounce+27952+52440+1787277+3901457@groups.io; dmarc=fail header.from= (p=none dis=none) header.from= Received: from web01.groups.io (web01.groups.io [66.175.222.12]) by mx.zohomail.com with SMTPS id 1576820093753664.1341897761678; Thu, 19 Dec 2019 21:34:53 -0800 (PST) Return-Path: X-Received: by 127.0.0.2 with SMTP id uMseYY1788612xeBJN4yvIvb; Thu, 19 Dec 2019 21:34:52 -0800 X-Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by mx.groups.io with SMTP id smtpd.web09.5751.1576820090871902675 for ; Thu, 19 Dec 2019 21:34:51 -0800 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False X-Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 19 Dec 2019 21:34:50 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.69,334,1571727600"; d="scan'208";a="213474489" X-Received: from ydong10-desktop.ccr.corp.intel.com ([10.239.158.133]) by fmsmga008.fm.intel.com with ESMTP; 19 Dec 2019 21:34:49 -0800 From: "Dong, Eric" To: devel@edk2.groups.io Cc: Ray Ni , Laszlo Ersek Subject: [edk2-devel] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Remove dependence between APs. Date: Fri, 20 Dec 2019 13:34:46 +0800 Message-Id: <20191220053446.1532-1-eric.dong@intel.com> MIME-Version: 1.0 Precedence: Bulk List-Unsubscribe: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,eric.dong@intel.com X-Gm-Message-State: Xcg1nlNfSBVNFjrC1qQWoYacx1787277AA= Content-Transfer-Encoding: quoted-printable DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=groups.io; q=dns/txt; s=20140610; t=1576820092; bh=4sLgXrEpQF/zZnB3/G8sNJKuR26OiWsbEI1QKzO7HtM=; h=Cc:Date:From:Reply-To:Subject:To; b=lC/BLCZ/8uB4IrOYutV+4ubZmaINYxaqHN+qTRq0hhPPTIqELibGvqcmsTs+64bD8J0 mcHkKtlPRVygNtsL0nolTwL97zPEGo2yIaX6bIl3gDtEaacsamECKeyspMtNE3e0xqUaj g0gU/vlskDdBl0MUz95hBiIVvkVGQ/MEuew= X-ZohoMail-DKIM: pass (identity @groups.io) Content-Type: text/plain; charset="utf-8" REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2268 In current implementation, when check whether APs called by StartUpAllAPs or StartUpThisAp, it checks the Tokens value used by other APs. Also the AP will update the Token value for itself if its task finished. In this case, the potential race condition issues happens for the tokens. Because of this, system may trig ASSERT during cycling test. This change enhance the code logic, add new attributes for the token to remove the reference for the tokens belongs to other APs. Cc: Ray Ni Cc: Laszlo Ersek Signed-off-by: Eric Dong --- UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 107 +++++++-------------- UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 4 +- 2 files changed, 36 insertions(+), 75 deletions(-) diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c b/UefiCpuPkg/PiSmmCpuDxe= Smm/MpService.c index 757f1056f7..bd5fdfd728 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c @@ -402,38 +402,6 @@ IsPresentAp ( *(mSmmMpSyncData->CpuData[CpuIndex].Present)); } =20 -/** - Check whether execute in single AP or all APs. - - Compare two Tokens used by different APs to know whether in StartAllAps = call. - - Whether is an valid AP base on AP's Present flag. - - @retval TRUE IN StartAllAps call. - @retval FALSE Not in StartAllAps call. - -**/ -BOOLEAN -InStartAllApsCall ( - VOID - ) -{ - UINTN ApIndex; - UINTN ApIndex2; - - for (ApIndex =3D mMaxNumberOfCpus; ApIndex-- > 0;) { - if (IsPresentAp (ApIndex) && (mSmmMpSyncData->CpuData[ApIndex].Token != =3D NULL)) { - for (ApIndex2 =3D ApIndex; ApIndex2-- > 0;) { - if (IsPresentAp (ApIndex2) && (mSmmMpSyncData->CpuData[ApIndex2].T= oken !=3D NULL)) { - return mSmmMpSyncData->CpuData[ApIndex2].Token =3D=3D mSmmMpSync= Data->CpuData[ApIndex].Token; - } - } - } - } - - return FALSE; -} - /** Clean up the status flags used during executing the procedure. =20 @@ -445,40 +413,20 @@ ReleaseToken ( IN UINTN CpuIndex ) { - UINTN Index; - BOOLEAN Released; + PROCEDURE_TOKEN *Token; =20 - if (InStartAllApsCall ()) { - // - // In Start All APs mode, make sure all APs have finished task. - // - if (WaitForAllAPsNotBusy (FALSE)) { - // - // Clean the flags update in the function call. - // - Released =3D FALSE; - for (Index =3D mMaxNumberOfCpus; Index-- > 0;) { - // - // Only In SMM APs need to be clean up. - // - if (mSmmMpSyncData->CpuData[Index].Present && mSmmMpSyncData->CpuD= ata[Index].Token !=3D NULL) { - if (!Released) { - ReleaseSpinLock (mSmmMpSyncData->CpuData[Index].Token); - Released =3D TRUE; - } - mSmmMpSyncData->CpuData[Index].Token =3D NULL; - } - } - } - } else { - // - // In single AP mode. - // - if (mSmmMpSyncData->CpuData[CpuIndex].Token !=3D NULL) { - ReleaseSpinLock (mSmmMpSyncData->CpuData[CpuIndex].Token); - mSmmMpSyncData->CpuData[CpuIndex].Token =3D NULL; + Token =3D mSmmMpSyncData->CpuData[CpuIndex].Token; + mSmmMpSyncData->CpuData[CpuIndex].Token =3D NULL; + + if (!Token->SingleAp) { + ReleaseSemaphore (&Token->FinishedApCount); + + if (Token->FinishedApCount < (UINT32)mMaxNumberOfCpus) { + return; } } + + ReleaseSpinLock (Token->ProcedureToken); } =20 /** @@ -912,12 +860,14 @@ APHandler ( *mSmmMpSyncData->CpuData[CpuIndex].Status =3D ProcedureStatus; } =20 + if (mSmmMpSyncData->CpuData[CpuIndex].Token !=3D NULL) { + ReleaseToken (CpuIndex); + } + // // Release BUSY // ReleaseSpinLock (mSmmMpSyncData->CpuData[CpuIndex].Busy); - - ReleaseToken (CpuIndex); } =20 if (SmmCpuFeaturesNeedConfigureMtrrs()) { @@ -1127,7 +1077,7 @@ IsTokenInUse ( @retval return the spin lock used as token. =20 **/ -SPIN_LOCK * +PROCEDURE_TOKEN * CreateToken ( VOID ) @@ -1170,10 +1120,12 @@ CreateToken ( ASSERT (ProcToken !=3D NULL); ProcToken->Signature =3D PROCEDURE_TOKEN_SIGNATURE; ProcToken->ProcedureToken =3D CpuToken; + ProcToken->FinishedApCount =3D 0; + ProcToken->SingleAp =3D TRUE; =20 InsertTailList (&gSmmCpuPrivate->TokenList, &ProcToken->Link); =20 - return CpuToken; + return ProcToken; } =20 /** @@ -1278,14 +1230,11 @@ InternalSmmStartupThisAp ( =20 AcquireSpinLock (mSmmMpSyncData->CpuData[CpuIndex].Busy); =20 - if (Token !=3D NULL) { - *Token =3D (MM_COMPLETION) CreateToken (); - } - mSmmMpSyncData->CpuData[CpuIndex].Procedure =3D Procedure; mSmmMpSyncData->CpuData[CpuIndex].Parameter =3D ProcArguments; if (Token !=3D NULL) { - mSmmMpSyncData->CpuData[CpuIndex].Token =3D (SPIN_LOCK *)(*Token); + mSmmMpSyncData->CpuData[CpuIndex].Token =3D CreateToken (); + *Token =3D (MM_COMPLETION) mSmmMpSyncData->CpuData[CpuIndex].Token->Pr= ocedureToken; } mSmmMpSyncData->CpuData[CpuIndex].Status =3D CpuStatus; if (mSmmMpSyncData->CpuData[CpuIndex].Status !=3D NULL) { @@ -1343,6 +1292,7 @@ InternalSmmStartupAllAPs ( { UINTN Index; UINTN CpuCount; + PROCEDURE_TOKEN *ProcToken; =20 if ((TimeoutInMicroseconds !=3D 0) && ((mSmmMp.Attributes & EFI_MM_MP_TI= MEOUT_SUPPORTED) =3D=3D 0)) { return EFI_INVALID_PARAMETER; @@ -1371,7 +1321,11 @@ InternalSmmStartupAllAPs ( } =20 if (Token !=3D NULL) { - *Token =3D (MM_COMPLETION) CreateToken (); + ProcToken =3D CreateToken (); + *Token =3D (MM_COMPLETION)ProcToken->ProcedureToken; + ProcToken->SingleAp =3D FALSE; + } else { + ProcToken =3D NULL; } =20 // @@ -1392,7 +1346,7 @@ InternalSmmStartupAllAPs ( mSmmMpSyncData->CpuData[Index].Procedure =3D (EFI_AP_PROCEDURE2) Pro= cedure; mSmmMpSyncData->CpuData[Index].Parameter =3D ProcedureArguments; if (Token !=3D NULL) { - mSmmMpSyncData->CpuData[Index].Token =3D (SPIN_LOCK *)(*Token); + mSmmMpSyncData->CpuData[Index].Token =3D ProcToken; } if (CPUStatus !=3D NULL) { mSmmMpSyncData->CpuData[Index].Status =3D &CPUStatus[Index]; @@ -1408,6 +1362,11 @@ InternalSmmStartupAllAPs ( if (CPUStatus !=3D NULL) { CPUStatus[Index] =3D EFI_NOT_STARTED; } + + // + // Increate the count to mark this AP as finished. + // + ReleaseSemaphore (&ProcToken->FinishedApCount); } } =20 diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmC= puDxeSmm/PiSmmCpuDxeSmm.h index 5c1a01e42b..0551539aee 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h @@ -213,6 +213,8 @@ typedef struct { LIST_ENTRY Link; =20 SPIN_LOCK *ProcedureToken; + BOOLEAN SingleAp; + volatile UINT32 FinishedApCount; } PROCEDURE_TOKEN; =20 #define PROCEDURE_TOKEN_FROM_LINK(a) CR (a, PROCEDURE_TOKEN, Link, PROCED= URE_TOKEN_SIGNATURE) @@ -407,7 +409,7 @@ typedef struct { volatile VOID *Parameter; volatile UINT32 *Run; volatile BOOLEAN *Present; - SPIN_LOCK *Token; + PROCEDURE_TOKEN *Token; EFI_STATUS *Status; } SMM_CPU_DATA_BLOCK; =20 --=20 2.23.0.windows.1 -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#52440): https://edk2.groups.io/g/devel/message/52440 Mute This Topic: https://groups.io/mt/68844978/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-