From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.byosoft.com.cn (mail.byosoft.com.cn [58.240.74.242]) by mx.groups.io with SMTP id smtpd.web09.14093.1632621665862124519 for ; Sat, 25 Sep 2021 19:01:07 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=none, err=permanent DNS error (domain: byosoft.com.cn, ip: 58.240.74.242, mailfrom: gaoliming@byosoft.com.cn) Received: from DESKTOPS6D0PVI ([58.246.60.130]) (envelope-sender ) by 192.168.6.13 with ESMTP for ; Sun, 26 Sep 2021 10:00:57 +0800 X-WM-Sender: gaoliming@byosoft.com.cn X-Originating-IP: 58.246.60.130 X-WM-AuthFlag: YES X-WM-AuthUser: gaoliming@byosoft.com.cn From: "gaoliming" To: "'Ni, Ray'" , , "'Chan, Amy'" , "'Andrew Fish'" Cc: "'Kinney, Michael D'" , "'Gao, Liming'" , "'Liu, Zhiguang'" , "'Wang, Jian J'" , "'Gao, Zhichao'" References: <001a01d7aa99$d744af80$85ce0e80$@byosoft.com.cn> <003401d7aaa1$c7166830$55433890$@byosoft.com.cn> <16A70FAC1585C7C1.27516@groups.io> In-Reply-To: Subject: =?UTF-8?B?5Zue5aSNOiBbZWRrMi1kZXZlbF0gUkZDOiBBZGQgQmFzZUxpYi9RdWlja1NvcnQgaW4gTWRlUGtn?= Date: Sun, 26 Sep 2021 10:00:57 +0800 Message-ID: <00cf01d7b27a$59145d20$0b3d1760$@byosoft.com.cn> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQLnLItH7GrFLJqnU7rRMhL6o/j0rALHrNCxAkD3tuQBn1kvPgNjcDkYAU1KPNYBRs4xIAJq1+G/qR5HOeA= Content-Type: multipart/alternative; boundary="----=_NextPart_000_00D0_01D7B2BD.673B46A0" Content-Language: zh-cn ------=_NextPart_000_00D0_01D7B2BD.673B46A0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Ray: I may suggest to always require BufferOneElement. The consumer code may n= ot know ElementSize. To avoid the error, the consumer must allocate buffer = for BufferOneElement. If so, BufferOneElement is the required parameter. =20 Thanks Liming =E5=8F=91=E4=BB=B6=E4=BA=BA: Ni, Ray =20 =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: 2021=E5=B9=B49=E6=9C=8824=E6=97=A5 11= :53 =E6=94=B6=E4=BB=B6=E4=BA=BA: devel@edk2.groups.io; Ni, Ray ; Chan, Amy ; gaoliming ; '= Andrew Fish' =E6=8A=84=E9=80=81: Kinney, Michael D ; 'Gao, L= iming' ; Liu, Zhiguang ; Wang= , Jian J ; Gao, Zhichao =E4=B8=BB=E9=A2=98: RE: [edk2-devel] RFC: Add BaseLib/QuickSort in MdePkg =20 More details on new QuickSort() API: The new API needs to carry additional parameter =E2=80=9CBufferOneElement= =E2=80=9D. This parameter gives QuickSort() the temporary buffer for swapping in sor= ting. It=E2=80=99s to avoid BaseLib depends on MemoryAllocationLib. =20 =E2=80=A6 @param [in] BufferOneElement When ElementSize > 256, caller needs to provi= de a buffer whose size equals to ElementSize. It=E2=80=99s used by Q= uickSort() for swapping in sorting. When ElementSize <=3D 256, QuickSort() uses a= local stack 256-byte buffer. =20 @retval EFI_INVALID_PARAMETER When (ElementSize > 256) and (BufferOneElemen= t =3D=3D NULL). =E2=80=A6 VOID EFIAPI QuickSort ( IN OUT VOID *BufferToSort, IN CONST UINTN ElementCount, IN CONST UINTN ElementSize, IN SORT_COMPARE CompareFunction, IN VOID *BufferOneElement OPTIONAL ); =20 Any comments? =20 =20 From: devel@edk2.groups.io > On Behalf Of Ni, Ray Sent: Wednesday, September 22, 2021 2:04 PM To: Chan, Amy >; gaoliming = >; 'Andrew Fish= ' >; 'edk2-devel-groups-io' > Cc: Kinney, Michael D >; 'Gao, Liming' >; Liu, Zhiguang >; Wang, Jian J >;= Gao, Zhichao > Subject: Re: [edk2-devel] RFC: Add BaseLib/QuickSort in MdePkg =20 I don=E2=80=99t see objection so far. So, the final solution is: 1. Add QuickSort() API to BaseLib in MdePkg. 2. Update existing MdeModulePkg/SortLib to use QuickSort() in the implement= ation (proposed by Andrew Fish and Liming Gao) 3. Update UefiCpuPkg to use QuickSortLib to remove improper dependency on M= deModulePkg Thanks, Ray =20 From: Ni, Ray=20 Sent: Thursday, September 16, 2021 10:48 AM To: Chan, Amy >; gaoliming = >; 'Andrew Fish= ' >; 'edk2-devel-groups-io' > Cc: Kinney, Michael D >; 'Gao, Liming' >; Liu, Zhiguang >; Wang, Jian J >;= Gao, Zhichao > Subject: RE: [edk2-devel] RFC: Add BaseLib/QuickSort in MdePkg =20 Amy, No. We only Add QuickSort() function API into BaseLib.h. =20 From: Chan, Amy >=20 Sent: Thursday, September 16, 2021 10:46 AM To: gaoliming >= ; 'Andrew Fish' >; 'edk2-devel-gr= oups-io' > Cc: Ni, Ray >; Kinney, Michael = D >; 'Gao, = Liming' >; Liu, Zhiguan= g >; Wang, Jian J <= jian.j.wang@intel.com >; Gao, Zhichao > Subject: RE: [edk2-devel] RFC: Add BaseLib/QuickSort in MdePkg =20 Just to double confirm, will we have the null instance of QuickSort in MdeP= kg? =20 Regards, Amy =20 From: gaoliming = >=20 Sent: Thursday, September 16, 2021 10:23 AM To: 'Andrew Fish' >; 'edk2-devel-= groups-io' > Cc: Ni, Ray >; Kinney, Michael = D >; 'Gao, = Liming' >; Liu, Zhiguan= g >; Wang, Jian J <= jian.j.wang@intel.com >; Gao, Zhichao >; Chan, Amy > Subject: =E5=9B=9E=E5=A4=8D: [edk2-devel] RFC: Add BaseLib/QuickSort in Mde= Pkg =20 Andrew: Thanks for your suggestion. I think your idea is better. We add new QuickS= ort() API to BaseLib, and update SortLib library instance to consume BaseLi= b QuickSort() API. This way has no change in current SortLib library class.= It is the compatible solution.=20 =20 Thanks Liming =E5=8F=91=E4=BB=B6=E4=BA=BA: Andrew Fish >=20 =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: 2021=E5=B9=B49=E6=9C=8816=E6=97=A5 10= :13 =E6=94=B6=E4=BB=B6=E4=BA=BA: edk2-devel-groups-io >; Liming Gao > =E6=8A=84=E9=80=81: Ni, Ray >; = Mike Kinney = >; Gao, Liming >; Liu,= Zhiguang >; Wang, = Jian J >; Gao, Zhicha= o >; Chan, Amy > =E4=B8=BB=E9=A2=98: Re: [edk2-devel] RFC: Add BaseLib/QuickSort in MdePkg =20 =20 =20 On Sep 15, 2021, at 6:26 PM, gaoliming > wrote: =20 Ray: SortLib has been added since 2015. I would suggest to still keep this libr= ary class. To resolve the package dependency, my proposal is to move the li= brary class header file SortLib.h from MdeModulePkg to MdePkg, and still ke= ep the library instance in MdeModulePkg. This proposal has no impact on the= existing platform.=20 =20 =20 If we add QuickSort() API to the BaseLib can we not just port the existing = MdeModulePkg/SortLib to use QuickSort() in the implementation? Or is there = some other way to add the new thing in a backward compatible way. =20 Thanks, =20 Andrew Fish =20 Thanks Liming =E5=8F=91=E4=BB=B6=E4=BA=BA: devel@edk2.grou= ps.io < devel@edk2.groups.io> =E4=BB=A3=E8=A1= =A8 Ni, Ray =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: 2021=E5=B9=B49=E6=9C=8814=E6=97=A5 14= :15 =E6=94=B6=E4=BB=B6=E4=BA=BA: Kinney, Michael D < michael.d.kinney@intel.com>; Gao, Liming < liming.gao@intel.com>; Liu, Zhiguang < zhiguang.liu@intel.com>; Wang, Jian J < = jian.j.wang@intel.com>; Gao, Zhichao < zhich= ao.gao@intel.com> =E6=8A=84=E9=80=81: devel@edk2.groups.io; Ch= an, Amy < amy.chan@intel.com> =E4=B8=BB=E9=A2=98: [edk2-devel] RFC: Add BaseLib/QuickSort in MdePkg =20 Hi package maintainers of MdePkg, MdeModulePkg and ShellPkg, community, =20 A commit ( UefiCpuPkg/CpuCacheInfoLib: Sort CpuCacheInfo array) to= UefiCpuPkg let UefiCpuPkg depend on MdeModulePkg because the SortLib class and instances a= re all in MdeModulePkg. =20 UefiCpuPkg depending on MdeModulePkg breaks the rule that =E2=80=9CUefiCpuP= kg should ONLY depend on MdePkg=E2=80=9D. =20 To address this issue, there are two approaches: 1. Duplicate the sort logic in UefiCpuPkg to not depend on MdeModulePkg/Sor= tLib 2. Add QuickSort() API to BaseLib in MdePkg. =20 Approach #2 (MdePkg/BaseLib/QuickSort) makes more sense because quick sort = is a standard algorithm. We encourage consumers to update their code to use the quick sort in MdePkg= and gradually deprecate today=E2=80=99s MdeModulePkg/SortLib. =20 If you don=E2=80=99t have concerns, I plan to: 1. =E2=80=9CAdd QuickSort() to BaseLib=E2=80=9D and update all existing con= sumers to use this API instead. VOID EFIAPI QuickSort ( IN OUT VOID *BufferToSort, IN CONST UINTN Count, IN CONST UINTN ElementSize, IN SORT_COMPARE CompareFunction ); =20 2. =E2=80=9CAdd new ShellPkg/SortCompareLib=E2=80=9D Background: ShellPkg requires to sort devicepath/string so 3 APIs in UefiSo= rtLib (DevicePathCompare, StringNoCaseCompare, StringCompare) are provided = for Shell usage. we can move the 3 APIs to the SortCompareLib and update Sh= ell code to use BaseLib/QuickSort directly, with the sort compare function = from SortCompareLib. =20 Any concerns? =20 Thanks, Ray =20 ------=_NextPart_000_00D0_01D7B2BD.673B46A0 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable

