[PATCH 0/2] atmel-mci and maybe android-goldfish broken on 4.18

[PATCH 0/2] atmel-mci and maybe android-goldfish broken on 4.18

[Ludovic Desroches]

Hi,

Dougas Gilbert noticed that atmel-mci was broken with the 4.18 release.
He found the culprit commit which is 5b4277814e3fd
("[PATCH 2/7] mmc: atmel-mci: use sg_copy_{from,to}_buffer"). The from/to
logic was inverted: sg_copy_to_buffer copies from an SG list to a linear
buffer so it can't replace a memcpy where the destination is the virtual
address of an SG buffer.

This patch was part of "make more host drivers highmem safe v2". It seems
there is the same logical error within the android-goldfish driver but I
couldn't check it on real hardware.

Ludovic Desroches (2):
  mmc: atmel-mci: fix bad logic of sg_copy_{from,to}_buffer conversion
  mmc: android-goldfish: fix bad logic of sg_copy_{from,to}_buffer
    conversion

 drivers/mmc/host/android-goldfish.c |  4 ++--
 drivers/mmc/host/atmel-mci.c        | 12 ++++++------
 2 files changed, 8 insertions(+), 8 deletions(-)

-- 
2.12.2

[PATCH 1/2] mmc: atmel-mci: fix bad logic of sg_copy_{from,to}_buffer conversion

[Ludovic Desroches]

The conversion to sg_copy_{from,to}_buffer has been done in the wrong
way. sg_copy_to_buffer is a copy from an SG list to a linear buffer so
it can't replace memcpy(buf + offset, &value, remaining) where buf is
the virtual address of the SG. Same for sg_copy_to_buffer but in the
opposite way.

Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
Suggested-by: Douglas Gilbert <dgilbert@interlog.com>
Fixes: 5b4277814e3f ("mmc: atmel-mci: use sg_copy_{from,to}_buffer")
---
 drivers/mmc/host/atmel-mci.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
