From: Robin Gong <yibin.gong@nxp.com> To: "vkoul@kernel.org" <vkoul@kernel.org>, "l.stach@pengutronix.de" <l.stach@pengutronix.de> Cc: "dmaengine@vger.kernel.org" <dmaengine@vger.kernel.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, dl-linux-imx <linux-imx@nxp.com> Subject: [v3,2/4] Revert "dmaengine: imx-sdma: alloclate bd memory from dma pool" Date: Tue, 6 Nov 2018 03:40:28 +0000 [thread overview] Message-ID: <1541504525-25720-3-git-send-email-yibin.gong@nxp.com> (raw) From: Lucas Stach <l.stach@pengutronix.de> This reverts commit fe5b85c656bc. The SDMA engine needs the descriptors to be contiguous in memory. As the dma pool API is only able to provide a single descriptor per alloc invocation there is no guarantee that multiple descriptors satisfy this requirement. Also the code in question is broken as it only allocates memory for a single descriptor, without looking at the number of descriptors required for the transfer, leading to out-of-bounds accesses when the descriptors are written. Signed-off-by: Lucas Stach <l.stach@pengutronix.de> Signed-off-by: Robin Gong <yibin.gong@nxp.com> --- drivers/dma/imx-sdma.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c index 3bca5e0..8d2fec8 100644 --- a/drivers/dma/imx-sdma.c +++ b/drivers/dma/imx-sdma.c @@ -24,7 +24,6 @@ #include <linux/spinlock.h> #include <linux/device.h> #include <linux/dma-mapping.h> -#include <linux/dmapool.h> #include <linux/firmware.h> #include <linux/slab.h> #include <linux/platform_device.h> @@ -376,7 +375,6 @@ struct sdma_channel { u32 shp_addr, per_addr; enum dma_status status; struct imx_dma_data data; - struct dma_pool *bd_pool; }; #define IMX_DMA_SG_LOOP BIT(0) @@ -1192,10 +1190,11 @@ static int sdma_request_channel0(struct sdma_engine *sdma) static int sdma_alloc_bd(struct sdma_desc *desc) { + u32 bd_size = desc->num_bd * sizeof(struct sdma_buffer_descriptor); int ret = 0; - desc->bd = dma_pool_alloc(desc->sdmac->bd_pool, GFP_ATOMIC, - &desc->bd_phys); + desc->bd = dma_zalloc_coherent(NULL, bd_size, &desc->bd_phys, + GFP_ATOMIC); if (!desc->bd) { ret = -ENOMEM; goto out; @@ -1206,7 +1205,9 @@ static int sdma_alloc_bd(struct sdma_desc *desc) static void sdma_free_bd(struct sdma_desc *desc) { - dma_pool_free(desc->sdmac->bd_pool, desc->bd, desc->bd_phys); + u32 bd_size = desc->num_bd * sizeof(struct sdma_buffer_descriptor); + + dma_free_coherent(NULL, bd_size, desc->bd, desc->bd_phys); } static void sdma_desc_free(struct virt_dma_desc *vd) @@ -1272,10 +1273,6 @@ static int sdma_alloc_chan_resources(struct dma_chan *chan) if (ret) goto disable_clk_ahb; - sdmac->bd_pool = dma_pool_create("bd_pool", chan->device->dev, - sizeof(struct sdma_buffer_descriptor), - 32, 0); - return 0; disable_clk_ahb: @@ -1304,9 +1301,6 @@ static void sdma_free_chan_resources(struct dma_chan *chan) clk_disable(sdma->clk_ipg); clk_disable(sdma->clk_ahb); - - dma_pool_destroy(sdmac->bd_pool); - sdmac->bd_pool = NULL; } static struct sdma_desc *sdma_transfer_init(struct sdma_channel *sdmac,
WARNING: multiple messages have this Message-ID (diff)
From: Robin Gong <yibin.gong@nxp.com> To: "vkoul@kernel.org" <vkoul@kernel.org>, "l.stach@pengutronix.de" <l.stach@pengutronix.de> Cc: "dmaengine@vger.kernel.org" <dmaengine@vger.kernel.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, dl-linux-imx <linux-imx@nxp.com> Subject: [PATCH v3 2/4] Revert "dmaengine: imx-sdma: alloclate bd memory from dma pool" Date: Tue, 6 Nov 2018 03:40:28 +0000 [thread overview] Message-ID: <1541504525-25720-3-git-send-email-yibin.gong@nxp.com> (raw) In-Reply-To: <1541504525-25720-1-git-send-email-yibin.gong@nxp.com> From: Lucas Stach <l.stach@pengutronix.de> This reverts commit fe5b85c656bc. The SDMA engine needs the descriptors to be contiguous in memory. As the dma pool API is only able to provide a single descriptor per alloc invocation there is no guarantee that multiple descriptors satisfy this requirement. Also the code in question is broken as it only allocates memory for a single descriptor, without looking at the number of descriptors required for the transfer, leading to out-of-bounds accesses when the descriptors are written. Signed-off-by: Lucas Stach <l.stach@pengutronix.de> Signed-off-by: Robin Gong <yibin.gong@nxp.com> --- drivers/dma/imx-sdma.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c index 3bca5e0..8d2fec8 100644 --- a/drivers/dma/imx-sdma.c +++ b/drivers/dma/imx-sdma.c @@ -24,7 +24,6 @@ #include <linux/spinlock.h> #include <linux/device.h> #include <linux/dma-mapping.h> -#include <linux/dmapool.h> #include <linux/firmware.h> #include <linux/slab.h> #include <linux/platform_device.h> @@ -376,7 +375,6 @@ struct sdma_channel { u32 shp_addr, per_addr; enum dma_status status; struct imx_dma_data data; - struct dma_pool *bd_pool; }; #define IMX_DMA_SG_LOOP BIT(0) @@ -1192,10 +1190,11 @@ static int sdma_request_channel0(struct sdma_engine *sdma) static int sdma_alloc_bd(struct sdma_desc *desc) { + u32 bd_size = desc->num_bd * sizeof(struct sdma_buffer_descriptor); int ret = 0; - desc->bd = dma_pool_alloc(desc->sdmac->bd_pool, GFP_ATOMIC, - &desc->bd_phys); + desc->bd = dma_zalloc_coherent(NULL, bd_size, &desc->bd_phys, + GFP_ATOMIC); if (!desc->bd) { ret = -ENOMEM; goto out; @@ -1206,7 +1205,9 @@ static int sdma_alloc_bd(struct sdma_desc *desc) static void sdma_free_bd(struct sdma_desc *desc) { - dma_pool_free(desc->sdmac->bd_pool, desc->bd, desc->bd_phys); + u32 bd_size = desc->num_bd * sizeof(struct sdma_buffer_descriptor); + + dma_free_coherent(NULL, bd_size, desc->bd, desc->bd_phys); } static void sdma_desc_free(struct virt_dma_desc *vd) @@ -1272,10 +1273,6 @@ static int sdma_alloc_chan_resources(struct dma_chan *chan) if (ret) goto disable_clk_ahb; - sdmac->bd_pool = dma_pool_create("bd_pool", chan->device->dev, - sizeof(struct sdma_buffer_descriptor), - 32, 0); - return 0; disable_clk_ahb: @@ -1304,9 +1301,6 @@ static void sdma_free_chan_resources(struct dma_chan *chan) clk_disable(sdma->clk_ipg); clk_disable(sdma->clk_ahb); - - dma_pool_destroy(sdmac->bd_pool); - sdmac->bd_pool = NULL; } static struct sdma_desc *sdma_transfer_init(struct sdma_channel *sdmac, -- 2.7.4
next reply other threads:[~2018-11-06 3:40 UTC|newest] Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-11-06 3:40 Robin Gong [this message] 2018-11-06 3:40 ` [PATCH v3 2/4] Revert "dmaengine: imx-sdma: alloclate bd memory from dma pool" Robin Gong -- strict thread matches above, loose matches on Subject: below -- 2018-11-06 3:40 [v3,4/4] dmaengine: imx-sdma: use GFP_NOWAIT for dma descriptor allocations Robin Gong 2018-11-06 3:40 ` [PATCH v3 4/4] " Robin Gong 2018-11-06 3:40 [v3,3/4] dmaengine: imx-sdma: implement channel termination via worker Robin Gong 2018-11-06 3:40 ` [PATCH v3 3/4] " Robin Gong 2018-11-06 3:40 [v3,1/4] Revert "dmaengine: imx-sdma: Use GFP_NOWAIT for dma allocations" Robin Gong 2018-11-06 3:40 ` [PATCH v3 1/4] " Robin Gong 2018-11-06 3:40 [PATCH v3 0/4] Correct dma pool for imx-sdma Robin Gong 2018-11-12 15:56 ` Lucas Stach 2018-11-20 9:08 ` Robin Gong 2018-12-04 8:11 ` Robin Gong 2018-12-05 8:23 ` Vinod Koul
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=1541504525-25720-3-git-send-email-yibin.gong@nxp.com \ --to=yibin.gong@nxp.com \ --cc=dmaengine@vger.kernel.org \ --cc=l.stach@pengutronix.de \ --cc=linux-imx@nxp.com \ --cc=linux-kernel@vger.kernel.org \ --cc=vkoul@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.