Ray:

=C2= =A0 I may suggest to always require BufferOneElement. The consumer code may= not know ElementSize. To avoid the error, the consumer must allocate buffe= r for BufferOneElement. If so, BufferOneElement is the required parameter.<= o:p>

 

Thanks

Limi= ng

=E5=8F=91=E4=BB= =B6=E4=BA=BA: Ni, Ray <ray.ni@in= tel.com>
=E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: 2021=E5=B9=B49=E6=9C=8824=E6=97=A5 11:53
=E6=94= =B6=E4=BB=B6=E4=BA=BA: dev= el@edk2.groups.io; Ni, Ray <ray.ni@intel.com>; Chan, Amy <amy.chan= @intel.com>; gaoliming <gaoliming@byosoft.com.cn>; 'Andrew Fish' &= lt;afish@apple.com>
=E6=8A=84=E9=80=81:<= /span> Kinney, Michael D <michael.d.kinney@intel.= com>; 'Gao, Liming' <liming.gao@intel.com>; Liu, Zhiguang <zhig= uang.liu@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Gao, Zh= ichao <zhichao.gao@intel.com>
=E4=B8=BB=E9=A2=98: RE: [edk2-devel] RFC: Add BaseLi= b/QuickSort in MdePkg

 

More details on new QuickSort() API:

  The new API needs to carry additional parameter =E2=80= =9CBufferOneElement=E2=80=9D.

  This parameter gives QuickSort() the temporary buffer for swapping = in sorting.

  It=E2=80= =99s to avoid BaseLib depends on MemoryAllocationLib.

