mips: cache: Bulletproof the code against cornercases

Some architectures, like MIPS, require remapping of the registers. Add the map_physmem() call to handle it.

Signed-off-by Marek Vasut <[email protected]>

Cc Daniel Schwierzeck <[email protected]>

Cc Hans de Goede <[email protected]>

Cc Masahiro Yamada <[email protected]>

Cc Alexey Brodkin <[email protected]>

---

Hi Marek,

Quoted 9 lines

Some architectures, like MIPS, require remapping of the registers. Add the map_physmem() call to handle it.

Signed-off-by Marek Vasut <[email protected]>

Cc Daniel Schwierzeck <[email protected]>

Cc Hans de Goede <[email protected]>

Cc Masahiro Yamada <[email protected]>

Cc Alexey Brodkin <[email protected]>

---

Probably I'm missing something but I cannot apply it
on top of current master:
------------------------->8----------------------
Applying: mips: cache: Bulletproof the code against cornercases
Applying: usb: ehci: Use map_physmem in ehci-generic
error: patch failed: drivers/usb/host/ehci-generic.c:5
error: drivers/usb/host/ehci-generic.c: patch does not apply Patch failed at 0002 usb: ehci: Use map_physmem in ehci-generic The copy of the patch that failed is found in:
/home/abrodkin/Projects/sources/git/u-boot/.git/rebase-apply/patch
When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort" ------------------------->8----------------------

Note I'm applying entire series but not that only patch.

Certain processor architectures, like MIPS, require that the USB structures and transfer buffers are passed with their PA to the USB controller. If VA is passed, the USB will not work. Add the necessary virt_to_phys() calls into the USB EHCI code to make it work.

(quoted diff) +2 -0

+make O=/home/swarren/shared/git_wa/tegra-uboot-flasher/u-boot/build-p2371-2180 -s p2371-2180_defconfig
+make O=/home/swarren/shared/git_wa/tegra-uboot-flasher/u-boot/build-p2371-2180 -s -j8

In file included from ../arch/arm/include/asm/byteorder.h:29:0,

                 from ../include/compiler.h:125,
                 from ../include/image.h:19,
                 from ../include/common.h:88,
                 from ../drivers/usb/host/ehci-hcd.c:10:
../drivers/usb/host/ehci-hcd.c: In function ?ehci_td_buffer?:
../drivers/usb/host/ehci-hcd.c:248:49: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
   td->qt_buffer[idx] = cpu_to_hc32(virt_to_phys((void *)addr));
                                                 ^
../include/linux/byteorder/little_endian.h:34:51: note: in definition of macro ?__cpu_to_le32?
 #define __cpu_to_le32(x) ((__force __le32)(__u32)(x))
                                                   ^
../drivers/usb/host/ehci-hcd.c:248:24: note: in expansion of macro ?cpu_to_hc32?
   td->qt_buffer[idx] = cpu_to_hc32(virt_to_phys((void *)addr));
                        ^

Quoted 5 lines · Marek Vasut

Certain processor architectures, like MIPS, require that the USB structures and transfer buffers are passed with their PA to the USB controller. If VA is passed, the USB will not work. Add the necessary virt_to_phys() calls into the USB EHCI code to make it work.

FYI, this causes some cast size warnings, e.g.:

Quoted 18 lines

+make O=/home/swarren/shared/git_wa/tegra-uboot-flasher/u-boot/build-p2371-2180

(quoted diff) +1 -1

-s p2371-2180_defconfig
+make

O=/home/swarren/shared/git_wa/tegra-uboot-flasher/u-boot/build-p2371-2180 -s -j8
In file included from ../arch/arm/include/asm/byteorder.h:29:0,

                 from ../include/compiler.h:125,
                 from ../include/image.h:19,
                 from ../include/common.h:88,
                 from ../drivers/usb/host/ehci-hcd.c:10:
../drivers/usb/host/ehci-hcd.c: In function ?ehci_td_buffer?:
../drivers/usb/host/ehci-hcd.c:248:49: warning: cast to pointer from
integer of different size [-Wint-to-pointer-cast]
   td->qt_buffer[idx] = cpu_to_hc32(virt_to_phys((void *)addr));
                                                 ^

../include/linux/byteorder/little_endian.h:34:51: note: in definition of macro ?__cpu_to_le32?

Quoted 6 lines

 #define __cpu_to_le32(x) ((__force __le32)(__u32)(x))
                                                   ^
