From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=pass header.i=@intel.onmicrosoft.com header.s=selector2-intel-onmicrosoft-com header.b=f8SgdyB4; spf=pass (domain: intel.com, ip: 134.134.136.100, mailfrom: michael.a.kubacki@intel.com) Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by groups.io with SMTP; Thu, 03 Oct 2019 11:53:23 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 03 Oct 2019 11:53:22 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.67,253,1566889200"; d="scan'208";a="367133395" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by orsmga005.jf.intel.com with ESMTP; 03 Oct 2019 11:53:21 -0700 Received: from fmsmsx153.amr.corp.intel.com (10.18.125.6) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 3 Oct 2019 11:53:21 -0700 Received: from FMSEDG002.ED.cps.intel.com (10.1.192.134) by FMSMSX153.amr.corp.intel.com (10.18.125.6) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 3 Oct 2019 11:53:20 -0700 Received: from NAM03-CO1-obe.outbound.protection.outlook.com (104.47.40.53) by edgegateway.intel.com (192.55.55.69) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 3 Oct 2019 11:53:21 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=kLbX81cnIFwQOjDu4g1Z5vqUuZ+MBPxPONsmZq75ZIM0M4sEwIs4WOMQQpF258K8LluZ9EO67cX4ODWxzk6de9e16JZ2xgmMY0d3czZRSZwNJRqRKH3IaB9x80ktcNwpOEs10P1CDstdLLFLWsgegaz9TkYy+czUX0rv4LkYQJwzb/JReaQK8HZZJlNeNY9CHrl6pAjTMruSfumCPe6pL9360wKeK3J9NnUtmjWpfGNDWZfqXkL8z1u010syjj1ELcDcah31LriY+1WTPpgryQdSmv+Ct8tq9ZUuilVhIaMxke0nu7miMj5fKBT39ZJ9lKL1TrH07SfdoMtgDmVY2Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=5jP/RjlluEhjZovgvR7f82hWSD6BNpJrtgqjRUyCJ+Y=; b=EQ3K6c+ea/0q/O6nCaO+GsWliwrPLTZY4Pazu0q4gCuHXabMeSF9yra8WW6OdUwp+WiMrQrMGrkD7gzpmWI2bulkeXB5aj3Fk6Ue8oUbEReI1rTm8XpLYMDq3UtjodnYughIJTk4+mn/TYMOW8uerQ0SMzRa8Qs2BaFIGMN8sSOfLGoX8G0ETymJQ96ufn+Tv3C743N2R0XGK5IPopbQiDD127bSe0EVPWZfP4E2CQNbFmJf3AyQRHi7jRazgG6L7067jjx/T5UWnKt3fN3H0L/dVIO2Li+msF2e4SXrEmLX3XnBAfCfCtegx+QyMGNDomzdQRncsFV3l4ulxe4JkA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel.onmicrosoft.com; s=selector2-intel-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=5jP/RjlluEhjZovgvR7f82hWSD6BNpJrtgqjRUyCJ+Y=; b=f8SgdyB4N4h3541GEj1zPwNl7AC9uB+6MbPkhv8IHE4iocaeXoXoYecKw4ETrF26oA1BKVAPKtn8vE6N+u0BJzKbv3djI60E3FI5SDn5LITCqPybwcCMs1QtALOUXyzw+9y2p+dkscbO/3FAPXF1JVemR50VZQqq1qKNq1uBeGI= Received: from DM6PR11MB3834.namprd11.prod.outlook.com (20.179.17.87) by DM6PR11MB3369.namprd11.prod.outlook.com (20.176.122.208) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2305.20; Thu, 3 Oct 2019 18:52:28 +0000 Received: from DM6PR11MB3834.namprd11.prod.outlook.com ([fe80::59cc:8a30:6b9e:584e]) by DM6PR11MB3834.namprd11.prod.outlook.com ([fe80::59cc:8a30:6b9e:584e%3]) with mapi id 15.20.2305.023; Thu, 3 Oct 2019 18:52:28 +0000 From: "Kubacki, Michael A" To: "Wu, Hao A" , "devel@edk2.groups.io" CC: "Bi, Dandan" , Ard Biesheuvel , "Dong, Eric" , Laszlo Ersek , "Gao, Liming" , "Kinney, Michael D" , "Ni, Ray" , "Wang, Jian J" , "Yao, Jiewen" Subject: Re: [PATCH V2 8/9] MdeModulePkg/Variable: Add RT GetNextVariableName() cache support Thread-Topic: [PATCH V2 8/9] MdeModulePkg/Variable: Add RT GetNextVariableName() cache support Thread-Index: AQHVdZ69aao24nyK0UqxTLIg6pa5BadIlNZAgAC1IGA= Date: Thu, 3 Oct 2019 18:52:28 +0000 Message-ID: References: <20190928014717.31372-1-michael.a.kubacki@intel.com> <20190928014717.31372-9-michael.a.kubacki@intel.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows x-ctpclassification: CTP_NT x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNWU3Y2E3ZjUtMDEzMy00ZjhlLTliNGItNjQ5ODAzMDJhMTMzIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiM2dyQ1ZqeGVodEE2dmF5b2RGeGtaRkl0NFpNaFZzYXg4OUx1V2tQVlZNaFNRTWM5Qm1SeE1oNmZyOFVESk5IeSJ9 dlp-reaction: no-action dlp-version: 11.2.0.6 authentication-results: spf=none (sender IP is ) smtp.mailfrom=michael.a.kubacki@intel.com; x-originating-ip: [134.134.136.217] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 65aa54f9-889d-4943-cc42-08d74832d6d8 x-ms-traffictypediagnostic: DM6PR11MB3369: x-ms-exchange-purlcount: 1 x-ld-processed: 46c98d88-e344-4ed4-8496-4ed7712e255d,ExtAddr x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:1824; x-forefront-prvs: 01792087B6 x-forefront-antispam-report: SFV:NSPM;SFS:(10019020)(39860400002)(346002)(396003)(376002)(366004)(136003)(189003)(199004)(13464003)(305945005)(74316002)(66066001)(478600001)(7736002)(186003)(446003)(53546011)(6116002)(26005)(8936002)(102836004)(6506007)(229853002)(7696005)(6436002)(86362001)(76116006)(4326008)(66946007)(3846002)(9686003)(55016002)(66446008)(64756008)(66556008)(6306002)(66476007)(76176011)(14454004)(6246003)(256004)(107886003)(11346002)(52536014)(5660300002)(316002)(25786009)(486006)(966005)(14444005)(99286004)(71190400001)(71200400001)(54906003)(2906002)(8676002)(2501003)(81156014)(81166006)(476003)(110136005)(33656002);DIR:OUT;SFP:1102;SCL:1;SRVR:DM6PR11MB3369;H:DM6PR11MB3834.namprd11.prod.outlook.com;FPR:;SPF:None;LANG:en;PTR:InfoNoRecords;MX:1;A:1; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: /lJDS9SrDZJGHLIOcueMtKExvtR+NmjMW/fura5OpQzLLA7menYeIJz3fVsi8HxQeiz0DJ5Am65TDaOj/FmsRwM+yzygpIsVOLAO7VHK95OK3QP/Mmi8VQHFlp6FOh6SWA/ucmAwOOlbgX6hRn4qnc6lnsvAX278L1qjjZ4+BaMHnRCT1YLtn8ujE08hmu87qgS3Em4NPBP6+CJg7GRnUpfR/HqV0yOPLPrBjVq7m/xaypt8ZyHrUD0vjKztUue8u+sVJy8zFsjoCc3We6711oj+jmrCNj2fHc809OnXIiPX5MoqCQmhilzSfIfksBpoxxJrHYh321DrJo1WlMNdNVTJVnA9uSLmD2ZdVU5oWFMXf6zNr7wWiz8xLlIE1rgkTrLHuArGbG4uVnroC/tirSrF37Nc1PH1B5S1nCV0PnB3fFqiVJotjyx+O09Qh2XMcZud6Mc7pww1TRvX+9AvOg== MIME-Version: 1.0 X-MS-Exchange-CrossTenant-Network-Message-Id: 65aa54f9-889d-4943-cc42-08d74832d6d8 X-MS-Exchange-CrossTenant-originalarrivaltime: 03 Oct 2019 18:52:28.4112 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: W5EOm80dlA88H6OFG2qKRjxEm+faDp8yHzABEhw/kE3VhXfwOcdibXfYpX5GVA+ulL2HkTQ7dA62B2w7GZxsgE/A4eQ8LMKSXi1pZezjBJo= X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR11MB3369 Return-Path: michael.a.kubacki@intel.com X-OriginatorOrg: intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable > > -Done: > > + mVariableRuntimeCacheReadLock =3D FALSE; >=20 >=20 > Similar to the previous patch (7/9), > if timeout occurs when acquiring the read lock, should this flag be set t= o > FALSE in such case? > Given that the runtime service can be invoked in a multi-threaded OS enviro= nment, it is possible that one thread could be stuck with the lock while another t= hread times out waiting to acquire the lock. In that case, I believe the global lock sh= ould not be released. I can move setting the flag to FALSE within the same conditional = block in which it is set. Thanks, Michael > -----Original Message----- > From: Wu, Hao A > Sent: Thursday, October 3, 2019 1:05 AM > To: Kubacki, Michael A ; > devel@edk2.groups.io > Cc: Bi, Dandan ; Ard Biesheuvel > ; Dong, Eric ; Laszlo Ers= ek > ; Gao, Liming ; Kinney, Michael > D ; Ni, Ray ; Wang, Jian J > ; Yao, Jiewen > Subject: RE: [PATCH V2 8/9] MdeModulePkg/Variable: Add RT > GetNextVariableName() cache support >=20 > > -----Original Message----- > > From: Kubacki, Michael A > > Sent: Saturday, September 28, 2019 9:47 AM > > To: devel@edk2.groups.io > > Cc: Bi, Dandan; Ard Biesheuvel; Dong, Eric; Laszlo Ersek; Gao, Liming; > > Kinney, Michael D; Ni, Ray; Wang, Jian J; Wu, Hao A; Yao, Jiewen > > Subject: [PATCH V2 8/9] MdeModulePkg/Variable: Add RT > > GetNextVariableName() cache support > > > > https://bugzilla.tianocore.org/show_bug.cgi?id=3D2220 > > > > This change implements the Runtime Service GetNextVariableName() > using > > the Runtime Cache in VariableSmmRuntimeDxe. Runtime Service calls to > > GetNextVariableName() will no longer trigger a SW SMI. > > > > Overall system performance and stability will be improved by > > eliminating an SMI for these calls as they typically result in a > > relatively large number of invocations to retrieve all variable names > > in all variable stores present. > > > > Cc: Dandan Bi > > Cc: Ard Biesheuvel > > Cc: Eric Dong > > Cc: Laszlo Ersek > > Cc: Liming Gao > > Cc: Michael D Kinney > > Cc: Ray Ni > > Cc: Jian J Wang > > Cc: Hao A Wu > > Cc: Jiewen Yao > > Signed-off-by: Michael Kubacki > > --- > > > > > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe > > .c | 118 +++++++++----------- > > 1 file changed, 50 insertions(+), 68 deletions(-) > > > > diff --git > > > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeD > > xe.c > > > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeD > > xe.c > > index 46f69765a4..bc3b56b0ce 100644 > > --- > > > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeD > > xe.c > > +++ > > > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeD > > xe.c > > @@ -799,87 +799,69 @@ RuntimeServiceGetNextVariableName ( > > IN OUT EFI_GUID *VendorGuid > > ) > > { > > - EFI_STATUS Status; > > - UINTN PayloadSize; > > - SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME > > *SmmGetNextVariableName; > > - UINTN OutVariableNameSize; > > - UINTN InVariableNameSize; > > + EFI_STATUS Status; > > + UINTN DelayIndex; > > + UINTN MaxLen; > > + UINTN VarNameSize; > > + VARIABLE_HEADER *VariablePtr; > > + VARIABLE_STORE_HEADER > > *VariableStoreHeader[VariableStoreTypeMax]; > > + > > + Status =3D EFI_NOT_FOUND; > > > > if (VariableNameSize =3D=3D NULL || VariableName =3D=3D NULL || Vend= orGuid > > =3D=3D > > NULL) { > > return EFI_INVALID_PARAMETER; > > } > > > > - OutVariableNameSize =3D *VariableNameSize; > > - InVariableNameSize =3D StrSize (VariableName); > > - SmmGetNextVariableName =3D NULL; > > - > > // > > - // If input string exceeds SMM payload limit. Return failure > > + // Calculate the possible maximum length of name string, including > > + the > > Null terminator. > > // > > - if (InVariableNameSize > mVariableBufferPayloadSize - OFFSET_OF > > (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name)) > { > > + MaxLen =3D *VariableNameSize / sizeof (CHAR16); if ((MaxLen =3D=3D = 0) || > > + (StrnLenS (VariableName, MaxLen) =3D=3D MaxLen)) { > > + // > > + // Null-terminator is not found in the first VariableNameSize > > + bytes of the > > input VariableName buffer, > > + // follow spec to return EFI_INVALID_PARAMETER. > > + // > > return EFI_INVALID_PARAMETER; > > } > > > > - AcquireLockOnlyAtBootTime(&mVariableServicesLock); > > + AcquireLockOnlyAtBootTime (&mVariableServicesLock); > > > > - // > > - // Init the communicate buffer. The buffer data size is: > > - // SMM_COMMUNICATE_HEADER_SIZE + > > SMM_VARIABLE_COMMUNICATE_HEADER_SIZE + PayloadSize. > > - // > > - if (OutVariableNameSize > mVariableBufferPayloadSize - OFFSET_OF > > (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name)) > { > > - // > > - // If output buffer exceed SMM payload limit. Trim output buffer t= o > SMM > > payload size > > - // > > - OutVariableNameSize =3D mVariableBufferPayloadSize - OFFSET_OF > > (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name); > > + for (DelayIndex =3D 0; mVariableRuntimeCacheReadLock && DelayIndex < > > VARIABLE_RT_CACHE_READ_LOCK_TIMEOUT; DelayIndex++) { > > + MicroSecondDelay (10); > > } > > - // > > - // Payload should be Guid + NameSize + MAX of Input & Output buffer > > - // > > - PayloadSize =3D OFFSET_OF > > (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name) > + MAX > > (OutVariableNameSize, InVariableNameSize); > > - > > - Status =3D InitCommunicateBuffer ((VOID > **)&SmmGetNextVariableName, > > PayloadSize, SMM_VARIABLE_FUNCTION_GET_NEXT_VARIABLE_NAME); > > - if (EFI_ERROR (Status)) { > > - goto Done; > > - } > > - ASSERT (SmmGetNextVariableName !=3D NULL); > > - > > - // > > - // SMM comm buffer->NameSize is buffer size for return string > > - // > > - SmmGetNextVariableName->NameSize =3D OutVariableNameSize; > > - > > - CopyGuid (&SmmGetNextVariableName->Guid, VendorGuid); > > - // > > - // Copy whole string > > - // > > - CopyMem (SmmGetNextVariableName->Name, VariableName, > > InVariableNameSize); > > - if (OutVariableNameSize > InVariableNameSize) { > > - ZeroMem ((UINT8 *) SmmGetNextVariableName->Name + > > InVariableNameSize, OutVariableNameSize - InVariableNameSize); > > - } > > - > > - // > > - // Send data to SMM > > - // > > - Status =3D SendCommunicateBuffer (PayloadSize); > > - > > - // > > - // Get data from SMM. > > - // > > - if (Status =3D=3D EFI_SUCCESS || Status =3D=3D EFI_BUFFER_TOO_SMALL)= { > > - // > > - // SMM CommBuffer NameSize can be a trimed value > > - // Only update VariableNameSize when needed > > - // > > - *VariableNameSize =3D SmmGetNextVariableName->NameSize; > > - } > > - if (EFI_ERROR (Status)) { > > - goto Done; > > + if (DelayIndex < VARIABLE_RT_CACHE_READ_LOCK_TIMEOUT) { > > + ASSERT (!mVariableRuntimeCacheReadLock); > > + > > + CheckForRuntimeCacheSync (); > > + mVariableRuntimeCacheReadLock =3D TRUE; > > + > > + if (!mVariableRuntimeCachePendingUpdate) { > > + // > > + // 0: Volatile, 1: HOB, 2: Non-Volatile. > > + // The index and attributes mapping must be kept in this order > > + as > > FindVariable > > + // makes use of this mapping to implement search algorithm. > > + // > > + VariableStoreHeader[VariableStoreTypeVolatile] =3D > > mVariableRuntimeVolatileCacheBuffer; > > + VariableStoreHeader[VariableStoreTypeHob] =3D > > mVariableRuntimeHobCacheBuffer; > > + VariableStoreHeader[VariableStoreTypeNv] =3D > > mVariableRuntimeNvCacheBuffer; > > + > > + Status =3D GetNextVariableEx (VariableName, VendorGuid, > > VariableStoreHeader, &VariablePtr); > > + if (!EFI_ERROR (Status)) { > > + VarNameSize =3D NameSizeOfVariable (VariablePtr); > > + ASSERT (VarNameSize !=3D 0); > > + if (VarNameSize <=3D *VariableNameSize) { > > + CopyMem (VariableName, GetVariableNamePtr (VariablePtr), > > VarNameSize); > > + CopyMem (VendorGuid, GetVendorGuidPtr (VariablePtr), sizeof > > (EFI_GUID)); > > + Status =3D EFI_SUCCESS; > > + } else { > > + Status =3D EFI_BUFFER_TOO_SMALL; > > + } > > + > > + *VariableNameSize =3D VarNameSize; > > + } > > + } > > } > > - > > - CopyGuid (VendorGuid, &SmmGetNextVariableName->Guid); > > - CopyMem (VariableName, SmmGetNextVariableName->Name, > > SmmGetNextVariableName->NameSize); > > - > > -Done: > > + mVariableRuntimeCacheReadLock =3D FALSE; >=20 >=20 > Similar to the previous patch (7/9), > if timeout occurs when acquiring the read lock, should this flag be set t= o > FALSE in such case? >=20 > Best Regards, > Hao Wu >=20 >=20 > > ReleaseLockOnlyAtBootTime (&mVariableServicesLock); > > return Status; > > } > > -- > > 2.16.2.windows.1 >=20