On 01/09/17 15:22, Duran, Leo wrote: > >> -----Original Message----- >> From: Gao, Liming [mailto:liming....@intel.com] >> Sent: Sunday, January 08, 2017 9:11 PM >> To: Duran, Leo <leo.du...@amd.com>; Justen, Jordan L >> <jordan.l.jus...@intel.com>; 'Laszlo Ersek' <ler...@redhat.com>; edk2- >> de...@lists.01.org >> Cc: Singh, Brijesh <brijesh.si...@amd.com>; Fan, Jeff <jeff....@intel.com>; >> Kinney, Michael D <michael.d.kin...@intel.com>; Ma, Maurice >> <maurice...@intel.com>; Agyeman, Prince <prince.agye...@intel.com>; >> Ni, Ruiyu <ruiyu...@intel.com>; Steele, Kelly <kelly.ste...@intel.com>; Wei, >> David <david....@intel.com>; Guo, Mang <mang....@intel.com> >> 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.du...@amd.com] >>> Sent: Sunday, January 08, 2017 1:17 AM >>> To: Justen, Jordan L <jordan.l.jus...@intel.com>; 'Laszlo Ersek' >>> <ler...@redhat.com>; Gao, Liming <liming....@intel.com>; edk2- >>> de...@lists.01.org >>> Cc: Singh, Brijesh <brijesh.si...@amd.com>; Fan, Jeff >>> <jeff....@intel.com>; Kinney, Michael D <michael.d.kin...@intel.com>; >>> Ma, Maurice <maurice...@intel.com>; Agyeman, Prince >>> <prince.agye...@intel.com>; Ni, Ruiyu <ruiyu...@intel.com>; Steele, >>> Kelly <kelly.ste...@intel.com>; Wei, David <david....@intel.com>; Guo, >>> Mang <mang....@intel.com> >>> 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.jus...@intel.com] >>>> Sent: Friday, January 06, 2017 6:50 PM >>>> To: Duran, Leo <leo.du...@amd.com>; 'Laszlo Ersek' >>>> <ler...@redhat.com>; Gao, Liming <liming....@intel.com>; >>>> edk2-devel@lists.01.org >>>> Cc: Singh, Brijesh <brijesh.si...@amd.com>; Fan, Jeff >>>> <jeff....@intel.com>; Kinney, Michael D <michael.d.kin...@intel.com>; >>>> Ma, Maurice <maurice...@intel.com>; Agyeman, Prince >>>> <prince.agye...@intel.com>; Ni, Ruiyu <ruiyu...@intel.com>; Steele, >>>> Kelly <kelly.ste...@intel.com>; Wei, David <david....@intel.com>; >>>> Guo, Mang <mang....@intel.com> >>>> Subject: RE: [PATCH v3 0/4] BaseIoFifoLib >>>> >>>> On 2017-01-06 07:23:47, Duran, Leo wrote: >>>>> >>>>> >>>>>> -----Original Message----- >>>>>> From: Laszlo Ersek [mailto:ler...@redhat.com] >>>>>> Sent: Friday, January 06, 2017 5:12 AM >>>>>> To: Gao, Liming <liming....@intel.com>; Duran, Leo >>>>>> <leo.du...@amd.com>; edk2-devel@lists.01.org >>>>>> <edk2-de...@ml01.01.org> >>>>>> Cc: Singh, Brijesh <brijesh.si...@amd.com>; Justen, Jordan L >>>>>> <jordan.l.jus...@intel.com>; Fan, Jeff <jeff....@intel.com>; >>>>>> Kinney, Michael D <michael.d.kin...@intel.com>; Ma, Maurice >>>>>> <maurice...@intel.com>; Agyeman, Prince >>>> <prince.agye...@intel.com>; >>>>>> Ni, Ruiyu <ruiyu...@intel.com>; Steele, Kelly >>>>>> <kelly.ste...@intel.com>; Wei, David <david....@intel.com>; Guo, >>>>>> Mang <mang....@intel.com> >>>>>> 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 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
