All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC] mmc: meson-gx: use memcpy_to/fromio for dram-access-quirk
@ 2021-06-08 15:33   ` Neil Armstrong
  0 siblings, 0 replies; 21+ messages in thread
From: Neil Armstrong @ 2021-06-08 15:33 UTC (permalink / raw)
  To: ulf.hansson, m.szyprowski
  Cc: khilman, jbrunet, martin.blumenstingl, linux-mmc, linux-amlogic,
	linux-arm-kernel, linux-kernel, Neil Armstrong, Mark Rutland

It has been reported that usage of memcpy() to/from an iomem mapping is invalid,
and and recent arm64 memcpy update [1] triggers a memory abort when dram-access-quirk
is used on the G12A/G12B platforms.

This adds a local sg_copy_to_buffer which makes usage of io versions of memcpy
when dram-access-quirk is enabled.

Fixes: acdc8e71d9bb ("mmc: meson-gx: add dram-access-quirk")
Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
Suggested-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>

[1] 285133040e6c ("arm64: Import latest memcpy()/memmove() implementation")

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
Hi Ulf, Marek, Mark,

I haven't tested the patch yet, but should fix issue reported at [2].

Neil

[2] https://lore.kernel.org/r/acb244ad-0759-5a96-c659-5c23003d3dcd@samsung.com

 drivers/mmc/host/meson-gx-mmc.c | 48 ++++++++++++++++++++++++++++++---
 1 file changed, 44 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index b8b771b643cc..89ff6038092d 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -742,6 +742,48 @@ static void meson_mmc_desc_chain_transfer(struct mmc_host *mmc, u32 cmd_cfg)
 	writel(start, host->regs + SD_EMMC_START);
 }
 
+/* local sg copy to buffer version with _to/fromio usage for dram_access_quirk */
+static void meson_mmc_copy_buffer(struct meson_host *host, struct mmc_data *data,
+				  size_t buflen, bool to_buffer)
+{
+	unsigned int sg_flags = SG_MITER_ATOMIC;
+	struct scatterlist *sgl = data->sg;
+	unsigned int nents = data->sg_len;
+	struct sg_mapping_iter miter;
+	void *buf = host->bounce_buf;
+	unsigned int offset = 0;
+
+	if (to_buffer)
+		sg_flags |= SG_MITER_FROM_SG;
+	else
+		sg_flags |= SG_MITER_TO_SG;
+
+	sg_miter_start(&miter, sgl, nents, sg_flags);
+
+	while ((offset < buflen) && sg_miter_next(&miter)) {
+		unsigned int len;
+
+		len = min(miter.length, buflen - offset);
+
+		/* When dram_access_quirk, the bounce buffer is a iomem mapping */
+		if (host->dram_access_quirk) {
+			if (to_buffer)
+				memcpy_toio(buf + offset, miter.addr, len);
+			else
+				memcpy_fromio(miter.addr, buf + offset, len);
+		} else {
+			if (to_buffer)
+				memcpy(buf + offset, miter.addr, len);
+			else
+				memcpy(miter.addr, buf + offset, len);
+		}
+
+		offset += len;
+	}
+
+	sg_miter_stop(&miter);
+}
+
 static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
 {
 	struct meson_host *host = mmc_priv(mmc);
@@ -785,8 +827,7 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
 		if (data->flags & MMC_DATA_WRITE) {
 			cmd_cfg |= CMD_CFG_DATA_WR;
 			WARN_ON(xfer_bytes > host->bounce_buf_size);
-			sg_copy_to_buffer(data->sg, data->sg_len,
-					  host->bounce_buf, xfer_bytes);
+			meson_mmc_copy_buffer(host, data, xfer_bytes, true);
 			dma_wmb();
 		}
 
@@ -955,8 +996,7 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
 	if (meson_mmc_bounce_buf_read(data)) {
 		xfer_bytes = data->blksz * data->blocks;
 		WARN_ON(xfer_bytes > host->bounce_buf_size);
-		sg_copy_from_buffer(data->sg, data->sg_len,
-				    host->bounce_buf, xfer_bytes);
+		meson_mmc_copy_buffer(host, data, xfer_bytes, false);
 	}
 
 	next_cmd = meson_mmc_get_next_command(cmd);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [RFC] mmc: meson-gx: use memcpy_to/fromio for dram-access-quirk
@ 2021-06-08 15:33   ` Neil Armstrong
  0 siblings, 0 replies; 21+ messages in thread
From: Neil Armstrong @ 2021-06-08 15:33 UTC (permalink / raw)
  To: ulf.hansson, m.szyprowski
  Cc: khilman, jbrunet, martin.blumenstingl, linux-mmc, linux-amlogic,
	linux-arm-kernel, linux-kernel, Neil Armstrong, Mark Rutland

It has been reported that usage of memcpy() to/from an iomem mapping is invalid,
and and recent arm64 memcpy update [1] triggers a memory abort when dram-access-quirk
is used on the G12A/G12B platforms.

This adds a local sg_copy_to_buffer which makes usage of io versions of memcpy
when dram-access-quirk is enabled.

Fixes: acdc8e71d9bb ("mmc: meson-gx: add dram-access-quirk")
Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
Suggested-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>

[1] 285133040e6c ("arm64: Import latest memcpy()/memmove() implementation")

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
Hi Ulf, Marek, Mark,

I haven't tested the patch yet, but should fix issue reported at [2].

Neil

[2] https://lore.kernel.org/r/acb244ad-0759-5a96-c659-5c23003d3dcd@samsung.com

 drivers/mmc/host/meson-gx-mmc.c | 48 ++++++++++++++++++++++++++++++---
 1 file changed, 44 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index b8b771b643cc..89ff6038092d 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -742,6 +742,48 @@ static void meson_mmc_desc_chain_transfer(struct mmc_host *mmc, u32 cmd_cfg)
 	writel(start, host->regs + SD_EMMC_START);
 }
 
