Re: [PATCH v3 0/4] BaseIoFifoLib

["Duran, Leo" <leo.duran@amd.com>]

> -----Original Message-----
> From: Gao, Liming [mailto:liming.gao@intel.com]
> Sent: Sunday, January 08, 2017 9:11 PM
> To: Duran, Leo <leo.duran@amd.com>; Justen, Jordan L
> <jordan.l.justen@intel.com>; 'Laszlo Ersek' <lersek@redhat.com>; edk2-
> devel@lists.01.org
> Cc: Singh, Brijesh <brijesh.singh@amd.com>; Fan, Jeff <jeff.fan@intel.com>;
> Kinney, Michael D <michael.d.kinney@intel.com>; Ma, Maurice
> <maurice.ma@intel.com>; Agyeman, Prince <prince.agyeman@intel.com>;
> Ni, Ruiyu <ruiyu.ni@intel.com>; Steele, Kelly <kelly.steele@intel.com>; Wei,
> David <david.wei@intel.com>; Guo, Mang <mang.guo@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'.
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 <jordan.l.justen@intel.com>; 'Laszlo Ersek'
> ><lersek@redhat.com>; Gao, Liming <liming.gao@intel.com>; edk2-
> >devel@lists.01.org
> >Cc: Singh, Brijesh <brijesh.singh@amd.com>; Fan, Jeff
> ><jeff.fan@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
> >Ma, Maurice <maurice.ma@intel.com>; Agyeman, Prince
> ><prince.agyeman@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Steele,
> >Kelly <kelly.steele@intel.com>; Wei, David <david.wei@intel.com>; Guo,
> >Mang <mang.guo@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.justen@intel.com]
> >> Sent: Friday, January 06, 2017 6:50 PM
> >> To: Duran, Leo <leo.duran@amd.com>; 'Laszlo Ersek'
> >> <lersek@redhat.com>; Gao, Liming <liming.gao@intel.com>;
> >> edk2-devel@lists.01.org
> >> Cc: Singh, Brijesh <brijesh.singh@amd.com>; Fan, Jeff
> >> <jeff.fan@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
> >> Ma, Maurice <maurice.ma@intel.com>; Agyeman, Prince
> >> <prince.agyeman@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Steele,
> >> Kelly <kelly.steele@intel.com>; Wei, David <david.wei@intel.com>;
> >> Guo, Mang <mang.guo@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:lersek@redhat.com]
> >> > > Sent: Friday, January 06, 2017 5:12 AM
> >> > > To: Gao, Liming <liming.gao@intel.com>; Duran, Leo
> >> > > <leo.duran@amd.com>; edk2-devel@lists.01.org
> >> > > <edk2-devel@ml01.01.org>
> >> > > Cc: Singh, Brijesh <brijesh.singh@amd.com>; Justen, Jordan L
> >> > > <jordan.l.justen@intel.com>; Fan, Jeff <jeff.fan@intel.com>;
> >> > > Kinney, Michael D <michael.d.kinney@intel.com>; Ma, Maurice
> >> > > <maurice.ma@intel.com>; Agyeman, Prince
> >> <prince.agyeman@intel.com>;
> >> > > Ni, Ruiyu <ruiyu.ni@intel.com>; Steele, Kelly
> >> > > <kelly.steele@intel.com>; Wei, David <david.wei@intel.com>; Guo,
> >> > > Mang <mang.guo@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