Re: [PATCH v2] MdeModulePkg/NonDiscoverablePciDeviceDxe: Allow partial FreeBuffer

["Jeff Brasen" <jbrasen@nvidia.com>]

Adding Leif/Ard to CC incase they have any comments on this patch.

Thanks
Jeff
> -----Original Message-----
> From: Jeff Brasen
> Sent: Friday, June 17, 2022 9:39 AM
> To: devel@edk2.groups.io
> Cc: hao.a.wu@intel.com; ray.ni@intel.com
> Subject: RE: [PATCH v2] MdeModulePkg/NonDiscoverablePciDeviceDxe:
> Allow partial FreeBuffer
> 
> Any thoughts on this patch?
> 
> > -----Original Message-----
> > From: Jeff Brasen <jbrasen@nvidia.com>
> > Sent: Monday, February 14, 2022 11:46 AM
> > To: devel@edk2.groups.io
> > Cc: hao.a.wu@intel.com; ray.ni@intel.com; Jeff Brasen
> > <jbrasen@nvidia.com>
> > Subject: [PATCH v2] MdeModulePkg/NonDiscoverablePciDeviceDxe: Allow
> > partial FreeBuffer
> >
> > Add support for partial free of non cached buffers.
> > If a request for less than the full size is requested new allocations
> > for the remaining head and tail of the buffer are added to the list.
> > Added verification that Buffer is EFI_PAGE_SIZE aligned.
> > The XHCI driver does this if the page size for the controller is >4KB.
> >
> > Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
> > ---
> >  .../NonDiscoverablePciDeviceIo.c              | 53 ++++++++++++++++++-
> >  1 file changed, 51 insertions(+), 2 deletions(-)
> >
> > diff --git
> >
> a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable
> > PciDeviceIo.c
> >
> b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable
> > PciDeviceIo.c
> > index c1c5c6267c..77809cfedf 100644
> > ---
> >
> a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable
> > PciDeviceIo.c
> > +++
> >
> b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable
> > Pc
> > +++ iDeviceIo.c
> > @@ -960,12 +960,23 @@ NonCoherentPciIoFreeBuffer (
> >    LIST_ENTRY                                   *Entry;
> >    EFI_STATUS                                   Status;
> >    NON_DISCOVERABLE_DEVICE_UNCACHED_ALLOCATION  *Alloc;
> > +  NON_DISCOVERABLE_DEVICE_UNCACHED_ALLOCATION  *AllocHead;
> > + NON_DISCOVERABLE_DEVICE_UNCACHED_ALLOCATION  *AllocTail;
> >    BOOLEAN                                      Found;
> > +  UINTN                                        StartPages;
> > +  UINTN                                        EndPages;
> > +
> > +  if (HostAddress != ALIGN_POINTER (HostAddress, EFI_PAGE_SIZE)) {
> > +    ASSERT_EFI_ERROR (EFI_INVALID_PARAMETER);
> > +    return EFI_INVALID_PARAMETER;
> > +  }
> >
> >    Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO (This);
> >
> >    Found = FALSE;
> >    Alloc = NULL;
> > +  AllocHead = NULL;
> > +  AllocTail = NULL;
> >
> >    //
> >    // Find the uncached allocation list entry associated @@ -976,9
> > +987,13 @@ NonCoherentPciIoFreeBuffer (
> >         Entry = Entry->ForwardLink)
> >    {
> >      Alloc = BASE_CR (Entry,
> > NON_DISCOVERABLE_DEVICE_UNCACHED_ALLOCATION, List);
> > -    if ((Alloc->HostAddress == HostAddress) && (Alloc->NumPages ==
> Pages))
> > {
> > +    StartPages = 0;
> > +    if (Alloc->HostAddress < HostAddress) {
> > +      StartPages = (HostAddress - Alloc->HostAddress) / EFI_PAGE_SIZE;
> > +    }
> > +    if ((Alloc->HostAddress <= HostAddress) && (Alloc->NumPages >=
> > + (Pages + StartPages))) {
> >        //
> > -      // We are freeing the exact allocation we were given
> > +      // We are freeing at least part of what we were given
> >        // before by AllocateBuffer()
> >        //
> >        Found = TRUE;
> > @@ -991,7 +1006,41 @@ NonCoherentPciIoFreeBuffer (
> >      return EFI_NOT_FOUND;
> >    }
> >
> > +  EndPages = Alloc->NumPages - (Pages + StartPages);
> > +
> > +  if (StartPages != 0) {
> > +    AllocHead = AllocatePool (sizeof *AllocHead);
> > +    if (AllocHead == NULL) {
> > +      return EFI_OUT_OF_RESOURCES;
> > +    }
> > +
> > +    AllocHead->HostAddress = Alloc->HostAddress;
> > +    AllocHead->NumPages = StartPages;
> > +    AllocHead->Attributes = Alloc->Attributes;  }
> > +
> > +  if (EndPages != 0) {
> > +    AllocTail = AllocatePool (sizeof *AllocTail);
> > +    if (AllocTail == NULL) {
> > +      return EFI_OUT_OF_RESOURCES;
> > +    }
> > +
> > +    AllocTail->HostAddress = Alloc->HostAddress + ((Pages +
> > + StartPages) *
> > EFI_PAGE_SIZE);
> > +    AllocTail->NumPages = EndPages;
> > +    AllocTail->Attributes = Alloc->Attributes;  }
> > +
> >    RemoveEntryList (&Alloc->List);
> > +  //
> > +  // Record this new sub allocations in the linked list, so we  //
> > + can restore the memory space attributes later  //  if (AllocHead !=
> > + NULL) {
> > +    InsertHeadList (&Dev->UncachedAllocationList, &AllocHead->List);
> > + } if (AllocTail != NULL) {
> > +    InsertHeadList (&Dev->UncachedAllocationList, &AllocTail->List);
> > + }
> >
> >    Status = gDS->SetMemorySpaceAttributes (
> >                    (EFI_PHYSICAL_ADDRESS)(UINTN)HostAddress,
> > --
> > 2.17.1