+/* local sg copy to buffer version with _to/fromio usage for dram_access_quirk */
+static void meson_mmc_copy_buffer(struct meson_host *host, struct mmc_data *data,
+				  size_t buflen, bool to_buffer)
+{
+	unsigned int sg_flags = SG_MITER_ATOMIC;
+	struct scatterlist *sgl = data->sg;
+	unsigned int nents = data->sg_len;
+	struct sg_mapping_iter miter;
+	void *buf = host->bounce_buf;
+	unsigned int offset = 0;
+
+	if (to_buffer)
+		sg_flags |= SG_MITER_FROM_SG;
+	else
+		sg_flags |= SG_MITER_TO_SG;
+
+	sg_miter_start(&miter, sgl, nents, sg_flags);
+
+	while ((offset < buflen) && sg_miter_next(&miter)) {
+		unsigned int len;
+
+		len = min(miter.length, buflen - offset);
+
+		/* When dram_access_quirk, the bounce buffer is a iomem mapping */
+		if (host->dram_access_quirk) {
+			if (to_buffer)
+				memcpy_toio(buf + offset, miter.addr, len);
+			else
+				memcpy_fromio(miter.addr, buf + offset, len);
+		} else {
+			if (to_buffer)
+				memcpy(buf + offset, miter.addr, len);
+			else
+				memcpy(miter.addr, buf + offset, len);
+		}
+
+		offset += len;
+	}
+
+	sg_miter_stop(&miter);
+}
+
 static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
 {
 	struct meson_host *host = mmc_priv(mmc);
@@ -785,8 +827,7 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
 		if (data->flags & MMC_DATA_WRITE) {
 			cmd_cfg |= CMD_CFG_DATA_WR;
 			WARN_ON(xfer_bytes > host->bounce_buf_size);
-			sg_copy_to_buffer(data->sg, data->sg_len,
-					  host->bounce_buf, xfer_bytes);
+			meson_mmc_copy_buffer(host, data, xfer_bytes, true);
 			dma_wmb();
 		}
 
@@ -955,8 +996,7 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
 	if (meson_mmc_bounce_buf_read(data)) {
 		xfer_bytes = data->blksz * data->blocks;
 		WARN_ON(xfer_bytes > host->bounce_buf_size);
-		sg_copy_from_buffer(data->sg, data->sg_len,
-				    host->bounce_buf, xfer_bytes);
+		meson_mmc_copy_buffer(host, data, xfer_bytes, false);
 	}
 
 	next_cmd = meson_mmc_get_next_command(cmd);
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [RFC] mmc: meson-gx: use memcpy_to/fromio for dram-access-quirk
@ 2021-06-08 15:33   ` Neil Armstrong
  0 siblings, 0 replies; 21+ messages in thread
From: Neil Armstrong @ 2021-06-08 15:33 UTC (permalink / raw)
  To: ulf.hansson, m.szyprowski
  Cc: khilman, jbrunet, martin.blumenstingl, linux-mmc, linux-amlogic,
	linux-arm-kernel, linux-kernel, Neil Armstrong, Mark Rutland

It has been reported that usage of memcpy() to/from an iomem mapping is invalid,
and and recent arm64 memcpy update [1] triggers a memory abort when dram-access-quirk
is used on the G12A/G12B platforms.

This adds a local sg_copy_to_buffer which makes usage of io versions of memcpy
when dram-access-quirk is enabled.

Fixes: acdc8e71d9bb ("mmc: meson-gx: add dram-access-quirk")
Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
Suggested-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>

[1] 285133040e6c ("arm64: Import latest memcpy()/memmove() implementation")

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
Hi Ulf, Marek, Mark,

I haven't tested the patch yet, but should fix issue reported at [2].

Neil

[2] https://lore.kernel.org/r/acb244ad-0759-5a96-c659-5c23003d3dcd@samsung.com

 drivers/mmc/host/meson-gx-mmc.c | 48 ++++++++++++++++++++++++++++++---
 1 file changed, 44 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index b8b771b643cc..89ff6038092d 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -742,6 +742,48 @@ static void meson_mmc_desc_chain_transfer(struct mmc_host *mmc, u32 cmd_cfg)
 	writel(start, host->regs + SD_EMMC_START);
 }
 
+/* local sg copy to buffer version with _to/fromio usage for dram_access_quirk */
+static void meson_mmc_copy_buffer(struct meson_host *host, struct mmc_data *data,
+				  size_t buflen, bool to_buffer)
+{
+	unsigned int sg_flags = SG_MITER_ATOMIC;
+	struct scatterlist *sgl = data->sg;
+	unsigned int nents = data->sg_len;
+	struct sg_mapping_iter miter;
+	void *buf = host->bounce_buf;
+	unsigned int offset = 0;
+
+	if (to_buffer)
+		sg_flags |= SG_MITER_FROM_SG;
+	else
+		sg_flags |= SG_MITER_TO_SG;
+
+	sg_miter_start(&miter, sgl, nents, sg_flags);
+
+	while ((offset < buflen) && sg_miter_next(&miter)) {
+		unsigned int len;
+
+		len = min(miter.length, buflen - offset);
+
+		/* When dram_access_quirk, the bounce buffer is a iomem mapping */
+		if (host->dram_access_quirk) {
+			if (to_buffer)
+				memcpy_toio(buf + offset, miter.addr, len);
+			else
+				memcpy_fromio(miter.addr, buf + offset, len);
+		} else {
+			if (to_buffer)
+				memcpy(buf + offset, miter.addr, len);
+			else
+				memcpy(miter.addr, buf + offset, len);
+		}
+
+		offset += len;
+	}
+
+	sg_miter_stop(&miter);
+}
+
 static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
 {
 	struct meson_host *host = mmc_priv(mmc);
@@ -785,8 +827,7 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
 		if (data->flags & MMC_DATA_WRITE) {
 			cmd_cfg |= CMD_CFG_DATA_WR;
 			WARN_ON(xfer_bytes > host->bounce_buf_size);
-			sg_copy_to_buffer(data->sg, data->sg_len,
-					  host->bounce_buf, xfer_bytes);
+			meson_mmc_copy_buffer(host, data, xfer_bytes, true);
 			dma_wmb();
 		}
 
@@ -955,8 +996,7 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
 	if (meson_mmc_bounce_buf_read(data)) {
 		xfer_bytes = data->blksz * data->blocks;
 		WARN_ON(xfer_bytes > host->bounce_buf_size);
-		sg_copy_from_buffer(data->sg, data->sg_len,
-				    host->bounce_buf, xfer_bytes);
+		meson_mmc_copy_buffer(host, data, xfer_bytes, false);
 	}
 
 	next_cmd = meson_mmc_get_next_command(cmd);
-- 
2.25.1


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [RFC] mmc: meson-gx: use memcpy_to/fromio for dram-access-quirk
  2021-06-08 15:33   ` Neil Armstrong
  (?)
@ 2021-06-08 15:50     ` Marek Szyprowski
  -1 siblings, 0 replies; 21+ messages in thread
From: Marek Szyprowski @ 2021-06-08 15:50 UTC (permalink / raw)
  To: Neil Armstrong, ulf.hansson
  Cc: khilman, jbrunet, martin.blumenstingl, linux-mmc, linux-amlogic,
	linux-arm-kernel, linux-kernel, Mark Rutland,
	Bartlomiej Zolnierkiewicz

Hi

On 08.06.2021 17:33, Neil Armstrong wrote:
> It has been reported that usage of memcpy() to/from an iomem mapping is invalid,
> and and recent arm64 memcpy update [1] triggers a memory abort when dram-access-quirk
> is used on the G12A/G12B platforms.
>
> This adds a local sg_copy_to_buffer which makes usage of io versions of memcpy
> when dram-access-quirk is enabled.
>
> Fixes: acdc8e71d9bb ("mmc: meson-gx: add dram-access-quirk")
> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Suggested-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>
> [1] 285133040e6c ("arm64: Import latest memcpy()/memmove() implementation")
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
> Hi Ulf, Marek, Mark,
>
> I haven't tested the patch yet, but should fix issue reported at [2].

Works fine here and fixed the issue.

Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

> Neil
>
> [2] https://lore.kernel.org/r/acb244ad-0759-5a96-c659-5c23003d3dcd@samsung.com
>
>   drivers/mmc/host/meson-gx-mmc.c | 48 ++++++++++++++++++++++++++++++---
>   1 file changed, 44 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index b8b771b643cc..89ff6038092d 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -742,6 +742,48 @@ static void meson_mmc_desc_chain_transfer(struct mmc_host *mmc, u32 cmd_cfg)
>   	writel(start, host->regs + SD_EMMC_START);
>   }
>   
> +/* local sg copy to buffer version with _to/fromio usage for dram_access_quirk */
> +static void meson_mmc_copy_buffer(struct meson_host *host, struct mmc_data *data,
> +				  size_t buflen, bool to_buffer)
> +{
> +	unsigned int sg_flags = SG_MITER_ATOMIC;
> +	struct scatterlist *sgl = data->sg;
> +	unsigned int nents = data->sg_len;
> +	struct sg_mapping_iter miter;
> +	void *buf = host->bounce_buf;
> +	unsigned int offset = 0;
> +
> +	if (to_buffer)
> +		sg_flags |= SG_MITER_FROM_SG;
> +	else
> +		sg_flags |= SG_MITER_TO_SG;
> +
> +	sg_miter_start(&miter, sgl, nents, sg_flags);
> +
> +	while ((offset < buflen) && sg_miter_next(&miter)) {
> +		unsigned int len;
> +
> +		len = min(miter.length, buflen - offset);
> +
> +		/* When dram_access_quirk, the bounce buffer is a iomem mapping */
> +		if (host->dram_access_quirk) {
> +			if (to_buffer)
> +				memcpy_toio(buf + offset, miter.addr, len);
> +			else
> +				memcpy_fromio(miter.addr, buf + offset, len);
> +		} else {
> +			if (to_buffer)
> +				memcpy(buf + offset, miter.addr, len);
> +			else
> +				memcpy(miter.addr, buf + offset, len);
> +		}
> +
> +		offset += len;
> +	}
> +
> +	sg_miter_stop(&miter);
> +}
> +
>   static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>   {
>   	struct meson_host *host = mmc_priv(mmc);
> @@ -785,8 +827,7 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>   		if (data->flags & MMC_DATA_WRITE) {
>   			cmd_cfg |= CMD_CFG_DATA_WR;
>   			WARN_ON(xfer_bytes > host->bounce_buf_size);
> -			sg_copy_to_buffer(data->sg, data->sg_len,
> -					  host->bounce_buf, xfer_bytes);
> +			meson_mmc_copy_buffer(host, data, xfer_bytes, true);
>   			dma_wmb();
>   		}
>   
> @@ -955,8 +996,7 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
>   	if (meson_mmc_bounce_buf_read(data)) {
>   		xfer_bytes = data->blksz * data->blocks;
>   		WARN_ON(xfer_bytes > host->bounce_buf_size);
> -		sg_copy_from_buffer(data->sg, data->sg_len,
> -				    host->bounce_buf, xfer_bytes);
> +		meson_mmc_copy_buffer(host, data, xfer_bytes, false);
>   	}
>   
>   	next_cmd = meson_mmc_get_next_command(cmd);

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC] mmc: meson-gx: use memcpy_to/fromio for dram-access-quirk
@ 2021-06-08 15:50     ` Marek Szyprowski
  0 siblings, 0 replies; 21+ messages in thread
From: Marek Szyprowski @ 2021-06-08 15:50 UTC (permalink / raw)
  To: Neil Armstrong, ulf.hansson
  Cc: khilman, jbrunet, martin.blumenstingl, linux-mmc, linux-amlogic,
	linux-arm-kernel, linux-kernel, Mark Rutland,
	Bartlomiej Zolnierkiewicz

Hi

On 08.06.2021 17:33, Neil Armstrong wrote:
> It has been reported that usage of memcpy() to/from an iomem mapping is invalid,
> and and recent arm64 memcpy update [1] triggers a memory abort when dram-access-quirk
> is used on the G12A/G12B platforms.
>
> This adds a local sg_copy_to_buffer which makes usage of io versions of memcpy
> when dram-access-quirk is enabled.
>
> Fixes: acdc8e71d9bb ("mmc: meson-gx: add dram-access-quirk")
> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Suggested-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>
> [1] 285133040e6c ("arm64: Import latest memcpy()/memmove() implementation")
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
> Hi Ulf, Marek, Mark,
>
> I haven't tested the patch yet, but should fix issue reported at [2].

Works fine here and fixed the issue.

Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

> Neil
>
> [2] https://lore.kernel.org/r/acb244ad-0759-5a96-c659-5c23003d3dcd@samsung.com
>
>   drivers/mmc/host/meson-gx-mmc.c | 48 ++++++++++++++++++++++++++++++---
>   1 file changed, 44 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index b8b771b643cc..89ff6038092d 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -742,6 +742,48 @@ static void meson_mmc_desc_chain_transfer(struct mmc_host *mmc, u32 cmd_cfg)
>   	writel(start, host->regs + SD_EMMC_START);
>   }
>   
> +/* local sg copy to buffer version with _to/fromio usage for dram_access_quirk */
> +static void meson_mmc_copy_buffer(struct meson_host *host, struct mmc_data *data,
> +				  size_t buflen, bool to_buffer)
> +{
> +	unsigned int sg_flags = SG_MITER_ATOMIC;
> +	struct scatterlist *sgl = data->sg;
> +	unsigned int nents = data->sg_len;
> +	struct sg_mapping_iter miter;
> +	void *buf = host->bounce_buf;
> +	unsigned int offset = 0;
> +
> +	if (to_buffer)
> +		sg_flags |= SG_MITER_FROM_SG;
> +	else
> +		sg_flags |= SG_MITER_TO_SG;
> +
> +	sg_miter_start(&miter, sgl, nents, sg_flags);
> +
> +	while ((offset < buflen) && sg_miter_next(&miter)) {
> +		unsigned int len;
> +
> +		len = min(miter.length, buflen - offset);
> +
> +		/* When dram_access_quirk, the bounce buffer is a iomem mapping */
> +		if (host->dram_access_quirk) {
> +			if (to_buffer)
> +				memcpy_toio(buf + offset, miter.addr, len);
> +			else
> +				memcpy_fromio(miter.addr, buf + offset, len);
> +		} else {
> +			if (to_buffer)
> +				memcpy(buf + offset, miter.addr, len);
> +			else
> +				memcpy(miter.addr, buf + offset, len);
> +		}
> +
> +		offset += len;
> +	}
> +
> +	sg_miter_stop(&miter);
> +}
> +
>   static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>   {
>   	struct meson_host *host = mmc_priv(mmc);
> @@ -785,8 +827,7 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>   		if (data->flags & MMC_DATA_WRITE) {
>   			cmd_cfg |= CMD_CFG_DATA_WR;
>   			WARN_ON(xfer_bytes > host->bounce_buf_size);
> -			sg_copy_to_buffer(data->sg, data->sg_len,
> -					  host->bounce_buf, xfer_bytes);
> +			meson_mmc_copy_buffer(host, data, xfer_bytes, true);
>   			dma_wmb();
>   		}
>   
> @@ -955,8 +996,7 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
>   	if (meson_mmc_bounce_buf_read(data)) {
>   		xfer_bytes = data->blksz * data->blocks;
>   		WARN_ON(xfer_bytes > host->bounce_buf_size);
> -		sg_copy_from_buffer(data->sg, data->sg_len,
> -				    host->bounce_buf, xfer_bytes);
> +		meson_mmc_copy_buffer(host, data, xfer_bytes, false);
>   	}
>   
>   	next_cmd = meson_mmc_get_next_command(cmd);

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC] mmc: meson-gx: use memcpy_to/fromio for dram-access-quirk
@ 2021-06-08 15:50     ` Marek Szyprowski
  0 siblings, 0 replies; 21+ messages in thread
From: Marek Szyprowski @ 2021-06-08 15:50 UTC (permalink / raw)
  To: Neil Armstrong, ulf.hansson
  Cc: khilman, jbrunet, martin.blumenstingl, linux-mmc, linux-amlogic,
	linux-arm-kernel, linux-kernel, Mark Rutland,
	Bartlomiej Zolnierkiewicz

Hi

On 08.06.2021 17:33, Neil Armstrong wrote:
> It has been reported that usage of memcpy() to/from an iomem mapping is invalid,
> and and recent arm64 memcpy update [1] triggers a memory abort when dram-access-quirk
> is used on the G12A/G12B platforms.
>
> This adds a local sg_copy_to_buffer which makes usage of io versions of memcpy
> when dram-access-quirk is enabled.
>
> Fixes: acdc8e71d9bb ("mmc: meson-gx: add dram-access-quirk")
> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Suggested-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>
> [1] 285133040e6c ("arm64: Import latest memcpy()/memmove() implementation")
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
> Hi Ulf, Marek, Mark,
>
> I haven't tested the patch yet, but should fix issue reported at [2].

Works fine here and fixed the issue.

Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

> Neil
>
> [2] https://lore.kernel.org/r/acb244ad-0759-5a96-c659-5c23003d3dcd@samsung.com
>
>   drivers/mmc/host/meson-gx-mmc.c | 48 ++++++++++++++++++++++++++++++---
>   1 file changed, 44 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index b8b771b643cc..89ff6038092d 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -742,6 +742,48 @@ static void meson_mmc_desc_chain_transfer(struct mmc_host *mmc, u32 cmd_cfg)
>   	writel(start, host->regs + SD_EMMC_START);
>   }
>   
> +/* local sg copy to buffer version with _to/fromio usage for dram_access_quirk */
> +static void meson_mmc_copy_buffer(struct meson_host *host, struct mmc_data *data,
> +				  size_t buflen, bool to_buffer)
> +{
> +	unsigned int sg_flags = SG_MITER_ATOMIC;
> +	struct scatterlist *sgl = data->sg;
> +	unsigned int nents = data->sg_len;
> +	struct sg_mapping_iter miter;
> +	void *buf = host->bounce_buf;
> +	unsigned int offset = 0;
> +
> +	if (to_buffer)
> +		sg_flags |= SG_MITER_FROM_SG;
> +	else
> +		sg_flags |= SG_MITER_TO_SG;
> +
> +	sg_miter_start(&miter, sgl, nents, sg_flags);
> +
> +	while ((offset < buflen) && sg_miter_next(&miter)) {
> +		unsigned int len;
> +
> +		len = min(miter.length, buflen - offset);
> +
> +		/* When dram_access_quirk, the bounce buffer is a iomem mapping */
> +		if (host->dram_access_quirk) {
> +			if (to_buffer)
> +				memcpy_toio(buf + offset, miter.addr, len);
> +			else
> +				memcpy_fromio(miter.addr, buf + offset, len);
> +		} else {
> +			if (to_buffer)
> +				memcpy(buf + offset, miter.addr, len);
> +			else
> +				memcpy(miter.addr, buf + offset, len);
> +		}
> +
> +		offset += len;
> +	}
> +
> +	sg_miter_stop(&miter);
> +}
> +
>   static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>   {
>   	struct meson_host *host = mmc_priv(mmc);
> @@ -785,8 +827,7 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>   		if (data->flags & MMC_DATA_WRITE) {
>   			cmd_cfg |= CMD_CFG_DATA_WR;
>   			WARN_ON(xfer_bytes > host->bounce_buf_size);
> -			sg_copy_to_buffer(data->sg, data->sg_len,
> -					  host->bounce_buf, xfer_bytes);
> +			meson_mmc_copy_buffer(host, data, xfer_bytes, true);
>   			dma_wmb();
>   		}
>   
> @@ -955,8 +996,7 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
>   	if (meson_mmc_bounce_buf_read(data)) {
>   		xfer_bytes = data->blksz * data->blocks;
>   		WARN_ON(xfer_bytes > host->bounce_buf_size);
> -		sg_copy_from_buffer(data->sg, data->sg_len,
> -				    host->bounce_buf, xfer_bytes);
> +		meson_mmc_copy_buffer(host, data, xfer_bytes, false);
>   	}
>   
>   	next_cmd = meson_mmc_get_next_command(cmd);

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC] mmc: meson-gx: use memcpy_to/fromio for dram-access-quirk
  2021-06-08 15:33   ` Neil Armstrong
                     ` (2 preceding siblings ...)
  (?)
@ 2021-06-08 19:35   ` kernel test robot
  -1 siblings, 0 replies; 21+ messages in thread
