| Message ID | 20190715031723.6375-1-huangfq.daxian@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
On 7/14/2019 11:17 PM, Fuqian Huang wrote: > In commit 518a2f1925c3 > ("dma-mapping: zero memory returned from dma_alloc_*"), > dma_alloc_coherent has already zeroed the memory. > So memset is not needed. > > Signed-off-by: Fuqian Huang <huangfq.daxian@gmail.com> I don't see SWIO or ARM64 IOMMU drivers getting impacted by the mentioned change above (518a2f1925c3). arch/alpha/kernel/pci_iommu.c | 2 +- arch/arc/mm/dma.c | 2 +- arch/c6x/mm/dma-coherent.c | 5 ++++- arch/m68k/kernel/dma.c | 2 +- arch/microblaze/mm/consistent.c | 2 +- arch/openrisc/kernel/dma.c | 2 +- arch/parisc/kernel/pci-dma.c | 4 ++-- arch/s390/pci/pci_dma.c | 2 +- arch/sparc/kernel/ioport.c | 2 +- arch/sparc/mm/io-unit.c | 2 +- arch/sparc/mm/iommu.c | 2 +- arch/xtensa/kernel/pci-dma.c | 2 +- drivers/misc/mic/host/mic_boot.c | 2 +- kernel/dma/virt.c | 2 +- 14 files changed, 18 insertions(+), 15 deletions(-) How does this new behavior apply globally?
Sinan Kaya <Okaya@kernel.org> 於 2019年7月15日週一 下午12:17寫道： > > On 7/14/2019 11:17 PM, Fuqian Huang wrote: > > In commit 518a2f1925c3 > > ("dma-mapping: zero memory returned from dma_alloc_*"), > > dma_alloc_coherent has already zeroed the memory. > > So memset is not needed. > > > > Signed-off-by: Fuqian Huang <huangfq.daxian@gmail.com> > > I don't see SWIO or ARM64 IOMMU drivers getting impacted by > the mentioned change above (518a2f1925c3). > > How does this new behavior apply globally? > In the last version patch set, I referenced the commit af7ddd8a627c ("Merge tag 'dma-mapping-4.21' of git://git.infradead.org/users/hch/dma-mapping") in the commit log. The merged commit mentions that "ensure dma_alloc_coherent returns zeroed memory to avoid kernel data leaks through userspace. We already did this for most common architectures, but this ensures we do it everywhere." dma_alloc_coherent has already zeroed the memory during allocation and the commit also deprecates dma_zalloc_coherent. Greg and other maintainer told me to use the actual commit rather than the merged commit. So I reference the commit that ensures the dma_alloc_coherent to returns zeroed memory every where. Maybe this belongs to the `most common achitectures` and is not impacted by the mentioned change. Should I rewrite the commit log? Just mention that dma_alloc_coherent has already zeroed the memory and not to reference the commit?
On 7/15/2019 1:43 AM, Fuqian Huang wrote: > Should I rewrite the commit log? Just mention that dma_alloc_coherent > has already > zeroed the memory and not to reference the commit? I'd like to hear from Robin Murphy that arm smmu driver follows this as well.
On 15/07/2019 16:17, Sinan Kaya wrote: > On 7/15/2019 1:43 AM, Fuqian Huang wrote: >> Should I rewrite the commit log? Just mention that dma_alloc_coherent >> has already >> zeroed the memory and not to reference the commit? > > I'd like to hear from Robin Murphy that arm smmu driver follows this as > well. I'd be lying if I said it did. ...but only because that's never been part of the SMMU driver's responsibility either way. The iommu-dma layer however, and thus the respective arm64 iommu_dma_ops, has always zeroed allocations right from its inception. 518a2f1925c3 was just cleaning up the last of the stragglers which *weren't* already clearing buffers anyway, such that we could formalise that behaviour into the API. Robin.
On 7/16/2019 7:35 AM, Robin Murphy wrote: > On 15/07/2019 16:17, Sinan Kaya wrote: >> On 7/15/2019 1:43 AM, Fuqian Huang wrote: >>> Should I rewrite the commit log? Just mention that dma_alloc_coherent >>> has already >>> zeroed the memory and not to reference the commit? >> >> I'd like to hear from Robin Murphy that arm smmu driver follows this as >> well. > > I'd be lying if I said it did. > > ...but only because that's never been part of the SMMU driver's > responsibility either way. The iommu-dma layer however, and thus the > respective arm64 iommu_dma_ops, has always zeroed allocations right from > its inception. > > 518a2f1925c3 was just cleaning up the last of the stragglers which > *weren't* already clearing buffers anyway, such that we could formalise > that behaviour into the API. Thanks for confirming the behavior for arm64 arch. Acked-by: Sinan Kaya <okaya@kernel.org>
On 15-07-19, 11:17, Fuqian Huang wrote: > In commit 518a2f1925c3 > ("dma-mapping: zero memory returned from dma_alloc_*"), > dma_alloc_coherent has already zeroed the memory. > So memset is not needed. Applied, thanks
diff --git a/drivers/dma/qcom/hidma_ll.c b/drivers/dma/qcom/hidma_ll.c index 5bf8b145c427..bb4471e84e48 100644 --- a/drivers/dma/qcom/hidma_ll.c +++ b/drivers/dma/qcom/hidma_ll.c @@ -749,7 +749,6 @@ struct hidma_lldev *hidma_ll_init(struct device *dev, u32 nr_tres, if (!lldev->tre_ring) return NULL; - memset(lldev->tre_ring, 0, (HIDMA_TRE_SIZE + 1) * nr_tres); lldev->tre_ring_size = HIDMA_TRE_SIZE * nr_tres; lldev->nr_tres = nr_tres; @@ -769,7 +768,6 @@ struct hidma_lldev *hidma_ll_init(struct device *dev, u32 nr_tres, if (!lldev->evre_ring) return NULL; - memset(lldev->evre_ring, 0, (HIDMA_EVRE_SIZE + 1) * nr_tres); lldev->evre_ring_size = HIDMA_EVRE_SIZE * nr_tres; /* the EVRE ring has to be EVRE_SIZE aligned */
In commit 518a2f1925c3 ("dma-mapping: zero memory returned from dma_alloc_*"), dma_alloc_coherent has already zeroed the memory. So memset is not needed. Signed-off-by: Fuqian Huang <huangfq.daxian@gmail.com> --- Changes in v3: - Use actual commit rather than the merge commit in the commit message drivers/dma/qcom/hidma_ll.c | 2 -- 1 file changed, 2 deletions(-)