[edk2-devel] [PATCH 1/2] EmbeddedPkg/NonCoherentDmaLib: implement support for DMA range limits
Leif Lindholm via Groups.Io
leif.lindholm=linaro.org at groups.io
Mon Nov 25 12:46:06 UTC 2019
On Thu, Nov 21, 2019 at 09:32:26 +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 <ard.biesheuvel at linaro.org>
> ---
> EmbeddedPkg/EmbeddedPkg.dec | 6 +
> EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.c | 176 +++++++++++++++++---
> EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.inf | 1 +
> 3 files changed, 158 insertions(+), 25 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..dd3d111e7eb9 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);
> +
> + if (Pages == 0) {
> + return NULL;
> + }
> + if (Alignment > EFI_PAGE_SIZE) {
> + //
> + // Calculate the total number of pages since alignment is larger than page
> + // size.
> + //
> + AlignmentMask = Alignment - 1;
> + RealPages = Pages + EFI_SIZE_TO_PAGES (Alignment);
> + //
> + // Make sure that Pages plus EFI_SIZE_TO_PAGES (Alignment) does not
> + // overflow.
> + //
> + ASSERT (RealPages > Pages);
> +
> + Memory = mDmaHostAddressLimit;
> + Status = gBS->AllocatePages (AllocateMaxAddress, MemoryType, RealPages,
> + &Memory);
> + if (EFI_ERROR (Status)) {
> + return NULL;
> + }
> + AlignedMemory = ((UINTN)Memory + AlignmentMask) & ~AlignmentMask;
> + UnalignedPages = EFI_SIZE_TO_PAGES (AlignedMemory - (UINTN)Memory);
> + if (UnalignedPages > 0) {
> + //
> + // Free first unaligned page(s).
> + //
> + Status = gBS->FreePages (Memory, UnalignedPages);
> + ASSERT_EFI_ERROR (Status);
> + }
> + Memory = AlignedMemory + EFI_PAGES_TO_SIZE (Pages);
> + UnalignedPages = RealPages - Pages - UnalignedPages;
> + if (UnalignedPages > 0) {
> + //
> + // Free last unaligned page(s).
> + //
> + Status = gBS->FreePages (Memory, UnalignedPages);
> + ASSERT_EFI_ERROR (Status);
> + }
> + } else {
> + //
> + // Do not over-allocate pages in this case.
> + //
> + Memory = mDmaHostAddressLimit;
> + Status = gBS->AllocatePages (AllocateMaxAddress, MemoryType, Pages,
> + &Memory);
> + if (EFI_ERROR (Status)) {
> + return NULL;
> + }
> + AlignedMemory = (UINTN)Memory;
> + }
> + return (VOID *)AlignedMemory;
> +}
> +
> /**
> Provides the DMA controller-specific addresses needed to access system memory.
>
> @@ -111,7 +209,22 @@ DmaMap (
> return EFI_OUT_OF_RESOURCES;
> }
>
> - if (Operation != MapOperationBusMasterRead &&
> + if (((UINTN)HostAddress + *NumberOfBytes) > mDmaHostAddressLimit) {
> +
> + if (Operation == MapOperationBusMasterCommonBuffer) {
> + goto CommonBufferError;
> + }
> +
> + AllocSize = ALIGN_VALUE (*NumberOfBytes, mCpu->DmaBufferAlignment);
> + Map->BufferAddress = InternalAllocateAlignedPages (EfiBootServicesData,
> + EFI_SIZE_TO_PAGES (AllocSize),
> + mCpu->DmaBufferAlignment);
> + if (Map->BufferAddress == NULL) {
> + Status = EFI_OUT_OF_RESOURCES;
> + goto FreeMapInfo;
> + }
> + *DeviceAddress = HostToDeviceAddress (Map->BufferAddress);
> + } else if (Operation != MapOperationBusMasterRead &&
> ((((UINTN)HostAddress & (mCpu->DmaBufferAlignment - 1)) != 0) ||
> ((*NumberOfBytes & (mCpu->DmaBufferAlignment - 1)) != 0))) {
>
> @@ -128,12 +241,7 @@ DmaMap (
> // on uncached buffers.
> //
> if (Operation == MapOperationBusMasterCommonBuffer) {
> - DEBUG ((DEBUG_ERROR,
> - "%a: Operation type 'MapOperationBusMasterCommonBuffer' is only "
> - "supported\non memory regions that were allocated using "
> - "DmaAllocateBuffer ()\n", __FUNCTION__));
> - Status = EFI_UNSUPPORTED;
> - goto FreeMapInfo;
> + goto CommonBufferError;
> }
>
> //
> @@ -154,14 +262,6 @@ DmaMap (
>
> Buffer = ALIGN_POINTER (Map->BufferAddress, mCpu->DmaBufferAlignment);
> *DeviceAddress = HostToDeviceAddress (Buffer);
> -
> - //
> - // Get rid of any dirty cachelines covering the double buffer. This
> - // prevents them from being written back unexpectedly, potentially
> - // overwriting the data we receive from the device.
> - //
> - mCpu->FlushDataCache (mCpu, (UINTN)Buffer, *NumberOfBytes,
> - EfiCpuFlushTypeWriteBack);
> } else {
> Map->DoubleBuffer = FALSE;
> }
> @@ -184,13 +284,13 @@ DmaMap (
> (GcdDescriptor.Attributes & (EFI_MEMORY_WB | EFI_MEMORY_WT)) == 0);
>
> DEBUG_CODE_END ();
> -
> - // Flush the Data Cache (should not have any effect if the memory region is
> - // uncached)
> - mCpu->FlushDataCache (mCpu, (UINTN)HostAddress, *NumberOfBytes,
> - EfiCpuFlushTypeWriteBackInvalidate);
> }
>
> + // Flush the Data Cache (should not have any effect if the memory region is
> + // uncached)
> + mCpu->FlushDataCache (mCpu, (UINTN)HostAddress, *NumberOfBytes,
> + EfiCpuFlushTypeWriteBack);
> +
> Map->HostAddress = (UINTN)HostAddress;
> Map->NumberOfBytes = *NumberOfBytes;
> Map->Operation = Operation;
> @@ -199,6 +299,12 @@ DmaMap (
>
> return EFI_SUCCESS;
>
> +CommonBufferError:
> + DEBUG ((DEBUG_ERROR,
> + "%a: Operation type 'MapOperationBusMasterCommonBuffer' is only "
> + "supported\non memory regions that were allocated using "
> + "DmaAllocateBuffer ()\n", __FUNCTION__));
> + Status = EFI_UNSUPPORTED;
> FreeMapInfo:
> FreePool (Map);
>
> @@ -229,6 +335,7 @@ DmaUnmap (
> MAP_INFO_INSTANCE *Map;
> EFI_STATUS Status;
> VOID *Buffer;
> + UINTN AllocSize;
>
> if (Mapping == NULL) {
> ASSERT (FALSE);
> @@ -238,7 +345,18 @@ DmaUnmap (
> Map = (MAP_INFO_INSTANCE *)Mapping;
>
> Status = EFI_SUCCESS;
> - if (Map->DoubleBuffer) {
> + if (((UINTN)Map->HostAddress + Map->NumberOfBytes) > mDmaHostAddressLimit) {
> +
> + mCpu->FlushDataCache (mCpu, (UINTN)Map->BufferAddress, Map->NumberOfBytes,
> + EfiCpuFlushTypeInvalidate);
> +
> + CopyMem ((VOID *)(UINTN)Map->HostAddress, Map->BufferAddress,
> + Map->NumberOfBytes);
> +
> + AllocSize = ALIGN_VALUE (Map->NumberOfBytes, mCpu->DmaBufferAlignment);
> + FreePages (Map->BufferAddress, EFI_SIZE_TO_PAGES (AllocSize));
> + } else if (Map->DoubleBuffer) {
> +
> ASSERT (Map->Operation == MapOperationBusMasterWrite);
>
> if (Map->Operation != MapOperationBusMasterWrite) {
> @@ -335,10 +453,9 @@ DmaAllocateAlignedBuffer (
> return EFI_INVALID_PARAMETER;
> }
>
> - if (MemoryType == EfiBootServicesData) {
> - Allocation = AllocateAlignedPages (Pages, Alignment);
> - } else if (MemoryType == EfiRuntimeServicesData) {
> - Allocation = AllocateAlignedRuntimePages (Pages, Alignment);
> + if (MemoryType == EfiBootServicesData ||
> + MemoryType == EfiRuntimeServicesData) {
> + Allocation = InternalAllocateAlignedPages (MemoryType, Pages, Alignment);
> } else {
> return EFI_INVALID_PARAMETER;
> }
> @@ -479,6 +596,15 @@ NonCoherentDmaLibConstructor (
> {
> InitializeListHead (&UncachedAllocationList);
>
> + //
> + // Ensure that the combination of DMA addressing offset and limit produces
> + // a sane value.
> + //
> + ASSERT (PcdGet64 (PcdDmaDeviceLimit) > PcdGet64 (PcdDmaDeviceOffset));
Is this worth turning into a hard conditional and error return rather
than an assert? The following statement will end up wrapping downwards
if this condition is not true.
> +
> + mDmaHostAddressLimit = PcdGet64 (PcdDmaDeviceLimit) -
> + PcdGet64 (PcdDmaDeviceOffset);
> +
/
Leif
> // Get the Cpu protocol for later use
> return gBS->LocateProtocol (&gEfiCpuArchProtocolGuid, NULL, (VOID **)&mCpu);
> }
> diff --git a/EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.inf b/EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.inf
> index 2db751afee91..1a21cfe4ff23 100644
> --- a/EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.inf
> +++ b/EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.inf
> @@ -38,6 +38,7 @@ [Protocols]
>
> [Pcd]
> gEmbeddedTokenSpaceGuid.PcdDmaDeviceOffset
> + gEmbeddedTokenSpaceGuid.PcdDmaDeviceLimit
>
> [Depex]
> gEfiCpuArchProtocolGuid
> --
> 2.17.1
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#51234): https://edk2.groups.io/g/devel/message/51234
Mute This Topic: https://groups.io/mt/61068915/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list