MdeModulePkg maintainers, Any comments on this? > -----Original Message----- > From: Ard Biesheuvel <a...@kernel.org> > Sent: Wednesday, September 21, 2022 10:32 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 Wed, 21 Sept 2022 at 18:27, Jeff Brasen via groups.io > <jbrasen=nvidia....@groups.io> wrote: > > > > Anything else needed to get this merged? > > > > That is up to the MdeModulePkg maintainers. > > > > -----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 (#94524): https://edk2.groups.io/g/devel/message/94524 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] -=-=-=-=-=-=-=-=-=-=-=-