../drivers/usb/host/ehci-hcd.c:248:24: note: in expansion of macro
?cpu_to_hc32?
   td->qt_buffer[idx] = cpu_to_hc32(virt_to_phys((void *)addr));
                        ^

(This is a 64-bit ARM platform, so I had CROSS_COMPILE=aarch64-linux-gnu-)

Quoted 27 lines · Stephen Warren

Quoted 5 lines · Marek Vasut

Certain processor architectures, like MIPS, require that the USB structures and transfer buffers are passed with their PA to the USB controller. If VA is passed, the USB will not work. Add the necessary virt_to_phys() calls into the USB EHCI code to make it work.

FYI, this causes some cast size warnings, e.g.:

Quoted 18 lines

+make O=/home/swarren/shared/git_wa/tegra-uboot-flasher/u-boot/build-p2371-2180

(quoted diff) +1 -1

-s p2371-2180_defconfig
+make

O=/home/swarren/shared/git_wa/tegra-uboot-flasher/u-boot/build-p2371-2180 -s -j8
In file included from ../arch/arm/include/asm/byteorder.h:29:0,

                  from ../include/compiler.h:125,
                  from ../include/image.h:19,
                  from ../include/common.h:88,
                  from ../drivers/usb/host/ehci-hcd.c:10:
../drivers/usb/host/ehci-hcd.c: In function ?ehci_td_buffer?:
../drivers/usb/host/ehci-hcd.c:248:49: warning: cast to pointer from
integer of different size [-Wint-to-pointer-cast]
    td->qt_buffer[idx] = cpu_to_hc32(virt_to_phys((void *)addr));
                                                  ^

../include/linux/byteorder/little_endian.h:34:51: note: in definition of macro ?__cpu_to_le32?

Quoted 8 lines

Quoted 6 lines

  #define __cpu_to_le32(x) ((__force __le32)(__u32)(x))
                                                    ^
../drivers/usb/host/ehci-hcd.c:248:24: note: in expansion of macro
?cpu_to_hc32?
    td->qt_buffer[idx] = cpu_to_hc32(virt_to_phys((void *)addr));
                         ^

(This is a 64-bit ARM platform, so I had CROSS_COMPILE=aarch64-linux-gnu-)

