From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f66.google.com (mail-wm1-f66.google.com [209.85.128.66]) by mx.groups.io with SMTP id smtpd.web09.10589.1574770294431389216 for ; Tue, 26 Nov 2019 04:11:34 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=zi8oKU8L; spf=pass (domain: linaro.org, ip: 209.85.128.66, mailfrom: ard.biesheuvel@linaro.org) Received: by mail-wm1-f66.google.com with SMTP id g206so2970931wme.1 for ; Tue, 26 Nov 2019 04:11:34 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=W5w/KjgTRbrKDg/uv/ZM6yNo2Whs3dTNAT+5mwlxteo=; b=zi8oKU8LF3stVHqTtTiSIB0RrYkqr3FcqqCqyPiKkp3ftD3fLdloTCYuu1zW2rLAB0 GV7QeLIsLBm5s2dgo4wDTLGbP5EQQ9X5IUTD0nOXku3Na2gL3/zvpxyOSdaXj31rA+Tl aVJiBpzZ3OJl+UPJgSA9KUVVHjJ2a2EvTTOnEn4JBJXHfzRbBsTSm7TJ+k5OjOg9qBWH KcETJ8Kv5jlqoTaHYRAPA1OHvlbTnBVZb9jnlPR/QgjCKS+ChPqb73a08vD44bDigauu O1UKjxCYrHXxcQEArPWXDCQ0fKbLkg/O292+iQBNh3gMtvdvQLbnjFQ5waEV/MKm/IT9 on9Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=W5w/KjgTRbrKDg/uv/ZM6yNo2Whs3dTNAT+5mwlxteo=; b=BbIfihE6zgAriV+XrhQuHxN+NXVV6vKH6ItPe1n5yMHOA3fQ7Iv12ekvWl+zZ1G6WM NJy4FPBvhor4EwI1iZ9ci7y8bztkNwx/Hq8uTSmpHB702CCtEGOxh3jFX/Jea/ibE0L8 Zr38N53DOOQj/ZVEOv50fPO8ICem053dAk2R6lcnXELonhn63VWLv/It8gFPvqZwzVEY UeA9GfhfEykhsqn70J9qcZSSI6w2T/JQR0RiqmaXUe3HbCqPikeNwWYaDnA/DlhncyMN f/nXweXTLyb6Hca6NOopVo8qkqWUsypzqdPDBXHJtUAKQLyzKFtqpN/8hv7qo/Ukt0u2 SC0A== X-Gm-Message-State: APjAAAXG+xtJcs69iGhuLdX3Td01KyscaM9dbmOryGxwyp2rOpSI0CYK IPW05xJrlCwsGJ208TrybvIgvQ7eY0GcAwdXo3NJNw== X-Google-Smtp-Source: APXvYqyfoZ9ZA/CGKiMoAWksLuNSpt3gYDzIZvhwvO1HC8ECy6ZfOn2E8o7hNoOmtqTPBKI56nKz163eo2Pb3ELSncg= X-Received: by 2002:a1c:3d08:: with SMTP id k8mr3797954wma.119.1574770292845; Tue, 26 Nov 2019 04:11:32 -0800 (PST) MIME-Version: 1.0 References: <20191125231242.12193-1-ard.biesheuvel@linaro.org> <20191125231242.12193-2-ard.biesheuvel@linaro.org> <20191126120456.GV7359@bivouac.eciton.net> In-Reply-To: <20191126120456.GV7359@bivouac.eciton.net> From: "Ard Biesheuvel" Date: Tue, 26 Nov 2019 13:11:34 +0100 Message-ID: Subject: Re: [PATCH v2 1/2] EmbeddedPkg/NonCoherentDmaLib: implement support for DMA range limits To: Leif Lindholm Cc: edk2-devel-groups-io Content-Type: text/plain; charset="UTF-8" On Tue, 26 Nov 2019 at 13:04, Leif Lindholm wrote: > > On Tue, Nov 26, 2019 at 00:12:41 +0100, Ard Biesheuvel wrote: > > Implement support for driving peripherals with limited DMA ranges to > > NonCoherentDmaLib, by adding a device address limit, and taking it, > > along with the device offset, into account when allocating or mapping > > DMA buffers. > > > > Signed-off-by: Ard Biesheuvel > > --- > > EmbeddedPkg/EmbeddedPkg.dec | 6 + > > EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.c | 165 ++++++++++++++++++-- > > EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.inf | 1 + > > 3 files changed, 160 insertions(+), 12 deletions(-) > > > > diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec > > index 8812a6db7c30..69922802f473 100644 > > --- a/EmbeddedPkg/EmbeddedPkg.dec > > +++ b/EmbeddedPkg/EmbeddedPkg.dec > > @@ -186,6 +186,12 @@ [PcdsFixedAtBuild.common, PcdsDynamic.common] > > # > > gEmbeddedTokenSpaceGuid.PcdDmaDeviceOffset|0x0|UINT64|0x0000058 > > > > + # > > + # Highest address value supported by the device for DMA addressing. Note > > + # that this value should be strictly greater than PcdDmaDeviceOffset. > > + # > > + gEmbeddedTokenSpaceGuid.PcdDmaDeviceLimit|0xFFFFFFFFFFFFFFFF|UINT64|0x000005A > > + > > # > > # Selection between DT and ACPI as a default > > # > > diff --git a/EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.c b/EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.c > > index 78220f6358aa..115345765435 100644 > > --- a/EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.c > > +++ b/EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.c > > @@ -40,6 +40,8 @@ typedef struct { > > STATIC EFI_CPU_ARCH_PROTOCOL *mCpu; > > STATIC LIST_ENTRY UncachedAllocationList; > > > > +STATIC PHYSICAL_ADDRESS mDmaHostAddressLimit; > > + > > STATIC > > PHYSICAL_ADDRESS > > HostToDeviceAddress ( > > @@ -49,6 +51,102 @@ HostToDeviceAddress ( > > return (PHYSICAL_ADDRESS)(UINTN)Address + PcdGet64 (PcdDmaDeviceOffset); > > } > > > > +/** > > + Allocates one or more 4KB pages of a certain memory type at a specified > > + alignment. > > + > > + Allocates the number of 4KB pages specified by Pages of a certain memory type > > + with an alignment specified by Alignment. The allocated buffer is returned. > > + If Pages is 0, then NULL is returned. If there is not enough memory at the > > + specified alignment remaining to satisfy the request, then NULL is returned. > > + If Alignment is not a power of two and Alignment is not zero, then ASSERT(). > > + If Pages plus EFI_SIZE_TO_PAGES (Alignment) overflows, then ASSERT(). > > + > > + @param MemoryType The type of memory to allocate. > > + @param Pages The number of 4 KB pages to allocate. > > + @param Alignment The requested alignment of the allocation. > > + Must be a power of two. > > + If Alignment is zero, then byte alignment is > > + used. > > + > > + @return A pointer to the allocated buffer or NULL if allocation fails. > > + > > +**/ > > +STATIC > > +VOID * > > +InternalAllocateAlignedPages ( > > + IN EFI_MEMORY_TYPE MemoryType, > > + IN UINTN Pages, > > + IN UINTN Alignment > > + ) > > +{ > > + EFI_STATUS Status; > > + EFI_PHYSICAL_ADDRESS Memory; > > + UINTN AlignedMemory; > > + UINTN AlignmentMask; > > + UINTN UnalignedPages; > > + UINTN RealPages; > > + > > + // > > + // Alignment must be a power of two or zero. > > + // > > + ASSERT ((Alignment & (Alignment - 1)) == 0); > > Sorry, slight mental glitch (and I realise this is in copied code) - > the above also matches for an Alignment of 1, which contradicts the > comment. Clearly requesting allocation explicitly aligned to 1 is a) > silly, b) the same as what happens for any value <= EFI_PAGE_SIZE > in the below code, and c) harmless, but could you update the comment? > > If so: > Acked-by: Leif Lindholm > Given that 1 == 2^0, it is also a power of 2, and so the comment is accurate imho