=

 

=E2=80=A6<= /span>

@param [in] BufferOneElement =  When ElementSize > 256, caller needs to provide a buffer whose siz= e
           &nbs= p;            &= nbsp;     equals to ElementSize. It=E2=80=99s used by Q= uickSort() for swapping in sorting.

         &nb= sp;            =         When ElementSize <=3D 256, Qu= ickSort() uses a local stack 256-byte buffer.

 

@retval EFI_INVALID_PARAMETER When (ElementSize >= 256) and (BufferOneElement =3D=3D NULL).

=E2=80=A6

VOID

EFIAPI<= /p>

QuickSort (

  IN&nbs= p;OUT   VOID         = ;            &n= bsp;   *BufferToSort,

  IN CONST UINTN     = ;            &n= bsp;      ElementCount,

  IN CONST UINTN  =             &nb= sp;         Elem= entSize,

  IN   = ;    SORT_COMPARE      &n= bsp;          CompareFunction,

  IN      = VOID           &nbs= p;             = *BufferOneElement OPTIONAL

  );

 

Any comments?

 

 

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of = Ni, Ray
Sent: Wednesday, September 22, 2021 2:04 PM
To:= Chan, Amy <amy.chan@intel.com= >; gaoliming <gaolimi= ng@byosoft.com.cn>; 'Andrew Fish' <afish@apple.com>; 'edk2-devel-groups-io' <devel@edk2.groups.io>
Cc: Kinney, Mi= chael D <michael.d.kinney@= intel.com>; 'Gao, Liming' <liming.gao@intel.com>; Liu, Zhiguang <zhiguang.liu@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Gao, Zhich= ao <zhichao.gao@intel.com&g= t;
Subject: Re: [edk2-devel] RFC: Add BaseLib/QuickSort in MdePkg=

