From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (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 9894C80289 for ; Thu, 9 Mar 2017 08:36:06 -0800 (PST) Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 12EDC80F97; Thu, 9 Mar 2017 16:36:07 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-228.phx2.redhat.com [10.3.116.228]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v29Ga4C4012882; Thu, 9 Mar 2017 11:36:05 -0500 To: "Duran, Leo" , "'Gao, Liming'" , "Justen, Jordan L" , "Kinney, Michael D" , "edk2-devel@lists.01.org" , Brijesh Singh References: <148884284887.29188.7643544710695103939.stgit@brijesh-build-machine> <148884287496.29188.5155874233993236979.stgit@brijesh-build-machine> <2c6593dd-12f5-9277-0c36-ffd2d6c2cc55@redhat.com> <148891718851.27104.2018366977522352345@jljusten-skl> <4A89E2EF3DFEDB4C8BFDE51014F606A14D6ECFA9@shsmsx102.ccr.corp.intel.com> <148899953450.16179.11727988878279840569@jljusten-skl> <4A89E2EF3DFEDB4C8BFDE51014F606A14D6ED505@shsmsx102.ccr.corp.intel.com> Cc: "Lendacky, Thomas" , "brijesh.sing@amd.com" From: Laszlo Ersek Message-ID: Date: Thu, 9 Mar 2017 17:36:02 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Thu, 09 Mar 2017 16:36:07 +0000 (UTC) Subject: Re: [RFC PATCH v1 4/5] OvmfPkg/BaseIoLibIntrinsic: import BaseIoLibIntrinsic package X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 09 Mar 2017 16:36:06 -0000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 03/09/17 16:36, Duran, Leo wrote: > OK, what I'm hearing is: > - Let's leave the Fifo routines in BaseIoLib (as we currently have) > - And let's add the SEV checks to BaseIoLib, so that OvmPkg (et al) can consume it as-is > > If so, I could put a patch-set together for that... Please confirm. Confirmed from my side. > BTW, I'd try to minimize the need for CPUID (e.g., check it once and cache the result) In a general purpose BASE library instance, you can't rely on memory (writeable memory) being available. The library instance could be linked into SEC or PEI phase modules that (generally speaking) have no writeable global variables. It's only an OvmfPkg specialty that PEI has writeable global variables. (And it comes with a non-intuitive downside: in the SMM-less build of OVMF, on S3 resume, the PEI global variables are not re-initialized to the values that the C language would require. This is something we always have to keep in mind.) I don't think the CPUID checks will have a huge performance impact, especially because (IIUC) you only have to customize the Fifo routines. The cost of the CPUID will be amortized over all the individual IO port accesses (--> traps) that you will perform individually anyway. IMO the FeaturePCD check (which will be evaluated at compile time) is a valid requirement, the CPUID result caching is not -- not until you can measure the CPUID's impact to be "grave" under "general" workloads. Even then, the separate DxeSmm instance can be added incrementally (as suggested by others). Thanks! Laszlo > > Leo > >> -----Original Message----- >> From: Gao, Liming [mailto:liming.gao@intel.com] >> Sent: Wednesday, March 08, 2017 7:48 PM >> To: Justen, Jordan L ; Kinney, Michael D >> ; edk2-devel@lists.01.org; Brijesh Singh >> ; Laszlo Ersek >> Cc: Lendacky, Thomas ; Duran, Leo >> ; brijesh.sing@amd.com >> Subject: RE: [RFC PATCH v1 4/5] OvmfPkg/BaseIoLibIntrinsic: import >> BaseIoLibIntrinsic package >> >> >> >>> -----Original Message----- >>> From: Justen, Jordan L >>> Sent: Thursday, March 9, 2017 2:59 AM >>> To: Gao, Liming ; Kinney, Michael D >>> ; edk2-devel@lists.01.org; Brijesh Singh >>> ; Laszlo Ersek >>> Cc: Thomas.Lendacky@amd.com; leo.duran@amd.com; >> brijesh.sing@amd.com >>> Subject: RE: [RFC PATCH v1 4/5] OvmfPkg/BaseIoLibIntrinsic: import >>> BaseIoLibIntrinsic package >>> >>> On 2017-03-08 07:41:58, Gao, Liming wrote: >>>> >>>>> -----Original Message----- >>>>> From: Justen, Jordan L >>>> >>>>> One other thought is, should we consider a DxeSmm alternative .inf >>>>> for BaseIoLibIntrinsic.inf? In that case we could use a global >>>>> variable to help out. Maybe this could prevent the concern that >>>>> might drive a new PCD to be added? >>>>> >>>>> -Jordan >>>> Current patch has stored the check state into data section. In PEI >>>> phase, the data section can't be written. So, every call will check >>>> CpuId. In DXE and SMM phase, the data section can be written. The >>>> first call will cache the check state. So, no DxeSmm INF is >>>> required. >>>> >>> >>> I don't think we can attempt to write a variable to memory in generic >>> SEC/PEI code. Some flash memory treat memory writes as an I/O for >>> programming purposes. I think we added >>> PcdGuidedExtractHandlerTableAddress for this reason. This is why I >>> suggested that maybe we could add a DXE/SMM .inf where we could >> assume >>> writeable global variables exit. >>> >>> -Jordan >> >> I get your point. So, I suggest to always check SEV in BaseIoLib. If people >> meet with the performance issue, DXE/SMM version IoLib can be added >> later. >> >