From: kernel test robot @ 2021-06-08 19:35 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 4393 bytes --]

Hi Neil,

[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on linus/master]
[also build test WARNING on v5.13-rc5 next-20210608]
[cannot apply to ulf.hansson-mmc/next mmc/mmc-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Neil-Armstrong/mmc-meson-gx-use-memcpy_to-fromio-for-dram-access-quirk/20210608-233729
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 614124bea77e452aa6df7a8714e8bc820b489922
config: microblaze-randconfig-s032-20210608 (attached as .config)
compiler: microblaze-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.3-341-g8af24329-dirty
        # https://github.com/0day-ci/linux/commit/2058382d3f2ac13e79a3402e8992bf41bca22748
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Neil-Armstrong/mmc-meson-gx-use-memcpy_to-fromio-for-dram-access-quirk/20210608-233729
        git checkout 2058382d3f2ac13e79a3402e8992bf41bca22748
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' W=1 ARCH=microblaze 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
>> drivers/mmc/host/meson-gx-mmc.c:774:49: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void volatile [noderef] __iomem *addr @@     got void * @@
   drivers/mmc/host/meson-gx-mmc.c:774:49: sparse:     expected void volatile [noderef] __iomem *addr
   drivers/mmc/host/meson-gx-mmc.c:774:49: sparse:     got void *
>> drivers/mmc/host/meson-gx-mmc.c:776:63: sparse: sparse: incorrect type in argument 2 (different address spaces) @@     expected void const volatile [noderef] __iomem *addr @@     got void * @@
   drivers/mmc/host/meson-gx-mmc.c:776:63: sparse:     expected void const volatile [noderef] __iomem *addr
   drivers/mmc/host/meson-gx-mmc.c:776:63: sparse:     got void *
   drivers/mmc/host/meson-gx-mmc.c:1222:34: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected void *bounce_buf @@     got void [noderef] __iomem * @@
   drivers/mmc/host/meson-gx-mmc.c:1222:34: sparse:     expected void *bounce_buf
   drivers/mmc/host/meson-gx-mmc.c:1222:34: sparse:     got void [noderef] __iomem *

vim +774 drivers/mmc/host/meson-gx-mmc.c

   747	
   748	/* local sg copy to buffer version with _to/fromio usage for dram_access_quirk */
   749	static void meson_mmc_copy_buffer(struct meson_host *host, struct mmc_data *data,
   750					  size_t buflen, bool to_buffer)
   751	{
   752		unsigned int sg_flags = SG_MITER_ATOMIC;
   753		struct scatterlist *sgl = data->sg;
   754		unsigned int nents = data->sg_len;
   755		struct sg_mapping_iter miter;
   756		void *buf = host->bounce_buf;
   757		unsigned int offset = 0;
   758	
   759		if (to_buffer)
   760			sg_flags |= SG_MITER_FROM_SG;
   761		else
   762			sg_flags |= SG_MITER_TO_SG;
   763	
   764		sg_miter_start(&miter, sgl, nents, sg_flags);
   765	
   766		while ((offset < buflen) && sg_miter_next(&miter)) {
   767			unsigned int len;
   768	
   769			len = min(miter.length, buflen - offset);
   770	
   771			/* When dram_access_quirk, the bounce buffer is a iomem mapping */
   772			if (host->dram_access_quirk) {
   773				if (to_buffer)
 > 774					memcpy_toio(buf + offset, miter.addr, len);
   775				else
 > 776					memcpy_fromio(miter.addr, buf + offset, len);
   777			} else {
   778				if (to_buffer)
   779					memcpy(buf + offset, miter.addr, len);
   780				else
   781					memcpy(miter.addr, buf + offset, len);
   782			}
   783	
   784			offset += len;
   785		}
   786	
   787		sg_miter_stop(&miter);
   788	}
   789	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 33525 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC] mmc: meson-gx: use memcpy_to/fromio for dram-access-quirk
  2021-06-08 15:33   ` Neil Armstrong
                     ` (3 preceding siblings ...)
  (?)
@ 2021-06-08 20:49   ` kernel test robot
  -1 siblings, 0 replies; 21+ messages in thread
From: kernel test robot @ 2021-06-08 20:49 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 4908 bytes --]

Hi Neil,

[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on linus/master]
[also build test WARNING on v5.13-rc5 next-20210608]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Neil-Armstrong/mmc-meson-gx-use-memcpy_to-fromio-for-dram-access-quirk/20210608-233729
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 614124bea77e452aa6df7a8714e8bc820b489922
config: mips-randconfig-s031-20210608 (attached as .config)
compiler: mips-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.3-341-g8af24329-dirty
        # https://github.com/0day-ci/linux/commit/2058382d3f2ac13e79a3402e8992bf41bca22748
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Neil-Armstrong/mmc-meson-gx-use-memcpy_to-fromio-for-dram-access-quirk/20210608-233729
        git checkout 2058382d3f2ac13e79a3402e8992bf41bca22748
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' W=1 ARCH=mips 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
   command-line: note: in included file:
   builtin:1:9: sparse: sparse: preprocessor token __ATOMIC_ACQUIRE redefined
   builtin:0:0: sparse: this was the original definition
   builtin:1:9: sparse: sparse: preprocessor token __ATOMIC_SEQ_CST redefined
   builtin:0:0: sparse: this was the original definition
   builtin:1:9: sparse: sparse: preprocessor token __ATOMIC_ACQ_REL redefined
   builtin:0:0: sparse: this was the original definition
   builtin:1:9: sparse: sparse: preprocessor token __ATOMIC_RELEASE redefined
   builtin:0:0: sparse: this was the original definition
>> drivers/mmc/host/meson-gx-mmc.c:774:49: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void volatile [noderef] __iomem *dst @@     got void * @@
   drivers/mmc/host/meson-gx-mmc.c:774:49: sparse:     expected void volatile [noderef] __iomem *dst
   drivers/mmc/host/meson-gx-mmc.c:774:49: sparse:     got void *
>> drivers/mmc/host/meson-gx-mmc.c:776:63: sparse: sparse: incorrect type in argument 2 (different address spaces) @@     expected void const volatile [noderef] __iomem *src @@     got void * @@
   drivers/mmc/host/meson-gx-mmc.c:776:63: sparse:     expected void const volatile [noderef] __iomem *src
   drivers/mmc/host/meson-gx-mmc.c:776:63: sparse:     got void *
   drivers/mmc/host/meson-gx-mmc.c:1222:34: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected void *bounce_buf @@     got void [noderef] __iomem * @@
   drivers/mmc/host/meson-gx-mmc.c:1222:34: sparse:     expected void *bounce_buf
   drivers/mmc/host/meson-gx-mmc.c:1222:34: sparse:     got void [noderef] __iomem *

vim +774 drivers/mmc/host/meson-gx-mmc.c

   747	
   748	/* local sg copy to buffer version with _to/fromio usage for dram_access_quirk */
   749	static void meson_mmc_copy_buffer(struct meson_host *host, struct mmc_data *data,
   750					  size_t buflen, bool to_buffer)
   751	{
   752		unsigned int sg_flags = SG_MITER_ATOMIC;
   753		struct scatterlist *sgl = data->sg;
   754		unsigned int nents = data->sg_len;
   755		struct sg_mapping_iter miter;
   756		void *buf = host->bounce_buf;
   757		unsigned int offset = 0;
   758	
   759		if (to_buffer)
   760			sg_flags |= SG_MITER_FROM_SG;
   761		else
   762			sg_flags |= SG_MITER_TO_SG;
   763	
   764		sg_miter_start(&miter, sgl, nents, sg_flags);
   765	
   766		while ((offset < buflen) && sg_miter_next(&miter)) {
   767			unsigned int len;
   768	
   769			len = min(miter.length, buflen - offset);
   770	
   771			/* When dram_access_quirk, the bounce buffer is a iomem mapping */
   772			if (host->dram_access_quirk) {
   773				if (to_buffer)
 > 774					memcpy_toio(buf + offset, miter.addr, len);
   775				else
 > 776					memcpy_fromio(miter.addr, buf + offset, len);
   777			} else {
   778				if (to_buffer)
   779					memcpy(buf + offset, miter.addr, len);
   780				else
   781					memcpy(miter.addr, buf + offset, len);
   782			}
   783	
   784			offset += len;
   785		}
   786	
   787		sg_miter_stop(&miter);
   788	}
   789	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 32594 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC] mmc: meson-gx: use memcpy_to/fromio for dram-access-quirk
  2021-06-08 15:33   ` Neil Armstrong
                     ` (4 preceding siblings ...)
  (?)
@ 2021-06-09  4:35   ` kernel test robot
  -1 siblings, 0 replies; 21+ messages in thread
From: kernel test robot @ 2021-06-09  4:35 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 4311 bytes --]

Hi Neil,

[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on linus/master]
[also build test WARNING on v5.13-rc5 next-20210608]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Neil-Armstrong/mmc-meson-gx-use-memcpy_to-fromio-for-dram-access-quirk/20210608-233729
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 614124bea77e452aa6df7a8714e8bc820b489922
config: arm64-randconfig-s032-20210608 (attached as .config)
compiler: aarch64-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.3-341-g8af24329-dirty
        # https://github.com/0day-ci/linux/commit/2058382d3f2ac13e79a3402e8992bf41bca22748
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Neil-Armstrong/mmc-meson-gx-use-memcpy_to-fromio-for-dram-access-quirk/20210608-233729
        git checkout 2058382d3f2ac13e79a3402e8992bf41bca22748
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' W=1 ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
>> drivers/mmc/host/meson-gx-mmc.c:774:33: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void volatile [noderef] __iomem * @@     got void * @@
   drivers/mmc/host/meson-gx-mmc.c:774:33: sparse:     expected void volatile [noderef] __iomem *
   drivers/mmc/host/meson-gx-mmc.c:774:33: sparse:     got void *
>> drivers/mmc/host/meson-gx-mmc.c:776:33: sparse: sparse: incorrect type in argument 2 (different address spaces) @@     expected void const volatile [noderef] __iomem * @@     got void * @@
   drivers/mmc/host/meson-gx-mmc.c:776:33: sparse:     expected void const volatile [noderef] __iomem *
   drivers/mmc/host/meson-gx-mmc.c:776:33: sparse:     got void *
   drivers/mmc/host/meson-gx-mmc.c:1222:34: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected void *bounce_buf @@     got void [noderef] __iomem * @@
   drivers/mmc/host/meson-gx-mmc.c:1222:34: sparse:     expected void *bounce_buf
   drivers/mmc/host/meson-gx-mmc.c:1222:34: sparse:     got void [noderef] __iomem *

