2019-07-15 03:18:56

by Fuqian Huang

[permalink] [raw]
Subject: [PATCH v3 04/24] dmaengine: qcom_hidma: Remove call to memset after dmam_alloc_coherent

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 <[email protected]>
---
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(-)

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 */
--
2.11.0


2019-07-15 04:19:00

by Sinan Kaya

[permalink] [raw]
Subject: Re: [PATCH v3 04/24] dmaengine: qcom_hidma: Remove call to memset after dmam_alloc_coherent

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 <[email protected]>

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?

2019-07-15 05:44:14

by Fuqian Huang

[permalink] [raw]
Subject: Re: [PATCH v3 04/24] dmaengine: qcom_hidma: Remove call to memset after dmam_alloc_coherent

Sinan Kaya <[email protected]> 於 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 <[email protected]>
>
> 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?

2019-07-15 15:18:25

by Sinan Kaya

[permalink] [raw]
Subject: Re: [PATCH v3 04/24] dmaengine: qcom_hidma: Remove call to memset after dmam_alloc_coherent

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.

2019-07-16 11:36:29

by Robin Murphy

[permalink] [raw]
Subject: Re: [PATCH v3 04/24] dmaengine: qcom_hidma: Remove call to memset after dmam_alloc_coherent

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.

2019-07-16 15:11:02

by Sinan Kaya

[permalink] [raw]
Subject: Re: [PATCH v3 04/24] dmaengine: qcom_hidma: Remove call to memset after dmam_alloc_coherent

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 <[email protected]>

2019-08-08 12:37:58

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH v3 04/24] dmaengine: qcom_hidma: Remove call to memset after dmam_alloc_coherent

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

--
~Vinod