[edk2] ArmPkg/ArmDmaLib: add support for fixed host-to-device DMA offset

Message ID 1478610295-22065-1-git-send-email-ard.biesheuvel@linaro.org
State New
Headers show

Commit Message

Ard Biesheuvel Nov. 8, 2016, 1:04 p.m.
Some devices, such as the Raspberry Pi3, have a fixed offset between memory
addresses as seen by the host and as seen by the other bus masters. So add
a new PCD that allows this fixed offset to be recorded, and to be used when
returning device addresses from the DmaLib mapping routines.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

---
 ArmPkg/ArmPkg.dec                      |  6 ++++++
 ArmPkg/Library/ArmDmaLib/ArmDmaLib.c   | 13 +++++++++++--
 ArmPkg/Library/ArmDmaLib/ArmDmaLib.inf |  1 +
 3 files changed, 18 insertions(+), 2 deletions(-)

-- 
2.7.4

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

Comments

Leif Lindholm Nov. 9, 2016, 9:13 p.m. | #1
On Tue, Nov 08, 2016 at 01:04:55PM +0000, Ard Biesheuvel wrote:
> Some devices, such as the Raspberry Pi3, have a fixed offset between memory

> addresses as seen by the host and as seen by the other bus masters. So add

> a new PCD that allows this fixed offset to be recorded, and to be used when

> returning device addresses from the DmaLib mapping routines.

>

> Contributed-under: TianoCore Contribution Agreement 1.0

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---

>  ArmPkg/ArmPkg.dec                      |  6 ++++++

>  ArmPkg/Library/ArmDmaLib/ArmDmaLib.c   | 13 +++++++++++--

>  ArmPkg/Library/ArmDmaLib/ArmDmaLib.inf |  1 +

>  3 files changed, 18 insertions(+), 2 deletions(-)

> 

> diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec

> index 3cdb5da3d4f3..b2d59a168337 100644

> --- a/ArmPkg/ArmPkg.dec

> +++ b/ArmPkg/ArmPkg.dec

> @@ -134,6 +134,12 @@ [PcdsFixedAtBuild.common]

>    gArmTokenSpaceGuid.PcdFdSize|0|UINT32|0x0000002C

>    gArmTokenSpaceGuid.PcdFvSize|0|UINT32|0x0000002E

>  

> +  #

> +  # Value to add to a host address to obtain a device address

> +  # (using unsigned 64-bit integer arithmetic on both ARM and AArch64)

> +  #

> +  gArmTokenSpaceGuid.PcdArmDmaDeviceOffset|0x0|UINT64|0x0000044

> +


Is the offset always positive?

>  [PcdsFixedAtBuild.common, PcdsPatchableInModule.common]

>    gArmTokenSpaceGuid.PcdFdBaseAddress|0|UINT64|0x0000002B

>    gArmTokenSpaceGuid.PcdFvBaseAddress|0|UINT64|0x0000002D

> diff --git a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c

> index d48d6ff6dbbb..d6dee78a254d 100644

> --- a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c

> +++ b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c

