[PATCH] Revert "arm64: dma: Drop cache invalidation from arch_dma_prep_coherent()"

[Manivannan Sadhasivam]

On Tue, Dec 06, 2022 at 10:34:03AM +0000, Will Deacon wrote:
> This reverts commit c44094eee32f32f175aadc0efcac449d99b1bbf7.
> 
> Although the semantics of the DMA API require only a clean operation
> here, it turns out that the Qualcomm 'qcom_q6v5_mss' remoteproc driver
> (ab)uses the DMA API for transferring the modem firmware to the secure
> world via calls to Trustzone [1].
> 
> Once the firmware buffer has changed hands, _any_ access from the
> non-secure side (i.e. Linux) will be detected on the bus and result in a
> full system reset [2]. Although this is possible even with this revert
> in place (due to speculative reads via the cacheable linear alias of
> memory), anecdotally the problem occurs considerably more frequently
> when the lines have not been invalidated, assumedly due to some
> micro-architectural interactions with the cache hierarchy.
> 
> Revert the offending change for now, along with a comment, so that the
> Qualcomm developers have time to fix the driver [3] to use a firmware
> buffer which does not have a cacheable alias in the linear map.
> 
> Link: https://lore.kernel.org/r/20221114110329.68413-1-manivannan.sadhasivam@linaro.org [1]
> Link: https://lore.kernel.org/r/CAMi1Hd3H2k1J8hJ6e-Miy5+nVDNzv6qQ3nN-9929B0GbHJkXEg@mail.gmail.com/ [2]
> Link: https://lore.kernel.org/r/20221206092152.GD15486@thinkpad [2]
> Reported-by: Amit Pundir <amit.pundir at linaro.org>
> Reported-by: Manivannan Sadhasivam <manivannan.sadhasivam at linaro.org>
> Cc: Catalin Marinas <catalin.marinas at arm.com>
> Cc: Thorsten Leemhuis <regressions at leemhuis.info>
> Cc: Sibi Sankar <quic_sibis at quicinc.com>
> Signed-off-by: Will Deacon <will at kernel.org>

Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam at linaro.org>

Thanks,
Mani

> ---
>  arch/arm64/mm/dma-mapping.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
> index 3cb101e8cb29..5240f6acad64 100644
> --- a/arch/arm64/mm/dma-mapping.c
> +++ b/arch/arm64/mm/dma-mapping.c
> @@ -36,7 +36,22 @@ void arch_dma_prep_coherent(struct page *page, size_t size)
>  {
>  	unsigned long start = (unsigned long)page_address(page);
>  
> -	dcache_clean_poc(start, start + size);
> +	/*
> +	 * The architecture only requires a clean to the PoC here in order to
> +	 * meet the requirements of the DMA API. However, some vendors (i.e.
> +	 * Qualcomm) abuse the DMA API for transferring buffers from the
> +	 * non-secure to the secure world, resetting the system if a non-secure
> +	 * access shows up after the buffer has been transferred:
> +	 *
> +	 * https://lore.kernel.org/r/20221114110329.68413-1-manivannan.sadhasivam@linaro.org
> +	 *
> +	 * Using clean+invalidate appears to make this issue less likely, but
> +	 * the drivers themselves still need fixing as the CPU could issue a
> +	 * speculative read from the buffer via the linear mapping irrespective
> +	 * of the cache maintenance we use. Once the drivers are fixed, we can
> +	 * relax this to a clean operation.
> +	 */
> +	dcache_clean_inval_poc(start, start + size);
>  }
>  
>  #ifdef CONFIG_IOMMU_DMA
> -- 
> 2.39.0.rc0.267.gcb52ba06e7-goog
> 

-- 
மணிவண்ணன் சதாசிவம்