vim +774 drivers/mmc/host/meson-gx-mmc.c

   747	
   748	/* local sg copy to buffer version with _to/fromio usage for dram_access_quirk */
   749	static void meson_mmc_copy_buffer(struct meson_host *host, struct mmc_data *data,
   750					  size_t buflen, bool to_buffer)
   751	{
   752		unsigned int sg_flags = SG_MITER_ATOMIC;
   753		struct scatterlist *sgl = data->sg;
   754		unsigned int nents = data->sg_len;
   755		struct sg_mapping_iter miter;
   756		void *buf = host->bounce_buf;
   757		unsigned int offset = 0;
   758	
   759		if (to_buffer)
   760			sg_flags |= SG_MITER_FROM_SG;
   761		else
   762			sg_flags |= SG_MITER_TO_SG;
   763	
   764		sg_miter_start(&miter, sgl, nents, sg_flags);
   765	
   766		while ((offset < buflen) && sg_miter_next(&miter)) {
   767			unsigned int len;
   768	
   769			len = min(miter.length, buflen - offset);
   770	
   771			/* When dram_access_quirk, the bounce buffer is a iomem mapping */
   772			if (host->dram_access_quirk) {
   773				if (to_buffer)
 > 774					memcpy_toio(buf + offset, miter.addr, len);
   775				else
 > 776					memcpy_fromio(miter.addr, buf + offset, len);
   777			} else {
   778				if (to_buffer)
   779					memcpy(buf + offset, miter.addr, len);
   780				else
   781					memcpy(miter.addr, buf + offset, len);
   782			}
   783	
   784			offset += len;
   785		}
   786	
   787		sg_miter_stop(&miter);
   788	}
   789	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 33882 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC] mmc: meson-gx: use memcpy_to/fromio for dram-access-quirk
  2021-06-08 15:50     ` Marek Szyprowski
  (?)
@ 2021-06-09 13:07       ` Neil Armstrong
  -1 siblings, 0 replies; 21+ messages in thread
From: Neil Armstrong @ 2021-06-09 13:07 UTC (permalink / raw)
  To: Marek Szyprowski, ulf.hansson
  Cc: khilman, jbrunet, martin.blumenstingl, linux-mmc, linux-amlogic,
	linux-arm-kernel, linux-kernel, Mark Rutland,
	Bartlomiej Zolnierkiewicz

Hi,

On 08/06/2021 17:50, Marek Szyprowski wrote:
> Hi
> 
> On 08.06.2021 17:33, Neil Armstrong wrote:
>> It has been reported that usage of memcpy() to/from an iomem mapping is invalid,
>> and and recent arm64 memcpy update [1] triggers a memory abort when dram-access-quirk
>> is used on the G12A/G12B platforms.
>>
>> This adds a local sg_copy_to_buffer which makes usage of io versions of memcpy
>> when dram-access-quirk is enabled.
>>
>> Fixes: acdc8e71d9bb ("mmc: meson-gx: add dram-access-quirk")
>> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> Suggested-by: Mark Rutland <mark.rutland@arm.com>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>
>> [1] 285133040e6c ("arm64: Import latest memcpy()/memmove() implementation")
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>> Hi Ulf, Marek, Mark,
>>
>> I haven't tested the patch yet, but should fix issue reported at [2].
> 
> Works fine here and fixed the issue.
> 
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

Thanks, I'll need to rework to pass an __iomem pointer to memcpy_to/fromio so sparse doesn't scream anymore.

Neil

> 
>> Neil
>>
>> [2] https://lore.kernel.org/r/acb244ad-0759-5a96-c659-5c23003d3dcd@samsung.com
>>
>>   drivers/mmc/host/meson-gx-mmc.c | 48 ++++++++++++++++++++++++++++++---
>>   1 file changed, 44 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
>> index b8b771b643cc..89ff6038092d 100644
>> --- a/drivers/mmc/host/meson-gx-mmc.c
>> +++ b/drivers/mmc/host/meson-gx-mmc.c
>> @@ -742,6 +742,48 @@ static void meson_mmc_desc_chain_transfer(struct mmc_host *mmc, u32 cmd_cfg)
>>   	writel(start, host->regs + SD_EMMC_START);
>>   }
>>   
>> +/* local sg copy to buffer version with _to/fromio usage for dram_access_quirk */
>> +static void meson_mmc_copy_buffer(struct meson_host *host, struct mmc_data *data,
>> +				  size_t buflen, bool to_buffer)
>> +{
>> +	unsigned int sg_flags = SG_MITER_ATOMIC;
>> +	struct scatterlist *sgl = data->sg;
>> +	unsigned int nents = data->sg_len;
>> +	struct sg_mapping_iter miter;
>> +	void *buf = host->bounce_buf;
>> +	unsigned int offset = 0;
>> +
>> +	if (to_buffer)
>> +		sg_flags |= SG_MITER_FROM_SG;
>> +	else
>> +		sg_flags |= SG_MITER_TO_SG;
>> +
>> +	sg_miter_start(&miter, sgl, nents, sg_flags);
>> +
>> +	while ((offset < buflen) && sg_miter_next(&miter)) {
>> +		unsigned int len;
>> +
>> +		len = min(miter.length, buflen - offset);
>> +
>> +		/* When dram_access_quirk, the bounce buffer is a iomem mapping */
>> +		if (host->dram_access_quirk) {
>> +			if (to_buffer)
>> +				memcpy_toio(buf + offset, miter.addr, len);
>> +			else
>> +				memcpy_fromio(miter.addr, buf + offset, len);
>> +		} else {
>> +			if (to_buffer)
>> +				memcpy(buf + offset, miter.addr, len);
>> +			else
>> +				memcpy(miter.addr, buf + offset, len);
>> +		}
>> +
>> +		offset += len;
>> +	}
>> +
>> +	sg_miter_stop(&miter);
>> +}
>> +
>>   static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>   {
>>   	struct meson_host *host = mmc_priv(mmc);
>> @@ -785,8 +827,7 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>   		if (data->flags & MMC_DATA_WRITE) {
>>   			cmd_cfg |= CMD_CFG_DATA_WR;
>>   			WARN_ON(xfer_bytes > host->bounce_buf_size);
>> -			sg_copy_to_buffer(data->sg, data->sg_len,
>> -					  host->bounce_buf, xfer_bytes);
>> +			meson_mmc_copy_buffer(host, data, xfer_bytes, true);
>>   			dma_wmb();
>>   		}
>>   
>> @@ -955,8 +996,7 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
>>   	if (meson_mmc_bounce_buf_read(data)) {
>>   		xfer_bytes = data->blksz * data->blocks;
>>   		WARN_ON(xfer_bytes > host->bounce_buf_size);
>> -		sg_copy_from_buffer(data->sg, data->sg_len,
>> -				    host->bounce_buf, xfer_bytes);
>> +		meson_mmc_copy_buffer(host, data, xfer_bytes, false);
>>   	}
>>   
>>   	next_cmd = meson_mmc_get_next_command(cmd);
> 
> Best regards
> 


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC] mmc: meson-gx: use memcpy_to/fromio for dram-access-quirk
@ 2021-06-09 13:07       ` Neil Armstrong
  0 siblings, 0 replies; 21+ messages in thread
From: Neil Armstrong @ 2021-06-09 13:07 UTC (permalink / raw)
  To: Marek Szyprowski, ulf.hansson
  Cc: khilman, jbrunet, martin.blumenstingl, linux-mmc, linux-amlogic,
	linux-arm-kernel, linux-kernel, Mark Rutland,
	Bartlomiej Zolnierkiewicz

Hi,

On 08/06/2021 17:50, Marek Szyprowski wrote:
> Hi
> 
> On 08.06.2021 17:33, Neil Armstrong wrote:
>> It has been reported that usage of memcpy() to/from an iomem mapping is invalid,
>> and and recent arm64 memcpy update [1] triggers a memory abort when dram-access-quirk
>> is used on the G12A/G12B platforms.
>>
>> This adds a local sg_copy_to_buffer which makes usage of io versions of memcpy
>> when dram-access-quirk is enabled.
>>
>> Fixes: acdc8e71d9bb ("mmc: meson-gx: add dram-access-quirk")
>> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> Suggested-by: Mark Rutland <mark.rutland@arm.com>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>
>> [1] 285133040e6c ("arm64: Import latest memcpy()/memmove() implementation")
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>> Hi Ulf, Marek, Mark,
>>
>> I haven't tested the patch yet, but should fix issue reported at [2].
> 
> Works fine here and fixed the issue.
> 
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

Thanks, I'll need to rework to pass an __iomem pointer to memcpy_to/fromio so sparse doesn't scream anymore.

Neil

> 
>> Neil
>>
>> [2] https://lore.kernel.org/r/acb244ad-0759-5a96-c659-5c23003d3dcd@samsung.com
>>
>>   drivers/mmc/host/meson-gx-mmc.c | 48 ++++++++++++++++++++++++++++++---
>>   1 file changed, 44 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
>> index b8b771b643cc..89ff6038092d 100644
>> --- a/drivers/mmc/host/meson-gx-mmc.c
>> +++ b/drivers/mmc/host/meson-gx-mmc.c
>> @@ -742,6 +742,48 @@ static void meson_mmc_desc_chain_transfer(struct mmc_host *mmc, u32 cmd_cfg)
>>   	writel(start, host->regs + SD_EMMC_START);
>>   }
>>   
>> +/* local sg copy to buffer version with _to/fromio usage for dram_access_quirk */
>> +static void meson_mmc_copy_buffer(struct meson_host *host, struct mmc_data *data,
>> +				  size_t buflen, bool to_buffer)
>> +{
>> +	unsigned int sg_flags = SG_MITER_ATOMIC;
>> +	struct scatterlist *sgl = data->sg;
>> +	unsigned int nents = data->sg_len;
>> +	struct sg_mapping_iter miter;
>> +	void *buf = host->bounce_buf;
>> +	unsigned int offset = 0;
>> +
>> +	if (to_buffer)
>> +		sg_flags |= SG_MITER_FROM_SG;
>> +	else
>> +		sg_flags |= SG_MITER_TO_SG;
>> +
>> +	sg_miter_start(&miter, sgl, nents, sg_flags);
>> +
>> +	while ((offset < buflen) && sg_miter_next(&miter)) {
>> +		unsigned int len;
>> +
>> +		len = min(miter.length, buflen - offset);
>> +
>> +		/* When dram_access_quirk, the bounce buffer is a iomem mapping */
>> +		if (host->dram_access_quirk) {
>> +			if (to_buffer)
>> +				memcpy_toio(buf + offset, miter.addr, len);
>> +			else
>> +				memcpy_fromio(miter.addr, buf + offset, len);
>> +		} else {
>> +			if (to_buffer)
>> +				memcpy(buf + offset, miter.addr, len);
>> +			else
>> +				memcpy(miter.addr, buf + offset, len);
>> +		}
>> +
>> +		offset += len;
>> +	}
>> +
>> +	sg_miter_stop(&miter);
>> +}
>> +
>>   static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>   {
>>   	struct meson_host *host = mmc_priv(mmc);
>> @@ -785,8 +827,7 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>   		if (data->flags & MMC_DATA_WRITE) {
>>   			cmd_cfg |= CMD_CFG_DATA_WR;
>>   			WARN_ON(xfer_bytes > host->bounce_buf_size);
>> -			sg_copy_to_buffer(data->sg, data->sg_len,
>> -					  host->bounce_buf, xfer_bytes);
>> +			meson_mmc_copy_buffer(host, data, xfer_bytes, true);
>>   			dma_wmb();
>>   		}
>>   
>> @@ -955,8 +996,7 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
>>   	if (meson_mmc_bounce_buf_read(data)) {
>>   		xfer_bytes = data->blksz * data->blocks;
>>   		WARN_ON(xfer_bytes > host->bounce_buf_size);
>> -		sg_copy_from_buffer(data->sg, data->sg_len,
>> -				    host->bounce_buf, xfer_bytes);
>> +		meson_mmc_copy_buffer(host, data, xfer_bytes, false);
>>   	}
>>   
>>   	next_cmd = meson_mmc_get_next_command(cmd);
> 
> Best regards
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC] mmc: meson-gx: use memcpy_to/fromio for dram-access-quirk
@ 2021-06-09 13:07       ` Neil Armstrong
  0 siblings, 0 replies; 21+ messages in thread
From: Neil Armstrong @ 2021-06-09 13:07 UTC (permalink / raw)
  To: Marek Szyprowski, ulf.hansson
  Cc: khilman, jbrunet, martin.blumenstingl, linux-mmc, linux-amlogic,
	linux-arm-kernel, linux-kernel, Mark Rutland,
	Bartlomiej Zolnierkiewicz

Hi,

