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

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

> 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
>>