On Tue, Apr 19, 2016 at 04:55:33PM +0200, Ard Biesheuvel wrote:
> DmaMap () operations of type MapOperationBusMasterCommonBuffer should
> return a mapping that is coherent between the CPU and the device. For
> this reason, the API only allows DmaMap () to be called with this operation
> type if the memory to be mapped was allocated by DmaAllocateBuffer (),
> which in this implementation guarantees the coherency by using uncached
> mappings on the CPU side.
>
> This means that, if we encounter a cached mapping in DmaMap () with this
> operation type, the code is either broken, or someone is violating the
> API, but simply proceeding with a double buffer makes no sense at all,
> and can only cause problems.
>
> So instead, actively reject this operation type for cached memory mappings.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <[email protected]>
> ---
> ArmPkg/Library/ArmDmaLib/ArmDmaLib.c | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c
> b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c
> index 7f6598318a91..7e518ed3b83e 100644
> --- a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c
> +++ b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c
> @@ -103,6 +103,18 @@ DmaMap (
> // 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__));
> + return EFI_UNSUPPORTED;
> + }
> +
> + //
> // If the buffer does not fill entire cache lines we must double
> buffer into
> // uncached memory. Device (PCI) address becomes uncached page.
> //
> @@ -112,7 +124,7 @@ DmaMap (
> return Status;
> }
>
> - if ((Operation == MapOperationBusMasterRead) || (Operation ==
> MapOperationBusMasterCommonBuffer)) {
> + if (Operation == MapOperationBusMasterRead) {
> CopyMem (Buffer, HostAddress, *NumberOfBytes);
> }
>
> @@ -168,7 +180,9 @@ DmaUnmap (
> Map = (MAP_INFO_INSTANCE *)Mapping;
>
> if (Map->DoubleBuffer) {
> - if ((Map->Operation == MapOperationBusMasterWrite) || (Map->Operation ==
> MapOperationBusMasterCommonBuffer)) {
> + ASSERT (Map->Operation != MapOperationBusMasterCommonBuffer);
Would it be more correct to return EFI_DEVICE_ERROR if this
condition became true?
> +
> + if (Map->Operation == MapOperationBusMasterWrite) {
> CopyMem ((VOID *)(UINTN)Map->HostAddress, (VOID
> *)(UINTN)Map->DeviceAddress, Map->NumberOfBytes);
> }
>
> --
> 2.5.0
>
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
