ArmPkg EmbeddedPkg: clean up DmaLib implementations

[edk2] [PATCH 2/6] EmbeddedPkg/CoherentDmaLib: add support for non-1:1 DMA translation

Posted by Ard Biesheuvel 7 years, 3 months ago

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

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 2/6] EmbeddedPkg/CoherentDmaLib: add support for non-1:1 DMA translation

Posted by Leif Lindholm 7 years, 3 months ago

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
> 
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 2/6] EmbeddedPkg/CoherentDmaLib: add support for non-1:1 DMA translation

Posted by Ard Biesheuvel 7 years, 3 months ago

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.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 2/6] EmbeddedPkg/CoherentDmaLib: add support for non-1:1 DMA translation

Posted by Leif Lindholm 7 years, 3 months ago

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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel