| Message ID | 20200414123136.441454-1-hch@lst.de (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 459b1f86f1cba7de813fbc335df476c111feec22 |
| Headers | show |
| Series | firmware: qcom_scm: fix bogous abuse of dma-direct internals | expand |
On Tue 14 Apr 05:31 PDT 2020, Christoph Hellwig wrote: > As far as the device is concerned the dma address is the physical > address. There is no need to convert it to a physical address, > especially not using dma-direct internals that are not available > to drivers and which will interact badly with IOMMUs. Last but not > least the commit introducing it claimed to just fix a type issue, > but actually changed behavior. > > Fixes: 6e37ccf78a532 ("firmware: qcom_scm: Use proper types for dma mappings") > Signed-off-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> and applied. Thanks, Bjorn > --- > drivers/firmware/qcom_scm.c | 9 +++------ > 1 file changed, 3 insertions(+), 6 deletions(-) > > diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c > index 059bb0fbae9e..4701487573f7 100644 > --- a/drivers/firmware/qcom_scm.c > +++ b/drivers/firmware/qcom_scm.c > @@ -6,7 +6,6 @@ > #include <linux/init.h> > #include <linux/cpumask.h> > #include <linux/export.h> > -#include <linux/dma-direct.h> > #include <linux/dma-mapping.h> > #include <linux/module.h> > #include <linux/types.h> > @@ -806,8 +805,7 @@ int qcom_scm_assign_mem(phys_addr_t mem_addr, size_t mem_sz, > struct qcom_scm_mem_map_info *mem_to_map; > phys_addr_t mem_to_map_phys; > phys_addr_t dest_phys; > - phys_addr_t ptr_phys; > - dma_addr_t ptr_dma; > + dma_addr_t ptr_phys; > size_t mem_to_map_sz; > size_t dest_sz; > size_t src_sz; > @@ -824,10 +822,9 @@ int qcom_scm_assign_mem(phys_addr_t mem_addr, size_t mem_sz, > ptr_sz = ALIGN(src_sz, SZ_64) + ALIGN(mem_to_map_sz, SZ_64) + > ALIGN(dest_sz, SZ_64); > > - ptr = dma_alloc_coherent(__scm->dev, ptr_sz, &ptr_dma, GFP_KERNEL); > + ptr = dma_alloc_coherent(__scm->dev, ptr_sz, &ptr_phys, GFP_KERNEL); > if (!ptr) > return -ENOMEM; > - ptr_phys = dma_to_phys(__scm->dev, ptr_dma); > > /* Fill source vmid detail */ > src = ptr; > @@ -855,7 +852,7 @@ int qcom_scm_assign_mem(phys_addr_t mem_addr, size_t mem_sz, > > ret = __qcom_scm_assign_mem(__scm->dev, mem_to_map_phys, mem_to_map_sz, > ptr_phys, src_sz, dest_phys, dest_sz); > - dma_free_coherent(__scm->dev, ptr_sz, ptr, ptr_dma); > + dma_free_coherent(__scm->dev, ptr_sz, ptr, ptr_phys); > if (ret) { > dev_err(__scm->dev, > "Assign memory protection call failed %d\n", ret); > -- > 2.25.1 >
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c index 059bb0fbae9e..4701487573f7 100644 --- a/drivers/firmware/qcom_scm.c +++ b/drivers/firmware/qcom_scm.c @@ -6,7 +6,6 @@ #include <linux/init.h> #include <linux/cpumask.h> #include <linux/export.h> -#include <linux/dma-direct.h> #include <linux/dma-mapping.h> #include <linux/module.h> #include <linux/types.h> @@ -806,8 +805,7 @@ int qcom_scm_assign_mem(phys_addr_t mem_addr, size_t mem_sz, struct qcom_scm_mem_map_info *mem_to_map; phys_addr_t mem_to_map_phys; phys_addr_t dest_phys; - phys_addr_t ptr_phys; - dma_addr_t ptr_dma; + dma_addr_t ptr_phys; size_t mem_to_map_sz; size_t dest_sz; size_t src_sz; @@ -824,10 +822,9 @@ int qcom_scm_assign_mem(phys_addr_t mem_addr, size_t mem_sz, ptr_sz = ALIGN(src_sz, SZ_64) + ALIGN(mem_to_map_sz, SZ_64) + ALIGN(dest_sz, SZ_64); - ptr = dma_alloc_coherent(__scm->dev, ptr_sz, &ptr_dma, GFP_KERNEL); + ptr = dma_alloc_coherent(__scm->dev, ptr_sz, &ptr_phys, GFP_KERNEL); if (!ptr) return -ENOMEM; - ptr_phys = dma_to_phys(__scm->dev, ptr_dma); /* Fill source vmid detail */ src = ptr; @@ -855,7 +852,7 @@ int qcom_scm_assign_mem(phys_addr_t mem_addr, size_t mem_sz, ret = __qcom_scm_assign_mem(__scm->dev, mem_to_map_phys, mem_to_map_sz, ptr_phys, src_sz, dest_phys, dest_sz); - dma_free_coherent(__scm->dev, ptr_sz, ptr, ptr_dma); + dma_free_coherent(__scm->dev, ptr_sz, ptr, ptr_phys); if (ret) { dev_err(__scm->dev, "Assign memory protection call failed %d\n", ret);
As far as the device is concerned the dma address is the physical address. There is no need to convert it to a physical address, especially not using dma-direct internals that are not available to drivers and which will interact badly with IOMMUs. Last but not least the commit introducing it claimed to just fix a type issue, but actually changed behavior. Fixes: 6e37ccf78a532 ("firmware: qcom_scm: Use proper types for dma mappings") Signed-off-by: Christoph Hellwig <hch@lst.de> --- drivers/firmware/qcom_scm.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)