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=ruiyu.ni@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 5C5132117D288 for ; Tue, 23 Oct 2018 02:35:23 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 23 Oct 2018 02:35:22 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,415,1534834800"; d="scan'208";a="83708879" Received: from ray-dev.ccr.corp.intel.com (HELO [10.239.9.11]) ([10.239.9.11]) by orsmga008.jf.intel.com with ESMTP; 23 Oct 2018 02:35:21 -0700 To: "Lohr, Paul A" , Laszlo Ersek , "edk2-devel@lists.01.org" Cc: "Kinney, Michael D" , "Yao, Jiewen" References: <20181022090333.95988-1-ruiyu.ni@intel.com> From: "Ni, Ruiyu" Message-ID: <1998f4ff-13e0-3918-db78-74ada10fddc0@Intel.com> Date: Tue, 23 Oct 2018 17:36:37 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: Subject: Re: [PATCH] MdeModulePkg/PiSmmIpl: Do not reset SMRAM to UC when CPU driver runs 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, 23 Oct 2018 09:35:23 -0000 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit On 10/23/2018 11:12 AM, Lohr, Paul A wrote: > Hello, > > Code to remove SMRAM = UC (line 650-ish) looks good. I would suggest adding some debug comments in the area it was removed. Thanks. debug message or comments? I guess you'd like to have some comments to say "SMRR is enabled by CPU SMM driver so no need to reset the SMRAM to UC in MTRR". Correct? > > Per #4, I also "think" the second SMRAM = UC should be removed, in addition to the SMRAM = WB in the same area. But this is under investigation now. Expect an update in 24-48 hours. I did some search in close source platform code and found out that the SMRAM is set to WB by default. So it turns out setting SMRAM to WB is unnecessary here. But this introduces a platform assumption. If platform doesn't set SMRAM to WB, removing the WB set here will cause system boot performance downgrade. Above is my understanding. Please post update here once you finish the investigation. > > Per #5, I too do not like the "CPU AP" comment, as I initially thought it referred to Application Processor 😊 Yes, please clean that AP comment up! Sure I will do that. > > > Paul A. Lohr – Server Firmware Enabling > 512.239.9073 (cell) > 512.794.5044 (work) >