On 08/06/2021 17:50, Marek Szyprowski wrote:
> Hi
> 
> On 08.06.2021 17:33, Neil Armstrong wrote:
>> It has been reported that usage of memcpy() to/from an iomem mapping is invalid,
>> and and recent arm64 memcpy update [1] triggers a memory abort when dram-access-quirk
>> is used on the G12A/G12B platforms.
>>
>> This adds a local sg_copy_to_buffer which makes usage of io versions of memcpy
>> when dram-access-quirk is enabled.
>>
>> Fixes: acdc8e71d9bb ("mmc: meson-gx: add dram-access-quirk")
>> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> Suggested-by: Mark Rutland <mark.rutland@arm.com>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>
>> [1] 285133040e6c ("arm64: Import latest memcpy()/memmove() implementation")
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>> Hi Ulf, Marek, Mark,
>>
>> I haven't tested the patch yet, but should fix issue reported at [2].
> 
> Works fine here and fixed the issue.
> 
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

Thanks, I'll need to rework to pass an __iomem pointer to memcpy_to/fromio so sparse doesn't scream anymore.

Neil

> 
>> Neil
>>
>> [2] https://lore.kernel.org/r/acb244ad-0759-5a96-c659-5c23003d3dcd@samsung.com
>>
>>   drivers/mmc/host/meson-gx-mmc.c | 48 ++++++++++++++++++++++++++++++---
>>   1 file changed, 44 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
>> index b8b771b643cc..89ff6038092d 100644
>> --- a/drivers/mmc/host/meson-gx-mmc.c
>> +++ b/drivers/mmc/host/meson-gx-mmc.c
>> @@ -742,6 +742,48 @@ static void meson_mmc_desc_chain_transfer(struct mmc_host *mmc, u32 cmd_cfg)
>>   	writel(start, host->regs + SD_EMMC_START);
>>   }
>>   
>> +/* local sg copy to buffer version with _to/fromio usage for dram_access_quirk */
>> +static void meson_mmc_copy_buffer(struct meson_host *host, struct mmc_data *data,
>> +				  size_t buflen, bool to_buffer)
>> +{
>> +	unsigned int sg_flags = SG_MITER_ATOMIC;
>> +	struct scatterlist *sgl = data->sg;
>> +	unsigned int nents = data->sg_len;
>> +	struct sg_mapping_iter miter;
>> +	void *buf = host->bounce_buf;
>> +	unsigned int offset = 0;
>> +
>> +	if (to_buffer)
>> +		sg_flags |= SG_MITER_FROM_SG;
>> +	else
>> +		sg_flags |= SG_MITER_TO_SG;
>> +
>> +	sg_miter_start(&miter, sgl, nents, sg_flags);
>> +
>> +	while ((offset < buflen) && sg_miter_next(&miter)) {
>> +		unsigned int len;
>> +
>> +		len = min(miter.length, buflen - offset);
>> +
>> +		/* When dram_access_quirk, the bounce buffer is a iomem mapping */
>> +		if (host->dram_access_quirk) {
>> +			if (to_buffer)
>> +				memcpy_toio(buf + offset, miter.addr, len);
>> +			else
>> +				memcpy_fromio(miter.addr, buf + offset, len);
>> +		} else {
>> +			if (to_buffer)
>> +				memcpy(buf + offset, miter.addr, len);
>> +			else
>> +				memcpy(miter.addr, buf + offset, len);
>> +		}
>> +
>> +		offset += len;
>> +	}
>> +
>> +	sg_miter_stop(&miter);
>> +}
>> +
>>   static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>   {
>>   	struct meson_host *host = mmc_priv(mmc);
>> @@ -785,8 +827,7 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>   		if (data->flags & MMC_DATA_WRITE) {
>>   			cmd_cfg |= CMD_CFG_DATA_WR;
>>   			WARN_ON(xfer_bytes > host->bounce_buf_size);
>> -			sg_copy_to_buffer(data->sg, data->sg_len,
>> -					  host->bounce_buf, xfer_bytes);
>> +			meson_mmc_copy_buffer(host, data, xfer_bytes, true);
>>   			dma_wmb();
>>   		}
>>   
>> @@ -955,8 +996,7 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
>>   	if (meson_mmc_bounce_buf_read(data)) {
>>   		xfer_bytes = data->blksz * data->blocks;
>>   		WARN_ON(xfer_bytes > host->bounce_buf_size);
>> -		sg_copy_from_buffer(data->sg, data->sg_len,
>> -				    host->bounce_buf, xfer_bytes);
>> +		meson_mmc_copy_buffer(host, data, xfer_bytes, false);
>>   	}
>>   
>>   	next_cmd = meson_mmc_get_next_command(cmd);
> 
> Best regards
> 


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC] mmc: meson-gx: use memcpy_to/fromio for dram-access-quirk
  2021-06-09 13:07       ` Neil Armstrong
  (?)
@ 2021-06-09 14:45         ` Robin Murphy
  -1 siblings, 0 replies; 21+ messages in thread
From: Robin Murphy @ 2021-06-09 14:45 UTC (permalink / raw)
  To: Neil Armstrong, Marek Szyprowski, ulf.hansson
  Cc: khilman, jbrunet, martin.blumenstingl, linux-mmc, linux-amlogic,
	linux-arm-kernel, linux-kernel, Mark Rutland,
	Bartlomiej Zolnierkiewicz

On 2021-06-09 14:07, Neil Armstrong wrote:
> Hi,
> 
> On 08/06/2021 17:50, Marek Szyprowski wrote:
>> Hi
>>
>> On 08.06.2021 17:33, Neil Armstrong wrote:
>>> It has been reported that usage of memcpy() to/from an iomem mapping is invalid,
>>> and and recent arm64 memcpy update [1] triggers a memory abort when dram-access-quirk
>>> is used on the G12A/G12B platforms.
>>>
>>> This adds a local sg_copy_to_buffer which makes usage of io versions of memcpy
>>> when dram-access-quirk is enabled.
>>>
>>> Fixes: acdc8e71d9bb ("mmc: meson-gx: add dram-access-quirk")
>>> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>> Suggested-by: Mark Rutland <mark.rutland@arm.com>
>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>>
>>> [1] 285133040e6c ("arm64: Import latest memcpy()/memmove() implementation")
>>>
>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>> ---
>>> Hi Ulf, Marek, Mark,
>>>
>>> I haven't tested the patch yet, but should fix issue reported at [2].
>>
>> Works fine here and fixed the issue.
>>
>> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> 
> Thanks, I'll need to rework to pass an __iomem pointer to memcpy_to/fromio so sparse doesn't scream anymore.

Hmm, might it be worth factoring out general sg_copy_{to,from}_iomem() 
helpers? From a quick grep I found at least mv_cesa_sg_copy() already 
doing essentially the same thing as meson_mmc_copy_buffer().

Robin.

> 
> Neil
> 
>>
>>> Neil
>>>
>>> [2] https://lore.kernel.org/r/acb244ad-0759-5a96-c659-5c23003d3dcd@samsung.com
>>>
>>>    drivers/mmc/host/meson-gx-mmc.c | 48 ++++++++++++++++++++++++++++++---
>>>    1 file changed, 44 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
>>> index b8b771b643cc..89ff6038092d 100644
>>> --- a/drivers/mmc/host/meson-gx-mmc.c
>>> +++ b/drivers/mmc/host/meson-gx-mmc.c
>>> @@ -742,6 +742,48 @@ static void meson_mmc_desc_chain_transfer(struct mmc_host *mmc, u32 cmd_cfg)
>>>    	writel(start, host->regs + SD_EMMC_START);
>>>    }
>>>    
>>> +/* local sg copy to buffer version with _to/fromio usage for dram_access_quirk */
>>> +static void meson_mmc_copy_buffer(struct meson_host *host, struct mmc_data *data,
>>> +				  size_t buflen, bool to_buffer)
>>> +{
>>> +	unsigned int sg_flags = SG_MITER_ATOMIC;
>>> +	struct scatterlist *sgl = data->sg;
>>> +	unsigned int nents = data->sg_len;
>>> +	struct sg_mapping_iter miter;
>>> +	void *buf = host->bounce_buf;
>>> +	unsigned int offset = 0;
>>> +
>>> +	if (to_buffer)
>>> +		sg_flags |= SG_MITER_FROM_SG;
>>> +	else
>>> +		sg_flags |= SG_MITER_TO_SG;
>>> +
>>> +	sg_miter_start(&miter, sgl, nents, sg_flags);
>>> +
>>> +	while ((offset < buflen) && sg_miter_next(&miter)) {
>>> +		unsigned int len;
>>> +
>>> +		len = min(miter.length, buflen - offset);
>>> +
>>> +		/* When dram_access_quirk, the bounce buffer is a iomem mapping */
>>> +		if (host->dram_access_quirk) {
>>> +			if (to_buffer)
>>> +				memcpy_toio(buf + offset, miter.addr, len);
>>> +			else
>>> +				memcpy_fromio(miter.addr, buf + offset, len);
>>> +		} else {
>>> +			if (to_buffer)
>>> +				memcpy(buf + offset, miter.addr, len);
>>> +			else
>>> +				memcpy(miter.addr, buf + offset, len);
>>> +		}
>>> +
>>> +		offset += len;
>>> +	}
>>> +
>>> +	sg_miter_stop(&miter);
>>> +}
>>> +
>>>    static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>>    {
>>>    	struct meson_host *host = mmc_priv(mmc);
>>> @@ -785,8 +827,7 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>>    		if (data->flags & MMC_DATA_WRITE) {
>>>    			cmd_cfg |= CMD_CFG_DATA_WR;
>>>    			WARN_ON(xfer_bytes > host->bounce_buf_size);
>>> -			sg_copy_to_buffer(data->sg, data->sg_len,
>>> -					  host->bounce_buf, xfer_bytes);
>>> +			meson_mmc_copy_buffer(host, data, xfer_bytes, true);
>>>    			dma_wmb();
>>>    		}
>>>    
>>> @@ -955,8 +996,7 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
>>>    	if (meson_mmc_bounce_buf_read(data)) {
>>>    		xfer_bytes = data->blksz * data->blocks;
>>>    		WARN_ON(xfer_bytes > host->bounce_buf_size);
>>> -		sg_copy_from_buffer(data->sg, data->sg_len,
>>> -				    host->bounce_buf, xfer_bytes);
>>> +		meson_mmc_copy_buffer(host, data, xfer_bytes, false);
>>>    	}
>>>    
>>>    	next_cmd = meson_mmc_get_next_command(cmd);
>>
>> Best regards
>>
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC] mmc: meson-gx: use memcpy_to/fromio for dram-access-quirk
@ 2021-06-09 14:45         ` Robin Murphy
  0 siblings, 0 replies; 21+ messages in thread
From: Robin Murphy @ 2021-06-09 14:45 UTC (permalink / raw)
  To: Neil Armstrong, Marek Szyprowski, ulf.hansson
  Cc: khilman, jbrunet, martin.blumenstingl, linux-mmc, linux-amlogic,
	linux-arm-kernel, linux-kernel, Mark Rutland,
	Bartlomiej Zolnierkiewicz

On 2021-06-09 14:07, Neil Armstrong wrote:
> Hi,
> 
> On 08/06/2021 17:50, Marek Szyprowski wrote:
>> Hi
>>
>> On 08.06.2021 17:33, Neil Armstrong wrote:
>>> It has been reported that usage of memcpy() to/from an iomem mapping is invalid,
>>> and and recent arm64 memcpy update [1] triggers a memory abort when dram-access-quirk
>>> is used on the G12A/G12B platforms.
>>>
>>> This adds a local sg_copy_to_buffer which makes usage of io versions of memcpy
>>> when dram-access-quirk is enabled.
>>>
>>> Fixes: acdc8e71d9bb ("mmc: meson-gx: add dram-access-quirk")
>>> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>> Suggested-by: Mark Rutland <mark.rutland@arm.com>
>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>>
>>> [1] 285133040e6c ("arm64: Import latest memcpy()/memmove() implementation")
>>>
>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>> ---
>>> Hi Ulf, Marek, Mark,
>>>
>>> I haven't tested the patch yet, but should fix issue reported at [2].
>>
>> Works fine here and fixed the issue.
>>
>> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> 
> Thanks, I'll need to rework to pass an __iomem pointer to memcpy_to/fromio so sparse doesn't scream anymore.

Hmm, might it be worth factoring out general sg_copy_{to,from}_iomem() 
helpers? From a quick grep I found at least mv_cesa_sg_copy() already 
doing essentially the same thing as meson_mmc_copy_buffer().

Robin.

> 
> Neil
> 
>>
>>> Neil
>>>
>>> [2] https://lore.kernel.org/r/acb244ad-0759-5a96-c659-5c23003d3dcd@samsung.com
>>>
>>>    drivers/mmc/host/meson-gx-mmc.c | 48 ++++++++++++++++++++++++++++++---
>>>    1 file changed, 44 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
>>> index b8b771b643cc..89ff6038092d 100644
>>> --- a/drivers/mmc/host/meson-gx-mmc.c
>>> +++ b/drivers/mmc/host/meson-gx-mmc.c
>>> @@ -742,6 +742,48 @@ static void meson_mmc_desc_chain_transfer(struct mmc_host *mmc, u32 cmd_cfg)
>>>    	writel(start, host->regs + SD_EMMC_START);
>>>    }
>>>    
>>> +/* local sg copy to buffer version with _to/fromio usage for dram_access_quirk */
>>> +static void meson_mmc_copy_buffer(struct meson_host *host, struct mmc_data *data,
>>> +				  size_t buflen, bool to_buffer)
>>> +{
>>> +	unsigned int sg_flags = SG_MITER_ATOMIC;
>>> +	struct scatterlist *sgl = data->sg;
>>> +	unsigned int nents = data->sg_len;
>>> +	struct sg_mapping_iter miter;
>>> +	void *buf = host->bounce_buf;
>>> +	unsigned int offset = 0;
>>> +
>>> +	if (to_buffer)
>>> +		sg_flags |= SG_MITER_FROM_SG;
>>> +	else
>>> +		sg_flags |= SG_MITER_TO_SG;
>>> +
>>> +	sg_miter_start(&miter, sgl, nents, sg_flags);
>>> +
>>> +	while ((offset < buflen) && sg_miter_next(&miter)) {
>>> +		unsigned int len;
>>> +
>>> +		len = min(miter.length, buflen - offset);
>>> +
>>> +		/* When dram_access_quirk, the bounce buffer is a iomem mapping */
>>> +		if (host->dram_access_quirk) {
>>> +			if (to_buffer)
>>> +				memcpy_toio(buf + offset, miter.addr, len);
>>> +			else
>>> +				memcpy_fromio(miter.addr, buf + offset, len);
>>> +		} else {
>>> +			if (to_buffer)
>>> +				memcpy(buf + offset, miter.addr, len);
>>> +			else
>>> +				memcpy(miter.addr, buf + offset, len);
>>> +		}
>>> +
>>> +		offset += len;
>>> +	}
>>> +
>>> +	sg_miter_stop(&miter);
>>> +}
>>> +
>>>    static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>>    {
>>>    	struct meson_host *host = mmc_priv(mmc);
>>> @@ -785,8 +827,7 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>>    		if (data->flags & MMC_DATA_WRITE) {
>>>    			cmd_cfg |= CMD_CFG_DATA_WR;
>>>    			WARN_ON(xfer_bytes > host->bounce_buf_size);
>>> -			sg_copy_to_buffer(data->sg, data->sg_len,
>>> -					  host->bounce_buf, xfer_bytes);
>>> +			meson_mmc_copy_buffer(host, data, xfer_bytes, true);
>>>    			dma_wmb();
>>>    		}
>>>    
>>> @@ -955,8 +996,7 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
>>>    	if (meson_mmc_bounce_buf_read(data)) {
>>>    		xfer_bytes = data->blksz * data->blocks;
>>>    		WARN_ON(xfer_bytes > host->bounce_buf_size);
>>> -		sg_copy_from_buffer(data->sg, data->sg_len,
>>> -				    host->bounce_buf, xfer_bytes);
>>> +		meson_mmc_copy_buffer(host, data, xfer_bytes, false);
>>>    	}
>>>    
>>>    	next_cmd = meson_mmc_get_next_command(cmd);
>>
>> Best regards
>>
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC] mmc: meson-gx: use memcpy_to/fromio for dram-access-quirk
@ 2021-06-09 14:45         ` Robin Murphy
  0 siblings, 0 replies; 21+ messages in thread
