> -----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.
-Jeff
>
>
> > > -----Original Message-----
> > > From: Jeff Brasen
> > > Sent: Friday, June 17, 2022 9:39 AM
> > > To: devel@edk2.groups.io
> > > Cc: hao.a...@intel.com; ray...@intel.com
> > > Subject: RE: [PATCH v2] MdeModulePkg/NonDiscoverablePciDeviceDxe:
> > > Allow partial FreeBuffer
> > >
> > > Any thoughts on this patch?
> > >
> > > > -----Original Message-----
> > > > From: Jeff Brasen <jbra...@nvidia.com>
> > > > Sent: Monday, February 14, 2022 11:46 AM
> > > > To: devel@edk2.groups.io
> > > > Cc: hao.a...@intel.com; ray...@intel.com; Jeff Brasen
> > > > <jbra...@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 <jbra...@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
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#92047): https://edk2.groups.io/g/devel/message/92047
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]
-=-=-=-=-=-=-=-=-=-=-=-
