* [PATCH 1/6] EmbeddedPkg: rename NullDmaLib to CoherentDmaLib
2017-08-30 8:21 [PATCH 0/6] ArmPkg EmbeddedPkg: clean up DmaLib implementations Ard Biesheuvel
@ 2017-08-30 8:21 ` Ard Biesheuvel
2017-08-30 10:46 ` Leif Lindholm
2017-08-30 8:21 ` [PATCH 2/6] EmbeddedPkg/CoherentDmaLib: add support for non-1:1 DMA translation Ard Biesheuvel
` (5 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Ard Biesheuvel @ 2017-08-30 8:21 UTC (permalink / raw)
To: edk2-devel, leif.lindholm; +Cc: Ard Biesheuvel
Rename NullDmaLib to CoherentDmaLib to better reflect its nature, and
to better convey the fact that it actually serves a useful purpose,
i.e., as a DmaLib library class resolution for drivers that control
hardware that may only be cache coherent or in some cases (i.e., on
some platforms but not on others).
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
EmbeddedPkg/EmbeddedPkg.dsc | 2 +-
EmbeddedPkg/Library/{NullDmaLib/NullDmaLib.c => CoherentDmaLib/CoherentDmaLib.c} | 0
EmbeddedPkg/Library/{NullDmaLib/NullDmaLib.inf => CoherentDmaLib/CoherentDmaLib.inf} | 20 ++++++--------------
3 files changed, 7 insertions(+), 15 deletions(-)
diff --git a/EmbeddedPkg/EmbeddedPkg.dsc b/EmbeddedPkg/EmbeddedPkg.dsc
index 4a34e34843ad..84c5a842e37e 100644
--- a/EmbeddedPkg/EmbeddedPkg.dsc
+++ b/EmbeddedPkg/EmbeddedPkg.dsc
@@ -250,7 +250,7 @@ [Components.common]
EmbeddedPkg/Library/TemplateResetSystemLib/TemplateResetSystemLib.inf
EmbeddedPkg/Library/TemplateRealTimeClockLib/TemplateRealTimeClockLib.inf
EmbeddedPkg/Library/LzmaHobCustomDecompressLib/LzmaHobCustomDecompressLib.inf
- EmbeddedPkg/Library/NullDmaLib/NullDmaLib.inf
+ EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.inf
EmbeddedPkg/Library/DxeDtPlatformDtbLoaderLibDefault/DxeDtPlatformDtbLoaderLibDefault.inf
EmbeddedPkg/Ebl/Ebl.inf
diff --git a/EmbeddedPkg/Library/NullDmaLib/NullDmaLib.c b/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c
similarity index 100%
rename from EmbeddedPkg/Library/NullDmaLib/NullDmaLib.c
rename to EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c
diff --git a/EmbeddedPkg/Library/NullDmaLib/NullDmaLib.inf b/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.inf
similarity index 78%
rename from EmbeddedPkg/Library/NullDmaLib/NullDmaLib.inf
rename to EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.inf
index 38261d5ede2b..c40a600cf6a3 100644
--- a/EmbeddedPkg/Library/NullDmaLib/NullDmaLib.inf
+++ b/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.inf
@@ -1,6 +1,8 @@
#/** @file
#
# Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.<BR>
+# Copyright (c) 2017, Linaro, Ltd. All rights reserved.<BR>
+#
# This program and the accompanying materials
# are licensed and made available under the terms and conditions of the BSD License
# which accompanies this distribution. The full text of the license may be found at
@@ -12,15 +14,15 @@
#**/
[Defines]
- INF_VERSION = 0x00010005
- BASE_NAME = NullDmaLib
+ INF_VERSION = 0x00010019
+ BASE_NAME = CoherentDmaLib
FILE_GUID = 0F2A0816-D319-4ee7-A6B8-D58524E4428F
MODULE_TYPE = BASE
VERSION_STRING = 1.0
LIBRARY_CLASS = DmaLib
-[Sources.common]
- NullDmaLib.c
+[Sources]
+ CoherentDmaLib.c
[Packages]
MdePkg/MdePkg.dec
@@ -29,13 +31,3 @@ [Packages]
[LibraryClasses]
DebugLib
MemoryAllocationLib
-
-
-[Protocols]
-
-[Guids]
-
-[Pcd]
-
-[Depex]
- TRUE
--
2.11.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 1/6] EmbeddedPkg: rename NullDmaLib to CoherentDmaLib
2017-08-30 8:21 ` [PATCH 1/6] EmbeddedPkg: rename NullDmaLib to CoherentDmaLib Ard Biesheuvel
@ 2017-08-30 10:46 ` Leif Lindholm
2017-08-30 10:52 ` Ard Biesheuvel
0 siblings, 1 reply; 17+ messages in thread
From: Leif Lindholm @ 2017-08-30 10:46 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel
On Wed, Aug 30, 2017 at 09:21:03AM +0100, Ard Biesheuvel wrote:
> Rename NullDmaLib to CoherentDmaLib to better reflect its nature, and
> to better convey the fact that it actually serves a useful purpose,
> i.e., as a DmaLib library class resolution for drivers that control
> hardware that may only be cache coherent or in some cases (i.e., on
> some platforms but not on others).
The above doesn't read very well (and I'm not 100% certain what it's
trying to say, so can't really propose an improvement).
No other issues with patch.
/
Leif
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> EmbeddedPkg/EmbeddedPkg.dsc | 2 +-
> EmbeddedPkg/Library/{NullDmaLib/NullDmaLib.c => CoherentDmaLib/CoherentDmaLib.c} | 0
> EmbeddedPkg/Library/{NullDmaLib/NullDmaLib.inf => CoherentDmaLib/CoherentDmaLib.inf} | 20 ++++++--------------
> 3 files changed, 7 insertions(+), 15 deletions(-)
>
> diff --git a/EmbeddedPkg/EmbeddedPkg.dsc b/EmbeddedPkg/EmbeddedPkg.dsc
> index 4a34e34843ad..84c5a842e37e 100644
> --- a/EmbeddedPkg/EmbeddedPkg.dsc
> +++ b/EmbeddedPkg/EmbeddedPkg.dsc
> @@ -250,7 +250,7 @@ [Components.common]
> EmbeddedPkg/Library/TemplateResetSystemLib/TemplateResetSystemLib.inf
> EmbeddedPkg/Library/TemplateRealTimeClockLib/TemplateRealTimeClockLib.inf
> EmbeddedPkg/Library/LzmaHobCustomDecompressLib/LzmaHobCustomDecompressLib.inf
> - EmbeddedPkg/Library/NullDmaLib/NullDmaLib.inf
> + EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.inf
> EmbeddedPkg/Library/DxeDtPlatformDtbLoaderLibDefault/DxeDtPlatformDtbLoaderLibDefault.inf
>
> EmbeddedPkg/Ebl/Ebl.inf
> diff --git a/EmbeddedPkg/Library/NullDmaLib/NullDmaLib.c b/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c
> similarity index 100%
> rename from EmbeddedPkg/Library/NullDmaLib/NullDmaLib.c
> rename to EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c
> diff --git a/EmbeddedPkg/Library/NullDmaLib/NullDmaLib.inf b/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.inf
> similarity index 78%
> rename from EmbeddedPkg/Library/NullDmaLib/NullDmaLib.inf
> rename to EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.inf
> index 38261d5ede2b..c40a600cf6a3 100644
> --- a/EmbeddedPkg/Library/NullDmaLib/NullDmaLib.inf
> +++ b/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.inf
> @@ -1,6 +1,8 @@
> #/** @file
> #
> # Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.<BR>
> +# Copyright (c) 2017, Linaro, Ltd. All rights reserved.<BR>
> +#
> # This program and the accompanying materials
> # are licensed and made available under the terms and conditions of the BSD License
> # which accompanies this distribution. The full text of the license may be found at
> @@ -12,15 +14,15 @@
> #**/
>
> [Defines]
> - INF_VERSION = 0x00010005
> - BASE_NAME = NullDmaLib
> + INF_VERSION = 0x00010019
> + BASE_NAME = CoherentDmaLib
> FILE_GUID = 0F2A0816-D319-4ee7-A6B8-D58524E4428F
> MODULE_TYPE = BASE
> VERSION_STRING = 1.0
> LIBRARY_CLASS = DmaLib
>
> -[Sources.common]
> - NullDmaLib.c
> +[Sources]
> + CoherentDmaLib.c
>
> [Packages]
> MdePkg/MdePkg.dec
> @@ -29,13 +31,3 @@ [Packages]
> [LibraryClasses]
> DebugLib
> MemoryAllocationLib
> -
> -
> -[Protocols]
> -
> -[Guids]
> -
> -[Pcd]
> -
> -[Depex]
> - TRUE
> --
> 2.11.0
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 1/6] EmbeddedPkg: rename NullDmaLib to CoherentDmaLib
2017-08-30 10:46 ` Leif Lindholm
@ 2017-08-30 10:52 ` Ard Biesheuvel
2017-08-30 11:37 ` Leif Lindholm
0 siblings, 1 reply; 17+ messages in thread
From: Ard Biesheuvel @ 2017-08-30 10:52 UTC (permalink / raw)
To: Leif Lindholm; +Cc: edk2-devel@lists.01.org
On 30 August 2017 at 11:46, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Wed, Aug 30, 2017 at 09:21:03AM +0100, Ard Biesheuvel wrote:
>> Rename NullDmaLib to CoherentDmaLib to better reflect its nature, and
>> to better convey the fact that it actually serves a useful purpose,
>> i.e., as a DmaLib library class resolution for drivers that control
>> hardware that may only be cache coherent or in some cases (i.e., on
>> some platforms but not on others).
>
> The above doesn't read very well (and I'm not 100% certain what it's
> trying to say, so can't really propose an improvement).
> No other issues with patch.
>
How about
"""
The name NullDmaLib suggests that this library is a placeholder that
only exists to fulfil formal dependencies on the DmaLib library class
without providing an actual implementation (*). This is not the case,
though: NullDmaLib does implement DmaLib fully, but doing so simply
requires very little effort on a cache coherent platform. So let's
rename it to CoherentDmaLib instead.
"""
* there are such instances in MdeModulePkg that do nothing and
ASSERT() in every function.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/6] EmbeddedPkg: rename NullDmaLib to CoherentDmaLib
2017-08-30 10:52 ` Ard Biesheuvel
@ 2017-08-30 11:37 ` Leif Lindholm
0 siblings, 0 replies; 17+ messages in thread
From: Leif Lindholm @ 2017-08-30 11:37 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel@lists.01.org
On Wed, Aug 30, 2017 at 11:52:26AM +0100, Ard Biesheuvel wrote:
> On 30 August 2017 at 11:46, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > On Wed, Aug 30, 2017 at 09:21:03AM +0100, Ard Biesheuvel wrote:
> >> Rename NullDmaLib to CoherentDmaLib to better reflect its nature, and
> >> to better convey the fact that it actually serves a useful purpose,
> >> i.e., as a DmaLib library class resolution for drivers that control
> >> hardware that may only be cache coherent or in some cases (i.e., on
> >> some platforms but not on others).
> >
> > The above doesn't read very well (and I'm not 100% certain what it's
> > trying to say, so can't really propose an improvement).
> > No other issues with patch.
> >
>
> How about
>
> """
> The name NullDmaLib suggests that this library is a placeholder that
> only exists to fulfil formal dependencies on the DmaLib library class
> without providing an actual implementation (*). This is not the case,
> though: NullDmaLib does implement DmaLib fully, but doing so simply
> requires very little effort on a cache coherent platform. So let's
> rename it to CoherentDmaLib instead.
> """
>
> * there are such instances in MdeModulePkg that do nothing and
> ASSERT() in every function.
Indeed.
Yes, this looks fine to me:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/6] EmbeddedPkg/CoherentDmaLib: add support for non-1:1 DMA translation
2017-08-30 8:21 [PATCH 0/6] ArmPkg EmbeddedPkg: clean up DmaLib implementations Ard Biesheuvel
2017-08-30 8:21 ` [PATCH 1/6] EmbeddedPkg: rename NullDmaLib to CoherentDmaLib Ard Biesheuvel
@ 2017-08-30 8:21 ` Ard Biesheuvel
2017-08-30 10:51 ` Leif Lindholm
2017-08-30 8:21 ` [PATCH 3/6] EmbeddedPkg: implement NonCoherentDmaLib based on ArmDmaLib Ard Biesheuvel
` (4 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Ard Biesheuvel @ 2017-08-30 8:21 UTC (permalink / raw)
To: edk2-devel, leif.lindholm; +Cc: Ard Biesheuvel
Bring CoherentDmaLib in line with ArmDmaLib, and add support for
defining a static offset between the host's and the bus master's
view of memory.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
EmbeddedPkg/EmbeddedPkg.dec | 7 +++++++
EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c | 10 +++++++++-
EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.inf | 3 +++
3 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
index 8ad2a84c045c..ccdf38e36a8c 100644
--- a/EmbeddedPkg/EmbeddedPkg.dec
+++ b/EmbeddedPkg/EmbeddedPkg.dec
@@ -208,3 +208,10 @@ [PcdsFixedAtBuild.X64]
[PcdsFixedAtBuild.common, PcdsDynamic.common]
gEmbeddedTokenSpaceGuid.PcdFdtDevicePaths|L""|VOID*|0x00000055
+
+ #
+ # Value to add to a host address to obtain a device address, using
+ # unsigned 64-bit integer arithmetic. This means we can rely on
+ # truncation on overflow to specify negative offsets.
+ #
+ gEmbeddedTokenSpaceGuid.PcdDmaDeviceOffset|0x0|UINT64|0x0000058
diff --git a/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c b/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c
index 4cbe349190a9..564db83c901c 100644
--- a/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c
+++ b/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c
@@ -19,6 +19,14 @@
#include <Library/MemoryAllocationLib.h>
+STATIC
+PHYSICAL_ADDRESS
+HostToDeviceAddress (
+ IN VOID *Address
+ )
+{
+ return (PHYSICAL_ADDRESS)(UINTN)Address + PcdGet64 (PcdDmaDeviceOffset);
+}
/**
Provides the DMA controller-specific addresses needed to access system memory.
@@ -50,7 +58,7 @@ DmaMap (
OUT VOID **Mapping
)
{
- *DeviceAddress = (PHYSICAL_ADDRESS)(UINTN)HostAddress;
+ *DeviceAddress = HostToDeviceAddress (HostAddress);
*Mapping = NULL;
return EFI_SUCCESS;
}
diff --git a/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.inf b/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.inf
index c40a600cf6a3..f64d780e16ed 100644
--- a/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.inf
+++ b/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.inf
@@ -31,3 +31,6 @@ [Packages]
[LibraryClasses]
DebugLib
MemoryAllocationLib
+
+[Pcd]
+ gEmbeddedTokenSpaceGuid.PcdDmaDeviceOffset
--
2.11.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 2/6] EmbeddedPkg/CoherentDmaLib: add support for non-1:1 DMA translation
2017-08-30 8:21 ` [PATCH 2/6] EmbeddedPkg/CoherentDmaLib: add support for non-1:1 DMA translation Ard Biesheuvel
@ 2017-08-30 10:51 ` Leif Lindholm
2017-08-30 10:54 ` Ard Biesheuvel
0 siblings, 1 reply; 17+ messages in thread
From: Leif Lindholm @ 2017-08-30 10:51 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel
On Wed, Aug 30, 2017 at 09:21:04AM +0100, Ard Biesheuvel wrote:
> Bring CoherentDmaLib in line with ArmDmaLib, and add support for
> defining a static offset between the host's and the bus master's
> view of memory.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> EmbeddedPkg/EmbeddedPkg.dec | 7 +++++++
> EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c | 10 +++++++++-
> EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.inf | 3 +++
> 3 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
> index 8ad2a84c045c..ccdf38e36a8c 100644
> --- a/EmbeddedPkg/EmbeddedPkg.dec
> +++ b/EmbeddedPkg/EmbeddedPkg.dec
> @@ -208,3 +208,10 @@ [PcdsFixedAtBuild.X64]
>
> [PcdsFixedAtBuild.common, PcdsDynamic.common]
> gEmbeddedTokenSpaceGuid.PcdFdtDevicePaths|L""|VOID*|0x00000055
> +
> + #
> + # Value to add to a host address to obtain a device address, using
> + # unsigned 64-bit integer arithmetic. This means we can rely on
> + # truncation on overflow to specify negative offsets.
Is that promotion-safe on 32-bit archs?
/
Leif
> + #
> + gEmbeddedTokenSpaceGuid.PcdDmaDeviceOffset|0x0|UINT64|0x0000058
> diff --git a/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c b/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c
> index 4cbe349190a9..564db83c901c 100644
> --- a/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c
> +++ b/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c
> @@ -19,6 +19,14 @@
> #include <Library/MemoryAllocationLib.h>
>
>
> +STATIC
> +PHYSICAL_ADDRESS
> +HostToDeviceAddress (
> + IN VOID *Address
> + )
> +{
> + return (PHYSICAL_ADDRESS)(UINTN)Address + PcdGet64 (PcdDmaDeviceOffset);
> +}
>
> /**
> Provides the DMA controller-specific addresses needed to access system memory.
> @@ -50,7 +58,7 @@ DmaMap (
> OUT VOID **Mapping
> )
> {
> - *DeviceAddress = (PHYSICAL_ADDRESS)(UINTN)HostAddress;
> + *DeviceAddress = HostToDeviceAddress (HostAddress);
> *Mapping = NULL;
> return EFI_SUCCESS;
> }
> diff --git a/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.inf b/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.inf
> index c40a600cf6a3..f64d780e16ed 100644
> --- a/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.inf
> +++ b/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.inf
> @@ -31,3 +31,6 @@ [Packages]
> [LibraryClasses]
> DebugLib
> MemoryAllocationLib
> +
> +[Pcd]
> + gEmbeddedTokenSpaceGuid.PcdDmaDeviceOffset
> --
> 2.11.0
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 2/6] EmbeddedPkg/CoherentDmaLib: add support for non-1:1 DMA translation
2017-08-30 10:51 ` Leif Lindholm
@ 2017-08-30 10:54 ` Ard Biesheuvel
2017-08-30 11:47 ` Leif Lindholm
0 siblings, 1 reply; 17+ messages in thread
From: Ard Biesheuvel @ 2017-08-30 10:54 UTC (permalink / raw)
To: Leif Lindholm; +Cc: edk2-devel@lists.01.org
On 30 August 2017 at 11:51, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Wed, Aug 30, 2017 at 09:21:04AM +0100, Ard Biesheuvel wrote:
>> Bring CoherentDmaLib in line with ArmDmaLib, and add support for
>> defining a static offset between the host's and the bus master's
>> view of memory.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>> EmbeddedPkg/EmbeddedPkg.dec | 7 +++++++
>> EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c | 10 +++++++++-
>> EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.inf | 3 +++
>> 3 files changed, 19 insertions(+), 1 deletion(-)
>>
>> diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
>> index 8ad2a84c045c..ccdf38e36a8c 100644
>> --- a/EmbeddedPkg/EmbeddedPkg.dec
>> +++ b/EmbeddedPkg/EmbeddedPkg.dec
>> @@ -208,3 +208,10 @@ [PcdsFixedAtBuild.X64]
>>
>> [PcdsFixedAtBuild.common, PcdsDynamic.common]
>> gEmbeddedTokenSpaceGuid.PcdFdtDevicePaths|L""|VOID*|0x00000055
>> +
>> + #
>> + # Value to add to a host address to obtain a device address, using
>> + # unsigned 64-bit integer arithmetic. This means we can rely on
>> + # truncation on overflow to specify negative offsets.
>
> Is that promotion-safe on 32-bit archs?
>
Yes. EFI_PHYSICAL_ADDRESS is always 64-bits, and so is this PCD, so
whether it is a 32-bit platform or not should not make any difference.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/6] EmbeddedPkg/CoherentDmaLib: add support for non-1:1 DMA translation
2017-08-30 10:54 ` Ard Biesheuvel
@ 2017-08-30 11:47 ` Leif Lindholm
0 siblings, 0 replies; 17+ messages in thread
From: Leif Lindholm @ 2017-08-30 11:47 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel@lists.01.org
On Wed, Aug 30, 2017 at 11:54:05AM +0100, Ard Biesheuvel wrote:
> On 30 August 2017 at 11:51, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > On Wed, Aug 30, 2017 at 09:21:04AM +0100, Ard Biesheuvel wrote:
> >> Bring CoherentDmaLib in line with ArmDmaLib, and add support for
> >> defining a static offset between the host's and the bus master's
> >> view of memory.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> ---
> >> EmbeddedPkg/EmbeddedPkg.dec | 7 +++++++
> >> EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c | 10 +++++++++-
> >> EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.inf | 3 +++
> >> 3 files changed, 19 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
> >> index 8ad2a84c045c..ccdf38e36a8c 100644
> >> --- a/EmbeddedPkg/EmbeddedPkg.dec
> >> +++ b/EmbeddedPkg/EmbeddedPkg.dec
> >> @@ -208,3 +208,10 @@ [PcdsFixedAtBuild.X64]
> >>
> >> [PcdsFixedAtBuild.common, PcdsDynamic.common]
> >> gEmbeddedTokenSpaceGuid.PcdFdtDevicePaths|L""|VOID*|0x00000055
> >> +
> >> + #
> >> + # Value to add to a host address to obtain a device address, using
> >> + # unsigned 64-bit integer arithmetic. This means we can rely on
> >> + # truncation on overflow to specify negative offsets.
> >
> > Is that promotion-safe on 32-bit archs?
>
> Yes. EFI_PHYSICAL_ADDRESS is always 64-bits, and so is this PCD, so
> whether it is a 32-bit platform or not should not make any difference.
Right.
Well, EFI_PHYSICAL_ADDRESS is. PHYSICAL_ADDRESS appears to also be
(they are not derived from each other).
+ return (PHYSICAL_ADDRESS)(UINTN)Address + PcdGet64 (PcdDmaDeviceOffset);
(I think I misparsed the above as
return (PHYSICAL_ADDRESS)((UINTN)Address + PcdGet64 (PcdDmaDeviceOffset));
)
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
/
Leif
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/6] EmbeddedPkg: implement NonCoherentDmaLib based on ArmDmaLib
2017-08-30 8:21 [PATCH 0/6] ArmPkg EmbeddedPkg: clean up DmaLib implementations Ard Biesheuvel
2017-08-30 8:21 ` [PATCH 1/6] EmbeddedPkg: rename NullDmaLib to CoherentDmaLib Ard Biesheuvel
2017-08-30 8:21 ` [PATCH 2/6] EmbeddedPkg/CoherentDmaLib: add support for non-1:1 DMA translation Ard Biesheuvel
@ 2017-08-30 8:21 ` Ard Biesheuvel
2017-08-30 13:05 ` Leif Lindholm
2017-08-30 8:21 ` [PATCH 4/6] Omap35xxPkg: switch to EmbeddedPkg's NonCoherentDmaLib Ard Biesheuvel
` (3 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Ard Biesheuvel @ 2017-08-30 8:21 UTC (permalink / raw)
To: edk2-devel, leif.lindholm; +Cc: Ard Biesheuvel
The non-coherent DmaLib implementation in ArmDmaLib no longer relies on
anything in ArmPkg. So clone it into EmbeddedPkg, and rename it to
NonCoherentDmaLib.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
EmbeddedPkg/EmbeddedPkg.dsc | 1 +
EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.c | 491 ++++++++++++++++++++
EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.inf | 50 ++
3 files changed, 542 insertions(+)
diff --git a/EmbeddedPkg/EmbeddedPkg.dsc b/EmbeddedPkg/EmbeddedPkg.dsc
index 84c5a842e37e..012721a332f4 100644
--- a/EmbeddedPkg/EmbeddedPkg.dsc
+++ b/EmbeddedPkg/EmbeddedPkg.dsc
@@ -251,6 +251,7 @@ [Components.common]
EmbeddedPkg/Library/TemplateRealTimeClockLib/TemplateRealTimeClockLib.inf
EmbeddedPkg/Library/LzmaHobCustomDecompressLib/LzmaHobCustomDecompressLib.inf
EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.inf
+ EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.inf
EmbeddedPkg/Library/DxeDtPlatformDtbLoaderLibDefault/DxeDtPlatformDtbLoaderLibDefault.inf
EmbeddedPkg/Ebl/Ebl.inf
diff --git a/EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.c b/EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.c
new file mode 100644
index 000000000000..08b9c017f426
--- /dev/null
+++ b/EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.c
@@ -0,0 +1,491 @@
+/** @file
+
+ Generic non-coherent implementation of DmaLib.h
+
+ Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.<BR>
+ Copyright (c) 2015 - 2017, Linaro, Ltd. All rights reserved.<BR>
+
+ This program and the accompanying materials are licensed and made
+ available under the terms and conditions of the BSD License which
+ accompanies this distribution. The full text of the license may be
+ found at http://opensource.org/licenses/bsd-license.php
+
+ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
+ IMPLIED.
+
+**/
+
+#include <PiDxe.h>
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+#include <Library/DmaLib.h>
+#include <Library/DxeServicesTableLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/IoLib.h>
+#include <Library/BaseMemoryLib.h>
+
+#include <Protocol/Cpu.h>
+
+typedef struct {
+ EFI_PHYSICAL_ADDRESS HostAddress;
+ VOID *BufferAddress;
+ UINTN NumberOfBytes;
+ DMA_MAP_OPERATION Operation;
+ BOOLEAN DoubleBuffer;
+} MAP_INFO_INSTANCE;
+
+
+typedef struct {
+ LIST_ENTRY Link;
+ VOID *HostAddress;
+ UINTN NumPages;
+ UINT64 Attributes;
+} UNCACHED_ALLOCATION;
+
+STATIC EFI_CPU_ARCH_PROTOCOL *mCpu;
+STATIC LIST_ENTRY UncachedAllocationList;
+
+STATIC
+PHYSICAL_ADDRESS
+HostToDeviceAddress (
+ IN VOID *Address
+ )
+{
+ return (PHYSICAL_ADDRESS)(UINTN)Address + PcdGet64 (PcdDmaDeviceOffset);
+}
+
+/**
+ Provides the DMA controller-specific addresses needed to access system memory.
+
+ Operation is relative to the DMA bus master.
+
+ @param Operation Indicates if the bus master is going to read or
+ write to system memory.
+ @param HostAddress The system memory address to map to the DMA
+ controller.
+ @param NumberOfBytes On input the number of bytes to map. On output
+ the number of bytes that were mapped.
+ @param DeviceAddress The resulting map address for the bus master
+ controller to use to access the host's
+ HostAddress.
+ @param Mapping A resulting value to pass to Unmap().
+
+ @retval EFI_SUCCESS The range was mapped for the returned
+ NumberOfBytes.
+ @retval EFI_UNSUPPORTED The HostAddress cannot be mapped as a common
+ buffer.
+ @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
+ @retval EFI_OUT_OF_RESOURCES The request could not be completed due to a lack
+ of resources.
+ @retval EFI_DEVICE_ERROR The system hardware could not map the requested
+ address.
+
+**/
+EFI_STATUS
+EFIAPI
+DmaMap (
+ IN DMA_MAP_OPERATION Operation,
+ IN VOID *HostAddress,
+ IN OUT UINTN *NumberOfBytes,
+ OUT PHYSICAL_ADDRESS *DeviceAddress,
+ OUT VOID **Mapping
+ )
+{
+ EFI_STATUS Status;
+ MAP_INFO_INSTANCE *Map;
+ VOID *Buffer;
+ EFI_GCD_MEMORY_SPACE_DESCRIPTOR GcdDescriptor;
+ UINTN AllocSize;
+
+ if (HostAddress == NULL ||
+ NumberOfBytes == NULL ||
+ DeviceAddress == NULL ||
+ Mapping == NULL ) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ if (Operation >= MapOperationMaximum) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ *DeviceAddress = HostToDeviceAddress (HostAddress);
+
+ // Remember range so we can flush on the other side
+ Map = AllocatePool (sizeof (MAP_INFO_INSTANCE));
+ if (Map == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ if (Operation != MapOperationBusMasterRead &&
+ ((((UINTN)HostAddress & (mCpu->DmaBufferAlignment - 1)) != 0) ||
+ ((*NumberOfBytes & (mCpu->DmaBufferAlignment - 1)) != 0))) {
+
+ // Get the cacheability of the region
+ Status = gDS->GetMemorySpaceDescriptor ((UINTN)HostAddress, &GcdDescriptor);
+ if (EFI_ERROR(Status)) {
+ goto FreeMapInfo;
+ }
+
+ // If the mapped buffer is not an uncached buffer
+ if ((GcdDescriptor.Attributes & (EFI_MEMORY_WB | EFI_MEMORY_WT)) != 0) {
+ //
+ // Operations of type MapOperationBusMasterCommonBuffer are only allowed
+ // 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;
+ }
+
+ //
+ // If the buffer does not fill entire cache lines we must double buffer
+ // into a suitably aligned allocation that allows us to invalidate the
+ // cache without running the risk of corrupting adjacent unrelated data.
+ // Note that pool allocations are guaranteed to be 8 byte aligned, so
+ // we only have to add (alignment - 8) worth of padding.
+ //
+ Map->DoubleBuffer = TRUE;
+ AllocSize = ALIGN_VALUE (*NumberOfBytes, mCpu->DmaBufferAlignment) +
+ (mCpu->DmaBufferAlignment - 8);
+ Map->BufferAddress = AllocatePool (AllocSize);
+ if (Map->BufferAddress == NULL) {
+ Status = EFI_OUT_OF_RESOURCES;
+ goto FreeMapInfo;
+ }
+
+ 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;
+ }
+ } else {
+ Map->DoubleBuffer = FALSE;
+
+ DEBUG_CODE_BEGIN ();
+
+ //
+ // The operation type check above only executes if the buffer happens to be
+ // misaligned with respect to CWG, but even if it is aligned, we should not
+ // allow arbitrary buffers to be used for creating consistent mappings.
+ // So duplicate the check here when running in DEBUG mode, just to assert
+ // that we are not trying to create a consistent mapping for cached memory.
+ //
+ Status = gDS->GetMemorySpaceDescriptor ((UINTN)HostAddress, &GcdDescriptor);
+ ASSERT_EFI_ERROR(Status);
+
+ ASSERT (Operation != MapOperationBusMasterCommonBuffer ||
+ (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);
+ }
+
+ Map->HostAddress = (UINTN)HostAddress;
+ Map->NumberOfBytes = *NumberOfBytes;
+ Map->Operation = Operation;
+
+ *Mapping = Map;
+
+ return EFI_SUCCESS;
+
+FreeMapInfo:
+ FreePool (Map);
+
+ return Status;
+}
+
+
+/**
+ Completes the DmaMapBusMasterRead(), DmaMapBusMasterWrite(), or
+ DmaMapBusMasterCommonBuffer() operation and releases any corresponding
+ resources.
+
+ @param Mapping The mapping value returned from DmaMap*().
+
+ @retval EFI_SUCCESS The range was unmapped.
+ @retval EFI_DEVICE_ERROR The data was not committed to the target system
+ memory.
+ @retval EFI_INVALID_PARAMETER An inconsistency was detected between the
+ mapping type and the DoubleBuffer field
+
+**/
+EFI_STATUS
+EFIAPI
+DmaUnmap (
+ IN VOID *Mapping
+ )
+{
+ MAP_INFO_INSTANCE *Map;
+ EFI_STATUS Status;
+ VOID *Buffer;
+
+ if (Mapping == NULL) {
+ ASSERT (FALSE);
+ return EFI_INVALID_PARAMETER;
+ }
+
+ Map = (MAP_INFO_INSTANCE *)Mapping;
+
+ Status = EFI_SUCCESS;
+ if (Map->DoubleBuffer) {
+ ASSERT (Map->Operation == MapOperationBusMasterWrite);
+
+ if (Map->Operation != MapOperationBusMasterWrite) {
+ Status = EFI_INVALID_PARAMETER;
+ } else {
+ Buffer = ALIGN_POINTER (Map->BufferAddress, mCpu->DmaBufferAlignment);
+
+ mCpu->FlushDataCache (mCpu, (UINTN)Buffer, Map->NumberOfBytes,
+ EfiCpuFlushTypeInvalidate);
+
+ CopyMem ((VOID *)(UINTN)Map->HostAddress, Buffer, Map->NumberOfBytes);
+
+ FreePool (Map->BufferAddress);
+ }
+ } else {
+ if (Map->Operation == MapOperationBusMasterWrite) {
+ //
+ // Make sure we read buffer from uncached memory and not the cache
+ //
+ mCpu->FlushDataCache (mCpu, Map->HostAddress, Map->NumberOfBytes,
+ EfiCpuFlushTypeInvalidate);
+ }
+ }
+
+ FreePool (Map);
+
+ return Status;
+}
+
+/**
+ Allocates pages that are suitable for an DmaMap() of type
+ MapOperationBusMasterCommonBuffer mapping.
+
+ @param MemoryType The type of memory to allocate,
+ EfiBootServicesData or EfiRuntimeServicesData.
+ @param Pages The number of pages to allocate.
+ @param HostAddress A pointer to store the base system memory
+ address of the allocated range.
+
+ @retval EFI_SUCCESS The requested memory pages were allocated.
+ @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
+ @retval EFI_OUT_OF_RESOURCES The memory pages could not be allocated.
+
+**/
+EFI_STATUS
+EFIAPI
+DmaAllocateBuffer (
+ IN EFI_MEMORY_TYPE MemoryType,
+ IN UINTN Pages,
+ OUT VOID **HostAddress
+ )
+{
+ return DmaAllocateAlignedBuffer (MemoryType, Pages, 0, HostAddress);
+}
+
+/**
+ Allocates pages that are suitable for an DmaMap() of type
+ MapOperationBusMasterCommonBuffer mapping, at the requested alignment.
+
+ @param MemoryType The type of memory to allocate,
+ EfiBootServicesData or EfiRuntimeServicesData.
+ @param Pages The number of pages to allocate.
+ @param Alignment Alignment in bytes of the base of the returned
+ buffer (must be a power of 2)
+ @param HostAddress A pointer to store the base system memory
+ address of the allocated range.
+
+ @retval EFI_SUCCESS The requested memory pages were allocated.
+ @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
+ @retval EFI_OUT_OF_RESOURCES The memory pages could not be allocated.
+
+**/
+EFI_STATUS
+EFIAPI
+DmaAllocateAlignedBuffer (
+ IN EFI_MEMORY_TYPE MemoryType,
+ IN UINTN Pages,
+ IN UINTN Alignment,
+ OUT VOID **HostAddress
+ )
+{
+ EFI_GCD_MEMORY_SPACE_DESCRIPTOR GcdDescriptor;
+ VOID *Allocation;
+ UINT64 MemType;
+ UNCACHED_ALLOCATION *Alloc;
+ EFI_STATUS Status;
+
+ if (Alignment == 0) {
+ Alignment = EFI_PAGE_SIZE;
+ }
+
+ if (HostAddress == NULL ||
+ (Alignment & (Alignment - 1)) != 0) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ if (MemoryType == EfiBootServicesData) {
+ Allocation = AllocateAlignedPages (Pages, Alignment);
+ } else if (MemoryType == EfiRuntimeServicesData) {
+ Allocation = AllocateAlignedRuntimePages (Pages, Alignment);
+ } else {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ if (Allocation == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ // Get the cacheability of the region
+ Status = gDS->GetMemorySpaceDescriptor ((UINTN)Allocation, &GcdDescriptor);
+ if (EFI_ERROR(Status)) {
+ goto FreeBuffer;
+ }
+
+ // Choose a suitable uncached memory type that is supported by the region
+ if (GcdDescriptor.Capabilities & EFI_MEMORY_WC) {
+ MemType = EFI_MEMORY_WC;
+ } else if (GcdDescriptor.Capabilities & EFI_MEMORY_UC) {
+ MemType = EFI_MEMORY_UC;
+ } else {
+ Status = EFI_UNSUPPORTED;
+ goto FreeBuffer;
+ }
+
+ Alloc = AllocatePool (sizeof *Alloc);
+ if (Alloc == NULL) {
+ goto FreeBuffer;
+ }
+
+ Alloc->HostAddress = Allocation;
+ Alloc->NumPages = Pages;
+ Alloc->Attributes = GcdDescriptor.Attributes;
+
+ InsertHeadList (&UncachedAllocationList, &Alloc->Link);
+
+ // Remap the region with the new attributes
+ Status = gDS->SetMemorySpaceAttributes ((PHYSICAL_ADDRESS)(UINTN)Allocation,
+ EFI_PAGES_TO_SIZE (Pages),
+ MemType);
+ if (EFI_ERROR (Status)) {
+ goto FreeAlloc;
+ }
+
+ Status = mCpu->FlushDataCache (mCpu,
+ (PHYSICAL_ADDRESS)(UINTN)Allocation,
+ EFI_PAGES_TO_SIZE (Pages),
+ EfiCpuFlushTypeInvalidate);
+ if (EFI_ERROR (Status)) {
+ goto FreeAlloc;
+ }
+
+ *HostAddress = Allocation;
+
+ return EFI_SUCCESS;
+
+FreeAlloc:
+ RemoveEntryList (&Alloc->Link);
+ FreePool (Alloc);
+
+FreeBuffer:
+ FreePages (Allocation, Pages);
+ return Status;
+}
+
+
+/**
+ Frees memory that was allocated with DmaAllocateBuffer().
+
+ @param Pages The number of pages to free.
+ @param HostAddress The base system memory address of the allocated
+ range.
+
+ @retval EFI_SUCCESS The requested memory pages were freed.
+ @retval EFI_INVALID_PARAMETER The memory range specified by HostAddress and
+ Pages was not allocated with
+ DmaAllocateBuffer().
+
+**/
+EFI_STATUS
+EFIAPI
+DmaFreeBuffer (
+ IN UINTN Pages,
+ IN VOID *HostAddress
+ )
+{
+ LIST_ENTRY *Link;
+ UNCACHED_ALLOCATION *Alloc;
+ BOOLEAN Found;
+ EFI_STATUS Status;
+
+ if (HostAddress == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ for (Link = GetFirstNode (&UncachedAllocationList), Found = FALSE;
+ !IsNull (&UncachedAllocationList, Link);
+ Link = GetNextNode (&UncachedAllocationList, Link)) {
+
+ Alloc = BASE_CR (Link, UNCACHED_ALLOCATION, Link);
+ if (Alloc->HostAddress == HostAddress && Alloc->NumPages == Pages) {
+ Found = TRUE;
+ break;
+ }
+ }
+
+ if (!Found) {
+ ASSERT (FALSE);
+ return EFI_INVALID_PARAMETER;
+ }
+
+ RemoveEntryList (&Alloc->Link);
+
+ Status = gDS->SetMemorySpaceAttributes ((PHYSICAL_ADDRESS)(UINTN)HostAddress,
+ EFI_PAGES_TO_SIZE (Pages),
+ Alloc->Attributes);
+ if (EFI_ERROR (Status)) {
+ goto FreeAlloc;
+ }
+
+ //
+ // If we fail to restore the original attributes, it is better to leak the
+ // memory than to return it to the heap
+ //
+ FreePages (HostAddress, Pages);
+
+FreeAlloc:
+ FreePool (Alloc);
+ return Status;
+}
+
+
+EFI_STATUS
+EFIAPI
+NonCoherentDmaLibConstructor (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ InitializeListHead (&UncachedAllocationList);
+
+ // 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
new file mode 100644
index 000000000000..9f430d6c3721
--- /dev/null
+++ b/EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.inf
@@ -0,0 +1,50 @@
+#/** @file
+#
+# Generic non-coherent implementation of DmaLib.h
+#
+# Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.<BR>
+# Copyright (c) 2015 - 2017, Linaro, Ltd. All rights reserved.<BR>
+#
+# This program and the accompanying materials are licensed and made
+# available under the terms and conditions of the BSD License which
+# accompanies this distribution. The full text of the license may be
+# found at http://opensource.org/licenses/bsd-license.php
+#
+# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
+# IMPLIED.
+#
+#**/
+
+[Defines]
+ INF_VERSION = 0x00010019
+ BASE_NAME = NonCoherentDmaLib
+ FILE_GUID = 43ad4920-db15-4e24-9889-2db568431fbd
+ MODULE_TYPE = DXE_DRIVER
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = DmaLib
+ CONSTRUCTOR = NonCoherentDmaLibConstructor
+
+[Sources]
+ NonCoherentDmaLib.c
+
+[Packages]
+ EmbeddedPkg/EmbeddedPkg.dec
+ MdePkg/MdePkg.dec
+
+[LibraryClasses]
+ BaseMemoryLib
+ DebugLib
+ DxeServicesTableLib
+ IoLib
+ MemoryAllocationLib
+ UefiBootServicesTableLib
+
+[Protocols]
+ gEfiCpuArchProtocolGuid
+
+[Pcd]
+ gEmbeddedTokenSpaceGuid.PcdDmaDeviceOffset
+
+[Depex]
+ gEfiCpuArchProtocolGuid
--
2.11.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 4/6] Omap35xxPkg: switch to EmbeddedPkg's NonCoherentDmaLib
2017-08-30 8:21 [PATCH 0/6] ArmPkg EmbeddedPkg: clean up DmaLib implementations Ard Biesheuvel
` (2 preceding siblings ...)
2017-08-30 8:21 ` [PATCH 3/6] EmbeddedPkg: implement NonCoherentDmaLib based on ArmDmaLib Ard Biesheuvel
@ 2017-08-30 8:21 ` Ard Biesheuvel
2017-08-30 8:21 ` [PATCH 5/6] BeagleBoardPkg: switch to generic non-coherent DmaLib Ard Biesheuvel
` (2 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Ard Biesheuvel @ 2017-08-30 8:21 UTC (permalink / raw)
To: edk2-devel, leif.lindholm; +Cc: Ard Biesheuvel
Replace the reference to the ARM specific ArmDmaLib with a reference
to the generic NonCoherentDmaLib.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
Omap35xxPkg/Omap35xxPkg.dsc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Omap35xxPkg/Omap35xxPkg.dsc b/Omap35xxPkg/Omap35xxPkg.dsc
index 941bc97060b9..c5d9746104e6 100644
--- a/Omap35xxPkg/Omap35xxPkg.dsc
+++ b/Omap35xxPkg/Omap35xxPkg.dsc
@@ -61,7 +61,7 @@ [LibraryClasses.common]
DxeServicesTableLib|MdePkg/Library/DxeServicesTableLib/DxeServicesTableLib.inf
UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
UefiApplicationEntryPoint|MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf
- DmaLib|ArmPkg/Library/ArmDmaLib/ArmDmaLib.inf
+ DmaLib|EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.inf
TimerLib|Omap35xxPkg/Library/Omap35xxTimerLib/Omap35xxTimerLib.inf
--
2.11.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 5/6] BeagleBoardPkg: switch to generic non-coherent DmaLib
2017-08-30 8:21 [PATCH 0/6] ArmPkg EmbeddedPkg: clean up DmaLib implementations Ard Biesheuvel
` (3 preceding siblings ...)
2017-08-30 8:21 ` [PATCH 4/6] Omap35xxPkg: switch to EmbeddedPkg's NonCoherentDmaLib Ard Biesheuvel
@ 2017-08-30 8:21 ` Ard Biesheuvel
2017-08-30 8:21 ` [PATCH 6/6] ArmPkg: remove ArmDmaLib Ard Biesheuvel
2017-08-30 13:06 ` [PATCH 0/6] ArmPkg EmbeddedPkg: clean up DmaLib implementations Leif Lindholm
6 siblings, 0 replies; 17+ messages in thread
From: Ard Biesheuvel @ 2017-08-30 8:21 UTC (permalink / raw)
To: edk2-devel, leif.lindholm; +Cc: Ard Biesheuvel
Replace the reference to the ARM specific ArmDmaLib with a reference
to the generic NonCoherentDmaLib.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
BeagleBoardPkg/BeagleBoardPkg.dsc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/BeagleBoardPkg/BeagleBoardPkg.dsc b/BeagleBoardPkg/BeagleBoardPkg.dsc
index 84aae84ff52d..30f6fd02e82f 100644
--- a/BeagleBoardPkg/BeagleBoardPkg.dsc
+++ b/BeagleBoardPkg/BeagleBoardPkg.dsc
@@ -122,7 +122,7 @@ [LibraryClasses.common]
GdbSerialLib|Omap35xxPkg/Library/GdbSerialLib/GdbSerialLib.inf
ArmDisassemblerLib|ArmPkg/Library/ArmDisassemblerLib/ArmDisassemblerLib.inf
DebugAgentLib|MdeModulePkg/Library/DebugAgentLibNull/DebugAgentLibNull.inf
- DmaLib|ArmPkg/Library/ArmDmaLib/ArmDmaLib.inf
+ DmaLib|EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.inf
NetLib|MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf
FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
--
2.11.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 6/6] ArmPkg: remove ArmDmaLib
2017-08-30 8:21 [PATCH 0/6] ArmPkg EmbeddedPkg: clean up DmaLib implementations Ard Biesheuvel
` (4 preceding siblings ...)
2017-08-30 8:21 ` [PATCH 5/6] BeagleBoardPkg: switch to generic non-coherent DmaLib Ard Biesheuvel
@ 2017-08-30 8:21 ` Ard Biesheuvel
2017-08-30 13:06 ` [PATCH 0/6] ArmPkg EmbeddedPkg: clean up DmaLib implementations Leif Lindholm
6 siblings, 0 replies; 17+ messages in thread
From: Ard Biesheuvel @ 2017-08-30 8:21 UTC (permalink / raw)
To: edk2-devel, leif.lindholm; +Cc: Ard Biesheuvel
Now that we have a generic DmaLib implementation for non-coherent DMA,
let's get rid of the ARM specific one.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
ArmPkg/ArmPkg.dsc | 2 -
ArmPkg/Library/ArmDmaLib/ArmDmaLib.c | 478 --------------------
ArmPkg/Library/ArmDmaLib/ArmDmaLib.inf | 49 --
3 files changed, 529 deletions(-)
diff --git a/ArmPkg/ArmPkg.dsc b/ArmPkg/ArmPkg.dsc
index ff2b0c074dc1..cf86f89bd702 100644
--- a/ArmPkg/ArmPkg.dsc
+++ b/ArmPkg/ArmPkg.dsc
@@ -72,7 +72,6 @@ [LibraryClasses.common]
ArmGenericTimerCounterLib|ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.inf
ArmSmcLib|ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
ArmDisassemblerLib|ArmPkg/Library/ArmDisassemblerLib/ArmDisassemblerLib.inf
- DmaLib|ArmPkg/Library/ArmDmaLib/ArmDmaLib.inf
UefiApplicationEntryPoint|MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf
PerformanceLib|MdePkg/Library/BasePerformanceLibNull/BasePerformanceLibNull.inf
@@ -106,7 +105,6 @@ [LibraryClasses.ARM, LibraryClasses.AARCH64]
[Components.common]
ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.inf
ArmPkg/Library/ArmDisassemblerLib/ArmDisassemblerLib.inf
- ArmPkg/Library/ArmDmaLib/ArmDmaLib.inf
ArmPkg/Library/BdsLib/BdsLib.inf
ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
ArmPkg/Library/DebugAgentSymbolsBaseLib/DebugAgentSymbolsBaseLib.inf
diff --git a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c
deleted file mode 100644
index 2a8cf0fe21a4..000000000000
--- a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c
+++ /dev/null
@@ -1,478 +0,0 @@
-/** @file
- Generic ARM implementation of DmaLib.h
-
- Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.<BR>
- Copyright (c) 2015 - 2017, Linaro, Ltd. All rights reserved.<BR>
-
- This program and the accompanying materials
- are licensed and made available under the terms and conditions of the BSD License
- which accompanies this distribution. The full text of the license may be found at
- http://opensource.org/licenses/bsd-license.php
-
- THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
- WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
-
-**/
-
-#include <PiDxe.h>
-#include <Library/BaseLib.h>
-#include <Library/DebugLib.h>
-#include <Library/DmaLib.h>
-#include <Library/DxeServicesTableLib.h>
-#include <Library/MemoryAllocationLib.h>
-#include <Library/UefiBootServicesTableLib.h>
-#include <Library/IoLib.h>
-#include <Library/BaseMemoryLib.h>
-
-#include <Protocol/Cpu.h>
-
-typedef struct {
- EFI_PHYSICAL_ADDRESS HostAddress;
- VOID *BufferAddress;
- UINTN NumberOfBytes;
- DMA_MAP_OPERATION Operation;
- BOOLEAN DoubleBuffer;
-} MAP_INFO_INSTANCE;
-
-
-typedef struct {
- LIST_ENTRY Link;
- VOID *HostAddress;
- UINTN NumPages;
- UINT64 Attributes;
-} UNCACHED_ALLOCATION;
-
-STATIC EFI_CPU_ARCH_PROTOCOL *mCpu;
-STATIC LIST_ENTRY UncachedAllocationList;
-
-STATIC
-PHYSICAL_ADDRESS
-HostToDeviceAddress (
- IN VOID *Address
- )
-{
- return (PHYSICAL_ADDRESS)(UINTN)Address + PcdGet64 (PcdArmDmaDeviceOffset);
-}
-
-/**
- Provides the DMA controller-specific addresses needed to access system memory.
-
- Operation is relative to the DMA bus master.
-
- @param Operation Indicates if the bus master is going to read or write to system memory.
- @param HostAddress The system memory address to map to the DMA controller.
- @param NumberOfBytes On input the number of bytes to map. On output the number of bytes
- that were mapped.
- @param DeviceAddress The resulting map address for the bus master controller to use to
- access the hosts HostAddress.
- @param Mapping A resulting value to pass to Unmap().
-
- @retval EFI_SUCCESS The range was mapped for the returned NumberOfBytes.
- @retval EFI_UNSUPPORTED The HostAddress cannot be mapped as a common buffer.
- @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
- @retval EFI_OUT_OF_RESOURCES The request could not be completed due to a lack of resources.
- @retval EFI_DEVICE_ERROR The system hardware could not map the requested address.
-
-**/
-EFI_STATUS
-EFIAPI
-DmaMap (
- IN DMA_MAP_OPERATION Operation,
- IN VOID *HostAddress,
- IN OUT UINTN *NumberOfBytes,
- OUT PHYSICAL_ADDRESS *DeviceAddress,
- OUT VOID **Mapping
- )
-{
- EFI_STATUS Status;
- MAP_INFO_INSTANCE *Map;
- VOID *Buffer;
- EFI_GCD_MEMORY_SPACE_DESCRIPTOR GcdDescriptor;
- UINTN AllocSize;
-
- if (HostAddress == NULL || NumberOfBytes == NULL || DeviceAddress == NULL || Mapping == NULL ) {
- return EFI_INVALID_PARAMETER;
- }
-
- if (Operation >= MapOperationMaximum) {
- return EFI_INVALID_PARAMETER;
- }
-
- *DeviceAddress = HostToDeviceAddress (HostAddress);
-
- // Remember range so we can flush on the other side
- Map = AllocatePool (sizeof (MAP_INFO_INSTANCE));
- if (Map == NULL) {
- return EFI_OUT_OF_RESOURCES;
- }
-
- if (Operation != MapOperationBusMasterRead &&
- ((((UINTN)HostAddress & (mCpu->DmaBufferAlignment - 1)) != 0) ||
- ((*NumberOfBytes & (mCpu->DmaBufferAlignment - 1)) != 0))) {
-
- // Get the cacheability of the region
- Status = gDS->GetMemorySpaceDescriptor ((UINTN)HostAddress, &GcdDescriptor);
- if (EFI_ERROR(Status)) {
- goto FreeMapInfo;
- }
-
- // If the mapped buffer is not an uncached buffer
- if ((GcdDescriptor.Attributes & (EFI_MEMORY_WB | EFI_MEMORY_WT)) != 0) {
- //
- // Operations of type MapOperationBusMasterCommonBuffer are only allowed
- // on uncached buffers.
- //
- if (Operation == MapOperationBusMasterCommonBuffer) {
- DEBUG ((EFI_D_ERROR,
- "%a: Operation type 'MapOperationBusMasterCommonBuffer' is only supported\n"
- "on memory regions that were allocated using DmaAllocateBuffer ()\n",
- __FUNCTION__));
- Status = EFI_UNSUPPORTED;
- goto FreeMapInfo;
- }
-
- //
- // If the buffer does not fill entire cache lines we must double buffer
- // into a suitably aligned allocation that allows us to invalidate the
- // cache without running the risk of corrupting adjacent unrelated data.
- // Note that pool allocations are guaranteed to be 8 byte aligned, so
- // we only have to add (alignment - 8) worth of padding.
- //
- Map->DoubleBuffer = TRUE;
- AllocSize = ALIGN_VALUE (*NumberOfBytes, mCpu->DmaBufferAlignment) +
- (mCpu->DmaBufferAlignment - 8);
- Map->BufferAddress = AllocatePool (AllocSize);
- if (Map->BufferAddress == NULL) {
- Status = EFI_OUT_OF_RESOURCES;
- goto FreeMapInfo;
- }
-
- 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;
- }
- } else {
- Map->DoubleBuffer = FALSE;
-
- DEBUG_CODE_BEGIN ();
-
- //
- // The operation type check above only executes if the buffer happens to be
- // misaligned with respect to CWG, but even if it is aligned, we should not
- // allow arbitrary buffers to be used for creating consistent mappings.
- // So duplicate the check here when running in DEBUG mode, just to assert
- // that we are not trying to create a consistent mapping for cached memory.
- //
- Status = gDS->GetMemorySpaceDescriptor ((UINTN)HostAddress, &GcdDescriptor);
- ASSERT_EFI_ERROR(Status);
-
- ASSERT (Operation != MapOperationBusMasterCommonBuffer ||
- (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);
- }
-
- Map->HostAddress = (UINTN)HostAddress;
- Map->NumberOfBytes = *NumberOfBytes;
- Map->Operation = Operation;
-
- *Mapping = Map;
-
- return EFI_SUCCESS;
-
-FreeMapInfo:
- FreePool (Map);
-
- return Status;
-}
-
-
-/**
- Completes the DmaMapBusMasterRead(), DmaMapBusMasterWrite(), or DmaMapBusMasterCommonBuffer()
- operation and releases any corresponding resources.
-
- @param Mapping The mapping value returned from DmaMap*().
-
- @retval EFI_SUCCESS The range was unmapped.
- @retval EFI_DEVICE_ERROR The data was not committed to the target system memory.
- @retval EFI_INVALID_PARAMETER An inconsistency was detected between the mapping type
- and the DoubleBuffer field
-
-**/
-EFI_STATUS
-EFIAPI
-DmaUnmap (
- IN VOID *Mapping
- )
-{
- MAP_INFO_INSTANCE *Map;
- EFI_STATUS Status;
- VOID *Buffer;
-
- if (Mapping == NULL) {
- ASSERT (FALSE);
- return EFI_INVALID_PARAMETER;
- }
-
- Map = (MAP_INFO_INSTANCE *)Mapping;
-
- Status = EFI_SUCCESS;
- if (Map->DoubleBuffer) {
- ASSERT (Map->Operation == MapOperationBusMasterWrite);
-
- if (Map->Operation != MapOperationBusMasterWrite) {
- Status = EFI_INVALID_PARAMETER;
- } else {
- Buffer = ALIGN_POINTER (Map->BufferAddress, mCpu->DmaBufferAlignment);
-
- mCpu->FlushDataCache (mCpu, (UINTN)Buffer, Map->NumberOfBytes,
- EfiCpuFlushTypeInvalidate);
-
- CopyMem ((VOID *)(UINTN)Map->HostAddress, Buffer, Map->NumberOfBytes);
-
- FreePool (Map->BufferAddress);
- }
- } else {
- if (Map->Operation == MapOperationBusMasterWrite) {
- //
- // Make sure we read buffer from uncached memory and not the cache
- //
- mCpu->FlushDataCache (mCpu, Map->HostAddress, Map->NumberOfBytes,
- EfiCpuFlushTypeInvalidate);
- }
- }
-
- FreePool (Map);
-
- return Status;
-}
-
-/**
- Allocates pages that are suitable for an DmaMap() of type MapOperationBusMasterCommonBuffer.
- mapping.
-
- @param MemoryType The type of memory to allocate, EfiBootServicesData or
- EfiRuntimeServicesData.
- @param Pages The number of pages to allocate.
- @param HostAddress A pointer to store the base system memory address of the
- allocated range.
-
- @retval EFI_SUCCESS The requested memory pages were allocated.
- @retval EFI_UNSUPPORTED Attributes is unsupported. The only legal attribute bits are
- MEMORY_WRITE_COMBINE and MEMORY_CACHED.
- @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
- @retval EFI_OUT_OF_RESOURCES The memory pages could not be allocated.
-
-**/
-EFI_STATUS
-EFIAPI
-DmaAllocateBuffer (
- IN EFI_MEMORY_TYPE MemoryType,
- IN UINTN Pages,
- OUT VOID **HostAddress
- )
-{
- return DmaAllocateAlignedBuffer (MemoryType, Pages, 0, HostAddress);
-}
-
-/**
- Allocates pages that are suitable for an DmaMap() of type
- MapOperationBusMasterCommonBuffer mapping, at the requested alignment.
-
- @param MemoryType The type of memory to allocate, EfiBootServicesData or
- EfiRuntimeServicesData.
- @param Pages The number of pages to allocate.
- @param Alignment Alignment in bytes of the base of the returned
- buffer (must be a power of 2)
- @param HostAddress A pointer to store the base system memory address of the
- allocated range.
-
- @retval EFI_SUCCESS The requested memory pages were allocated.
- @retval EFI_UNSUPPORTED Attributes is unsupported. The only legal attribute bits are
- MEMORY_WRITE_COMBINE and MEMORY_CACHED.
- @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
- @retval EFI_OUT_OF_RESOURCES The memory pages could not be allocated.
-
-**/
-EFI_STATUS
-EFIAPI
-DmaAllocateAlignedBuffer (
- IN EFI_MEMORY_TYPE MemoryType,
- IN UINTN Pages,
- IN UINTN Alignment,
- OUT VOID **HostAddress
- )
-{
- EFI_GCD_MEMORY_SPACE_DESCRIPTOR GcdDescriptor;
- VOID *Allocation;
- UINT64 MemType;
- UNCACHED_ALLOCATION *Alloc;
- EFI_STATUS Status;
-
- if (Alignment == 0) {
- Alignment = EFI_PAGE_SIZE;
- }
-
- if (HostAddress == NULL ||
- (Alignment & (Alignment - 1)) != 0) {
- return EFI_INVALID_PARAMETER;
- }
-
- if (MemoryType == EfiBootServicesData) {
- Allocation = AllocateAlignedPages (Pages, Alignment);
- } else if (MemoryType == EfiRuntimeServicesData) {
- Allocation = AllocateAlignedRuntimePages (Pages, Alignment);
- } else {
- return EFI_INVALID_PARAMETER;
- }
-
- if (Allocation == NULL) {
- return EFI_OUT_OF_RESOURCES;
- }
-
- // Get the cacheability of the region
- Status = gDS->GetMemorySpaceDescriptor ((UINTN)Allocation, &GcdDescriptor);
- if (EFI_ERROR(Status)) {
- goto FreeBuffer;
- }
-
- // Choose a suitable uncached memory type that is supported by the region
- if (GcdDescriptor.Capabilities & EFI_MEMORY_WC) {
- MemType = EFI_MEMORY_WC;
- } else if (GcdDescriptor.Capabilities & EFI_MEMORY_UC) {
- MemType = EFI_MEMORY_UC;
- } else {
- Status = EFI_UNSUPPORTED;
- goto FreeBuffer;
- }
-
- Alloc = AllocatePool (sizeof *Alloc);
- if (Alloc == NULL) {
- goto FreeBuffer;
- }
-
- Alloc->HostAddress = Allocation;
- Alloc->NumPages = Pages;
- Alloc->Attributes = GcdDescriptor.Attributes;
-
- InsertHeadList (&UncachedAllocationList, &Alloc->Link);
-
- // Remap the region with the new attributes
- Status = gDS->SetMemorySpaceAttributes ((PHYSICAL_ADDRESS)(UINTN)Allocation,
- EFI_PAGES_TO_SIZE (Pages),
- MemType);
- if (EFI_ERROR (Status)) {
- goto FreeAlloc;
- }
-
- Status = mCpu->FlushDataCache (mCpu,
- (PHYSICAL_ADDRESS)(UINTN)Allocation,
- EFI_PAGES_TO_SIZE (Pages),
- EfiCpuFlushTypeInvalidate);
- if (EFI_ERROR (Status)) {
- goto FreeAlloc;
- }
-
- *HostAddress = Allocation;
-
- return EFI_SUCCESS;
-
-FreeAlloc:
- RemoveEntryList (&Alloc->Link);
- FreePool (Alloc);
-
-FreeBuffer:
- FreePages (Allocation, Pages);
- return Status;
-}
-
-
-/**
- Frees memory that was allocated with DmaAllocateBuffer().
-
- @param Pages The number of pages to free.
- @param HostAddress The base system memory address of the allocated range.
-
- @retval EFI_SUCCESS The requested memory pages were freed.
- @retval EFI_INVALID_PARAMETER The memory range specified by HostAddress and Pages
- was not allocated with DmaAllocateBuffer().
-
-**/
-EFI_STATUS
-EFIAPI
-DmaFreeBuffer (
- IN UINTN Pages,
- IN VOID *HostAddress
- )
-{
- LIST_ENTRY *Link;
- UNCACHED_ALLOCATION *Alloc;
- BOOLEAN Found;
- EFI_STATUS Status;
-
- if (HostAddress == NULL) {
- return EFI_INVALID_PARAMETER;
- }
-
- for (Link = GetFirstNode (&UncachedAllocationList), Found = FALSE;
- !IsNull (&UncachedAllocationList, Link);
- Link = GetNextNode (&UncachedAllocationList, Link)) {
-
- Alloc = BASE_CR (Link, UNCACHED_ALLOCATION, Link);
- if (Alloc->HostAddress == HostAddress && Alloc->NumPages == Pages) {
- Found = TRUE;
- break;
- }
- }
-
- if (!Found) {
- ASSERT (FALSE);
- return EFI_INVALID_PARAMETER;
- }
-
- RemoveEntryList (&Alloc->Link);
-
- Status = gDS->SetMemorySpaceAttributes ((PHYSICAL_ADDRESS)(UINTN)HostAddress,
- EFI_PAGES_TO_SIZE (Pages),
- Alloc->Attributes);
- if (EFI_ERROR (Status)) {
- goto FreeAlloc;
- }
-
- //
- // If we fail to restore the original attributes, it is better to leak the
- // memory than to return it to the heap
- //
- FreePages (HostAddress, Pages);
-
-FreeAlloc:
- FreePool (Alloc);
- return Status;
-}
-
-
-EFI_STATUS
-EFIAPI
-ArmDmaLibConstructor (
- IN EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE *SystemTable
- )
-{
- InitializeListHead (&UncachedAllocationList);
-
- // Get the Cpu protocol for later use
- return gBS->LocateProtocol (&gEfiCpuArchProtocolGuid, NULL, (VOID **)&mCpu);
-}
diff --git a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.inf b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.inf
deleted file mode 100644
index e33ed92c9d20..000000000000
--- a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.inf
+++ /dev/null
@@ -1,49 +0,0 @@
-#/** @file
-#
-# Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.<BR>
-# This program and the accompanying materials
-# are licensed and made available under the terms and conditions of the BSD License
-# which accompanies this distribution. The full text of the license may be found at
-# http://opensource.org/licenses/bsd-license.php
-#
-# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
-# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
-#
-#**/
-
-[Defines]
- INF_VERSION = 0x00010005
- BASE_NAME = ArmDmaLib
- FILE_GUID = F1BD6B36-B705-43aa-8A28-33F58ED85EFB
- MODULE_TYPE = UEFI_DRIVER
- VERSION_STRING = 1.0
- LIBRARY_CLASS = DmaLib
- CONSTRUCTOR = ArmDmaLibConstructor
-
-[Sources.common]
- ArmDmaLib.c
-
-[Packages]
- MdePkg/MdePkg.dec
- EmbeddedPkg/EmbeddedPkg.dec
- ArmPkg/ArmPkg.dec
-
-
-[LibraryClasses]
- DebugLib
- DxeServicesTableLib
- UefiBootServicesTableLib
- MemoryAllocationLib
- IoLib
- BaseMemoryLib
-
-[Protocols]
- gEfiCpuArchProtocolGuid
-
-[Guids]
-
-[Pcd]
- gArmTokenSpaceGuid.PcdArmDmaDeviceOffset
-
-[Depex]
- gEfiCpuArchProtocolGuid
--
2.11.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 0/6] ArmPkg EmbeddedPkg: clean up DmaLib implementations
2017-08-30 8:21 [PATCH 0/6] ArmPkg EmbeddedPkg: clean up DmaLib implementations Ard Biesheuvel
` (5 preceding siblings ...)
2017-08-30 8:21 ` [PATCH 6/6] ArmPkg: remove ArmDmaLib Ard Biesheuvel
@ 2017-08-30 13:06 ` Leif Lindholm
2017-08-30 13:17 ` Ard Biesheuvel
6 siblings, 1 reply; 17+ messages in thread
From: Leif Lindholm @ 2017-08-30 13:06 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel
On Wed, Aug 30, 2017 at 09:21:02AM +0100, Ard Biesheuvel wrote:
> Currently, we have two DmaLib implementations: a cache coherent one called
> 'NullDmaLib' residing in EmbeddedPkg, and a non-cache coherent one called
> 'ArmDmaLib', residinh in ArmPkg.
>
> In both cases, this is slightly awkward: NullDmaLib suggests no functionality
> whatsoever, which is slightly misleading because 'nothing' is the correct
> action in case of cache coherent DMA, rather than a lack of action. As for
> ArmDmalib, this was never specific to ARM, and no longer depends on anything
> that ArmPkg provides, so it does not really belong in ArmPkg anymore.
>
> So let's rename them to CoherentDmaLib and NonCoherentDmaLib, respectively,
> and move that latter into EmbeddedPkg where it arguably belongs. To align
> the two further, add support for non-1:1 DMA mappings to CoherentDmaLib as
> well.
>
> Note that the final patch can only be merged after out-of-tree platforms
> have switched from ArmDmaLib to NonCoherentDmaLib.
For 4-6/6:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> Ard Biesheuvel (6):
> EmbeddedPkg: rename NullDmaLib to CoherentDmaLib
> EmbeddedPkg/CoherentDmaLib: add support for non-1:1 DMA translation
> EmbeddedPkg: implement NonCoherentDmaLib based on ArmDmaLib
> Omap35xxPkg: switch to EmbeddedPkg's NonCoherentDmaLib
> BeagleBoardPkg: switch to generic non-coherent DmaLib
> ArmPkg: remove ArmDmaLib
>
> ArmPkg/ArmPkg.dsc | 2 -
> ArmPkg/Library/ArmDmaLib/ArmDmaLib.inf | 49 ---------
> BeagleBoardPkg/BeagleBoardPkg.dsc | 2 +-
> EmbeddedPkg/EmbeddedPkg.dec | 7 ++
> EmbeddedPkg/EmbeddedPkg.dsc | 3 +-
> EmbeddedPkg/Library/{NullDmaLib/NullDmaLib.c => CoherentDmaLib/CoherentDmaLib.c} | 10 +-
> EmbeddedPkg/Library/{NullDmaLib/NullDmaLib.inf => CoherentDmaLib/CoherentDmaLib.inf} | 19 ++--
> ArmPkg/Library/ArmDmaLib/ArmDmaLib.c => EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.c | 105 +++++++++++---------
> EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.inf | 50 ++++++++++
> Omap35xxPkg/Omap35xxPkg.dsc | 2 +-
> 10 files changed, 136 insertions(+), 113 deletions(-)
> delete mode 100644 ArmPkg/Library/ArmDmaLib/ArmDmaLib.inf
> rename EmbeddedPkg/Library/{NullDmaLib/NullDmaLib.c => CoherentDmaLib/CoherentDmaLib.c} (94%)
> rename EmbeddedPkg/Library/{NullDmaLib/NullDmaLib.inf => CoherentDmaLib/CoherentDmaLib.inf} (75%)
> rename ArmPkg/Library/ArmDmaLib/ArmDmaLib.c => EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.c (82%)
> create mode 100644 EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.inf
>
> --
> 2.11.0
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 0/6] ArmPkg EmbeddedPkg: clean up DmaLib implementations
2017-08-30 13:06 ` [PATCH 0/6] ArmPkg EmbeddedPkg: clean up DmaLib implementations Leif Lindholm
@ 2017-08-30 13:17 ` Ard Biesheuvel
2017-09-01 12:01 ` Ard Biesheuvel
0 siblings, 1 reply; 17+ messages in thread
From: Ard Biesheuvel @ 2017-08-30 13:17 UTC (permalink / raw)
To: Leif Lindholm; +Cc: edk2-devel@lists.01.org
On 30 August 2017 at 14:06, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Wed, Aug 30, 2017 at 09:21:02AM +0100, Ard Biesheuvel wrote:
>> Currently, we have two DmaLib implementations: a cache coherent one called
>> 'NullDmaLib' residing in EmbeddedPkg, and a non-cache coherent one called
>> 'ArmDmaLib', residinh in ArmPkg.
>>
>> In both cases, this is slightly awkward: NullDmaLib suggests no functionality
>> whatsoever, which is slightly misleading because 'nothing' is the correct
>> action in case of cache coherent DMA, rather than a lack of action. As for
>> ArmDmalib, this was never specific to ARM, and no longer depends on anything
>> that ArmPkg provides, so it does not really belong in ArmPkg anymore.
>>
>> So let's rename them to CoherentDmaLib and NonCoherentDmaLib, respectively,
>> and move that latter into EmbeddedPkg where it arguably belongs. To align
>> the two further, add support for non-1:1 DMA mappings to CoherentDmaLib as
>> well.
>>
>> Note that the final patch can only be merged after out-of-tree platforms
>> have switched from ArmDmaLib to NonCoherentDmaLib.
>
> For 4-6/6:
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>
Thanks,
#1 - #5 merged as
7385d2543e2a EmbeddedPkg: rename NullDmaLib to CoherentDmaLib
0bcb80106762 EmbeddedPkg/CoherentDmaLib: add support for non-1:1 DMA translation
723102c72fb0 EmbeddedPkg: implement NonCoherentDmaLib based on ArmDmaLib
c878cd95e132 Omap35xxPkg: switch to EmbeddedPkg's NonCoherentDmaLib
877f4460b3e3 BeagleBoardPkg: switch to generic non-coherent DmaLib
#6 needs to wait until the edk2-platforms changes are in.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/6] ArmPkg EmbeddedPkg: clean up DmaLib implementations
2017-08-30 13:17 ` Ard Biesheuvel
@ 2017-09-01 12:01 ` Ard Biesheuvel
0 siblings, 0 replies; 17+ messages in thread
From: Ard Biesheuvel @ 2017-09-01 12:01 UTC (permalink / raw)
To: Leif Lindholm; +Cc: edk2-devel@lists.01.org
On 30 August 2017 at 14:17, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 30 August 2017 at 14:06, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>> On Wed, Aug 30, 2017 at 09:21:02AM +0100, Ard Biesheuvel wrote:
>>> Currently, we have two DmaLib implementations: a cache coherent one called
>>> 'NullDmaLib' residing in EmbeddedPkg, and a non-cache coherent one called
>>> 'ArmDmaLib', residinh in ArmPkg.
>>>
>>> In both cases, this is slightly awkward: NullDmaLib suggests no functionality
>>> whatsoever, which is slightly misleading because 'nothing' is the correct
>>> action in case of cache coherent DMA, rather than a lack of action. As for
>>> ArmDmalib, this was never specific to ARM, and no longer depends on anything
>>> that ArmPkg provides, so it does not really belong in ArmPkg anymore.
>>>
>>> So let's rename them to CoherentDmaLib and NonCoherentDmaLib, respectively,
>>> and move that latter into EmbeddedPkg where it arguably belongs. To align
>>> the two further, add support for non-1:1 DMA mappings to CoherentDmaLib as
>>> well.
>>>
>>> Note that the final patch can only be merged after out-of-tree platforms
>>> have switched from ArmDmaLib to NonCoherentDmaLib.
>>
>> For 4-6/6:
>> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>>
>
> Thanks,
>
> #1 - #5 merged as
>
> 7385d2543e2a EmbeddedPkg: rename NullDmaLib to CoherentDmaLib
> 0bcb80106762 EmbeddedPkg/CoherentDmaLib: add support for non-1:1 DMA translation
> 723102c72fb0 EmbeddedPkg: implement NonCoherentDmaLib based on ArmDmaLib
> c878cd95e132 Omap35xxPkg: switch to EmbeddedPkg's NonCoherentDmaLib
> 877f4460b3e3 BeagleBoardPkg: switch to generic non-coherent DmaLib
>
> #6 needs to wait until the edk2-platforms changes are in.
#6 pushed as 63ed4d2757eb
Thanks.
^ permalink raw reply [flat|nested] 17+ messages in thread