From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.88; helo=mga01.intel.com; envelope-from=star.zeng@intel.com; receiver=edk2-devel@lists.01.org Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) (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 6DF27211B5A57 for ; Tue, 15 Jan 2019 02:11:10 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 15 Jan 2019 02:11:09 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,481,1539673200"; d="scan'208";a="311937348" Received: from shzintpr02.sh.intel.com (HELO [10.253.24.32]) ([10.239.4.160]) by fmsmga005.fm.intel.com with ESMTP; 15 Jan 2019 02:11:08 -0800 To: Laszlo Ersek , "Wang, Jian J" , "edk2-devel@lists.01.org" Cc: "Wu, Hao A" , star.zeng@intel.com References: <1547479196-40248-1-git-send-email-star.zeng@intel.com> <1547479196-40248-7-git-send-email-star.zeng@intel.com> <6d46ce75-4a17-dc89-373e-84b4891034ec@redhat.com> From: "Zeng, Star" Message-ID: <98b6a8a5-2586-890a-b9a9-5540396090a7@intel.com> Date: Tue, 15 Jan 2019 18:10:38 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 MIME-Version: 1.0 In-Reply-To: <6d46ce75-4a17-dc89-373e-84b4891034ec@redhat.com> Subject: Re: [PATCH V2 06/15] MdeModulePkg Variable: Remove CacheOffset in UpdateVariable() X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 15 Jan 2019 10:11:10 -0000 X-List-Received-Date: Tue, 15 Jan 2019 10:11:10 -0000 X-List-Received-Date: Tue, 15 Jan 2019 10:11:10 -0000 X-List-Received-Date: Tue, 15 Jan 2019 10:11:10 -0000 X-List-Received-Date: Tue, 15 Jan 2019 10:11:10 -0000 X-List-Received-Date: Tue, 15 Jan 2019 10:11:10 -0000 X-List-Received-Date: Tue, 15 Jan 2019 10:11:10 -0000 X-List-Received-Date: Tue, 15 Jan 2019 10:11:10 -0000 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit On 2019/1/15 17:58, Laszlo Ersek wrote: > On 01/15/19 09:04, Wang, Jian J wrote: >> Star, >> >> Just a tiny comment below. With it's addressed, >> >> Reviewed-by: Jian J Wang >> >>> -----Original Message----- >>> From: Zeng, Star >>> Sent: Monday, January 14, 2019 11:20 PM >>> To: edk2-devel@lists.01.org >>> Cc: Zeng, Star ; Wang, Jian J ; >>> Wu, Hao A ; Laszlo Ersek >>> Subject: [PATCH V2 06/15] MdeModulePkg Variable: Remove CacheOffset in >>> UpdateVariable() >>> >>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1323 >>> Merge EmuVariable and Real variable driver. >>> >>> CacheOffset could be removed in UpdateVariable() after >>> // >>> // update the memory copy of Flash region. >>> // >>> CopyMem ( >>> (UINT8 *)mNvVariableCache + CacheOffset, >>> (UINT8 *)NextVariable, VarSize >>> ); >>> >>> is moved to be before mVariableModuleGlobal->NonVolatileLastVariableOffset >>> value is updated, like right before >>> >>> mVariableModuleGlobal->NonVolatileLastVariableOffset += >>> HEADER_ALIGN (VarSize); >>> >>> This patch prepares for adding emulated variable NV mode >>> support in VariableRuntimeDxe. >>> >>> Cc: Jian J Wang >>> Cc: Hao Wu >>> Cc: Laszlo Ersek >>> Contributed-under: TianoCore Contribution Agreement 1.1 >>> Signed-off-by: Star Zeng >>> --- >>> MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 11 +++++------ >>> 1 file changed, 5 insertions(+), 6 deletions(-) >>> >>> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c >>> b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c >>> index 424f92a53757..4d524db30fec 100644 >>> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c >>> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c >>> @@ -2139,7 +2139,6 @@ UpdateVariable ( >>> VARIABLE_POINTER_TRACK *Variable; >>> VARIABLE_POINTER_TRACK NvVariable; >>> VARIABLE_STORE_HEADER *VariableStoreHeader; >>> - UINTN CacheOffset; >>> UINT8 *BufferForMerge; >>> UINTN MergedBufSize; >>> BOOLEAN DataReady; >>> @@ -2577,7 +2576,6 @@ UpdateVariable ( >>> // >>> // Step 1: >>> // >>> - CacheOffset = mVariableModuleGlobal->NonVolatileLastVariableOffset; >>> Status = UpdateVariableStore ( >>> &mVariableModuleGlobal->VariableGlobal, >>> FALSE, >>> @@ -2643,6 +2641,11 @@ UpdateVariable ( >>> goto Done; >>> } >>> >>> + // >>> + // update the memory copy of Flash region. >>> + // >> >> The first character of comment is not capitalized. > > In this particular case, I agree that such a change can be added to the > patch. While it does not pertain to the actual work done here, the patch > itself is so small (which is a good thing!) that the comment > capitalization would not cause confusion. > > Still I suggest mentioning it briefly in the commit message too. Make sense, I can do that. Thanks, Star > > Thanks > Laszlo > > >> >>> + CopyMem ((UINT8 *)mNvVariableCache + mVariableModuleGlobal- >>>> NonVolatileLastVariableOffset, (UINT8 *)NextVariable, VarSize); >>> + >>> mVariableModuleGlobal->NonVolatileLastVariableOffset += HEADER_ALIGN >>> (VarSize); >>> >>> if ((Attributes & EFI_VARIABLE_HARDWARE_ERROR_RECORD) != 0) { >>> @@ -2653,10 +2656,6 @@ UpdateVariable ( >>> mVariableModuleGlobal->CommonUserVariableTotalSize += >>> HEADER_ALIGN (VarSize); >>> } >>> } >>> - // >>> - // update the memory copy of Flash region. >>> - // >>> - CopyMem ((UINT8 *)mNvVariableCache + CacheOffset, (UINT8 >>> *)NextVariable, VarSize); >>> } else { >>> // >>> // Create a volatile variable. >>> -- >>> 2.7.0.windows.1 >>