From: Robin Murphy @ 2021-06-09 14:45 UTC (permalink / raw)
  To: Neil Armstrong, Marek Szyprowski, ulf.hansson
  Cc: khilman, jbrunet, martin.blumenstingl, linux-mmc, linux-amlogic,
	linux-arm-kernel, linux-kernel, Mark Rutland,
	Bartlomiej Zolnierkiewicz

On 2021-06-09 14:07, Neil Armstrong wrote:
> Hi,
> 
> On 08/06/2021 17:50, Marek Szyprowski wrote:
>> Hi
>>
>> On 08.06.2021 17:33, Neil Armstrong wrote:
>>> It has been reported that usage of memcpy() to/from an iomem mapping is invalid,
>>> and and recent arm64 memcpy update [1] triggers a memory abort when dram-access-quirk
>>> is used on the G12A/G12B platforms.
>>>
>>> This adds a local sg_copy_to_buffer which makes usage of io versions of memcpy
>>> when dram-access-quirk is enabled.
>>>
>>> Fixes: acdc8e71d9bb ("mmc: meson-gx: add dram-access-quirk")
>>> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>> Suggested-by: Mark Rutland <mark.rutland@arm.com>
>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>>
>>> [1] 285133040e6c ("arm64: Import latest memcpy()/memmove() implementation")
>>>
>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>> ---
>>> Hi Ulf, Marek, Mark,
>>>
>>> I haven't tested the patch yet, but should fix issue reported at [2].
>>
>> Works fine here and fixed the issue.
>>
>> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> 
> Thanks, I'll need to rework to pass an __iomem pointer to memcpy_to/fromio so sparse doesn't scream anymore.

Hmm, might it be worth factoring out general sg_copy_{to,from}_iomem() 
helpers? From a quick grep I found at least mv_cesa_sg_copy() already 
doing essentially the same thing as meson_mmc_copy_buffer().

Robin.

> 
> Neil
> 
>>
>>> Neil
>>>
>>> [2] https://lore.kernel.org/r/acb244ad-0759-5a96-c659-5c23003d3dcd@samsung.com
>>>
>>>    drivers/mmc/host/meson-gx-mmc.c | 48 ++++++++++++++++++++++++++++++---
>>>    1 file changed, 44 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
>>> index b8b771b643cc..89ff6038092d 100644
>>> --- a/drivers/mmc/host/meson-gx-mmc.c
>>> +++ b/drivers/mmc/host/meson-gx-mmc.c
>>> @@ -742,6 +742,48 @@ static void meson_mmc_desc_chain_transfer(struct mmc_host *mmc, u32 cmd_cfg)
>>>    	writel(start, host->regs + SD_EMMC_START);
>>>    }
>>>    
>>> +/* local sg copy to buffer version with _to/fromio usage for dram_access_quirk */
>>> +static void meson_mmc_copy_buffer(struct meson_host *host, struct mmc_data *data,
>>> +				  size_t buflen, bool to_buffer)
>>> +{
>>> +	unsigned int sg_flags = SG_MITER_ATOMIC;
>>> +	struct scatterlist *sgl = data->sg;
>>> +	unsigned int nents = data->sg_len;
>>> +	struct sg_mapping_iter miter;
>>> +	void *buf = host->bounce_buf;
>>> +	unsigned int offset = 0;
>>> +
>>> +	if (to_buffer)
>>> +		sg_flags |= SG_MITER_FROM_SG;
>>> +	else
>>> +		sg_flags |= SG_MITER_TO_SG;
>>> +
>>> +	sg_miter_start(&miter, sgl, nents, sg_flags);
>>> +
>>> +	while ((offset < buflen) && sg_miter_next(&miter)) {
>>> +		unsigned int len;
>>> +
>>> +		len = min(miter.length, buflen - offset);
>>> +
>>> +		/* When dram_access_quirk, the bounce buffer is a iomem mapping */
>>> +		if (host->dram_access_quirk) {
>>> +			if (to_buffer)
>>> +				memcpy_toio(buf + offset, miter.addr, len);
>>> +			else
>>> +				memcpy_fromio(miter.addr, buf + offset, len);
>>> +		} else {
>>> +			if (to_buffer)
>>> +				memcpy(buf + offset, miter.addr, len);
>>> +			else
>>> +				memcpy(miter.addr, buf + offset, len);
>>> +		}
>>> +
>>> +		offset += len;
>>> +	}
>>> +
>>> +	sg_miter_stop(&miter);
>>> +}
>>> +
>>>    static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>>    {
>>>    	struct meson_host *host = mmc_priv(mmc);
>>> @@ -785,8 +827,7 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>>    		if (data->flags & MMC_DATA_WRITE) {
>>>    			cmd_cfg |= CMD_CFG_DATA_WR;
>>>    			WARN_ON(xfer_bytes > host->bounce_buf_size);
>>> -			sg_copy_to_buffer(data->sg, data->sg_len,
>>> -					  host->bounce_buf, xfer_bytes);
>>> +			meson_mmc_copy_buffer(host, data, xfer_bytes, true);
>>>    			dma_wmb();
>>>    		}
>>>    
>>> @@ -955,8 +996,7 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
>>>    	if (meson_mmc_bounce_buf_read(data)) {
>>>    		xfer_bytes = data->blksz * data->blocks;
>>>    		WARN_ON(xfer_bytes > host->bounce_buf_size);
>>> -		sg_copy_from_buffer(data->sg, data->sg_len,
>>> -				    host->bounce_buf, xfer_bytes);
>>> +		meson_mmc_copy_buffer(host, data, xfer_bytes, false);
>>>    	}
>>>    
>>>    	next_cmd = meson_mmc_get_next_command(cmd);
>>
>> Best regards
>>
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC] mmc: meson-gx: use memcpy_to/fromio for dram-access-quirk
  2021-06-09 14:45         ` Robin Murphy
  (?)
@ 2021-06-09 14:55           ` Neil Armstrong
  -1 siblings, 0 replies; 21+ messages in thread
From: Neil Armstrong @ 2021-06-09 14:55 UTC (permalink / raw)
  To: Robin Murphy, Marek Szyprowski, ulf.hansson
  Cc: khilman, jbrunet, martin.blumenstingl, linux-mmc, linux-amlogic,
	linux-arm-kernel, linux-kernel, Mark Rutland,
	Bartlomiej Zolnierkiewicz

Hi Robin,

On 09/06/2021 16:45, Robin Murphy wrote:
> On 2021-06-09 14:07, Neil Armstrong wrote:
>> Hi,
>>
>> On 08/06/2021 17:50, Marek Szyprowski wrote:
>>> Hi
>>>
>>> On 08.06.2021 17:33, Neil Armstrong wrote:
>>>> It has been reported that usage of memcpy() to/from an iomem mapping is invalid,
>>>> and and recent arm64 memcpy update [1] triggers a memory abort when dram-access-quirk
>>>> is used on the G12A/G12B platforms.
>>>>
>>>> This adds a local sg_copy_to_buffer which makes usage of io versions of memcpy
>>>> when dram-access-quirk is enabled.
>>>>
>>>> Fixes: acdc8e71d9bb ("mmc: meson-gx: add dram-access-quirk")
>>>> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>>> Suggested-by: Mark Rutland <mark.rutland@arm.com>
>>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>>>
>>>> [1] 285133040e6c ("arm64: Import latest memcpy()/memmove() implementation")
>>>>
>>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>>> ---
>>>> Hi Ulf, Marek, Mark,
>>>>
>>>> I haven't tested the patch yet, but should fix issue reported at [2].
>>>
>>> Works fine here and fixed the issue.
>>>
>>> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>
>> Thanks, I'll need to rework to pass an __iomem pointer to memcpy_to/fromio so sparse doesn't scream anymore.
> 
> Hmm, might it be worth factoring out general sg_copy_{to,from}_iomem() helpers? From a quick grep I found at least mv_cesa_sg_copy() already doing essentially the same thing as meson_mmc_copy_buffer().

It's definitely worth it, but since we need a quick fix, we should have meson_mmc_copy_buffer() as a fix then we should definitely move to sg_copy_{to,from}_iomem() helpers

Neil

