[edk2-devel] [PATCH v2] MdeModulePkg/NonDiscoverablePciDeviceDxe: Allow partial FreeBuffer

[Ard Biesheuvel]

On Fri, 5 Aug 2022 at 18:56, Jeff Brasen via groups.io
<jbrasen=nvidia.com at groups.io> wrote:
>
>
>
> > -----Original Message-----
> > From: Ard Biesheuvel <ardb at kernel.org>
> > Sent: Tuesday, August 2, 2022 10:51 AM
> > To: Jeff Brasen <jbrasen at nvidia.com>
> > Cc: devel at edk2.groups.io; hao.a.wu at intel.com; ray.ni at intel.com;
> > quic_llindhol at quicinc.com; ardb+tianocore at 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 <jbrasen at nvidia.com> wrote:
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Ard Biesheuvel <ardb at kernel.org>
> > > > Sent: Friday, July 29, 2022 9:48 AM
> > > > To: Jeff Brasen <jbrasen at nvidia.com>
> > > > Cc: devel at edk2.groups.io; hao.a.wu at intel.com; ray.ni at intel.com;
> > > > quic_llindhol at quicinc.com; ardb+tianocore at 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 <jbrasen at 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.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#92432): https://edk2.groups.io/g/devel/message/92432
Mute This Topic: https://groups.io/mt/89143704/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-