Anything else needed to get this merged? > -----Original Message----- > From: Ard Biesheuvel <a...@kernel.org> > Sent: Thursday, September 8, 2022 9:55 AM > To: Jeff Brasen <jbra...@nvidia.com> > Cc: devel@edk2.groups.io; hao.a...@intel.com; ray...@intel.com; > quic_llind...@quicinc.com; ardb+tianoc...@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 <jbra...@nvidia.com> wrote: > > > > > > > > > -----Original Message----- > > > From: Ard Biesheuvel <a...@kernel.org> > > > Sent: Monday, August 15, 2022 8:42 AM > > > To: devel@edk2.groups.io; Jeff Brasen <jbra...@nvidia.com> > > > Cc: hao.a...@intel.com; ray...@intel.com; quic_llind...@quicinc.com; > > > ardb+tianoc...@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 > > > <jbrasen=nvidia....@groups.io> wrote: > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > From: Ard Biesheuvel <a...@kernel.org> > > > > > Sent: Tuesday, August 2, 2022 10:51 AM > > > > > To: Jeff Brasen <jbra...@nvidia.com> > > > > > Cc: devel@edk2.groups.io; hao.a...@intel.com; ray...@intel.com; > > > > > quic_llind...@quicinc.com; ardb+tianoc...@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 <jbra...@nvidia.com> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > From: Ard Biesheuvel <a...@kernel.org> > > > > > > > Sent: Friday, July 29, 2022 9:48 AM > > > > > > > To: Jeff Brasen <jbra...@nvidia.com> > > > > > > > Cc: devel@edk2.groups.io; hao.a...@intel.com; > > > > > > > ray...@intel.com; quic_llind...@quicinc.com; > > > > > > > ardb+tianoc...@kernel.org > > > > > > > Subject: Re: [PATCH v2] > MdeModulePkg/NonDiscoverablePciDeviceDxe: > > > > > > > Allow partial FreeBuffer > > > > > > > > > > > > > > External email: Use caution opening links or attachments > > > > > > > > > > > > > > > > > > > > > On Thu, 28 Jul 2022 at 13:25, Jeff Brasen <jbra...@nvidia.com> > wrote: > > > > > > > > > > > > > > > > > > > > > > > > Adding Leif/Ard to CC incase they have any comments on 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 count 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 is > > > > > > 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 specific detection. > > > > > > > > > > > > > > > > Ah interesting. Would it help if the allocate routine aligned > > > > > allocations to their size? > > > > > > > > The PciIo->AllocateBuffer function doesn't support passing the > > > > 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 end 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 aligning 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 the alignment and then > frees pages at the beginning and end of the allocation. I am not sure we > really > could change this without adding 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 <a...@kernel.org>
-=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#94082): https://edk2.groups.io/g/devel/message/94082 Mute This Topic: https://groups.io/mt/89143704/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