<= o:p> 

I don=E2=80=99t see obj= ection so far.

So, the final = solution is:

  1. Add QuickSort() API to BaseLib in MdePkg.
  2. Update existing MdeModulePkg/SortLib to use QuickSort() i= n the implementation (proposed by Andrew Fish and Liming Gao)
  3. Update UefiCpuPkg to use QuickSortLib to remove improper dependency on Mde= ModulePkg

Thanks,<= /o:p>

Ray

 

From: Ni, Ray
Sent: Thursday, Septemb= er 16, 2021 10:48 AM
To: Chan, Amy <amy.chan@intel.com>; gaoliming <gaoliming@byosoft.com.cn>; 'Andrew Fish' <= afish@apple.com>; 'edk2-devel-gro= ups-io' <devel@edk2.groups.io>
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; 'Gao, Liming' <liming.gao@intel.com>; Liu, Zhiguan= g <Zhiguang.Liu@intel.com&= gt;; Wang, Jian J <jian.j.wang@= intel.com>; Gao, Zhichao <zhichao.gao@intel.com>
Subject: RE: [edk2-devel] RFC: Ad= d BaseLib/QuickSort in MdePkg

 

Amy,

No. We only Add Qu= ickSort() function API into BaseLib.h.

 

From: Chan, Amy <amy.chan@intel.com>
Sent: Thursday, September 1= 6, 2021 10:46 AM
To: gaoliming <gaoliming@byosoft.com.cn>; 'Andrew Fish' <afish@apple.com>; 'edk2-devel-groups-io'= <devel@edk2.groups.io>Cc: Ni, Ray <ray.ni@intel.c= om>; Kinney, Michael D <michael.d.kinney@intel.com>; 'Gao, Liming' <liming.gao@intel.com>; Liu, Zhiguang <zhiguang.liu@intel.com>; Wang= , Jian J <jian.j.wang@intel.com= >; Gao, Zhichao <zhichao= .gao@intel.com>
Subject: RE: [edk2-devel] RFC: Add BaseLib= /QuickSort in MdePkg

=  

Jus= t to double confirm, will we have the null instance of QuickSort in MdePkg?=

 

=

Regards,

Amy

 

From: gaoliming <gaoliming@byos= oft.com.cn>
Sent: Thursday, September 16, 2021 10:23 AMTo: 'Andrew Fish' <afish@app= le.com>; 'edk2-devel-groups-io' <devel@edk2.groups.io>
Cc: Ni, Ray <ray.ni@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com&g= t;; 'Gao, Liming' <liming.gao@in= tel.com>; Liu, Zhiguang <zhiguang.liu@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; Chan, Amy <= ;amy.chan@intel.com>
Sub= ject: =E5=9B=9E=E5=A4= =8D: [edk2-devel] RFC: Add BaseLib/QuickSort in MdePkg

 =

Andrew:

 <= span lang=3DEN-US style=3D'font-size:10.5pt;font-family:=E7=AD=89=E7=BA=BF'= >Thanks for your suggestion. I think your idea is better. We add new QuickS= ort() API to BaseLib, and update SortLib library instance to consume BaseLi= b QuickSort() API. This way has no change in current SortLib library class.= It is the compatible solution.

