From: Robin Murphy <robin.murphy@arm.com> To: Will Deacon <will@kernel.org>, Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> Cc: catalin.marinas@arm.com, amit.pundir@linaro.org, andersson@kernel.org, quic_sibis@quicinc.com, sumit.semwal@linaro.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] Revert "arm64: dma: Drop cache invalidation from arch_dma_prep_coherent()" Date: Mon, 14 Nov 2022 15:14:21 +0000 [thread overview] Message-ID: <1659929b-1372-cea6-5840-c58369a4252d@arm.com> (raw) In-Reply-To: <20221114141109.GG30263@willie-the-truck> On 2022-11-14 14:11, Will Deacon wrote: > On Mon, Nov 14, 2022 at 04:33:29PM +0530, Manivannan Sadhasivam wrote: >> This reverts commit c44094eee32f32f175aadc0efcac449d99b1bbf7. >> >> As reported by Amit [1], dropping cache invalidation from >> arch_dma_prep_coherent() triggers a crash on the Qualcomm SM8250 platform >> (most probably on other Qcom platforms too). The reason is, Qcom >> qcom_q6v5_mss driver copies the firmware metadata and shares it with modem >> for validation. The modem has a secure block (XPU) that will trigger a >> whole system crash if the shared memory is accessed by the CPU while modem >> is poking at it. >> >> To avoid this issue, the qcom_q6v5_mss driver allocates a chunk of memory >> with no kernel mapping, vmap's it, copies the firmware metadata and >> unvmap's it. Finally the address is then shared with modem for metadata >> validation [2]. >> >> Now because of the removal of cache invalidation from >> arch_dma_prep_coherent(), there will be cache lines associated with this >> memory even after sharing with modem. So when the CPU accesses it, the XPU >> violation gets triggered. > > This last past is a non-sequitur: the buffer is no longer mapped on the CPU > side, so how would the CPU access it? Right, for the previous change to have made a difference the offending part of this buffer must be present in some cache somewhere *before* the DMA buffer allocation completes. Clearly that driver is completely broken though. If the DMA allocation came from a no-map carveout vma_dma_alloc_from_dev_coherent() then the vmap() shenanigans wouldn't work, so if it backed by struct pages then the whole dance is still pointless because *a cacheable linear mapping exists*, and it's just relying on the reduced chance that anything's going to re-fetch the linear map address after those pages have been allocated, exactly as I called out previously[1]. Robin. [1] https://lore.kernel.org/linux-arm-kernel/97fface8-e40e-072c-4335-c94094884e93@arm.com/ > As I just replied to Amit, we need more information about what this > "access" is and how it is being detected. > > Will
WARNING: multiple messages have this Message-ID (diff)
From: Robin Murphy <robin.murphy@arm.com> To: Will Deacon <will@kernel.org>, Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> Cc: catalin.marinas@arm.com, amit.pundir@linaro.org, andersson@kernel.org, quic_sibis@quicinc.com, sumit.semwal@linaro.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] Revert "arm64: dma: Drop cache invalidation from arch_dma_prep_coherent()" Date: Mon, 14 Nov 2022 15:14:21 +0000 [thread overview] Message-ID: <1659929b-1372-cea6-5840-c58369a4252d@arm.com> (raw) In-Reply-To: <20221114141109.GG30263@willie-the-truck> On 2022-11-14 14:11, Will Deacon wrote: > On Mon, Nov 14, 2022 at 04:33:29PM +0530, Manivannan Sadhasivam wrote: >> This reverts commit c44094eee32f32f175aadc0efcac449d99b1bbf7. >> >> As reported by Amit [1], dropping cache invalidation from >> arch_dma_prep_coherent() triggers a crash on the Qualcomm SM8250 platform >> (most probably on other Qcom platforms too). The reason is, Qcom >> qcom_q6v5_mss driver copies the firmware metadata and shares it with modem >> for validation. The modem has a secure block (XPU) that will trigger a >> whole system crash if the shared memory is accessed by the CPU while modem >> is poking at it. >> >> To avoid this issue, the qcom_q6v5_mss driver allocates a chunk of memory >> with no kernel mapping, vmap's it, copies the firmware metadata and >> unvmap's it. Finally the address is then shared with modem for metadata >> validation [2]. >> >> Now because of the removal of cache invalidation from >> arch_dma_prep_coherent(), there will be cache lines associated with this >> memory even after sharing with modem. So when the CPU accesses it, the XPU >> violation gets triggered. > > This last past is a non-sequitur: the buffer is no longer mapped on the CPU > side, so how would the CPU access it? Right, for the previous change to have made a difference the offending part of this buffer must be present in some cache somewhere *before* the DMA buffer allocation completes. Clearly that driver is completely broken though. If the DMA allocation came from a no-map carveout vma_dma_alloc_from_dev_coherent() then the vmap() shenanigans wouldn't work, so if it backed by struct pages then the whole dance is still pointless because *a cacheable linear mapping exists*, and it's just relying on the reduced chance that anything's going to re-fetch the linear map address after those pages have been allocated, exactly as I called out previously[1]. Robin. [1] https://lore.kernel.org/linux-arm-kernel/97fface8-e40e-072c-4335-c94094884e93@arm.com/ > As I just replied to Amit, we need more information about what this > "access" is and how it is being detected. > > Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2022-11-14 15:14 UTC|newest] Thread overview: 57+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-11-14 11:03 [PATCH] Revert "arm64: dma: Drop cache invalidation from arch_dma_prep_coherent()" Manivannan Sadhasivam 2022-11-14 11:03 ` Manivannan Sadhasivam 2022-11-14 11:29 ` Manivannan Sadhasivam 2022-11-14 11:29 ` Manivannan Sadhasivam 2022-11-14 14:11 ` Will Deacon 2022-11-14 14:11 ` Will Deacon 2022-11-14 15:14 ` Robin Murphy [this message] 2022-11-14 15:14 ` Robin Murphy 2022-11-14 17:38 ` Catalin Marinas 2022-11-14 17:38 ` Catalin Marinas 2022-11-18 10:54 ` Manivannan Sadhasivam 2022-11-18 10:54 ` Manivannan Sadhasivam 2022-11-18 12:33 ` Will Deacon 2022-11-18 12:33 ` Will Deacon 2022-11-21 6:42 ` Manivannan Sadhasivam 2022-11-21 6:42 ` Manivannan Sadhasivam 2022-11-21 10:12 ` Sibi Sankar 2022-11-21 10:12 ` Sibi Sankar 2022-11-24 11:55 ` Catalin Marinas 2022-11-24 11:55 ` Catalin Marinas 2022-12-01 9:29 ` Thorsten Leemhuis 2022-12-01 9:29 ` Thorsten Leemhuis 2022-12-01 17:45 ` Catalin Marinas 2022-12-01 17:45 ` Catalin Marinas 2022-12-02 8:26 ` Amit Pundir 2022-12-02 8:26 ` Amit Pundir 2022-12-02 8:54 ` Thorsten Leemhuis 2022-12-02 8:54 ` Thorsten Leemhuis 2022-12-02 10:03 ` Will Deacon 2022-12-02 10:03 ` Will Deacon 2022-12-02 10:34 ` Thorsten Leemhuis 2022-12-02 10:34 ` Thorsten Leemhuis 2022-12-02 16:10 ` Greg KH 2022-12-02 16:10 ` Greg KH 2022-12-02 16:27 ` Thorsten Leemhuis 2022-12-02 16:27 ` Thorsten Leemhuis 2022-12-02 16:32 ` Greg KH 2022-12-02 16:32 ` Greg KH 2022-12-02 17:14 ` Manivannan Sadhasivam 2022-12-02 17:14 ` Manivannan Sadhasivam 2022-12-05 14:24 ` Will Deacon 2022-12-05 14:24 ` Will Deacon 2022-12-06 9:21 ` Manivannan Sadhasivam 2022-12-06 9:21 ` Manivannan Sadhasivam 2022-12-06 9:58 ` Will Deacon 2022-12-06 9:58 ` Will Deacon 2022-12-02 10:54 ` Manivannan Sadhasivam 2022-12-02 10:54 ` Manivannan Sadhasivam 2022-11-28 5:44 ` Thorsten Leemhuis 2022-11-28 5:44 ` Thorsten Leemhuis 2022-11-28 8:15 ` Manivannan Sadhasivam 2022-11-28 8:15 ` Manivannan Sadhasivam 2022-12-08 4:59 ` Leonard Lausen 2022-12-08 4:59 ` Leonard Lausen 2022-12-06 10:34 Will Deacon 2022-12-06 11:24 ` Manivannan Sadhasivam 2022-12-06 17:50 ` Catalin Marinas
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=1659929b-1372-cea6-5840-c58369a4252d@arm.com \ --to=robin.murphy@arm.com \ --cc=amit.pundir@linaro.org \ --cc=andersson@kernel.org \ --cc=catalin.marinas@arm.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=manivannan.sadhasivam@linaro.org \ --cc=quic_sibis@quicinc.com \ --cc=sumit.semwal@linaro.org \ --cc=will@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.