From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from walk.intel-email.com (walk.intel-email.com [101.227.64.242]) by mx.groups.io with SMTP id smtpd.web09.993.1664501538820400812 for ; Thu, 29 Sep 2022 18:32:19 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@byosoft.com.cn header.s=cloud-union header.b=JI7dplt1; spf=pass (domain: byosoft.com.cn, ip: 101.227.64.242, mailfrom: gaoliming@byosoft.com.cn) Received: from walk.intel-email.com (localhost [127.0.0.1]) by walk.intel-email.com (Postfix) with ESMTP id 32F03CD1F863 for ; Fri, 30 Sep 2022 09:32:16 +0800 (CST) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=byosoft.com.cn; s=cloud-union; t=1664501536; bh=cUqiM/qohP2o06WGP664YA/kmRAjbameuOJ4tHFVnUE=; h=From:To:Cc:References:In-Reply-To:Subject:Date; b=JI7dplt1DmxYVTeAVkkH/iiXI+aSCOn9Q0CqFSXLAjBuUa090zgbqdWK3zKlWrYIb 61rq7q8QFy0XCl1U0zNVAn9776L4JKWQdFl/leWYsK6pr5DN2hpKPsqZ86BDzOpB5Y TRPgP4FxcVfLQsTn/3O57i8z4Wx82qH6C7rfpBVs= Received: from localhost (localhost [127.0.0.1]) by walk.intel-email.com (Postfix) with ESMTP id 2EA5BCD1F821 for ; Fri, 30 Sep 2022 09:32:16 +0800 (CST) Received: from walk.intel-email.com (localhost [127.0.0.1]) by walk.intel-email.com (Postfix) with ESMTP id 03B50CD1F818 for ; Fri, 30 Sep 2022 09:32:16 +0800 (CST) Authentication-Results: walk.intel-email.com; none Received: from mail.byosoft.com.cn (mail.byosoft.com.cn [58.240.74.242]) by walk.intel-email.com (Postfix) with SMTP id 89FF7CD1F84D for ; Fri, 30 Sep 2022 09:32:12 +0800 (CST) Received: from DESKTOPS6D0PVI ([58.246.60.130]) (envelope-sender ) by 192.168.6.13 with ESMTP for ; Fri, 30 Sep 2022 09:32:09 +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: , , "'Ard Biesheuvel'" Cc: , , , , References: In-Reply-To: Subject: =?UTF-8?B?5Zue5aSNOiBbZWRrMi1kZXZlbF0gW1BBVENIIHYyXSBNZGVNb2R1bGVQa2cvTm9uRGlzY292ZXJhYmxlUGNpRGV2aWNlRHhlOiBBbGxvdyBwYXJ0aWFsIEZyZWVCdWZmZXI=?= Date: Fri, 30 Sep 2022 09:32:09 +0800 Message-ID: <01f301d8d46c$75962400$60c26c00$@byosoft.com.cn> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQIQvrZj1ZTEghLzKbXKkxQ/JEtg0gI4MgVBAjLGwJICqb4u9QIC7LI1AvLhf4cBvriomAJlIBFgAhsdaD8CIX+6WgLZTB4lAcDOHqwDCG3CYqym7yLw Sender: "gaoliming" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Content-Language: zh-cn Jeff:=20 I have no comments for this change. Acked-by: Liming Gao Thanks Liming > -----=E9=82=AE=E4=BB=B6=E5=8E=9F=E4=BB=B6----- > =E5=8F=91=E4=BB=B6=E4=BA=BA: devel@edk2.groups.io = =E4=BB=A3=E8=A1=A8 Jeff Brasen > via groups.io > =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: 2022=E5=B9=B49=E6=9C=8830=E6=97=A5 = 0:20 > =E6=94=B6=E4=BB=B6=E4=BA=BA: Ard Biesheuvel ; devel@edk2= .groups.io > =E6=8A=84=E9=80=81: hao.a.wu@intel.com; ray.ni@intel.com; quic_llindhol@q= uicinc.com; > ardb+tianocore@kernel.org; jian.j.wang@intel.com; gaoliming > > =E4=B8=BB=E9=A2=98: Re: [edk2-devel] [PATCH v2] > MdeModulePkg/NonDiscoverablePciDeviceDxe: Allow partial FreeBuffer >=20 > MdeModulePkg maintainers, Any comments on this? >=20 > > -----Original Message----- > > From: Ard Biesheuvel > > Sent: Wednesday, September 21, 2022 10:32 AM > > To: devel@edk2.groups.io; Jeff Brasen > > Cc: hao.a.wu@intel.com; ray.ni@intel.com; quic_llindhol@quicinc.com; > > ardb+tianocore@kernel.org > > Subject: Re: [edk2-devel] [PATCH v2] > > MdeModulePkg/NonDiscoverablePciDeviceDxe: Allow partial FreeBuffer > > > > External email: Use caution opening links or attachments > > > > > > On Wed, 21 Sept 2022 at 18:27, Jeff Brasen via groups.io > > wrote: > > > > > > Anything else needed to get this merged? > > > > > > > That is up to the MdeModulePkg maintainers. > > > > > > -----Original Message----- > > > > From: Ard Biesheuvel > > > > Sent: Thursday, September 8, 2022 9:55 AM > > > > To: Jeff Brasen > > > > Cc: devel@edk2.groups.io; hao.a.wu@intel.com; ray.ni@intel.com; > > > > quic_llindhol@quicinc.com; ardb+tianocore@kernel.org > > > > Subject: Re: [edk2-devel] [PATCH v2] > > > > MdeModulePkg/NonDiscoverablePciDeviceDxe: Allow partial FreeBuffer > > > > > > > > External email: Use caution opening links or attachments > > > > > > > > > > > > On Thu, 8 Sept 2022 at 17:39, Jeff Brasen wrot= e: > > > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > From: Ard Biesheuvel > > > > > > Sent: Monday, August 15, 2022 8:42 AM > > > > > > To: devel@edk2.groups.io; Jeff Brasen > > > > > > Cc: hao.a.wu@intel.com; ray.ni@intel.com; > > > > > > quic_llindhol@quicinc.com; > > > > > > ardb+tianocore@kernel.org > > > > > > Subject: Re: [edk2-devel] [PATCH v2] > > > > > > MdeModulePkg/NonDiscoverablePciDeviceDxe: Allow partial > > > > > > FreeBuffer > > > > > > > > > > > > External email: Use caution opening links or attachments > > > > > > > > > > > > > > > > > > On Fri, 5 Aug 2022 at 18:56, Jeff Brasen via groups.io > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > > From: Ard Biesheuvel > > > > > > > > Sent: Tuesday, August 2, 2022 10:51 AM > > > > > > > > To: Jeff Brasen > > > > > > > > Cc: devel@edk2.groups.io; hao.a.wu@intel.com; > > > > > > > > ray.ni@intel.com; quic_llindhol@quicinc.com; > > > > > > > > ardb+tianocore@kernel.org > > > > > > > > Subject: Re: [PATCH v2] > > MdeModulePkg/NonDiscoverablePciDeviceDxe: > > > > > > > > Allow partial FreeBuffer > > > > > > > > > > > > > > > > External email: Use caution opening links or attachments > > > > > > > > > > > > > > > > > > > > > > > > On Tue, 2 Aug 2022 at 17:32, Jeff Brasen > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > > > > From: Ard Biesheuvel > > > > > > > > > > Sent: Friday, July 29, 2022 9:48 AM > > > > > > > > > > To: Jeff Brasen > > > > > > > > > > Cc: devel@edk2.groups.io; hao.a.wu@intel.com; > > > > > > > > > > ray.ni@intel.com; quic_llindhol@quicinc.com; > > > > > > > > > > ardb+tianocore@kernel.org > > > > > > > > > > Subject: Re: [PATCH v2] > > > > MdeModulePkg/NonDiscoverablePciDeviceDxe: > > > > > > > > > > Allow partial FreeBuffer > > > > > > > > > > > > > > > > > > > > External email: Use caution opening links or attachment= s > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Thu, 28 Jul 2022 at 13:25, Jeff Brasen > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Adding Leif/Ard to CC incase they have any comments o= n > > this patch. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > This generally looks ok to me. I just wonder if it > > > > > > > > > > wouldn't be simpler to reuse the existing allocation > > > > > > > > > > descriptor if it is not being freed entirely. Given the > > > > > > > > > > [presumably] the most common case is to allocate and > > > > > > > > > > then free some pages at the end, lowering the page coun= t > > > > > > > > > > on the existing descriptor would cover most cases, and > > > > > > > > > > we'd only need to allocate new ones if pages are being > > > > > > > > > > freed at the start or in > > > > > > > > the middle. > > > > > > > > > > > > > > > > > > There is often freeing at the beginning as well as this i= s > > > > > > > > > being used to create > > > > > > > > a 64K aligned section of memory in the case. So it over > > > > > > > > allocates and the free's some at the beginning and the end. > > > > > > > > I could probably make it detect and use that but figured > > > > > > > > this code would support all cases and required less case sp= ecific > > detection. > > > > > > > > > > > > > > > > > > > > > > > > > Ah interesting. Would it help if the allocate routine > > > > > > > > aligned allocations to their size? > > > > > > > > > > > > > > The PciIo->AllocateBuffer function doesn't support passing th= e > > > > > > > request in so > > > > > > we would need to know that info beforehand. The current calling > > > > > > in the XHCI driver does a free at the beginning and then the en= d > > > > > > of the buffer so we could the existing allocation tracker but > > > > > > figured it would be better to correct the function just in case > > > > > > someone called it to free > > > > in the middle. > > > > > > > > > > > > > > > > > > > What I was wondering is whether such allocations are themselves > > > > > > multiples of 64k. This is perhaps orthogonal to the issue this > > > > > > patch addresses, as we'' still need to deal with partial free > > > > > > calls regardless. But I was curious whether XHCI in particular, > > > > > > and perhaps more generally, we could streamline this by alignin= g > > > > > > all allocations > > > > to a log2 upper bound of their sizes. > > > > > > > > > > Xhci code > > > > > > (https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Bus/Pci/ > > > > Xhci > > > > Dxe/UsbHcMem.c#L604) in allocation requested is greater the > > > > EFI_PAGE_SIZE allocates number of requested pages plus pages for th= e > > > > alignment and then frees pages at the beginning and end of the > > > > allocation. I am not sure we really could change this without addi= ng an > > alignment field to the PciIo protocol. > > > > > > > > > > Is there anything else you would like to change on this patch? > > > > > > > > > > > > > No. Thanks for the clarification. > > > > > > > > Reviewed-by: Ard Biesheuvel > > > > > > > > > > > > > > > >=20 >=20 >=20 >=20