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.43; helo=mga05.intel.com; envelope-from=ray.ni@intel.com; receiver=edk2-devel@lists.01.org Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) (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 76094209574CA for ; Mon, 25 Feb 2019 00:24:33 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 25 Feb 2019 00:24:33 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.58,410,1544515200"; d="scan'208";a="302299618" Received: from ray-dev.ccr.corp.intel.com (HELO [10.239.9.37]) ([10.239.9.37]) by orsmga005.jf.intel.com with ESMTP; 25 Feb 2019 00:24:31 -0800 To: Laszlo Ersek , "edk2-devel@lists.01.org" Cc: "Bi, Dandan" , "Wu, Hao A" , "Wang, Jian J" , Sean Brogan , "Zeng, Star" References: <20190221104112.14995-1-lersek@redhat.com> <20190221104112.14995-2-lersek@redhat.com> <734D49CCEBEEF84792F5B80ED585239D5C02CEB3@SHSMSX104.ccr.corp.intel.com> <4e739d1b-44a5-f2a2-3f0f-84f3c700e2b2@redhat.com> From: "Ni, Ray" Message-ID: <0a59efb2-45cb-51b4-60d6-9cf3a125c9cb@Intel.com> Date: Mon, 25 Feb 2019 16:27:14 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.5.0 MIME-Version: 1.0 In-Reply-To: <4e739d1b-44a5-f2a2-3f0f-84f3c700e2b2@redhat.com> Subject: Re: [PATCH v3 1/5] MdeModulePkg/UefiBootManagerLib: fix LoadImage/StartImage status code rep. 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: Mon, 25 Feb 2019 08:24:34 -0000 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit On 2/23/2019 1:16 AM, Laszlo Ersek wrote: > Yes, I was fully aware of that. > > However: > > The issue is that, in the BmReportLoadFailure() function, we do some > work*before* we call REPORT_STATUS_CODE_EX(). We have an ASSERT(), a > ZeroMem(), and a field assignment. > > If status code reporting is disabled for EFI_ERROR_CODE in the platform, > then said work will be wasted. We can optimize this by checking for > ReportErrorCodeEnabled() up-front, because we know for sure that later > on we will report the status code with EFI_ERROR_CODE type. > > In other words, this approach is similar to DEBUG_CODE(). In some cases, > logging a piece of information with DEBUG() takes non-trivial > computation. And it would be a waste, for example in RELEASE builds, to > perform the computation, and then throw away only the result (the log > message). Therefore the DEBUG_CODE macro is used, and the whole work is > eliminated in RELEASE builds. > > The idea is the same here. If the compiler can statically deduce that > ReportErrorCodeEnabled() will always return FALSE -- for example because > the ReportStatusCodeLib instance in question looks at > "PcdReportStatusCodePropertyMask", and the PCD is Fixed-at-Build, and > the corresponding bit is clear --, then the compiler can eliminate the > entire BmReportLoadFailure() function. This is good for both flash usage > and for performance. > > I'm fine either way, but first, please confirm again that you really > want me to remove the ReportErrorCodeEnabled() check, before pushing. > > Thanks! > Laszlo Thanks for the explanation. I am fine with both. Reviewed-by: Ray Ni -- Thanks, Ray