From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C0F0BEB64DC for ; Thu, 15 Jun 2023 10:13:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S245312AbjFOKNk (ORCPT ); Thu, 15 Jun 2023 06:13:40 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34198 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S239841AbjFOKNh (ORCPT ); Thu, 15 Jun 2023 06:13:37 -0400 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 9E9E12710; Thu, 15 Jun 2023 03:13:35 -0700 (PDT) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 87FDC1FB; Thu, 15 Jun 2023 03:14:19 -0700 (PDT) Received: from [10.57.85.251] (unknown [10.57.85.251]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id DABFA3F71E; Thu, 15 Jun 2023 03:13:32 -0700 (PDT) Message-ID: <36565295-ebaa-2a66-3389-ba5eb714ab34@arm.com> Date: Thu, 15 Jun 2023 11:13:27 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:102.0) Gecko/20100101 Thunderbird/102.12.0 Subject: Re: [PATCH] Revert "Revert "Revert "arm64: dma: Drop cache invalidation from arch_dma_prep_coherent()""" Content-Language: en-GB To: Douglas Anderson , Will Deacon Cc: andersson@kernel.org, amit.pundir@linaro.org, linux-arm-msm@vger.kernel.org, konrad.dybcio@somainline.org, Sibi Sankar , Manivannan Sadhasivam , sumit.semwal@linaro.org, Stephen Boyd , Catalin Marinas , Manivannan Sadhasivam , Marc Zyngier , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org References: <20230614165904.1.I279773c37e2c1ed8fbb622ca6d1397aea0023526@changeid> From: Robin Murphy In-Reply-To: <20230614165904.1.I279773c37e2c1ed8fbb622ca6d1397aea0023526@changeid> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2023-06-15 00:59, Douglas Anderson wrote: > This reverts commit 7bd6680b47fa4cd53ee1047693c09825e212a6f5. > > When booting a sc7180-trogdor based device on mainline, I see errors > that look like this: > > qcom_scm firmware:scm: Assign memory protection call failed -22 > qcom_rmtfs_mem 94600000.memory: assign memory failed > qcom_rmtfs_mem: probe of 94600000.memory failed with error -22 > > The device still boots OK, but WiFi doesn't work. > > The failure only seems to happen when > CONFIG_INIT_ON_ALLOC_DEFAULT_ON=y. When I don't have that set then > everything is peachy. Presumably something about the extra > initialization disagrees with the change to drop cache invalidation. AFAICS init_on_alloc essentially just adds __GFP_ZERO to the page allocation. This should make no difference to a DMA allocation given that dma_alloc_attrs explicitly zeros its allocation anyway. However... for the non-coherent case, the DMA API's memset will be done through the non-cacheable remap, while __GFP_ZERO can leave behind cached zeros for the linear map alias. Thus what I assume must be happening here is that "DMA" from the firmware is still making cacheable accesses to the buffer and getting those zeros instead of whatever actual data which was subsequently written non-cacheably direct to RAM. So either the firmware still needs fixing to make non-cacheable accesses, or the SCM driver needs to correctly describe it as coherent. Thanks, Robin. > Fixes: 7bd6680b47fa ("Revert "Revert "arm64: dma: Drop cache invalidation from arch_dma_prep_coherent()""") > Signed-off-by: Douglas Anderson > --- > > 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