=  

Thanks

Liming

=E5=8F=91=E4=BB=B6=E4=BA=BA: Andrew Fish <afish@apple.= com>
=E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: 2021=E5=B9=B49=E6=9C=8816=E6=97=A5 10:13
=E6=94= =B6=E4=BB=B6=E4=BA=BA: edk= 2-devel-groups-io <devel@edk2.gr= oups.io>; Liming Gao <gaoliming@byosoft.com.cn>
=E6=8A=84=E9=80=81: Ni, Ray <ray.ni@intel.com>; Mike Kinney <michael.d.kinney@intel.com>; Gao, Liming = <liming.gao@intel.com>; L= iu, Zhiguang <zhiguang.liu@int= el.com>; Wang, Jian J <j= ian.j.wang@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; Chan, Amy <amy.chan@intel.com>
=E4=B8=BB=E9= =A2=98: Re: [edk2-devel] R= FC: Add BaseLib/QuickSort in MdePkg

 

 

 <= /o:p>

=

On Sep 15, 2021, at 6:26 PM, g= aoliming <gaoliming@byosoft.= com.cn> wrote:

 

= Ray:

 SortLib has been add= ed since 2015. I would suggest to still keep this library class. To resolve= the package dependency, my proposal is to move the library class header fi= le SortLib.h from MdeModulePkg to MdePkg, and still keep the library instan= ce in MdeModulePkg. This proposal has no impact on the existing platform. 

 <= span lang=3DEN-US style=3D'font-size:11.0pt;font-family:"Calibri",sans-seri= f'>

 

If we add QuickSort() API to the BaseLib can we not= just port the existing MdeModulePkg/SortLib to use QuickSort() in the= implementation? Or is there some other way to add the new thing in a backw= ard compatible way.

 

Thanks,

 

Andrew Fish

&nb= sp;

Thanks

Liming

 

Hi package maintainers of MdePkg, MdeModulePkg and ShellPkg, comm= unity,

 =

A commit (UefiCpuPkg/CpuCacheInfoLib: Sort Cpu= CacheInfo array) to UefiCpuPkg let
UefiCpuPkg depend on MdeMo= dulePkg because the SortLib class and instances are all in MdeModulePkg.

 

UefiCpuPkg depending on MdeModu= lePkg breaks the rule that =E2=80=9CUefiCpuPkg should ONLY depend on MdePkg= =E2=80=9D.

 =

To address this i= ssue, there are two approaches:

  1. Duplicate the sort= logic in UefiCpuPkg to not depend on MdeModulePkg/SortLib
  2. Add QuickSort() API to BaseLib in MdePkg.

 

Approach #2 (MdePkg/BaseLib/Qui= ckSort) makes more sense because quick sort is a standard algorithm.

We encourage consumers t= o update their code to use the quick sort in MdePkg and gradually deprecate= today=E2=80=99s MdeModulePkg/SortLib.

 

If you don=E2=80=99t have concerns, I plan to:<= /p>

  1. =E2=80=9CAdd QuickSort() to BaseLib=E2=80=9D and update all existing= consumers to use this API instead.

VO= ID

=

EFIAPI

QuickSort (<= /o:p>

  IN OUT VOID  =             &nb= sp;    *BufferToSort,<= span lang=3DEN-US style=3D'font-size:11.0pt;font-family:"Calibri",sans-seri= f'>

  IN CONST UIN= TN            &= nbsp;   Count,

  IN CONST UINTN  =             &nb= sp; ElementSize,

  IN       = SORT_COMPARE         CompareFu= nction

  );<= span lang=3DEN-US style=3D'font-size:11.0pt;font-family:"Calibri",sans-seri= f'>

 

  1. =E2=80=9CAdd new ShellPkg/SortCompareLib=E2=80=9D

= Background: ShellPkg requires to sort devicepath/string so 3 APIs in Ue= fiSortLib (DevicePathCompare, StringNoCaseCompare, StringCompare) are provi= ded for Shell usage. we can move the 3 APIs to the SortCompareLib and updat= e Shell code to use BaseLib/QuickSort directly, with the sort compare funct= ion from SortCompareLib.

 

Any = concerns?

 =

Thanks,

Ray

&= nbsp;

------=_NextPart_000_00D0_01D7B2BD.673B46A0--