From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f66.google.com (mail-wr1-f66.google.com [209.85.221.66]) by mx.groups.io with SMTP id smtpd.web11.10864.1574770972270970155 for ; Tue, 26 Nov 2019 04:22:52 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=yvaaP0//; spf=pass (domain: linaro.org, ip: 209.85.221.66, mailfrom: leif.lindholm@linaro.org) Received: by mail-wr1-f66.google.com with SMTP id t2so22219893wrr.1 for ; Tue, 26 Nov 2019 04:22:52 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=i1SGPvXu3qE06HWsJNqYaba2SOzaP8QGzI/O4/8o1VY=; b=yvaaP0//Yh+u4zhZcHcUH8Aw7BXxlyRGONTVsTN8KFLBRdCfQ2iUJVpZS0bWP1WGkr xzIRhGNuuq+O2Y8lehGl9oaL87qvcFCPqJeRHX3VvaHHNm9WQKJbdnBs/yDg3JlE6QRU fdnbn21jCWAOszOyoi3exLSCt+yuW0bRUiyVbobxpHiLoHHzyiyB7eNj9f6TlFfF0E4v HFu+9Ai44cI14P6ajf2mrwsyIxIEO5RrhaiCn47VoDEEnjb/QETyPceUGW1fquQYIgSN EKxdC6o2hxBfEjuqvhr/y9IsFb2+ch+qpf48OEG/GYws659Wzf2iECnUN8N9bdceZ574 bvQQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=i1SGPvXu3qE06HWsJNqYaba2SOzaP8QGzI/O4/8o1VY=; b=n+yjgDwYTw5d44G7G4c3fAoyrfWCNPG0l4UdHVyac2AIdn529RCS9NJKFRUJXvNsyM ZNaBQ+OncyO4EuXwAmoo3h6z3ghIbAKB3vCp6wgSQliFcBNVIi/QcorVMz9WJ4AlZf6A GyHxhk4V5nMXJjf5yv97pvey9n5lf+gnR6PO/hF0T4p8Rxqax/gHeamMTwP78cnTG5v+ zcu4DzrOduqyiUR+8D226yN8K/oWAZ73NJQXyRHyod+e7SbWvEJYQd7W6PuuAo8p6cR4 E6IVgup/J/9IUvxsIFCwvRYDX0m1XpN6CE/EP/ShqpQSioweOtE2+LaY1XxygWLiVmzy xuMg== X-Gm-Message-State: APjAAAURj55KDlbk9IuYwuvI1IxpOliUuWO6fqiX/yGm5BdH2MUqNc4e NQGKeCH7w+kFyhyEw5Q8j62LHQ== X-Google-Smtp-Source: APXvYqzUYfXuk4c4xb9LNedMqsCzMpcNEErl4y7fge5Y5bLSiRBBGFRwFxJQNBu5trpf3BUuK6Tckw== X-Received: by 2002:a5d:6a4c:: with SMTP id t12mr28520599wrw.141.1574770970649; Tue, 26 Nov 2019 04:22:50 -0800 (PST) Return-Path: Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id t134sm2998928wmt.24.2019.11.26.04.22.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 26 Nov 2019 04:22:49 -0800 (PST) Date: Tue, 26 Nov 2019 12:22:48 +0000 From: "Leif Lindholm" To: Ard Biesheuvel Cc: edk2-devel-groups-io Subject: Re: [PATCH v2 1/2] EmbeddedPkg/NonCoherentDmaLib: implement support for DMA range limits Message-ID: <20191126122248.GW7359@bivouac.eciton.net> References: <20191125231242.12193-1-ard.biesheuvel@linaro.org> <20191125231242.12193-2-ard.biesheuvel@linaro.org> <20191126120456.GV7359@bivouac.eciton.net> MIME-Version: 1.0 In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Tue, Nov 26, 2019 at 13:11:34 +0100, Ard Biesheuvel wrote: > 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 I ... guess that's true. Sure. / Leif