Tom reported this to me too, sorry :-(

Do you have an idea how to fix this too? I am now installing arm64 toolchain.

Do you know about some nice arm64 board with USB for testing?

Quoted 4 lines

Tom reported this to me too, sorry :-(

Do you have an idea how to fix this too? I am now installing arm64
toolchain.

I haven't looked at it yet. I've seen similar problems in the bast handled by casting between integer types before casting to pointers e.g. something like the following guess:

uint32_t pa; void *p = (void *)(uintptr_t)pa;

Quoted 1 line

Do you know about some nice arm64 board with USB for testing?

There's always the Jetson TX1; it is the p2371-2180 that I was building above:

http://www.nvidia.com/object/jetson-tx1-dev-kit.html

On 02/26/2016 07:16 PM, Stephen Warren wrote:

Hi!

[...]

Quoted 5 lines

Quoted 4 lines

Tom reported this to me too, sorry :-(

Do you have an idea how to fix this too? I am now installing arm64
toolchain.

Quoted 5 lines

I haven't looked at it yet. I've seen similar problems in the bast handled by casting between integer types before casting to pointers e.g. something like the following guess:

uint32_t pa; void *p = (void *)(uintptr_t)pa;

I just tried this, it should do the trick. Can you give it a spin ?

Quoted 15 lines · Marek Vasut

On 02/26/2016 07:16 PM, Stephen Warren wrote:

Hi!

[...]

Quoted 9 lines

Quoted 4 lines

Tom reported this to me too, sorry :-(

Do you have an idea how to fix this too? I am now installing arm64
toolchain.

I haven't looked at it yet. I've seen similar problems in the bast handled by casting between integer types before casting to pointers e.g. something like the following guess:

Quoted 3 lines

Quoted 1 line

uint32_t pa; void *p = (void *)(uintptr_t)pa;

I just tried this, it should do the trick. Can you give it a spin ?

That does the trick, thanks.

This patch makes sure that the flush/invalidate_dcache_range() functions can handle corner-case calls like this -- invalidate_dcache_range(0, 0, 0); This call is valid and is happily produced by USB EHCI code for example. The expected behavior of the cache function(s) in this case is that they will do no operation, since the size is zero.

The current implementation though has a problem where such invocation will result in a hard CPU hang. This is because under such conditions, where the start_addr = 0 and stop = 0, the addr = 0 and aend = 0xffffffe0 . The loop will then try to iterate over the entire address space, which in itself is wrong. But iterating over the entire address space might also hit some odd address which will cause bus hang. The later happens on the Atheros MIPS.

Signed-off-by Marek Vasut <[email protected]>

Cc Daniel Schwierzeck <[email protected]>

Cc Hans de Goede <[email protected]>

---
 arch/mips/lib/cache.c | 8 ++++++++
 1 file changed, 8 insertions(+)

arch/mips/lib/cache.c +0 -0

index bf8ff59..7482005 100644

arch/mips/lib/cache.c +4 -0

 	while (1) {
 		mips_cache(HIT_WRITEBACK_INV_D, addr);
 		if (addr == aend)

@@ -109,6 +113,10 @@ void invalidate_dcache_range(ulong start_addr, ulong stop)
 	const void *addr = (const void *)(start_addr & ~(lsize - 1));
 	const void *aend = (const void *)((stop - 1) & ~(lsize - 1));
 
+	/* aend will be miscalculated when size is zero, so we return here */
+	if (start_addr == stop)
+		return;
+
 	while (1) {
 		mips_cache(HIT_INVALIDATE_D, addr);
 		if (addr == aend)

Am 27.01.2016 um 03:13 schrieb Marek Vasut:

Quoted 31 lines

This patch makes sure that the flush/invalidate_dcache_range() functions can handle corner-case calls like this -- invalidate_dcache_range(0, 0, 0); This call is valid and is happily produced by USB EHCI code for example. The expected behavior of the cache function(s) in this case is that they will do no operation, since the size is zero.

The current implementation though has a problem where such invocation will result in a hard CPU hang. This is because under such conditions, where the start_addr = 0 and stop = 0, the addr = 0 and aend = 0xffffffe0 . The loop will then try to iterate over the entire address space, which in itself is wrong. But iterating over the entire address space might also hit some odd address which will cause bus hang. The later happens on the Atheros MIPS.

Signed-off-by Marek Vasut <[email protected]>

Cc Daniel Schwierzeck <[email protected]>

Cc Hans de Goede <[email protected]>

---

 arch/mips/lib/cache.c | 8 ++++++++
 1 file changed, 8 insertions(+)

arch/mips/lib/cache.c +0 -0

index bf8ff59..7482005 100644
@@ -95,6 +95,10 @@ void flush_dcache_range(ulong start_addr, ulong stop)
 
 	const void *addr = (const void *)(start_addr & ~(lsize - 1));
 	const void *aend = (const void *)((stop - 1) & ~(lsize - 1));
 

arch/mips/lib/cache.c void flush_dcache_range(ulong start_addr, ulong stop) · line 101

Quoted 4 lines

arch/mips/lib/cache.c +4 -0

+	/* aend will be miscalculated when size is zero, so we return here */
+	if (start_addr == stop)
+		return;
+

you could additionally move the initialization of addr and aend behind this check like it's done in flush_cache()? That would save some CPU cycles.

Quoted 16 lines

arch/mips/lib/cache.c +0 -0

 	while (1) {
 

arch/mips/lib/cache.c +4 -0

 		mips_cache(HIT_WRITEBACK_INV_D, addr);
 		if (addr == aend)

@@ -109,6 +113,10 @@ void invalidate_dcache_range(ulong start_addr, ulong
 stop)
 
 	const void *addr = (const void *)(start_addr & ~(lsize - 1));
 	const void *aend = (const void *)((stop - 1) & ~(lsize - 1));
 
+	/* aend will be miscalculated when size is zero, so we return here */
+	if (start_addr == stop)
+		return;
+
 

arch/mips/lib/cache.c void invalidate_dcache_range(ulong start_addr, ulong stop) · line 122

Quoted 4 lines

arch/mips/lib/cache.c +0 -0

 	while (1) {
 
 		mips_cache(HIT_INVALIDATE_D, addr);
 		if (addr == aend)

This patch makes sure that the flush/invalidate_dcache_range() functions can handle corner-case calls like this -- invalidate_dcache_range(0, 0, 0); This call is valid and is happily produced by USB EHCI code for example. The expected behavior of the cache function(s) in this case is that they will do no operation, since the size is zero.

The current implementation though has a problem where such invocation will result in a hard CPU hang. This is because under such conditions, where the start_addr = 0 and stop = 0, the addr = 0 and aend = 0xffffffe0 . The loop will then try to iterate over the entire address space, which in itself is wrong. But iterating over the entire address space might also hit some odd address which will cause bus hang. The later happens on the Atheros MIPS.

Signed-off-by Marek Vasut <[email protected]>

Cc Daniel Schwierzeck <[email protected]>

Cc Hans de Goede <[email protected]>

---
 arch/mips/lib/cache.c | 8 ++++++++
 1 file changed, 8 insertions(+)

2016-01-27 3:13 GMT+01:00 Marek Vasut <[email protected]>:

Quoted 19 lines

This patch makes sure that the flush/invalidate_dcache_range() functions can handle corner-case calls like this -- invalidate_dcache_range(0, 0, 0); This call is valid and is happily produced by USB EHCI code for example. The expected behavior of the cache function(s) in this case is that they will do no operation, since the size is zero.

The current implementation though has a problem where such invocation will result in a hard CPU hang. This is because under such conditions, where the start_addr = 0 and stop = 0, the addr = 0 and aend = 0xffffffe0 . The loop will then try to iterate over the entire address space, which in itself is wrong. But iterating over the entire address space might also hit some odd address which will cause bus hang. The later happens on the Atheros MIPS.

Signed-off-by Marek Vasut <[email protected]>

Cc Daniel Schwierzeck <[email protected]>

Cc Hans de Goede <[email protected]>

---

Quoted 2 lines

 arch/mips/lib/cache.c | 8 ++++++++
 1 file changed, 8 insertions(+)

applied to u-boot-mips, thanks

Quoted 20 lines

2016-01-27 3:13 GMT+01:00 Marek Vasut <[email protected]>:

Quoted 19 lines

This patch makes sure that the flush/invalidate_dcache_range() functions can handle corner-case calls like this -- invalidate_dcache_range(0, 0, 0); This call is valid and is happily produced by USB EHCI code for example. The expected behavior of the cache function(s) in this case is that they will do no operation, since the size is zero.

The current implementation though has a problem where such invocation will result in a hard CPU hang. This is because under such conditions, where the start_addr = 0 and stop = 0, the addr = 0 and aend = 0xffffffe0 . The loop will then try to iterate over the entire address space, which in itself is wrong. But iterating over the entire address space might also hit some odd address which will cause bus hang. The later happens on the Atheros MIPS.

Signed-off-by Marek Vasut <[email protected]>

Cc Daniel Schwierzeck <[email protected]>

Cc Hans de Goede <[email protected]>

---

Quoted 4 lines

Quoted 2 lines

 arch/mips/lib/cache.c | 8 ++++++++
 1 file changed, 8 insertions(+)

applied to u-boot-mips, thanks

Thanks! I'll pick the remaining four, ok ?

2016-02-01 22:31 GMT+01:00 Marek Vasut <[email protected]>:

Quoted 22 lines · Daniel Schwierzeck

Quoted 21 lines

2016-01-27 3:13 GMT+01:00 Marek Vasut <[email protected]>:

Quoted 20 lines

This patch makes sure that the flush/invalidate_dcache_range() functions can handle corner-case calls like this -- invalidate_dcache_range(0, 0, 0); This call is valid and is happily produced by USB EHCI code for example. The expected behavior of the cache function(s) in this case is that they will do no operation, since the size is zero.

The current implementation though has a problem where such invocation will result in a hard CPU hang. This is because under such conditions, where the start_addr = 0 and stop = 0, the addr = 0 and aend = 0xffffffe0 . The loop will then try to iterate over the entire address space, which in itself is wrong. But iterating over the entire address space might also hit some odd address which will cause bus hang. The later happens on the Atheros MIPS.

Signed-off-by Marek Vasut <[email protected]>

Cc Daniel Schwierzeck <[email protected]>

Cc Hans de Goede <[email protected]>

---

Quoted 6 lines

Quoted 4 lines

Quoted 2 lines

 arch/mips/lib/cache.c | 8 ++++++++
 1 file changed, 8 insertions(+)

applied to u-boot-mips, thanks

Thanks! I'll pick the remaining four, ok ?

fine with me. I don't want to break USB ;)