> 
> Robin.
> 
>>
>> Neil
>>
>>>
>>>> Neil
>>>>
>>>> [2] https://lore.kernel.org/r/acb244ad-0759-5a96-c659-5c23003d3dcd@samsung.com
>>>>
>>>>    drivers/mmc/host/meson-gx-mmc.c | 48 ++++++++++++++++++++++++++++++---
>>>>    1 file changed, 44 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
>>>> index b8b771b643cc..89ff6038092d 100644
>>>> --- a/drivers/mmc/host/meson-gx-mmc.c
>>>> +++ b/drivers/mmc/host/meson-gx-mmc.c
>>>> @@ -742,6 +742,48 @@ static void meson_mmc_desc_chain_transfer(struct mmc_host *mmc, u32 cmd_cfg)
>>>>        writel(start, host->regs + SD_EMMC_START);
>>>>    }
>>>>    +/* local sg copy to buffer version with _to/fromio usage for dram_access_quirk */
>>>> +static void meson_mmc_copy_buffer(struct meson_host *host, struct mmc_data *data,
>>>> +                  size_t buflen, bool to_buffer)
>>>> +{
>>>> +    unsigned int sg_flags = SG_MITER_ATOMIC;
>>>> +    struct scatterlist *sgl = data->sg;
>>>> +    unsigned int nents = data->sg_len;
>>>> +    struct sg_mapping_iter miter;
>>>> +    void *buf = host->bounce_buf;
>>>> +    unsigned int offset = 0;
>>>> +
>>>> +    if (to_buffer)
>>>> +        sg_flags |= SG_MITER_FROM_SG;
>>>> +    else
>>>> +        sg_flags |= SG_MITER_TO_SG;
>>>> +
>>>> +    sg_miter_start(&miter, sgl, nents, sg_flags);
>>>> +
>>>> +    while ((offset < buflen) && sg_miter_next(&miter)) {
>>>> +        unsigned int len;
>>>> +
>>>> +        len = min(miter.length, buflen - offset);
>>>> +
>>>> +        /* When dram_access_quirk, the bounce buffer is a iomem mapping */
>>>> +        if (host->dram_access_quirk) {
>>>> +            if (to_buffer)
>>>> +                memcpy_toio(buf + offset, miter.addr, len);
>>>> +            else
>>>> +                memcpy_fromio(miter.addr, buf + offset, len);
>>>> +        } else {
>>>> +            if (to_buffer)
>>>> +                memcpy(buf + offset, miter.addr, len);
>>>> +            else
>>>> +                memcpy(miter.addr, buf + offset, len);
>>>> +        }
>>>> +
>>>> +        offset += len;
>>>> +    }
>>>> +
>>>> +    sg_miter_stop(&miter);
>>>> +}
>>>> +
>>>>    static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>>>    {
>>>>        struct meson_host *host = mmc_priv(mmc);
>>>> @@ -785,8 +827,7 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>>>            if (data->flags & MMC_DATA_WRITE) {
>>>>                cmd_cfg |= CMD_CFG_DATA_WR;
>>>>                WARN_ON(xfer_bytes > host->bounce_buf_size);
>>>> -            sg_copy_to_buffer(data->sg, data->sg_len,
>>>> -                      host->bounce_buf, xfer_bytes);
>>>> +            meson_mmc_copy_buffer(host, data, xfer_bytes, true);
>>>>                dma_wmb();
>>>>            }
>>>>    @@ -955,8 +996,7 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
>>>>        if (meson_mmc_bounce_buf_read(data)) {
>>>>            xfer_bytes = data->blksz * data->blocks;
>>>>            WARN_ON(xfer_bytes > host->bounce_buf_size);
>>>> -        sg_copy_from_buffer(data->sg, data->sg_len,
>>>> -                    host->bounce_buf, xfer_bytes);
>>>> +        meson_mmc_copy_buffer(host, data, xfer_bytes, false);
>>>>        }
>>>>           next_cmd = meson_mmc_get_next_command(cmd);
>>>
>>> Best regards
>>>
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC] mmc: meson-gx: use memcpy_to/fromio for dram-access-quirk
@ 2021-06-09 14:55           ` Neil Armstrong
  0 siblings, 0 replies; 21+ messages in thread
From: Neil Armstrong @ 2021-06-09 14:55 UTC (permalink / raw)
  To: Robin Murphy, Marek Szyprowski, ulf.hansson
  Cc: khilman, jbrunet, martin.blumenstingl, linux-mmc, linux-amlogic,
	linux-arm-kernel, linux-kernel, Mark Rutland,
	Bartlomiej Zolnierkiewicz

Hi Robin,

On 09/06/2021 16:45, Robin Murphy wrote:
> On 2021-06-09 14:07, Neil Armstrong wrote:
>> Hi,
>>
>> On 08/06/2021 17:50, Marek Szyprowski wrote:
>>> Hi
>>>
>>> On 08.06.2021 17:33, Neil Armstrong wrote:
>>>> It has been reported that usage of memcpy() to/from an iomem mapping is invalid,
>>>> and and recent arm64 memcpy update [1] triggers a memory abort when dram-access-quirk
>>>> is used on the G12A/G12B platforms.
>>>>
>>>> This adds a local sg_copy_to_buffer which makes usage of io versions of memcpy
>>>> when dram-access-quirk is enabled.
>>>>
>>>> Fixes: acdc8e71d9bb ("mmc: meson-gx: add dram-access-quirk")
>>>> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>>> Suggested-by: Mark Rutland <mark.rutland@arm.com>
>>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>>>
>>>> [1] 285133040e6c ("arm64: Import latest memcpy()/memmove() implementation")
>>>>
>>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>>> ---
>>>> Hi Ulf, Marek, Mark,
>>>>
>>>> I haven't tested the patch yet, but should fix issue reported at [2].
>>>
>>> Works fine here and fixed the issue.
>>>
>>> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>
>> Thanks, I'll need to rework to pass an __iomem pointer to memcpy_to/fromio so sparse doesn't scream anymore.
> 
> Hmm, might it be worth factoring out general sg_copy_{to,from}_iomem() helpers? From a quick grep I found at least mv_cesa_sg_copy() already doing essentially the same thing as meson_mmc_copy_buffer().

It's definitely worth it, but since we need a quick fix, we should have meson_mmc_copy_buffer() as a fix then we should definitely move to sg_copy_{to,from}_iomem() helpers

Neil

> 
> Robin.
> 
>>
>> Neil
>>
>>>
>>>> Neil
>>>>
>>>> [2] https://lore.kernel.org/r/acb244ad-0759-5a96-c659-5c23003d3dcd@samsung.com
>>>>
>>>>    drivers/mmc/host/meson-gx-mmc.c | 48 ++++++++++++++++++++++++++++++---
>>>>    1 file changed, 44 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
>>>> index b8b771b643cc..89ff6038092d 100644
>>>> --- a/drivers/mmc/host/meson-gx-mmc.c
>>>> +++ b/drivers/mmc/host/meson-gx-mmc.c
>>>> @@ -742,6 +742,48 @@ static void meson_mmc_desc_chain_transfer(struct mmc_host *mmc, u32 cmd_cfg)
>>>>        writel(start, host->regs + SD_EMMC_START);
>>>>    }
>>>>    +/* local sg copy to buffer version with _to/fromio usage for dram_access_quirk */
>>>> +static void meson_mmc_copy_buffer(struct meson_host *host, struct mmc_data *data,
>>>> +                  size_t buflen, bool to_buffer)
>>>> +{
>>>> +    unsigned int sg_flags = SG_MITER_ATOMIC;
>>>> +    struct scatterlist *sgl = data->sg;
>>>> +    unsigned int nents = data->sg_len;
>>>> +    struct sg_mapping_iter miter;
>>>> +    void *buf = host->bounce_buf;
>>>> +    unsigned int offset = 0;
>>>> +
>>>> +    if (to_buffer)
>>>> +        sg_flags |= SG_MITER_FROM_SG;
>>>> +    else
>>>> +        sg_flags |= SG_MITER_TO_SG;
>>>> +
>>>> +    sg_miter_start(&miter, sgl, nents, sg_flags);
>>>> +
>>>> +    while ((offset < buflen) && sg_miter_next(&miter)) {
>>>> +        unsigned int len;
>>>> +
>>>> +        len = min(miter.length, buflen - offset);
>>>> +
>>>> +        /* When dram_access_quirk, the bounce buffer is a iomem mapping */
>>>> +        if (host->dram_access_quirk) {
>>>> +            if (to_buffer)
>>>> +                memcpy_toio(buf + offset, miter.addr, len);
>>>> +            else
>>>> +                memcpy_fromio(miter.addr, buf + offset, len);
>>>> +        } else {
>>>> +            if (to_buffer)
>>>> +                memcpy(buf + offset, miter.addr, len);
>>>> +            else
>>>> +                memcpy(miter.addr, buf + offset, len);
>>>> +        }
>>>> +
>>>> +        offset += len;
>>>> +    }
>>>> +
>>>> +    sg_miter_stop(&miter);
>>>> +}
>>>> +
>>>>    static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>>>    {
>>>>        struct meson_host *host = mmc_priv(mmc);
>>>> @@ -785,8 +827,7 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>>>            if (data->flags & MMC_DATA_WRITE) {
>>>>                cmd_cfg |= CMD_CFG_DATA_WR;
>>>>                WARN_ON(xfer_bytes > host->bounce_buf_size);
>>>> -            sg_copy_to_buffer(data->sg, data->sg_len,
>>>> -                      host->bounce_buf, xfer_bytes);
>>>> +            meson_mmc_copy_buffer(host, data, xfer_bytes, true);
>>>>                dma_wmb();
>>>>            }
>>>>    @@ -955,8 +996,7 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
>>>>        if (meson_mmc_bounce_buf_read(data)) {
>>>>            xfer_bytes = data->blksz * data->blocks;
>>>>            WARN_ON(xfer_bytes > host->bounce_buf_size);
>>>> -        sg_copy_from_buffer(data->sg, data->sg_len,
>>>> -                    host->bounce_buf, xfer_bytes);
>>>> +        meson_mmc_copy_buffer(host, data, xfer_bytes, false);
>>>>        }
>>>>           next_cmd = meson_mmc_get_next_command(cmd);
>>>
>>> Best regards
>>>
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC] mmc: meson-gx: use memcpy_to/fromio for dram-access-quirk
@ 2021-06-09 14:55           ` Neil Armstrong
  0 siblings, 0 replies; 21+ messages in thread
From: Neil Armstrong @ 2021-06-09 14:55 UTC (permalink / raw)
  To: Robin Murphy, Marek Szyprowski, ulf.hansson
  Cc: khilman, jbrunet, martin.blumenstingl, linux-mmc, linux-amlogic,
	linux-arm-kernel, linux-kernel, Mark Rutland,
	Bartlomiej Zolnierkiewicz

Hi Robin,

On 09/06/2021 16:45, Robin Murphy wrote:
> On 2021-06-09 14:07, Neil Armstrong wrote:
>> Hi,
>>
>> On 08/06/2021 17:50, Marek Szyprowski wrote:
>>> Hi
>>>
>>> On 08.06.2021 17:33, Neil Armstrong wrote:
>>>> It has been reported that usage of memcpy() to/from an iomem mapping is invalid,
>>>> and and recent arm64 memcpy update [1] triggers a memory abort when dram-access-quirk
>>>> is used on the G12A/G12B platforms.
>>>>
>>>> This adds a local sg_copy_to_buffer which makes usage of io versions of memcpy
>>>> when dram-access-quirk is enabled.
>>>>
>>>> Fixes: acdc8e71d9bb ("mmc: meson-gx: add dram-access-quirk")
>>>> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>>> Suggested-by: Mark Rutland <mark.rutland@arm.com>
>>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>>>
>>>> [1] 285133040e6c ("arm64: Import latest memcpy()/memmove() implementation")
>>>>
>>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>>> ---
>>>> Hi Ulf, Marek, Mark,
>>>>
>>>> I haven't tested the patch yet, but should fix issue reported at [2].
>>>
>>> Works fine here and fixed the issue.
>>>
>>> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>
>> Thanks, I'll need to rework to pass an __iomem pointer to memcpy_to/fromio so sparse doesn't scream anymore.
> 
> Hmm, might it be worth factoring out general sg_copy_{to,from}_iomem() helpers? From a quick grep I found at least mv_cesa_sg_copy() already doing essentially the same thing as meson_mmc_copy_buffer().

It's definitely worth it, but since we need a quick fix, we should have meson_mmc_copy_buffer() as a fix then we should definitely move to sg_copy_{to,from}_iomem() helpers

Neil

