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 0363F81AD5 for ; Mon, 9 Jan 2017 07:13:52 -0800 (PST) Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (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 7EE713B717; Mon, 9 Jan 2017 15:13:52 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-74.phx2.redhat.com [10.3.116.74]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v09FDmbF001578; Mon, 9 Jan 2017 10:13:49 -0500 To: "Duran, Leo" , "'Gao, Liming'" , "Justen, Jordan L" , "edk2-devel@lists.01.org" References: <1483652965-14357-1-git-send-email-leo.duran@amd.com> <4A89E2EF3DFEDB4C8BFDE51014F606A14D6CA2BD@shsmsx102.ccr.corp.intel.com> <9bf84287-cdec-b5c0-26c6-16f5cd1e453f@redhat.com> <148375017874.13659.3185969071926290103@jljusten-ivb> <4A89E2EF3DFEDB4C8BFDE51014F606A14D6CAC20@shsmsx102.ccr.corp.intel.com> Cc: "Singh, Brijesh" , "Fan, Jeff" , "Kinney, Michael D" , "Ma, Maurice" , "Agyeman, Prince" , "Ni, Ruiyu" , "Steele, Kelly" , "Wei, David" , "Guo, Mang" From: Laszlo Ersek Message-ID: <07b6ae9d-fa79-6e9e-2320-8cbef093d8a7@redhat.com> Date: Mon, 9 Jan 2017 16:13:47 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Mon, 09 Jan 2017 15:13:52 +0000 (UTC) Subject: Re: [PATCH v3 0/4] BaseIoFifoLib 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: Mon, 09 Jan 2017 15:13:52 -0000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit On 01/09/17 15:22, Duran, Leo wrote: > >> -----Original Message----- >> From: Gao, Liming [mailto:liming.gao@intel.com] >> Sent: Sunday, January 08, 2017 9:11 PM >> To: Duran, Leo ; Justen, Jordan L >> ; 'Laszlo Ersek' ; edk2- >> devel@lists.01.org >> Cc: Singh, Brijesh ; Fan, Jeff ; >> Kinney, Michael D ; Ma, Maurice >> ; Agyeman, Prince ; >> Ni, Ruiyu ; Steele, Kelly ; Wei, >> David ; Guo, Mang >> Subject: RE: [PATCH v3 0/4] BaseIoFifoLib >> >> Leo: >> IoLib Library class is designed from the functionality, not code >> implementation. So, many IO operations are included in this library class. If >> developers want to use IO API, they only need to check IoLib library class. >> After add new APIs, we need to update all IoLib library instances to >> implement them. And, if any library API implementation has the different >> version, the full library instance will have to be copied to another instance. I >> know your concern is to duplicate the library implementation. But, I think this >> is the separate topic to optimize the library implementation and reuse the >> same source file. Other library instances may have the same issue. So, I >> suggest you submit bugzilla for this optimization request. We will figure out >> the solution and review it in this mail list. >> >> Thanks >> Liming > [Duran, Leo] > Hi Liming, > > I'm not sure I follow what you mean by an 'optimization request'. I think under "optimization", Liming means eliminating code duplication between IoLib library instances. That is, if I understand correctly, Liming and Jordan suggest that you first add the new interfaces to the IoLib class header, add (--> duplicate) the implementation to all library instances, then file a BZ about eliminating code duplication between the library instances. This is my understanding anyway. Thanks Laszlo > At present IoLIb does *not* include the Fifo routines that I've referred to, so I'm simply proposing to wrap the Fifo routines into in a library. > Moreover, as you just said, I’m also proposing not using IoLib to avoid having to duplicate all of the functionality in IoLib. > > Can you please give me a bit more detail as to what the 'optimization request' would be? > (i.e., should that request read exactly as I've proposed so far, proposing the creation of an IoFifoLib?) > I'll submit Bugzilla once I better understand what needs to be in it. > > Thanks, > Leo > > > >>> -----Original Message----- >>> From: Duran, Leo [mailto:leo.duran@amd.com] >>> Sent: Sunday, January 08, 2017 1:17 AM >>> To: Justen, Jordan L ; 'Laszlo Ersek' >>> ; Gao, Liming ; edk2- >>> devel@lists.01.org >>> Cc: Singh, Brijesh ; Fan, Jeff >>> ; Kinney, Michael D ; >>> Ma, Maurice ; Agyeman, Prince >>> ; Ni, Ruiyu ; Steele, >>> Kelly ; Wei, David ; Guo, >>> Mang >>> Subject: RE: [PATCH v3 0/4] BaseIoFifoLib >>> >>> Jordan, Liming, et al, >>> >>> It turns out that the runtime enablement of SEV feature that I referred >>> to can be detected in hardware; so instead of requiring 'driver' code >>> to set a dynamic PCD, the override Fifo routines could do a runtime >>> check like this: >>> >>> // In override version of the Fifo library >>> fifo_foo() >>> { >>> If (SEV_Enabled()) { >>> // don't use REP ins/outs >>> } else { >>> // use REP ins/outs >>> } >>> } >>> In essence we already have a hardware-based dynamic PCD, so the idea is >>> to leverage it. >>> >>> And since we're interested in overriding just the Fifo routines, it >>> would make better sense to keep them in a separate library (as proposed in >> the patch set). >>> Leo. >>> >>>> -----Original Message----- >>>> From: Jordan Justen [mailto:jordan.l.justen@intel.com] >>>> Sent: Friday, January 06, 2017 6:50 PM >>>> To: Duran, Leo ; 'Laszlo Ersek' >>>> ; Gao, Liming ; >>>> edk2-devel@lists.01.org >>>> Cc: Singh, Brijesh ; Fan, Jeff >>>> ; Kinney, Michael D ; >>>> Ma, Maurice ; Agyeman, Prince >>>> ; Ni, Ruiyu ; Steele, >>>> Kelly ; Wei, David ; >>>> Guo, Mang >>>> Subject: RE: [PATCH v3 0/4] BaseIoFifoLib >>>> >>>> On 2017-01-06 07:23:47, Duran, Leo wrote: >>>>> >>>>> >>>>>> -----Original Message----- >>>>>> From: Laszlo Ersek [mailto:lersek@redhat.com] >>>>>> Sent: Friday, January 06, 2017 5:12 AM >>>>>> To: Gao, Liming ; Duran, Leo >>>>>> ; edk2-devel@lists.01.org >>>>>> >>>>>> Cc: Singh, Brijesh ; Justen, Jordan L >>>>>> ; Fan, Jeff ; >>>>>> Kinney, Michael D ; Ma, Maurice >>>>>> ; Agyeman, Prince >>>> ; >>>>>> Ni, Ruiyu ; Steele, Kelly >>>>>> ; Wei, David ; Guo, >>>>>> Mang >>>>>> Subject: Re: [PATCH v3 0/4] BaseIoFifoLib >>>>>> >>>>>> On 01/06/17 07:02, Gao, Liming wrote: >>>>>>> Leo: >>>>>>> FifoIo is one width type of EFI_CPU_IO_PROTOCOL_WIDTH. So, how >>>>>>> about add new APIs into IoLib together with other Io APIs? If >>>>>>> so, no new library class is required. Platform DSC files are >>>>>>> not required to be changed. >>>>>> >>>>>> But then all of the IoLib instances will have to be extended too: >>>>>> >>>>>> IntelFrameworkPkg/Library/DxeIoLibCpuIo/DxeIoLibCpuIo.inf >>>>>> MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf >>>>>> MdePkg/Library/DxeIoLibCpuIo2/DxeIoLibCpuIo2.inf >>>>>> MdePkg/Library/DxeIoLibEsal/DxeIoLibEsal.inf >>>>>> MdePkg/Library/PeiIoLibCpuIo/PeiIoLibCpuIo.inf >>>>>> MdePkg/Library/SmmIoLibSmmCpuIo2/SmmIoLibSmmCpuIo2.inf >>>>>> >>>>>> Thanks, >>>>>> Laszlo >>>>>> >>>>> [Duran, Leo] Correct. >>>>> As I mentioned, one of the reasons for the new IoFifo library is to >>>>> be able to override it without having to duplicate the complete IoLib. >>>>> >>>> >>>> I agree with Liming about adding the functions to IoLib instead. >>>> >>>> Perhaps a PCD could be added to control if rep i/o instructions are >>>> used. >>>> >>>> -Jordan