[U-Boot] [PATCH 1/5] mips: cache: Bulletproof the code against cornercases
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 marex@denx.de Cc: Daniel Schwierzeck daniel.schwierzeck@gmail.com Cc: Hans de Goede hdegoede@redhat.com --- arch/mips/lib/cache.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/arch/mips/lib/cache.c b/arch/mips/lib/cache.c index bf8ff59..7482005 100644 --- a/arch/mips/lib/cache.c +++ b/arch/mips/lib/cache.c @@ -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));
+ /* aend will be miscalculated when size is zero, so we return here */ + if (start_addr == stop) + return; + 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)
Some architectures, like MIPS, require remapping of the registers. Add the map_physmem() call to handle it.
Signed-off-by: Marek Vasut marex@denx.de Cc: Daniel Schwierzeck daniel.schwierzeck@gmail.com Cc: Hans de Goede hdegoede@redhat.com Cc: Masahiro Yamada yamada.masahiro@socionext.com Cc: Alexey Brodkin abrodkin@synopsys.com --- drivers/usb/host/ehci-generic.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/ehci-generic.c b/drivers/usb/host/ehci-generic.c index 1292caa..6d58ef8 100644 --- a/drivers/usb/host/ehci-generic.c +++ b/drivers/usb/host/ehci-generic.c @@ -5,6 +5,7 @@ */
#include <common.h> +#include <asm/io.h> #include <dm.h> #include "ehci.h"
@@ -19,9 +20,10 @@ struct generic_ehci {
static int ehci_usb_probe(struct udevice *dev) { - struct ehci_hccr *hccr = (struct ehci_hccr *)dev_get_addr(dev); + struct ehci_hccr *hccr; struct ehci_hcor *hcor;
+ hccr = map_physmem(dev_get_addr(dev), 0x100, MAP_NOCACHE); hcor = (struct ehci_hcor *)((uintptr_t)hccr + HC_LENGTH(ehci_readl(&hccr->cr_capbase)));
Hi Marek,
On Wed, 2016-01-27 at 03:14 +0100, Marek Vasut wrote:
Some architectures, like MIPS, require remapping of the registers. Add the map_physmem() call to handle it.
Signed-off-by: Marek Vasut marex@denx.de Cc: Daniel Schwierzeck daniel.schwierzeck@gmail.com Cc: Hans de Goede hdegoede@redhat.com Cc: Masahiro Yamada yamada.masahiro@socionext.com Cc: Alexey Brodkin abrodkin@synopsys.com
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.
-Alexey
On Wednesday, January 27, 2016 at 03:21:17 PM, Alexey Brodkin wrote:
Hi Marek,
Hi,
On Wed, 2016-01-27 at 03:14 +0100, Marek Vasut wrote:
Some architectures, like MIPS, require remapping of the registers. Add the map_physmem() call to handle it.
Signed-off-by: Marek Vasut marex@denx.de Cc: Daniel Schwierzeck daniel.schwierzeck@gmail.com Cc: Hans de Goede hdegoede@redhat.com Cc: Masahiro Yamada yamada.masahiro@socionext.com Cc: Alexey Brodkin abrodkin@synopsys.com
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.
Probably because of 4feefdcfe916113ac6e1837ea22857f25fe1771c , that's easy to fix though.
Best regards, 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.
Signed-off-by: Marek Vasut marex@denx.de Cc: Daniel Schwierzeck daniel.schwierzeck@gmail.com Cc: Hans de Goede hdegoede@redhat.com --- drivers/usb/host/ehci-hcd.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index c664b16..b3eb08d 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -245,7 +245,7 @@ static int ehci_td_buffer(struct qTD *td, void *buf, size_t sz)
idx = 0; while (idx < QT_BUFFER_CNT) { - td->qt_buffer[idx] = cpu_to_hc32(addr); + td->qt_buffer[idx] = cpu_to_hc32(virt_to_phys((void *)addr)); td->qt_buffer_hi[idx] = 0; next = (addr + EHCI_PAGE_SIZE) & ~(EHCI_PAGE_SIZE - 1); delta = next - addr; @@ -398,7 +398,7 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer, * qh_overlay.qt_next ...... 13-10 H * - qh_overlay.qt_altnext */ - qh->qh_link = cpu_to_hc32((unsigned long)&ctrl->qh_list | QH_LINK_TYPE_QH); + qh->qh_link = cpu_to_hc32(virt_to_phys(&ctrl->qh_list) | QH_LINK_TYPE_QH); c = (dev->speed != USB_SPEED_HIGH) && !usb_pipeendpoint(pipe); maxpacket = usb_maxpacket(dev, pipe); endpt = QH_ENDPT1_RL(8) | QH_ENDPT1_C(c) | @@ -415,7 +415,6 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer, qh->qh_overlay.qt_altnext = cpu_to_hc32(QT_NEXT_TERMINATE);
tdp = &qh->qh_overlay.qt_next; - if (req != NULL) { /* * Setup request qTD (3.5 in ehci-r10.pdf) @@ -438,7 +437,7 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer, goto fail; } /* Update previous qTD! */ - *tdp = cpu_to_hc32((unsigned long)&qtd[qtd_counter]); + *tdp = cpu_to_hc32(virt_to_phys(&qtd[qtd_counter])); tdp = &qtd[qtd_counter++].qt_next; toggle = 1; } @@ -497,7 +496,7 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer, goto fail; } /* Update previous qTD! */ - *tdp = cpu_to_hc32((unsigned long)&qtd[qtd_counter]); + *tdp = cpu_to_hc32(virt_to_phys(&qtd[qtd_counter])); tdp = &qtd[qtd_counter++].qt_next; /* * Data toggle has to be adjusted since the qTD transfer @@ -528,11 +527,11 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer, QT_TOKEN_STATUS(QT_TOKEN_STATUS_ACTIVE); qtd[qtd_counter].qt_token = cpu_to_hc32(token); /* Update previous qTD! */ - *tdp = cpu_to_hc32((unsigned long)&qtd[qtd_counter]); + *tdp = cpu_to_hc32(virt_to_phys(&qtd[qtd_counter])); tdp = &qtd[qtd_counter++].qt_next; }
- ctrl->qh_list.qh_link = cpu_to_hc32((unsigned long)qh | QH_LINK_TYPE_QH); + ctrl->qh_list.qh_link = cpu_to_hc32(virt_to_phys(qh) | QH_LINK_TYPE_QH);
/* Flush dcache */ flush_dcache_range((unsigned long)&ctrl->qh_list, @@ -542,7 +541,7 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer, ALIGN_END_ADDR(struct qTD, qtd, qtd_count));
/* Set async. queue head pointer. */ - ehci_writel(&ctrl->hcor->or_asynclistaddr, (unsigned long)&ctrl->qh_list); + ehci_writel(&ctrl->hcor->or_asynclistaddr, virt_to_phys(&ctrl->qh_list));
usbsts = ehci_readl(&ctrl->hcor->or_usbsts); ehci_writel(&ctrl->hcor->or_usbsts, (usbsts & 0x3f)); @@ -989,7 +988,7 @@ static int ehci_common_init(struct ehci_ctrl *ctrl, uint tweaks)
/* Set head of reclaim list */ memset(qh_list, 0, sizeof(*qh_list)); - qh_list->qh_link = cpu_to_hc32((unsigned long)qh_list | QH_LINK_TYPE_QH); + qh_list->qh_link = cpu_to_hc32(virt_to_phys(qh_list) | QH_LINK_TYPE_QH); qh_list->qh_endpt1 = cpu_to_hc32(QH_ENDPT1_H(1) | QH_ENDPT1_EPS(USB_SPEED_HIGH)); qh_list->qh_overlay.qt_next = cpu_to_hc32(QT_NEXT_TERMINATE); @@ -1001,7 +1000,7 @@ static int ehci_common_init(struct ehci_ctrl *ctrl, uint tweaks) ALIGN_END_ADDR(struct QH, qh_list, 1));
/* Set async. queue head pointer. */ - ehci_writel(&ctrl->hcor->or_asynclistaddr, (unsigned long)qh_list); + ehci_writel(&ctrl->hcor->or_asynclistaddr, virt_to_phys(qh_list));
/* * Set up periodic list
On 01/26/2016 07:14 PM, Marek Vasut wrote:
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.:
+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)); ^
(This is a 64-bit ARM platform, so I had CROSS_COMPILE=aarch64-linux-gnu-)
On 02/26/2016 05:48 PM, Stephen Warren wrote:
On 01/26/2016 07:14 PM, Marek Vasut wrote:
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.:
+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)); ^
(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?
On 02/26/2016 09:55 AM, Marek Vasut wrote:
On 02/26/2016 05:48 PM, Stephen Warren wrote:
On 01/26/2016 07:14 PM, Marek Vasut wrote:
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.:
+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)); ^
(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.
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;
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:
On 02/26/2016 07:16 PM, Stephen Warren wrote:
Hi!
[...]
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;
I just tried this, it should do the trick. Can you give it a spin ?
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index 8f259be..0113c6c 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -237,7 +237,7 @@ static int ehci_shutdown(struct ehci_ctrl *ctrl) static int ehci_td_buffer(struct qTD *td, void *buf, size_t sz) { uint32_t delta, next; - uint32_t addr = (unsigned long)buf; + unsigned long addr = (unsigned long)buf; int idx;
if (addr != ALIGN(addr, ARCH_DMA_MINALIGN))
I am a bit worried about the u32 values all around the place. If the buffer would be above 4GiB, we might have a problem.
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:
If you have one you don't need ... ;-)
On 02/26/2016 11:44 AM, Marek Vasut wrote:
On 02/26/2016 07:16 PM, Stephen Warren wrote:
Hi!
[...]
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;
I just tried this, it should do the trick. Can you give it a spin ?
That does the trick, thanks.
On 02/26/2016 08:12 PM, Stephen Warren wrote:
On 02/26/2016 11:44 AM, Marek Vasut wrote:
On 02/26/2016 07:16 PM, Stephen Warren wrote:
Hi!
[...]
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;
I just tried this, it should do the trick. Can you give it a spin ?
That does the trick, thanks.
Thanks! I'll finish buildman builds (I set it up, finally, yay!) and submit a proper patch.
If the USB EHCI is configured for little endian MMIO, make sure to clear the USBMODE_BE flag from the USBMODE register.
Signed-off-by: Marek Vasut marex@denx.de Cc: Daniel Schwierzeck daniel.schwierzeck@gmail.com Cc: Hans de Goede hdegoede@redhat.com --- drivers/usb/host/ehci-hcd.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index b3eb08d..8f259be 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -134,6 +134,8 @@ static void ehci_set_usbmode(struct ehci_ctrl *ctrl) tmp |= USBMODE_CM_HC; #if defined(CONFIG_EHCI_MMIO_BIG_ENDIAN) tmp |= USBMODE_BE; +#else + tmp &= ~USBMODE_BE; #endif ehci_writel(reg_ptr, tmp); }
Add explicit cpu_to_be32()/be32_to_cpu() conversion to BE EHCI I/O accessors to align them with their LE counterpart. No functional change.
Signed-off-by: Marek Vasut marex@denx.de Cc: Daniel Schwierzeck daniel.schwierzeck@gmail.com Cc: Hans de Goede hdegoede@redhat.com --- drivers/usb/host/ehci.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h index b41c04a..826b3fe 100644 --- a/drivers/usb/host/ehci.h +++ b/drivers/usb/host/ehci.h @@ -102,8 +102,9 @@ struct usb_linux_config_descriptor { } __attribute__ ((packed));
#if defined CONFIG_EHCI_DESC_BIG_ENDIAN -#define ehci_readl(x) (*((volatile u32 *)(x))) -#define ehci_writel(a, b) (*((volatile u32 *)(a)) = ((volatile u32)b)) +#define ehci_readl(x) cpu_to_be32((*((volatile u32 *)(x)))) +#define ehci_writel(a, b) (*((volatile u32 *)(a)) = \ + cpu_to_be32(((volatile u32)b))) #else #define ehci_readl(x) cpu_to_le32((*((volatile u32 *)(x)))) #define ehci_writel(a, b) (*((volatile u32 *)(a)) = \
Am 27.01.2016 um 03:13 schrieb Marek Vasut:
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 marex@denx.de Cc: Daniel Schwierzeck daniel.schwierzeck@gmail.com Cc: Hans de Goede hdegoede@redhat.com
arch/mips/lib/cache.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/arch/mips/lib/cache.c b/arch/mips/lib/cache.c index bf8ff59..7482005 100644 --- a/arch/mips/lib/cache.c +++ b/arch/mips/lib/cache.c @@ -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));
- /* 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.
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)
On Wednesday, January 27, 2016 at 03:56:36 PM, Daniel Schwierzeck wrote:
Am 27.01.2016 um 03:13 schrieb Marek Vasut:
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 marex@denx.de Cc: Daniel Schwierzeck daniel.schwierzeck@gmail.com Cc: Hans de Goede hdegoede@redhat.com
arch/mips/lib/cache.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/arch/mips/lib/cache.c b/arch/mips/lib/cache.c index bf8ff59..7482005 100644 --- a/arch/mips/lib/cache.c +++ b/arch/mips/lib/cache.c @@ -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));
- /* 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.
The compiler would do the reordering anyway, it's not volatile so it's placement would not save any cycles.
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)
Best regards, Marek Vasut
2016-01-27 3:13 GMT+01:00 Marek Vasut marex@denx.de:
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 marex@denx.de Cc: Daniel Schwierzeck daniel.schwierzeck@gmail.com Cc: Hans de Goede hdegoede@redhat.com
arch/mips/lib/cache.c | 8 ++++++++ 1 file changed, 8 insertions(+)
applied to u-boot-mips, thanks
On Monday, February 01, 2016 at 10:29:51 PM, Daniel Schwierzeck wrote:
2016-01-27 3:13 GMT+01:00 Marek Vasut marex@denx.de:
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 marex@denx.de Cc: Daniel Schwierzeck daniel.schwierzeck@gmail.com Cc: Hans de Goede hdegoede@redhat.com
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 ?
Best regards, Marek Vasut
2016-02-01 22:31 GMT+01:00 Marek Vasut marex@denx.de:
On Monday, February 01, 2016 at 10:29:51 PM, Daniel Schwierzeck wrote:
2016-01-27 3:13 GMT+01:00 Marek Vasut marex@denx.de:
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 marex@denx.de Cc: Daniel Schwierzeck daniel.schwierzeck@gmail.com Cc: Hans de Goede hdegoede@redhat.com
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 ;)
On Monday, February 01, 2016 at 10:40:26 PM, Daniel Schwierzeck wrote:
2016-02-01 22:31 GMT+01:00 Marek Vasut marex@denx.de:
On Monday, February 01, 2016 at 10:29:51 PM, Daniel Schwierzeck wrote:
2016-01-27 3:13 GMT+01:00 Marek Vasut marex@denx.de:
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 marex@denx.de Cc: Daniel Schwierzeck daniel.schwierzeck@gmail.com Cc: Hans de Goede hdegoede@redhat.com
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 ;)
;-)
Best regards, Marek Vasut
participants (4)
-
Alexey Brodkin -
Daniel Schwierzeck -
Marek Vasut -
Stephen Warren