> 
> Robin.
> 
>>
>> Neil
>>
>>>
>>>> Neil
>>>>
>>>> [2] https://lore.kernel.org/r/acb244ad-0759-5a96-c659-5c23003d3dcd@samsung.com
>>>>
>>>>    drivers/mmc/host/meson-gx-mmc.c | 48 ++++++++++++++++++++++++++++++---
>>>>    1 file changed, 44 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
>>>> index b8b771b643cc..89ff6038092d 100644
>>>> --- a/drivers/mmc/host/meson-gx-mmc.c
>>>> +++ b/drivers/mmc/host/meson-gx-mmc.c
>>>> @@ -742,6 +742,48 @@ static void meson_mmc_desc_chain_transfer(struct mmc_host *mmc, u32 cmd_cfg)
>>>>        writel(start, host->regs + SD_EMMC_START);
>>>>    }
>>>>    +/* local sg copy to buffer version with _to/fromio usage for dram_access_quirk */
>>>> +static void meson_mmc_copy_buffer(struct meson_host *host, struct mmc_data *data,
>>>> +                  size_t buflen, bool to_buffer)
>>>> +{
>>>> +    unsigned int sg_flags = SG_MITER_ATOMIC;
>>>> +    struct scatterlist *sgl = data->sg;
>>>> +    unsigned int nents = data->sg_len;
>>>> +    struct sg_mapping_iter miter;
>>>> +    void *buf = host->bounce_buf;
>>>> +    unsigned int offset = 0;
>>>> +
>>>> +    if (to_buffer)
>>>> +        sg_flags |= SG_MITER_FROM_SG;
>>>> +    else
>>>> +        sg_flags |= SG_MITER_TO_SG;
>>>> +
>>>> +    sg_miter_start(&miter, sgl, nents, sg_flags);
>>>> +
>>>> +    while ((offset < buflen) && sg_miter_next(&miter)) {
>>>> +        unsigned int len;
>>>> +
>>>> +        len = min(miter.length, buflen - offset);
>>>> +
>>>> +        /* When dram_access_quirk, the bounce buffer is a iomem mapping */
>>>> +        if (host->dram_access_quirk) {
>>>> +            if (to_buffer)
>>>> +                memcpy_toio(buf + offset, miter.addr, len);
>>>> +            else
>>>> +                memcpy_fromio(miter.addr, buf + offset, len);
>>>> +        } else {
>>>> +            if (to_buffer)
>>>> +                memcpy(buf + offset, miter.addr, len);
>>>> +            else
>>>> +                memcpy(miter.addr, buf + offset, len);
>>>> +        }
>>>> +
>>>> +        offset += len;
>>>> +    }
>>>> +
>>>> +    sg_miter_stop(&miter);
>>>> +}
>>>> +
>>>>    static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>>>    {
>>>>        struct meson_host *host = mmc_priv(mmc);
>>>> @@ -785,8 +827,7 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>>>>            if (data->flags & MMC_DATA_WRITE) {
>>>>                cmd_cfg |= CMD_CFG_DATA_WR;
>>>>                WARN_ON(xfer_bytes > host->bounce_buf_size);
>>>> -            sg_copy_to_buffer(data->sg, data->sg_len,
>>>> -                      host->bounce_buf, xfer_bytes);
>>>> +            meson_mmc_copy_buffer(host, data, xfer_bytes, true);
>>>>                dma_wmb();
>>>>            }
>>>>    @@ -955,8 +996,7 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
>>>>        if (meson_mmc_bounce_buf_read(data)) {
>>>>            xfer_bytes = data->blksz * data->blocks;
>>>>            WARN_ON(xfer_bytes > host->bounce_buf_size);
>>>> -        sg_copy_from_buffer(data->sg, data->sg_len,
>>>> -                    host->bounce_buf, xfer_bytes);
>>>> +        meson_mmc_copy_buffer(host, data, xfer_bytes, false);
>>>>        }
>>>>           next_cmd = meson_mmc_get_next_command(cmd);
>>>
>>> Best regards
>>>
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC] mmc: meson-gx: use memcpy_to/fromio for dram-access-quirk
  2021-06-09 14:55           ` Neil Armstrong
  (?)
@ 2021-06-09 15:01             ` Robin Murphy
  -1 siblings, 0 replies; 21+ messages in thread
From: Robin Murphy @ 2021-06-09 15:01 UTC (permalink / raw)
  To: Neil Armstrong, Marek Szyprowski, ulf.hansson
  Cc: khilman, jbrunet, martin.blumenstingl, linux-mmc, linux-amlogic,
	linux-arm-kernel, linux-kernel, Mark Rutland,
	Bartlomiej Zolnierkiewicz

On 2021-06-09 15:55, Neil Armstrong wrote:
> Hi Robin,
> 
> On 09/06/2021 16:45, Robin Murphy wrote:
>> On 2021-06-09 14:07, Neil Armstrong wrote:
>>> Hi,
>>>
>>> On 08/06/2021 17:50, Marek Szyprowski wrote:
>>>> Hi
>>>>
>>>> On 08.06.2021 17:33, Neil Armstrong wrote:
>>>>> It has been reported that usage of memcpy() to/from an iomem mapping is invalid,
>>>>> and and recent arm64 memcpy update [1] triggers a memory abort when dram-access-quirk
>>>>> is used on the G12A/G12B platforms.
>>>>>
>>>>> This adds a local sg_copy_to_buffer which makes usage of io versions of memcpy
>>>>> when dram-access-quirk is enabled.
>>>>>
>>>>> Fixes: acdc8e71d9bb ("mmc: meson-gx: add dram-access-quirk")
>>>>> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>>>> Suggested-by: Mark Rutland <mark.rutland@arm.com>
>>>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>>>>
>>>>> [1] 285133040e6c ("arm64: Import latest memcpy()/memmove() implementation")
>>>>>
>>>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>>>> ---
>>>>> Hi Ulf, Marek, Mark,
>>>>>
>>>>> I haven't tested the patch yet, but should fix issue reported at [2].
>>>>
>>>> Works fine here and fixed the issue.
>>>>
>>>> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>>
>>> Thanks, I'll need to rework to pass an __iomem pointer to memcpy_to/fromio so sparse doesn't scream anymore.
>>
>> Hmm, might it be worth factoring out general sg_copy_{to,from}_iomem() helpers? From a quick grep I found at least mv_cesa_sg_copy() already doing essentially the same thing as meson_mmc_copy_buffer().
> 
> It's definitely worth it, but since we need a quick fix, we should have meson_mmc_copy_buffer() as a fix then we should definitely move to sg_copy_{to,from}_iomem() helpers

Oh, that makes sense for sure - I was just doing some general thinking 
out loud before I forget about the whole thing :)

Cheers,
Robin.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC] mmc: meson-gx: use memcpy_to/fromio for dram-access-quirk
@ 2021-06-09 15:01             ` Robin Murphy
  0 siblings, 0 replies; 21+ messages in thread
From: Robin Murphy @ 2021-06-09 15:01 UTC (permalink / raw)
  To: Neil Armstrong, Marek Szyprowski, ulf.hansson
  Cc: khilman, jbrunet, martin.blumenstingl, linux-mmc, linux-amlogic,
	linux-arm-kernel, linux-kernel, Mark Rutland,
	Bartlomiej Zolnierkiewicz

On 2021-06-09 15:55, Neil Armstrong wrote:
> Hi Robin,
> 
> On 09/06/2021 16:45, Robin Murphy wrote:
>> On 2021-06-09 14:07, Neil Armstrong wrote:
>>> Hi,
>>>
>>> On 08/06/2021 17:50, Marek Szyprowski wrote:
>>>> Hi
>>>>
>>>> On 08.06.2021 17:33, Neil Armstrong wrote:
>>>>> It has been reported that usage of memcpy() to/from an iomem mapping is invalid,
>>>>> and and recent arm64 memcpy update [1] triggers a memory abort when dram-access-quirk
>>>>> is used on the G12A/G12B platforms.
>>>>>
>>>>> This adds a local sg_copy_to_buffer which makes usage of io versions of memcpy
>>>>> when dram-access-quirk is enabled.
>>>>>
>>>>> Fixes: acdc8e71d9bb ("mmc: meson-gx: add dram-access-quirk")
>>>>> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>>>> Suggested-by: Mark Rutland <mark.rutland@arm.com>
>>>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>>>>
>>>>> [1] 285133040e6c ("arm64: Import latest memcpy()/memmove() implementation")
>>>>>
>>>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>>>> ---
>>>>> Hi Ulf, Marek, Mark,
>>>>>
>>>>> I haven't tested the patch yet, but should fix issue reported at [2].
>>>>
>>>> Works fine here and fixed the issue.
>>>>
>>>> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>>
>>> Thanks, I'll need to rework to pass an __iomem pointer to memcpy_to/fromio so sparse doesn't scream anymore.
>>
>> Hmm, might it be worth factoring out general sg_copy_{to,from}_iomem() helpers? From a quick grep I found at least mv_cesa_sg_copy() already doing essentially the same thing as meson_mmc_copy_buffer().
> 
> It's definitely worth it, but since we need a quick fix, we should have meson_mmc_copy_buffer() as a fix then we should definitely move to sg_copy_{to,from}_iomem() helpers

Oh, that makes sense for sure - I was just doing some general thinking 
out loud before I forget about the whole thing :)

Cheers,
Robin.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC] mmc: meson-gx: use memcpy_to/fromio for dram-access-quirk
@ 2021-06-09 15:01             ` Robin Murphy
  0 siblings, 0 replies; 21+ messages in thread
From: Robin Murphy @ 2021-06-09 15:01 UTC (permalink / raw)
  To: Neil Armstrong, Marek Szyprowski, ulf.hansson
  Cc: khilman, jbrunet, martin.blumenstingl, linux-mmc, linux-amlogic,
	linux-arm-kernel, linux-kernel, Mark Rutland,
	Bartlomiej Zolnierkiewicz

On 2021-06-09 15:55, Neil Armstrong wrote:
> Hi Robin,
> 
> On 09/06/2021 16:45, Robin Murphy wrote:
>> On 2021-06-09 14:07, Neil Armstrong wrote:
>>> Hi,
>>>
>>> On 08/06/2021 17:50, Marek Szyprowski wrote:
>>>> Hi
>>>>
>>>> On 08.06.2021 17:33, Neil Armstrong wrote:
>>>>> It has been reported that usage of memcpy() to/from an iomem mapping is invalid,
>>>>> and and recent arm64 memcpy update [1] triggers a memory abort when dram-access-quirk
>>>>> is used on the G12A/G12B platforms.
>>>>>
>>>>> This adds a local sg_copy_to_buffer which makes usage of io versions of memcpy
>>>>> when dram-access-quirk is enabled.
>>>>>
>>>>> Fixes: acdc8e71d9bb ("mmc: meson-gx: add dram-access-quirk")
>>>>> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>>>> Suggested-by: Mark Rutland <mark.rutland@arm.com>
>>>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>>>>
>>>>> [1] 285133040e6c ("arm64: Import latest memcpy()/memmove() implementation")
>>>>>
>>>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>>>> ---
>>>>> Hi Ulf, Marek, Mark,
>>>>>
>>>>> I haven't tested the patch yet, but should fix issue reported at [2].
>>>>
>>>> Works fine here and fixed the issue.
>>>>
>>>> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>>
>>> Thanks, I'll need to rework to pass an __iomem pointer to memcpy_to/fromio so sparse doesn't scream anymore.
>>
>> Hmm, might it be worth factoring out general sg_copy_{to,from}_iomem() helpers? From a quick grep I found at least mv_cesa_sg_copy() already doing essentially the same thing as meson_mmc_copy_buffer().
> 
> It's definitely worth it, but since we need a quick fix, we should have meson_mmc_copy_buffer() as a fix then we should definitely move to sg_copy_{to,from}_iomem() helpers

Oh, that makes sense for sure - I was just doing some general thinking 
out loud before I forget about the whole thing :)

Cheers,
Robin.

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2021-06-09 16:20 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20210608153357eucas1p24cbc7a2ddb00beada8cdd51ae2337c53@eucas1p2.samsung.com>
2021-06-08 15:33 ` [RFC] mmc: meson-gx: use memcpy_to/fromio for dram-access-quirk Neil Armstrong
2021-06-08 15:33   ` Neil Armstrong
2021-06-08 15:33   ` Neil Armstrong
2021-06-08 15:50   ` Marek Szyprowski
2021-06-08 15:50     ` Marek Szyprowski
2021-06-08 15:50     ` Marek Szyprowski
2021-06-09 13:07     ` Neil Armstrong
2021-06-09 13:07       ` Neil Armstrong
2021-06-09 13:07       ` Neil Armstrong
2021-06-09 14:45       ` Robin Murphy
2021-06-09 14:45         ` Robin Murphy
2021-06-09 14:45         ` Robin Murphy
2021-06-09 14:55         ` Neil Armstrong
2021-06-09 14:55           ` Neil Armstrong
2021-06-09 14:55           ` Neil Armstrong
2021-06-09 15:01           ` Robin Murphy
2021-06-09 15:01             ` Robin Murphy
2021-06-09 15:01             ` Robin Murphy
2021-06-08 19:35   ` kernel test robot
2021-06-08 20:49   ` kernel test robot
2021-06-09  4:35   ` kernel test robot

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.