Use the Interval value from isoc/intr endpoint descriptor, no need
minus one. But the original code doesn't cause transfer error for
normal cases, due to the interval is less than the host request.
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
drivers/usb/mtu3/mtu3_gadget.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
There is a seldom issue that the controller access invalid address
and trigger devapc or emimpu violation. That is due to memory access
is out of order and cause gpd data is not correct.
Make sure GPD is fully written before giving it to HW by setting its
HWO.
Fixes: 48e0d3735aa5 ("usb: mtu3: supports new QMU format")
Cc: stable@vger.kernel.org
Reported-by: Eddie Hung <redacted>
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
drivers/usb/mtu3/mtu3_qmu.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
@@ -273,6 +273,8 @@ static int mtu3_prepare_tx_gpd(struct mtu3_ep *mep, struct mtu3_request *mreq)gpd->dw3_info|=cpu_to_le32(GPD_EXT_FLAG_ZLP);}+/* make sure GPD is fully written before giving it to HW */+mb();gpd->dw0_info|=cpu_to_le32(GPD_FLAGS_IOC|GPD_FLAGS_HWO);mreq->gpd=gpd;
@@ -306,6 +308,8 @@ static int mtu3_prepare_rx_gpd(struct mtu3_ep *mep, struct mtu3_request *mreq)gpd->next_gpd=cpu_to_le32(lower_32_bits(enq_dma));ext_addr|=GPD_EXT_NGP(mtu,upper_32_bits(enq_dma));gpd->dw3_info=cpu_to_le32(ext_addr);+/* make sure GPD is fully written before giving it to HW */+mb();gpd->dw0_info|=cpu_to_le32(GPD_FLAGS_IOC|GPD_FLAGS_HWO);mreq->gpd=gpd;
@@ -445,7 +449,8 @@ static void qmu_tx_zlp_error_handler(struct mtu3 *mtu, u8 epnum)return;}mtu3_setbits(mbase,MU3D_EP_TXCR0(mep->epnum),TX_TXPKTRDY);-+/* make sure GPD is fully written before giving it to HW */+mb();/* by pass the current GDP */gpd_current->dw0_info|=cpu_to_le32(GPD_FLAGS_BPS|GPD_FLAGS_HWO);
--
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Thu, Dec 09, 2021 at 11:14:23AM +0800, Chunfeng Yun wrote:
quoted hunk
There is a seldom issue that the controller access invalid address
and trigger devapc or emimpu violation. That is due to memory access
is out of order and cause gpd data is not correct.
Make sure GPD is fully written before giving it to HW by setting its
HWO.
Fixes: 48e0d3735aa5 ("usb: mtu3: supports new QMU format")
Cc: stable@vger.kernel.org
Reported-by: Eddie Hung <redacted>
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
drivers/usb/mtu3/mtu3_qmu.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
@@ -273,6 +273,8 @@ static int mtu3_prepare_tx_gpd(struct mtu3_ep *mep, struct mtu3_request *mreq)gpd->dw3_info|=cpu_to_le32(GPD_EXT_FLAG_ZLP);}+/* make sure GPD is fully written before giving it to HW */+mb();
So this means you are using mmio for this structure? If so, shouldn't
you be using normal io memory read/write calls as well and not just
"raw" pointers like this:
Are you sure this is ok?
Sprinkling around mb() calls is almost never the correct solution.
If you need to ensure that a write succeeds, shouldn't you do a read
from it afterward? Many busses require this, doesn't yours?
quoted hunk
mreq->gpd = gpd;
@@ -306,6 +308,8 @@ static int mtu3_prepare_rx_gpd(struct mtu3_ep *mep, struct mtu3_request *mreq) gpd->next_gpd = cpu_to_le32(lower_32_bits(enq_dma)); ext_addr |= GPD_EXT_NGP(mtu, upper_32_bits(enq_dma)); gpd->dw3_info = cpu_to_le32(ext_addr);+ /* make sure GPD is fully written before giving it to HW */+ mb();
Again, mb(); does not ensure that memory-mapped i/o actually hits the
HW. Or if it does on your platform, how?
mb() is a compiler barrier, not a memory write to a bus barrier. Please
read Documentation/memory-barriers.txt for more details.
thanks,
greg k-h
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Mon, 2021-12-13 at 15:18 +0100, Greg Kroah-Hartman wrote:
On Thu, Dec 09, 2021 at 11:14:23AM +0800, Chunfeng Yun wrote:
quoted
There is a seldom issue that the controller access invalid address
and trigger devapc or emimpu violation. That is due to memory
access
is out of order and cause gpd data is not correct.
Make sure GPD is fully written before giving it to HW by setting
its
HWO.
Fixes: 48e0d3735aa5 ("usb: mtu3: supports new QMU format")
Cc: stable@vger.kernel.org
Reported-by: Eddie Hung <redacted>
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
drivers/usb/mtu3/mtu3_qmu.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/mtu3/mtu3_qmu.c
b/drivers/usb/mtu3/mtu3_qmu.c
index 3f414f91b589..34bb5ac67efe 100644
@@ -273,6 +273,8 @@ static int mtu3_prepare_tx_gpd(struct mtu3_ep
*mep, struct mtu3_request *mreq)
gpd->dw3_info |= cpu_to_le32(GPD_EXT_FLAG_ZLP);
}
+ /* make sure GPD is fully written before giving it to HW */
+ mb();
So this means you are using mmio for this structure?
No, it's a noncached memory.
If so, shouldn't
you be using normal io memory read/write calls as well and not just
"raw" pointers like this:
Are you sure this is ok?
Sprinkling around mb() calls is almost never the correct solution.
If you need to ensure that a write succeeds, shouldn't you do a read
from it afterward? Many busses require this, doesn't yours?
It works for register access.
Here is noncache memory access, add mb(), just want to prohibite both
the compiler and CPU from reordering read/writes.
quoted
mreq->gpd = gpd;
@@ -306,6 +308,8 @@ static int mtu3_prepare_rx_gpd(struct mtu3_ep
*mep, struct mtu3_request *mreq)
gpd->next_gpd = cpu_to_le32(lower_32_bits(enq_dma));
ext_addr |= GPD_EXT_NGP(mtu, upper_32_bits(enq_dma));
gpd->dw3_info = cpu_to_le32(ext_addr);
+ /* make sure GPD is fully written before giving it to HW */
+ mb();
Again, mb(); does not ensure that memory-mapped i/o actually hits the
HW. Or if it does on your platform, how?
Maybe the comment is misleading, I'll change it, here just want to
prevent reordering of compiler and cpu.
mb() is a compiler barrier, not a memory write to a bus
barrier. Please
read Documentation/memory-barriers.txt for more details.
On Thu, Dec 09, 2021 at 11:14:24AM +0800, Chunfeng Yun wrote:
This is caused by uninitialization of list_head.
BUG: KASAN: use-after-free in __list_del_entry_valid+0x34/0xe4
Call trace:
dump_backtrace+0x0/0x298
show_stack+0x24/0x34
dump_stack+0x130/0x1a8
print_address_description+0x88/0x56c
__kasan_report+0x1b8/0x2a0
kasan_report+0x14/0x20
__asan_load8+0x9c/0xa0
__list_del_entry_valid+0x34/0xe4
mtu3_req_complete+0x4c/0x300 [mtu3]
mtu3_gadget_stop+0x168/0x448 [mtu3]
usb_gadget_unregister_driver+0x204/0x3a0
unregister_gadget_item+0x44/0xa4
Reported-by: Yuwen Ng <redacted>
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
drivers/usb/mtu3/mtu3_gadget.c | 1 +
1 file changed, 1 insertion(+)
What commit does this fix? Should it go to stable kernels?
thanks,
greg k-h
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Thu, Dec 09, 2021 at 11:14:22AM +0800, Chunfeng Yun wrote:
Use the Interval value from isoc/intr endpoint descriptor, no need
minus one. But the original code doesn't cause transfer error for
normal cases, due to the interval is less than the host request.
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
drivers/usb/mtu3/mtu3_gadget.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
On Mon, 2021-12-13 at 15:20 +0100, Greg Kroah-Hartman wrote:
On Thu, Dec 09, 2021 at 11:14:22AM +0800, Chunfeng Yun wrote:
quoted
Use the Interval value from isoc/intr endpoint descriptor, no need
minus one. But the original code doesn't cause transfer error for
normal cases, due to the interval is less than the host request.
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
drivers/usb/mtu3/mtu3_gadget.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
What commit does this fix?
The interval between transfers is less than the Interval value, I add
it in commit massage when send out v2.
Thanks
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel