Re: [PATCH edk2-platforms v2] Silicon/SynQuacer: enable coherent DMA for NETSEC and eMMC

[Leif Lindholm <leif.lindholm@linaro.org>]

On Thu, Nov 30, 2017 at 06:53:55PM +0000, Ard Biesheuvel wrote:
> As it turns out, it is surprisingly easy to configure both the NETSEC
> and eMMC devices as cache coherent for DMA, given that they are both
> behind the same SMMU which is already configured in passthrough mode
> by the firmware running on the SCP.
> 
> So update the static SMMU configuration to make memory accesses performed
> by these devices inner shareable inner/outer writeback cacheable, which
> makes them cache coherent with the CPUs.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Looks fine to me:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
(If you want to hold back for Tested-by:s, feel free to.)

> ---
> v2: update DeveloperBox as well
>     update commit log to clarify that the SMMU is configured in pass through
>     mode by the firmware running on the Cortex-M3 SCP
> 
>  Platform/Socionext/DeveloperBox/DeveloperBox.dsc                                          |  2 +-
>  Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc                              |  2 +-
>  Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi                                     |  2 ++
>  Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c                                    |  2 +-
>  Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c                             | 23 ++++++++++++++++++++
>  Silicon/Socionext/SynQuacer/Include/Platform/MemoryMap.h                                  |  4 ++++
>  Silicon/Socionext/SynQuacer/Library/SynQuacerMemoryInitPeiLib/SynQuacerMemoryInitPeiLib.c |  3 +++
>  7 files changed, 35 insertions(+), 3 deletions(-)
> 
> diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> index 7ca9fb104311..e1183b529b73 100644
> --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> @@ -600,7 +600,7 @@ [Components.common]
>    NetworkPkg/HttpBootDxe/HttpBootDxe.inf
>    Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.inf {
>      <LibraryClasses>
> -      DmaLib|EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.inf
> +      DmaLib|EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.inf
>    }
>  
>    #
> diff --git a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
> index 10da45ad2837..637f6d414d24 100644
> --- a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
> +++ b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
> @@ -592,7 +592,7 @@ [Components.common]
>    NetworkPkg/HttpBootDxe/HttpBootDxe.inf
>    Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.inf {
>      <LibraryClasses>
> -      DmaLib|EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.inf
> +      DmaLib|EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.inf
>    }
>  
>    #
> diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
> index 5e663c59efbd..ec784c70afe7 100644
> --- a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
> +++ b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
> @@ -456,6 +456,7 @@
>              max-speed = <1000>;
>              max-frame-size = <9000>;
>              phy-handle = <&ethphy0>;
> +            dma-coherent;
>  
>              #address-cells = <1>;
>              #size-cells = <0>;
> @@ -557,6 +558,7 @@
>          fujitsu,cmd-dat-delay-select;
>          clocks = <&clk_alw_c_0 &clk_alw_b_0>;
>          clock-names = "core", "iface";
> +        dma-coherent;
>          status = "disabled";
>      };
>  };
> diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c
> index e577076ada4f..5a99283977fb 100644
> --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c
> +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c
> @@ -191,7 +191,7 @@ RegisterEmmc (
>  
>    Status = RegisterNonDiscoverableMmioDevice (
>               NonDiscoverableDeviceTypeSdhci,
> -             NonDiscoverableDeviceDmaTypeNonCoherent,
> +             NonDiscoverableDeviceDmaTypeCoherent,
>               NULL,
>               &mSdMmcControllerHandle,
>               1,
> diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c
> index c64ccf3b3c30..7e7c790a6113 100644
> --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c
> +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c
> @@ -123,6 +123,27 @@ FreeDevice:
>    return Status;
>  }
>  
> +#define SMMU_SCR0                         0x0
> +#define SMMU_SCR0_SHCFG_INNER             (0x2 << 22)
> +#define SMMU_SCR0_MTCFG                   (0x1 << 20)
> +#define SMMU_SCR0_MEMATTR_INNER_OUTER_WB  (0xf << 16)
> +
> +STATIC
> +VOID
> +SmmuEnableCoherentDma (
> +  VOID
> +  )
> +{
> +  //
> +  // The SCB SMMU (MMU-500) is shared between the NETSEC and eMMC devices, and
> +  // is configured in passthrough mode by default. Let's set the global memory
> +  // type override as well, so that all memory accesses by these devices are
> +  // inner shareable inner/outer writeback cacheable.
> +  //
> +  MmioOr32 (SYNQUACER_SCB_SMMU_BASE + SMMU_SCR0,
> +    SMMU_SCR0_SHCFG_INNER | SMMU_SCR0_MTCFG | SMMU_SCR0_MEMATTR_INNER_OUTER_WB);
> +}
> +
>  EFI_STATUS
>  EFIAPI
>  PlatformDxeEntryPoint (
> @@ -174,5 +195,7 @@ PlatformDxeEntryPoint (
>    Status = RegisterEmmc ();
>    ASSERT_EFI_ERROR (Status);
>  
> +  SmmuEnableCoherentDma ();
> +
>    return EFI_SUCCESS;
>  }
> diff --git a/Silicon/Socionext/SynQuacer/Include/Platform/MemoryMap.h b/Silicon/Socionext/SynQuacer/Include/Platform/MemoryMap.h
> index 3c7bd58866cc..f43adcc8607f 100644
> --- a/Silicon/Socionext/SynQuacer/Include/Platform/MemoryMap.h
> +++ b/Silicon/Socionext/SynQuacer/Include/Platform/MemoryMap.h
> @@ -65,4 +65,8 @@
>  #define SYNQUACER_PCIE_BASE             0x58200000
>  #define SYNQUACER_PCIE_SIZE             0x00200000
>  
> +// SCB SMMU
> +#define SYNQUACER_SCB_SMMU_BASE         0x52E00000
> +#define SYNQUACER_SCB_SMMU_SIZE         SIZE_64KB
> +
>  #endif
> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerMemoryInitPeiLib/SynQuacerMemoryInitPeiLib.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerMemoryInitPeiLib/SynQuacerMemoryInitPeiLib.c
> index a640b3e0c0d1..1402ecafce4a 100644
> --- a/Silicon/Socionext/SynQuacer/Library/SynQuacerMemoryInitPeiLib/SynQuacerMemoryInitPeiLib.c
> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerMemoryInitPeiLib/SynQuacerMemoryInitPeiLib.c
> @@ -115,6 +115,9 @@ STATIC CONST ARM_MEMORY_REGION_DESCRIPTOR mVirtualMemoryTable[] = {
>                       FixedPcdGet32 (PcdFlashNvStorageFtwWorkingSize)),
>    ARM_DEVICE_REGION (FixedPcdGet32 (PcdFlashNvStorageFtwSpareBase),
>                       FixedPcdGet32 (PcdFlashNvStorageFtwSpareSize)),
> +
> +  // NETSEC/eMMC SMMU
> +  ARM_DEVICE_REGION (SYNQUACER_SCB_SMMU_BASE, SYNQUACER_SCB_SMMU_SIZE),
>  };
>  
>  STATIC
> -- 
> 2.11.0
>