index 5aa2c9404e92..be53044086c7 100644
--- a/drivers/mmc/host/atmel-mci.c
+++ b/drivers/mmc/host/atmel-mci.c
@@ -1976,7 +1976,7 @@ static void atmci_read_data_pio(struct atmel_mci *host)
 	do {
 		value = atmci_readl(host, ATMCI_RDR);
 		if (likely(offset + 4 <= sg->length)) {
-			sg_pcopy_to_buffer(sg, 1, &value, sizeof(u32), offset);
+			sg_pcopy_from_buffer(sg, 1, &value, sizeof(u32), offset);
 
 			offset += 4;
 			nbytes += 4;
@@ -1993,7 +1993,7 @@ static void atmci_read_data_pio(struct atmel_mci *host)
 		} else {
 			unsigned int remaining = sg->length - offset;
 
-			sg_pcopy_to_buffer(sg, 1, &value, remaining, offset);
+			sg_pcopy_from_buffer(sg, 1, &value, remaining, offset);
 			nbytes += remaining;
 
 			flush_dcache_page(sg_page(sg));
@@ -2003,7 +2003,7 @@ static void atmci_read_data_pio(struct atmel_mci *host)
 				goto done;
 
 			offset = 4 - remaining;
-			sg_pcopy_to_buffer(sg, 1, (u8 *)&value + remaining,
+			sg_pcopy_from_buffer(sg, 1, (u8 *)&value + remaining,
 					offset, 0);
 			nbytes += offset;
 		}
@@ -2042,7 +2042,7 @@ static void atmci_write_data_pio(struct atmel_mci *host)
 
 	do {
 		if (likely(offset + 4 <= sg->length)) {
-			sg_pcopy_from_buffer(sg, 1, &value, sizeof(u32), offset);
+			sg_pcopy_to_buffer(sg, 1, &value, sizeof(u32), offset);
 			atmci_writel(host, ATMCI_TDR, value);
 
 			offset += 4;
@@ -2059,7 +2059,7 @@ static void atmci_write_data_pio(struct atmel_mci *host)
 			unsigned int remaining = sg->length - offset;
 
 			value = 0;
-			sg_pcopy_from_buffer(sg, 1, &value, remaining, offset);
+			sg_pcopy_to_buffer(sg, 1, &value, remaining, offset);
 			nbytes += remaining;
 
 			host->sg = sg = sg_next(sg);
@@ -2070,7 +2070,7 @@ static void atmci_write_data_pio(struct atmel_mci *host)
 			}
 
 			offset = 4 - remaining;
-			sg_pcopy_from_buffer(sg, 1, (u8 *)&value + remaining,
+			sg_pcopy_to_buffer(sg, 1, (u8 *)&value + remaining,
 					offset, 0);
 			atmci_writel(host, ATMCI_TDR, value);
 			nbytes += offset;
-- 
2.12.2

[PATCH 2/2] mmc: android-goldfish: fix bad logic of sg_copy_{from,to}_buffer conversion

[Ludovic Desroches]

The conversion to sg_copy_{from,to}_buffer has been done in the wrong
way. sg_copy_to_buffer is a copy from an SG list to a linear buffer so
it can't replace memcpy(dest, host->virt_base, data->sg->length) where
dest is the virtual address of the SG. Same for sg_copy_from_buffer
but in the opposite way.

Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
Suggested-by: Douglas Gilbert <dgilbert@interlog.com>
Fixes: 53d7e098ba08 ("mmc: android-goldfish: use sg_copy_{from,to}_buffer")
---
 drivers/mmc/host/android-goldfish.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/android-goldfish.c b/drivers/mmc/host/android-goldfish.c
index 294de177632c..61e4e2a213c9 100644
--- a/drivers/mmc/host/android-goldfish.c
+++ b/drivers/mmc/host/android-goldfish.c
@@ -217,7 +217,7 @@ static void goldfish_mmc_xfer_done(struct goldfish_mmc_host *host,
 			 * We don't really have DMA, so we need
 			 * to copy from our platform driver buffer
 			 */
-			sg_copy_to_buffer(data->sg, 1, host->virt_base,
+			sg_copy_from_buffer(data->sg, 1, host->virt_base,
 					data->sg->length);
 		}
 		host->data->bytes_xfered += data->sg->length;
@@ -393,7 +393,7 @@ static void goldfish_mmc_prepare_data(struct goldfish_mmc_host *host,
 		 * We don't really have DMA, so we need to copy to our
 		 * platform driver buffer
 		 */
-		sg_copy_from_buffer(data->sg, 1, host->virt_base,
+		sg_copy_to_buffer(data->sg, 1, host->virt_base,
 				data->sg->length);
 	}
 }
-- 
2.12.2

Re: [PATCH 0/2] atmel-mci and maybe android-goldfish broken on 4.18

[Ulf Hansson]

On 20 August 2018 at 10:54, Ludovic Desroches
<ludovic.desroches@microchip.com> wrote:
> Hi,
>
> Dougas Gilbert noticed that atmel-mci was broken with the 4.18 release.
> He found the culprit commit which is 5b4277814e3fd
> ("[PATCH 2/7] mmc: atmel-mci: use sg_copy_{from,to}_buffer"). The from/to
> logic was inverted: sg_copy_to_buffer copies from an SG list to a linear
> buffer so it can't replace a memcpy where the destination is the virtual
> address of an SG buffer.
>
> This patch was part of "make more host drivers highmem safe v2". It seems
> there is the same logical error within the android-goldfish driver but I
> couldn't check it on real hardware.
>
> Ludovic Desroches (2):
>   mmc: atmel-mci: fix bad logic of sg_copy_{from,to}_buffer conversion
>   mmc: android-goldfish: fix bad logic of sg_copy_{from,to}_buffer
>     conversion
>
>  drivers/mmc/host/android-goldfish.c |  4 ++--
>  drivers/mmc/host/atmel-mci.c        | 12 ++++++------
>  2 files changed, 8 insertions(+), 8 deletions(-)
>

Thanks, applied for fixes!

Kind regards
Uffe