[RFC] mmc: meson-gx: use memcpy_to/fromio for dram-access-quirk

[RFC] mmc: meson-gx: use memcpy_to/fromio for dram-access-quirk

[Neil Armstrong]

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

Re: [RFC] mmc: meson-gx: use memcpy_to/fromio for dram-access-quirk

[Marek Szyprowski]

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

Re: [RFC] mmc: meson-gx: use memcpy_to/fromio for dram-access-quirk

[Neil Armstrong]

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
> 

Re: [RFC] mmc: meson-gx: use memcpy_to/fromio for dram-access-quirk

[Robin Murphy]

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
> 

Re: [RFC] mmc: meson-gx: use memcpy_to/fromio for dram-access-quirk

[Neil Armstrong]

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
>>

Re: [RFC] mmc: meson-gx: use memcpy_to/fromio for dram-access-quirk

[Robin Murphy]

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.