> @@ -39,6 +39,15 @@ typedef struct {

>  EFI_CPU_ARCH_PROTOCOL      *gCpu;

>  UINTN                      gCacheAlignment = 0;

>  

> +STATIC

> +PHYSICAL_ADDRESS

> +HostToDeviceAddress (

> +  IN  PHYSICAL_ADDRESS  HostAddress

> +  )

> +{

> +  return HostAddress + PcdGet64 (PcdArmDmaDeviceOffset);

> +}

> +

>  /**

>    Provides the DMA controller-specific addresses needed to access system memory.

>  

> @@ -82,7 +91,7 @@ DmaMap (

>      return EFI_INVALID_PARAMETER;

>    }

>  

> -  *DeviceAddress = ConvertToPhysicalAddress (HostAddress);

> +  *DeviceAddress = HostToDeviceAddress (ConvertToPhysicalAddress (HostAddress));

>  

>    // Remember range so we can flush on the other side

>    Map = AllocatePool (sizeof (MAP_INFO_INSTANCE));

> @@ -129,7 +138,7 @@ DmaMap (

>          CopyMem (Buffer, HostAddress, *NumberOfBytes);

>        }

>  

> -      *DeviceAddress = (PHYSICAL_ADDRESS)(UINTN)Buffer;

> +      *DeviceAddress = HostToDeviceAddress ((PHYSICAL_ADDRESS)(UINTN)Buffer);

>      } else {

>        Map->DoubleBuffer  = FALSE;

>      }

> diff --git a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.inf b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.inf

> index 95c13006eaac..36c23f486974 100644

> --- a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.inf

> +++ b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.inf

> @@ -46,6 +46,7 @@ [Protocols]

>  [Guids]

>  

>  [Pcd]

> +  gArmTokenSpaceGuid.PcdArmDmaDeviceOffset

>  

>  [Depex]

>    gEfiCpuArchProtocolGuid

> -- 

> 2.7.4

> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel Nov. 9, 2016, 9:31 p.m. | #2
> On 10 Nov 2016, at 01:13, Leif Lindholm <leif.lindholm@linaro.org> wrote:

> 

>> On Tue, Nov 08, 2016 at 01:04:55PM +0000, Ard Biesheuvel wrote:

>> Some devices, such as the Raspberry Pi3, have a fixed offset between memory

>> addresses as seen by the host and as seen by the other bus masters. So add

>> a new PCD that allows this fixed offset to be recorded, and to be used when

>> returning device addresses from the DmaLib mapping routines.

>> 

>> Contributed-under: TianoCore Contribution Agreement 1.0

>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

>> ---

>> ArmPkg/ArmPkg.dec                      |  6 ++++++

>> ArmPkg/Library/ArmDmaLib/ArmDmaLib.c   | 13 +++++++++++--

>> ArmPkg/Library/ArmDmaLib/ArmDmaLib.inf |  1 +

>> 3 files changed, 18 insertions(+), 2 deletions(-)

>> 

>> diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec

>> index 3cdb5da3d4f3..b2d59a168337 100644

>> --- a/ArmPkg/ArmPkg.dec

>> +++ b/ArmPkg/ArmPkg.dec

>> @@ -134,6 +134,12 @@ [PcdsFixedAtBuild.common]

>>   gArmTokenSpaceGuid.PcdFdSize|0|UINT32|0x0000002C

>>   gArmTokenSpaceGuid.PcdFvSize|0|UINT32|0x0000002E

>> 

>> +  #

>> +  # Value to add to a host address to obtain a device address

>> +  # (using unsigned 64-bit integer arithmetic on both ARM and AArch64)

>> +  #

>> +  gArmTokenSpaceGuid.PcdArmDmaDeviceOffset|0x0|UINT64|0x0000044

>> +

> 

> Is the offset always positive?

> 


No, hence the comment (which apparently deserves some clarification)

The idea is that, even on a pure 32-bit system, a negative offset can be expressed by wrapping unsigned integer arithmetic, e.g., 0xffffffff_fffff000 for -4 kB


>> [PcdsFixedAtBuild.common, PcdsPatchableInModule.common]

>>   gArmTokenSpaceGuid.PcdFdBaseAddress|0|UINT64|0x0000002B

>>   gArmTokenSpaceGuid.PcdFvBaseAddress|0|UINT64|0x0000002D

>> diff --git a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c

>> index d48d6ff6dbbb..d6dee78a254d 100644

>> --- a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c

>> +++ b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c

>> @@ -39,6 +39,15 @@ typedef struct {

>> EFI_CPU_ARCH_PROTOCOL      *gCpu;

>> UINTN                      gCacheAlignment = 0;

>> 

>> +STATIC

>> +PHYSICAL_ADDRESS

>> +HostToDeviceAddress (

>> +  IN  PHYSICAL_ADDRESS  HostAddress

>> +  )

>> +{

>> +  return HostAddress + PcdGet64 (PcdArmDmaDeviceOffset);

>> +}

>> +

>> /**

>>   Provides the DMA controller-specific addresses needed to access system memory.

>> 

>> @@ -82,7 +91,7 @@ DmaMap (

>>     return EFI_INVALID_PARAMETER;

>>   }

>> 

>> -  *DeviceAddress = ConvertToPhysicalAddress (HostAddress);

>> +  *DeviceAddress = HostToDeviceAddress (ConvertToPhysicalAddress (HostAddress));

>> 

>>   // Remember range so we can flush on the other side

>>   Map = AllocatePool (sizeof (MAP_INFO_INSTANCE));

>> @@ -129,7 +138,7 @@ DmaMap (

>>         CopyMem (Buffer, HostAddress, *NumberOfBytes);

>>       }

>> 

>> -      *DeviceAddress = (PHYSICAL_ADDRESS)(UINTN)Buffer;

>> +      *DeviceAddress = HostToDeviceAddress ((PHYSICAL_ADDRESS)(UINTN)Buffer);

>>     } else {

>>       Map->DoubleBuffer  = FALSE;

>>     }

>> diff --git a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.inf b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.inf

>> index 95c13006eaac..36c23f486974 100644

>> --- a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.inf

>> +++ b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.inf

>> @@ -46,6 +46,7 @@ [Protocols]

>> [Guids]

>> 

>> [Pcd]

>> +  gArmTokenSpaceGuid.PcdArmDmaDeviceOffset

>> 

>> [Depex]

>>   gEfiCpuArchProtocolGuid

>> -- 

>> 2.7.4

>> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Leif Lindholm Nov. 10, 2016, 12:03 a.m. | #3
On Thu, Nov 10, 2016 at 01:31:38AM +0400, Ard Biesheuvel wrote:
> 

> 

> > On 10 Nov 2016, at 01:13, Leif Lindholm <leif.lindholm@linaro.org> wrote:

> > 

> >> On Tue, Nov 08, 2016 at 01:04:55PM +0000, Ard Biesheuvel wrote:

> >> Some devices, such as the Raspberry Pi3, have a fixed offset between memory

> >> addresses as seen by the host and as seen by the other bus masters. So add

> >> a new PCD that allows this fixed offset to be recorded, and to be used when

> >> returning device addresses from the DmaLib mapping routines.

> >> 

> >> Contributed-under: TianoCore Contribution Agreement 1.0

> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> >> ---

> >> ArmPkg/ArmPkg.dec                      |  6 ++++++

> >> ArmPkg/Library/ArmDmaLib/ArmDmaLib.c   | 13 +++++++++++--

> >> ArmPkg/Library/ArmDmaLib/ArmDmaLib.inf |  1 +

> >> 3 files changed, 18 insertions(+), 2 deletions(-)

> >> 

> >> diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec

> >> index 3cdb5da3d4f3..b2d59a168337 100644

> >> --- a/ArmPkg/ArmPkg.dec

> >> +++ b/ArmPkg/ArmPkg.dec

> >> @@ -134,6 +134,12 @@ [PcdsFixedAtBuild.common]

> >>   gArmTokenSpaceGuid.PcdFdSize|0|UINT32|0x0000002C

> >>   gArmTokenSpaceGuid.PcdFvSize|0|UINT32|0x0000002E

> >> 

> >> +  #

> >> +  # Value to add to a host address to obtain a device address

> >> +  # (using unsigned 64-bit integer arithmetic on both ARM and AArch64)

> >> +  #

> >> +  gArmTokenSpaceGuid.PcdArmDmaDeviceOffset|0x0|UINT64|0x0000044

> >> +

> > 

> > Is the offset always positive?

> > 

> 

> No, hence the comment (which apparently deserves some clarification)

> 

> The idea is that, even on a pure 32-bit system, a negative offset

> can be expressed by wrapping unsigned integer arithmetic, e.g.,

> 0xffffffff_fffff000 for -4 kB


Ah, ok. Yes, I can use something more obvious in the comment anyway :)
Although I think the word 'wrap' making an appearance would have been
sufficient.

/
    Leif

> >> [PcdsFixedAtBuild.common, PcdsPatchableInModule.common]

> >>   gArmTokenSpaceGuid.PcdFdBaseAddress|0|UINT64|0x0000002B

> >>   gArmTokenSpaceGuid.PcdFvBaseAddress|0|UINT64|0x0000002D

> >> diff --git a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c

> >> index d48d6ff6dbbb..d6dee78a254d 100644

> >> --- a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c

> >> +++ b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c

> >> @@ -39,6 +39,15 @@ typedef struct {

> >> EFI_CPU_ARCH_PROTOCOL      *gCpu;

> >> UINTN                      gCacheAlignment = 0;

> >> 

> >> +STATIC

> >> +PHYSICAL_ADDRESS

> >> +HostToDeviceAddress (

> >> +  IN  PHYSICAL_ADDRESS  HostAddress

> >> +  )

> >> +{

> >> +  return HostAddress + PcdGet64 (PcdArmDmaDeviceOffset);

> >> +}

> >> +

> >> /**

> >>   Provides the DMA controller-specific addresses needed to access system memory.

> >> 

> >> @@ -82,7 +91,7 @@ DmaMap (

> >>     return EFI_INVALID_PARAMETER;

> >>   }

> >> 

> >> -  *DeviceAddress = ConvertToPhysicalAddress (HostAddress);

> >> +  *DeviceAddress = HostToDeviceAddress (ConvertToPhysicalAddress (HostAddress));

> >> 

> >>   // Remember range so we can flush on the other side

> >>   Map = AllocatePool (sizeof (MAP_INFO_INSTANCE));

> >> @@ -129,7 +138,7 @@ DmaMap (

> >>         CopyMem (Buffer, HostAddress, *NumberOfBytes);

> >>       }

> >> 

> >> -      *DeviceAddress = (PHYSICAL_ADDRESS)(UINTN)Buffer;

> >> +      *DeviceAddress = HostToDeviceAddress ((PHYSICAL_ADDRESS)(UINTN)Buffer);

> >>     } else {

> >>       Map->DoubleBuffer  = FALSE;

> >>     }

> >> diff --git a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.inf b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.inf

> >> index 95c13006eaac..36c23f486974 100644

> >> --- a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.inf

> >> +++ b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.inf

> >> @@ -46,6 +46,7 @@ [Protocols]

> >> [Guids]

> >> 

> >> [Pcd]

> >> +  gArmTokenSpaceGuid.PcdArmDmaDeviceOffset

> >> 

> >> [Depex]

> >>   gEfiCpuArchProtocolGuid

> >> -- 

> >> 2.7.4

> >> 

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

Patch

diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
index 3cdb5da3d4f3..b2d59a168337 100644
--- a/ArmPkg/ArmPkg.dec
+++ b/ArmPkg/ArmPkg.dec
@@ -134,6 +134,12 @@  [PcdsFixedAtBuild.common]
   gArmTokenSpaceGuid.PcdFdSize|0|UINT32|0x0000002C
   gArmTokenSpaceGuid.PcdFvSize|0|UINT32|0x0000002E
 
+  #
+  # Value to add to a host address to obtain a device address
+  # (using unsigned 64-bit integer arithmetic on both ARM and AArch64)
+  #
+  gArmTokenSpaceGuid.PcdArmDmaDeviceOffset|0x0|UINT64|0x0000044
+
 [PcdsFixedAtBuild.common, PcdsPatchableInModule.common]
   gArmTokenSpaceGuid.PcdFdBaseAddress|0|UINT64|0x0000002B
   gArmTokenSpaceGuid.PcdFvBaseAddress|0|UINT64|0x0000002D
diff --git a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c
index d48d6ff6dbbb..d6dee78a254d 100644
--- a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c
+++ b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c
@@ -39,6 +39,15 @@  typedef struct {
 EFI_CPU_ARCH_PROTOCOL      *gCpu;
 UINTN                      gCacheAlignment = 0;
 
+STATIC
+PHYSICAL_ADDRESS
+HostToDeviceAddress (
+  IN  PHYSICAL_ADDRESS  HostAddress
+  )
+{
+  return HostAddress + PcdGet64 (PcdArmDmaDeviceOffset);
+}
+
 /**
   Provides the DMA controller-specific addresses needed to access system memory.
 
@@ -82,7 +91,7 @@  DmaMap (
     return EFI_INVALID_PARAMETER;
   }
 
-  *DeviceAddress = ConvertToPhysicalAddress (HostAddress);
+  *DeviceAddress = HostToDeviceAddress (ConvertToPhysicalAddress (HostAddress));
 
   // Remember range so we can flush on the other side
   Map = AllocatePool (sizeof (MAP_INFO_INSTANCE));
@@ -129,7 +138,7 @@  DmaMap (
         CopyMem (Buffer, HostAddress, *NumberOfBytes);
       }
 
-      *DeviceAddress = (PHYSICAL_ADDRESS)(UINTN)Buffer;
+      *DeviceAddress = HostToDeviceAddress ((PHYSICAL_ADDRESS)(UINTN)Buffer);
     } else {
       Map->DoubleBuffer  = FALSE;
     }
diff --git a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.inf b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.inf
index 95c13006eaac..36c23f486974 100644
--- a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.inf
+++ b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.inf
@@ -46,6 +46,7 @@  [Protocols]
 [Guids]
 
 [Pcd]
+  gArmTokenSpaceGuid.PcdArmDmaDeviceOffset
 
 [Depex]
   gEfiCpuArchProtocolGuid