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=ruiyu.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 A6A6121AE30DB for ; Tue, 18 Sep 2018 01:33:36 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 18 Sep 2018 01:33:36 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.53,389,1531810800"; d="scan'208";a="92697003" Received: from ray-dev.ccr.corp.intel.com (HELO [10.239.9.8]) ([10.239.9.8]) by orsmga002.jf.intel.com with ESMTP; 18 Sep 2018 01:33:35 -0700 To: Laszlo Ersek , "Duran, Leo" , "edk2-devel@lists.01.org" Cc: "Dong, Eric" References: <1536680498-6554-1-git-send-email-leo.duran@amd.com> <1536680498-6554-2-git-send-email-leo.duran@amd.com> <17c6d6d1-2655-fe06-a8b9-f48141bfb0d7@redhat.com> <610eaa55-c87b-5e0c-4f87-5c1e79ffc5ba@redhat.com> <12abd990-3b08-9159-e7a9-ffd7eb7282b3@redhat.com> <734D49CCEBEEF84792F5B80ED585239D5BE07168@SHSMSX104.ccr.corp.intel.com> <981751ac-68a0-ea2c-7985-2562d1916560@Intel.com> <85b907c0-1d7d-98f1-6e86-6bb3a3f86ffb@redhat.com> From: "Ni, Ruiyu" Message-ID: <5349de2c-8a15-c599-f966-84b87a517453@Intel.com> Date: Tue, 18 Sep 2018 16:34:28 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <85b907c0-1d7d-98f1-6e86-6bb3a3f86ffb@redhat.com> Subject: Re: [PATCH] UefiCpuPkg/MtrrLib: Add flag to skip disabling MTRRs prior to MTRR change. 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, 18 Sep 2018 08:33:36 -0000 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit On 9/18/2018 12:38 AM, Laszlo Ersek wrote: > On 09/17/18 18:20, Duran, Leo wrote: >> >>> -----Original Message----- >>> From: Ni, Ruiyu >>> Sent: Thursday, September 13, 2018 11:44 PM >>> To: Duran, Leo ; Laszlo Ersek ; >>> edk2-devel@lists.01.org >>> Cc: Dong, Eric >>> Subject: Re: [edk2] [PATCH] UefiCpuPkg/MtrrLib: Add flag to skip disabling >>> MTRRs prior to MTRR change. >>> >>> On 9/14/2018 3:31 AM, Duran, Leo wrote: >>>> >>>> >>>>> -----Original Message----- >>>>> From: Ni, Ruiyu >>>>> Sent: Wednesday, September 12, 2018 9:39 PM >>>>> To: Duran, Leo ; Laszlo Ersek >>> ; >>>>> edk2-devel@lists.01.org >>>>> Cc: Dong, Eric >>>>> Subject: RE: [PATCH] UefiCpuPkg/MtrrLib: Add flag to skip disabling >>>>> MTRRs prior to MTRR change. >>>>> >>>>> Leo, >>>>> Sorry I was in leave yesterday so didn't see the mail. >>>>> Which MSRs are shared? Only the >>> MSR_IA32_MTRR_DEF_TYPE_REGISTER? >>>>> Or all the MSRs that configures the CPU MTRR setting? >>>>> >>>> >>>> Hi Ray, >>>> The MTTR config MSRs are also shared by threads within a core. >>>> >>> >>> Hi Leo, >>> Do you think that fixing the caller is more proper? >> >> Hi Ray, >> Actually, >> The proposed PCD is the simplest solution, as that works for us and does not change the existing (default) flow. >> >> That is, >> I'd prefer making a decision about the PCD in platform-specific code, rather than introducing complex detection and heuristics at the caller level in EDK2 (just for AMD). >> >> So, please approve the PCD. Leo, I agree with you on the first part "the PCD is the simplest solution". But this really looks like a workaround of the real issue. For a multiple-socket system, it may contain S sockets, each socket contains C cores and each core contains T threads. In summary the system contains S * C * T threads. As you said all threads inside a core share the MTRR setting. Do all cores inside a socket share the MTRR setting? Do all sockets share the MTRR setting? If one of the answer of above questions is "no", how can we configure the PCD? > > - From my side, if it works for you, it works for me. (The general trend > has been to avoid adding more PCDs to the "core" package DEC files, but > I'm 100% neutral on that.) > > Laszlo > Laszlo, Thanks for pointing out the general trend. Yes less PCDs are very welcomed. To me, PCD is no different from protocol. And even worse, because it's very easily to be over-used. But I am not sure whether a PCD has to be introduced for this issue. Maybe even we choose to fix the caller, the PCD is still needed. I am not sure. -- Thanks, Ray