Re: [PATCH v3] usb: dwc3: gadget: Prevent EP queuing while stopping transfers
On Tue, Mar 23, 2021 at 11:53 PM Wesley Cheng wrote: > > > > On 3/23/2021 10:27 AM, Andy Shevchenko wrote: > > On Tue, Mar 23, 2021 at 1:19 AM Wesley Cheng wrote: > >> > >> Hi Andy, > >> > >> On 3/22/2021 2:14 PM, Andy Shevchenko wrote: > >>> On Mon, Mar 22, 2021 at 10:06 PM Wesley Cheng > >>> wrote: > > Hi Andy, > > On 3/22/2021 12:34 PM, Andy Shevchenko wrote: > > On Mon, Mar 22, 2021 at 8:49 PM Wesley Cheng > > wrote: > >> > >> Hi Andy, > >> > >> On 3/22/2021 5:48 AM, Andy Shevchenko wrote: > >>> On Fri, Mar 12, 2021 at 2:01 AM Wesley Cheng > >>> wrote: > > In the situations where the DWC3 gadget stops active transfers, once > calling the dwc3_gadget_giveback(), there is a chance where a > function > driver can queue a new USB request in between the time where the dwc3 > lock has been released and re-aquired. This occurs after we've > already > issued an ENDXFER command. When the stop active transfers continues > to remove USB requests from all dep lists, the newly added request > will > also be removed, while controller still has an active TRB for it. > This can lead to the controller accessing an unmapped memory address. > > Fix this by ensuring parameters to prevent EP queuing are set before > calling the stop active transfers API. > >>> > >>> > >>> commit f09ddcfcb8c569675066337adac2ac205113471f > >>> Author: Wesley Cheng > >>> Date: Thu Mar 11 15:59:02 2021 -0800 > >>> > >>>usb: dwc3: gadget: Prevent EP queuing while stopping transfers > >>> > >>> effectively broke my gadget setup. > >>> > >>> The output of the kernel (followed by non responsive state of USB > >>> controller): > >>> > >>> [ 195.228586] using random self ethernet address > >>> [ 195.233104] using random host ethernet address > >>> [ 195.245306] usb0: HOST MAC aa:bb:cc:dd:ee:f2 > >>> [ 195.249732] usb0: MAC aa:bb:cc:dd:ee:f1 > >>> # [ 195.773594] IPv6: ADDRCONF(NETDEV_CHANGE): usb0: link becomes > >>> ready > >>> [ 195.780585] [ cut here ] > >>> [ 195.785217] dwc3 dwc3.0.auto: No resource for ep2in > >>> [ 195.790162] WARNING: CPU: 0 PID: 217 at > >>> drivers/usb/dwc3/gadget.c:360 dwc3_send_gadget_ep_cmd+0x4b9/0x670 > >>> [ 195.799760] Modules linked in: usb_f_eem u_ether libcomposite > >>> brcmfmac brcmutil mmc_block pwm_lpss_pci pwm_lps > >>> s snd_sof_pci_intel_tng snd_sof_pci snd_sof_acpi_intel_byt > >>> snd_sof_intel_ipc snd_sof_acpi snd_sof snd_sof_nocodec > >>> spi_pxa2xx_platform snd_sof_xtensa_dsp spi_pxa2xx_pci > >>> extcon_intel_mrfld intel_mrfld_adc sdhci_pci cqhci sdhci m > >>> mc_core intel_mrfld_pwrbtn intel_soc_pmic_mrfld hci_uart btbcm btintel > >>> [ 195.835604] CPU: 0 PID: 217 Comm: irq/16-dwc3 Not tainted > >>> 5.12.0-rc4+ #60 > >>> [ 195.842403] Hardware name: Intel Corporation Merrifield/BODEGA BAY, > >>> BIOS 542 2015.01.21:18.19.48 > >>> [ 195.851191] RIP: 0010:dwc3_send_gadget_ep_cmd+0x4b9/0x670 > >>> [ 195.856608] Code: cd 00 00 00 44 89 44 24 20 48 89 4c 24 18 e8 ee > >>> f7 e4 ff 48 8b 4c 24 18 4c 89 f2 48 c7 c7 b9 > >>> ed 4f a0 48 89 c6 e8 ef 24 43 00 <0f> 0b 41 be ea ff ff ff 44 8b 44 24 > >>> 20 e9 80 fc ff ff 41 83 fe 92 > >>> [ 195.875381] RSP: :a53c00373ba8 EFLAGS: 00010086 > >>> [ 195.880617] RAX: RBX: 1387 RCX: > >>> dfff > >>> [ 195.887755] RDX: dfff RSI: ffea RDI: > >>> > >>> [ 195.894893] RBP: 9ce8c8f2b028 R08: a0732288 R09: > >>> 9ffb > >>> [ 195.902034] R10: e000 R11: 3fff R12: > >>> 00041006 > >>> [ 195.909170] R13: a53c00373c24 R14: 9ce8c11dadb0 R15: > >>> 9ce8c2861700 > >>> [ 195.916310] FS: () GS:9ce8fe20() > >>> knlGS: > >>> [ 195.924409] CS: 0010 DS: ES: CR0: 80050033 > >>> [ 195.930161] CR2: f7f694a0 CR3: 38e0c000 CR4: > >>> 001006f0 > >>> [ 195.937300] Call Trace: > >>> [ 195.939755] __dwc3_gadget_ep_enable+0x2d4/0x4e0 > >>> [ 195.944393] ? dwc3_remove_requests.constprop.0+0x86/0x170 > >> > >> Odd that this change would affect the USB enablment path, as they were > >> focused on the pullup disable path. Would you happen to have any > >> downstream changes on top of v5.12-rc4 we could review to see if they > >> are still required? (ie where is the dwc3_remove_requests() coming from > >> during ep enable) > > > > You may check my branch [1] on GH. Basically you may be interested in > > the commit: > > 0f86df1294
Re: [PATCH v3] usb: dwc3: gadget: Prevent EP queuing while stopping transfers
Hi Wesley, On 23.03.2021 22:53, Wesley Cheng wrote: > On 3/23/2021 10:27 AM, Andy Shevchenko wrote: >> On Tue, Mar 23, 2021 at 1:19 AM Wesley Cheng wrote: >>> On 3/22/2021 2:14 PM, Andy Shevchenko wrote: On Mon, Mar 22, 2021 at 10:06 PM Wesley Cheng wrote: > On 3/22/2021 12:34 PM, Andy Shevchenko wrote: >> On Mon, Mar 22, 2021 at 8:49 PM Wesley Cheng >> wrote: >>> On 3/22/2021 5:48 AM, Andy Shevchenko wrote: On Fri, Mar 12, 2021 at 2:01 AM Wesley Cheng wrote: > In the situations where the DWC3 gadget stops active transfers, once > calling the dwc3_gadget_giveback(), there is a chance where a function > driver can queue a new USB request in between the time where the dwc3 > lock has been released and re-aquired. This occurs after we've > already > issued an ENDXFER command. When the stop active transfers continues > to remove USB requests from all dep lists, the newly added request > will > also be removed, while controller still has an active TRB for it. > This can lead to the controller accessing an unmapped memory address. > > Fix this by ensuring parameters to prevent EP queuing are set before > calling the stop active transfers API. commit f09ddcfcb8c569675066337adac2ac205113471f Author: Wesley Cheng Date: Thu Mar 11 15:59:02 2021 -0800 usb: dwc3: gadget: Prevent EP queuing while stopping transfers effectively broke my gadget setup. The output of the kernel (followed by non responsive state of USB controller): [ 195.228586] using random self ethernet address [ 195.233104] using random host ethernet address [ 195.245306] usb0: HOST MAC aa:bb:cc:dd:ee:f2 [ 195.249732] usb0: MAC aa:bb:cc:dd:ee:f1 # [ 195.773594] IPv6: ADDRCONF(NETDEV_CHANGE): usb0: link becomes ready [ 195.780585] [ cut here ] [ 195.785217] dwc3 dwc3.0.auto: No resource for ep2in [ 195.790162] WARNING: CPU: 0 PID: 217 at drivers/usb/dwc3/gadget.c:360 dwc3_send_gadget_ep_cmd+0x4b9/0x670 [ 195.799760] Modules linked in: usb_f_eem u_ether libcomposite brcmfmac brcmutil mmc_block pwm_lpss_pci pwm_lps s snd_sof_pci_intel_tng snd_sof_pci snd_sof_acpi_intel_byt snd_sof_intel_ipc snd_sof_acpi snd_sof snd_sof_nocodec spi_pxa2xx_platform snd_sof_xtensa_dsp spi_pxa2xx_pci extcon_intel_mrfld intel_mrfld_adc sdhci_pci cqhci sdhci m mc_core intel_mrfld_pwrbtn intel_soc_pmic_mrfld hci_uart btbcm btintel [ 195.835604] CPU: 0 PID: 217 Comm: irq/16-dwc3 Not tainted 5.12.0-rc4+ #60 [ 195.842403] Hardware name: Intel Corporation Merrifield/BODEGA BAY, BIOS 542 2015.01.21:18.19.48 [ 195.851191] RIP: 0010:dwc3_send_gadget_ep_cmd+0x4b9/0x670 [ 195.856608] Code: cd 00 00 00 44 89 44 24 20 48 89 4c 24 18 e8 ee f7 e4 ff 48 8b 4c 24 18 4c 89 f2 48 c7 c7 b9 ed 4f a0 48 89 c6 e8 ef 24 43 00 <0f> 0b 41 be ea ff ff ff 44 8b 44 24 20 e9 80 fc ff ff 41 83 fe 92 [ 195.875381] RSP: :a53c00373ba8 EFLAGS: 00010086 [ 195.880617] RAX: RBX: 1387 RCX: dfff [ 195.887755] RDX: dfff RSI: ffea RDI: [ 195.894893] RBP: 9ce8c8f2b028 R08: a0732288 R09: 9ffb [ 195.902034] R10: e000 R11: 3fff R12: 00041006 [ 195.909170] R13: a53c00373c24 R14: 9ce8c11dadb0 R15: 9ce8c2861700 [ 195.916310] FS: () GS:9ce8fe20() knlGS: [ 195.924409] CS: 0010 DS: ES: CR0: 80050033 [ 195.930161] CR2: f7f694a0 CR3: 38e0c000 CR4: 001006f0 [ 195.937300] Call Trace: [ 195.939755] __dwc3_gadget_ep_enable+0x2d4/0x4e0 [ 195.944393] ? dwc3_remove_requests.constprop.0+0x86/0x170 >>> Odd that this change would affect the USB enablment path, as they were >>> focused on the pullup disable path. Would you happen to have any >>> downstream changes on top of v5.12-rc4 we could review to see if they >>> are still required? (ie where is the dwc3_remove_requests() coming from >>> during ep enable) >> You may check my branch [1] on GH. Basically you may be interested in >> the commit: >> 0f86df1294ee7523060cc16eafaf4898c693eab0 REVERTME: usb: dwc3: gadget: >> skip endpoints ep[18]{in,out} >> Otherwise it's a clean v5.12-rc4 with a revert and another USB PHY >> suspend fix (which also shouldn't affect this). > Ca
Re: [PATCH v3] usb: dwc3: gadget: Prevent EP queuing while stopping transfers
On 3/23/2021 10:27 AM, Andy Shevchenko wrote: > On Tue, Mar 23, 2021 at 1:19 AM Wesley Cheng wrote: >> >> Hi Andy, >> >> On 3/22/2021 2:14 PM, Andy Shevchenko wrote: >>> On Mon, Mar 22, 2021 at 10:06 PM Wesley Cheng wrote: Hi Andy, On 3/22/2021 12:34 PM, Andy Shevchenko wrote: > On Mon, Mar 22, 2021 at 8:49 PM Wesley Cheng > wrote: >> >> Hi Andy, >> >> On 3/22/2021 5:48 AM, Andy Shevchenko wrote: >>> On Fri, Mar 12, 2021 at 2:01 AM Wesley Cheng >>> wrote: In the situations where the DWC3 gadget stops active transfers, once calling the dwc3_gadget_giveback(), there is a chance where a function driver can queue a new USB request in between the time where the dwc3 lock has been released and re-aquired. This occurs after we've already issued an ENDXFER command. When the stop active transfers continues to remove USB requests from all dep lists, the newly added request will also be removed, while controller still has an active TRB for it. This can lead to the controller accessing an unmapped memory address. Fix this by ensuring parameters to prevent EP queuing are set before calling the stop active transfers API. >>> >>> >>> commit f09ddcfcb8c569675066337adac2ac205113471f >>> Author: Wesley Cheng >>> Date: Thu Mar 11 15:59:02 2021 -0800 >>> >>>usb: dwc3: gadget: Prevent EP queuing while stopping transfers >>> >>> effectively broke my gadget setup. >>> >>> The output of the kernel (followed by non responsive state of USB >>> controller): >>> >>> [ 195.228586] using random self ethernet address >>> [ 195.233104] using random host ethernet address >>> [ 195.245306] usb0: HOST MAC aa:bb:cc:dd:ee:f2 >>> [ 195.249732] usb0: MAC aa:bb:cc:dd:ee:f1 >>> # [ 195.773594] IPv6: ADDRCONF(NETDEV_CHANGE): usb0: link becomes ready >>> [ 195.780585] [ cut here ] >>> [ 195.785217] dwc3 dwc3.0.auto: No resource for ep2in >>> [ 195.790162] WARNING: CPU: 0 PID: 217 at >>> drivers/usb/dwc3/gadget.c:360 dwc3_send_gadget_ep_cmd+0x4b9/0x670 >>> [ 195.799760] Modules linked in: usb_f_eem u_ether libcomposite >>> brcmfmac brcmutil mmc_block pwm_lpss_pci pwm_lps >>> s snd_sof_pci_intel_tng snd_sof_pci snd_sof_acpi_intel_byt >>> snd_sof_intel_ipc snd_sof_acpi snd_sof snd_sof_nocodec >>> spi_pxa2xx_platform snd_sof_xtensa_dsp spi_pxa2xx_pci >>> extcon_intel_mrfld intel_mrfld_adc sdhci_pci cqhci sdhci m >>> mc_core intel_mrfld_pwrbtn intel_soc_pmic_mrfld hci_uart btbcm btintel >>> [ 195.835604] CPU: 0 PID: 217 Comm: irq/16-dwc3 Not tainted >>> 5.12.0-rc4+ #60 >>> [ 195.842403] Hardware name: Intel Corporation Merrifield/BODEGA BAY, >>> BIOS 542 2015.01.21:18.19.48 >>> [ 195.851191] RIP: 0010:dwc3_send_gadget_ep_cmd+0x4b9/0x670 >>> [ 195.856608] Code: cd 00 00 00 44 89 44 24 20 48 89 4c 24 18 e8 ee >>> f7 e4 ff 48 8b 4c 24 18 4c 89 f2 48 c7 c7 b9 >>> ed 4f a0 48 89 c6 e8 ef 24 43 00 <0f> 0b 41 be ea ff ff ff 44 8b 44 24 >>> 20 e9 80 fc ff ff 41 83 fe 92 >>> [ 195.875381] RSP: :a53c00373ba8 EFLAGS: 00010086 >>> [ 195.880617] RAX: RBX: 1387 RCX: >>> dfff >>> [ 195.887755] RDX: dfff RSI: ffea RDI: >>> >>> [ 195.894893] RBP: 9ce8c8f2b028 R08: a0732288 R09: >>> 9ffb >>> [ 195.902034] R10: e000 R11: 3fff R12: >>> 00041006 >>> [ 195.909170] R13: a53c00373c24 R14: 9ce8c11dadb0 R15: >>> 9ce8c2861700 >>> [ 195.916310] FS: () GS:9ce8fe20() >>> knlGS: >>> [ 195.924409] CS: 0010 DS: ES: CR0: 80050033 >>> [ 195.930161] CR2: f7f694a0 CR3: 38e0c000 CR4: >>> 001006f0 >>> [ 195.937300] Call Trace: >>> [ 195.939755] __dwc3_gadget_ep_enable+0x2d4/0x4e0 >>> [ 195.944393] ? dwc3_remove_requests.constprop.0+0x86/0x170 >> >> Odd that this change would affect the USB enablment path, as they were >> focused on the pullup disable path. Would you happen to have any >> downstream changes on top of v5.12-rc4 we could review to see if they >> are still required? (ie where is the dwc3_remove_requests() coming from >> during ep enable) > > You may check my branch [1] on GH. Basically you may be interested in > the commit: > 0f86df1294ee7523060cc16eafaf4898c693eab0 REVERTME: usb: dwc3: gadget: > skip endpoints ep[18]{in,out} > Otherwise it's a clean v5.12-rc4 with a revert and another USB PHY > suspend fix (which also shouldn't affect this). Can you link your GH reference? >>> >>> Oops, sorry. >>> Here we are: >>>
Re: [PATCH v3] usb: dwc3: gadget: Prevent EP queuing while stopping transfers
On 3/23/2021 10:27 AM, Andy Shevchenko wrote: > On Tue, Mar 23, 2021 at 1:19 AM Wesley Cheng wrote: >> >> Hi Andy, >> >> On 3/22/2021 2:14 PM, Andy Shevchenko wrote: >>> On Mon, Mar 22, 2021 at 10:06 PM Wesley Cheng wrote: Hi Andy, On 3/22/2021 12:34 PM, Andy Shevchenko wrote: > On Mon, Mar 22, 2021 at 8:49 PM Wesley Cheng > wrote: >> >> Hi Andy, >> >> On 3/22/2021 5:48 AM, Andy Shevchenko wrote: >>> On Fri, Mar 12, 2021 at 2:01 AM Wesley Cheng >>> wrote: In the situations where the DWC3 gadget stops active transfers, once calling the dwc3_gadget_giveback(), there is a chance where a function driver can queue a new USB request in between the time where the dwc3 lock has been released and re-aquired. This occurs after we've already issued an ENDXFER command. When the stop active transfers continues to remove USB requests from all dep lists, the newly added request will also be removed, while controller still has an active TRB for it. This can lead to the controller accessing an unmapped memory address. Fix this by ensuring parameters to prevent EP queuing are set before calling the stop active transfers API. >>> >>> >>> commit f09ddcfcb8c569675066337adac2ac205113471f >>> Author: Wesley Cheng >>> Date: Thu Mar 11 15:59:02 2021 -0800 >>> >>>usb: dwc3: gadget: Prevent EP queuing while stopping transfers >>> >>> effectively broke my gadget setup. >>> >>> The output of the kernel (followed by non responsive state of USB >>> controller): >>> >>> [ 195.228586] using random self ethernet address >>> [ 195.233104] using random host ethernet address >>> [ 195.245306] usb0: HOST MAC aa:bb:cc:dd:ee:f2 >>> [ 195.249732] usb0: MAC aa:bb:cc:dd:ee:f1 >>> # [ 195.773594] IPv6: ADDRCONF(NETDEV_CHANGE): usb0: link becomes ready >>> [ 195.780585] [ cut here ] >>> [ 195.785217] dwc3 dwc3.0.auto: No resource for ep2in >>> [ 195.790162] WARNING: CPU: 0 PID: 217 at >>> drivers/usb/dwc3/gadget.c:360 dwc3_send_gadget_ep_cmd+0x4b9/0x670 >>> [ 195.799760] Modules linked in: usb_f_eem u_ether libcomposite >>> brcmfmac brcmutil mmc_block pwm_lpss_pci pwm_lps >>> s snd_sof_pci_intel_tng snd_sof_pci snd_sof_acpi_intel_byt >>> snd_sof_intel_ipc snd_sof_acpi snd_sof snd_sof_nocodec >>> spi_pxa2xx_platform snd_sof_xtensa_dsp spi_pxa2xx_pci >>> extcon_intel_mrfld intel_mrfld_adc sdhci_pci cqhci sdhci m >>> mc_core intel_mrfld_pwrbtn intel_soc_pmic_mrfld hci_uart btbcm btintel >>> [ 195.835604] CPU: 0 PID: 217 Comm: irq/16-dwc3 Not tainted >>> 5.12.0-rc4+ #60 >>> [ 195.842403] Hardware name: Intel Corporation Merrifield/BODEGA BAY, >>> BIOS 542 2015.01.21:18.19.48 >>> [ 195.851191] RIP: 0010:dwc3_send_gadget_ep_cmd+0x4b9/0x670 >>> [ 195.856608] Code: cd 00 00 00 44 89 44 24 20 48 89 4c 24 18 e8 ee >>> f7 e4 ff 48 8b 4c 24 18 4c 89 f2 48 c7 c7 b9 >>> ed 4f a0 48 89 c6 e8 ef 24 43 00 <0f> 0b 41 be ea ff ff ff 44 8b 44 24 >>> 20 e9 80 fc ff ff 41 83 fe 92 >>> [ 195.875381] RSP: :a53c00373ba8 EFLAGS: 00010086 >>> [ 195.880617] RAX: RBX: 1387 RCX: >>> dfff >>> [ 195.887755] RDX: dfff RSI: ffea RDI: >>> >>> [ 195.894893] RBP: 9ce8c8f2b028 R08: a0732288 R09: >>> 9ffb >>> [ 195.902034] R10: e000 R11: 3fff R12: >>> 00041006 >>> [ 195.909170] R13: a53c00373c24 R14: 9ce8c11dadb0 R15: >>> 9ce8c2861700 >>> [ 195.916310] FS: () GS:9ce8fe20() >>> knlGS: >>> [ 195.924409] CS: 0010 DS: ES: CR0: 80050033 >>> [ 195.930161] CR2: f7f694a0 CR3: 38e0c000 CR4: >>> 001006f0 >>> [ 195.937300] Call Trace: >>> [ 195.939755] __dwc3_gadget_ep_enable+0x2d4/0x4e0 >>> [ 195.944393] ? dwc3_remove_requests.constprop.0+0x86/0x170 >> >> Odd that this change would affect the USB enablment path, as they were >> focused on the pullup disable path. Would you happen to have any >> downstream changes on top of v5.12-rc4 we could review to see if they >> are still required? (ie where is the dwc3_remove_requests() coming from >> during ep enable) > > You may check my branch [1] on GH. Basically you may be interested in > the commit: > 0f86df1294ee7523060cc16eafaf4898c693eab0 REVERTME: usb: dwc3: gadget: > skip endpoints ep[18]{in,out} > Otherwise it's a clean v5.12-rc4 with a revert and another USB PHY > suspend fix (which also shouldn't affect this). Can you link your GH reference? >>> >>> Oops, sorry. >>> Here we are: >>>
Re: [PATCH v3] usb: dwc3: gadget: Prevent EP queuing while stopping transfers
On Tue, Mar 23, 2021 at 1:19 AM Wesley Cheng wrote: > > Hi Andy, > > On 3/22/2021 2:14 PM, Andy Shevchenko wrote: > > On Mon, Mar 22, 2021 at 10:06 PM Wesley Cheng wrote: > >> > >> Hi Andy, > >> > >> On 3/22/2021 12:34 PM, Andy Shevchenko wrote: > >>> On Mon, Mar 22, 2021 at 8:49 PM Wesley Cheng > >>> wrote: > > Hi Andy, > > On 3/22/2021 5:48 AM, Andy Shevchenko wrote: > > On Fri, Mar 12, 2021 at 2:01 AM Wesley Cheng > > wrote: > >> > >> In the situations where the DWC3 gadget stops active transfers, once > >> calling the dwc3_gadget_giveback(), there is a chance where a function > >> driver can queue a new USB request in between the time where the dwc3 > >> lock has been released and re-aquired. This occurs after we've already > >> issued an ENDXFER command. When the stop active transfers continues > >> to remove USB requests from all dep lists, the newly added request will > >> also be removed, while controller still has an active TRB for it. > >> This can lead to the controller accessing an unmapped memory address. > >> > >> Fix this by ensuring parameters to prevent EP queuing are set before > >> calling the stop active transfers API. > > > > > > commit f09ddcfcb8c569675066337adac2ac205113471f > > Author: Wesley Cheng > > Date: Thu Mar 11 15:59:02 2021 -0800 > > > >usb: dwc3: gadget: Prevent EP queuing while stopping transfers > > > > effectively broke my gadget setup. > > > > The output of the kernel (followed by non responsive state of USB > > controller): > > > > [ 195.228586] using random self ethernet address > > [ 195.233104] using random host ethernet address > > [ 195.245306] usb0: HOST MAC aa:bb:cc:dd:ee:f2 > > [ 195.249732] usb0: MAC aa:bb:cc:dd:ee:f1 > > # [ 195.773594] IPv6: ADDRCONF(NETDEV_CHANGE): usb0: link becomes ready > > [ 195.780585] [ cut here ] > > [ 195.785217] dwc3 dwc3.0.auto: No resource for ep2in > > [ 195.790162] WARNING: CPU: 0 PID: 217 at > > drivers/usb/dwc3/gadget.c:360 dwc3_send_gadget_ep_cmd+0x4b9/0x670 > > [ 195.799760] Modules linked in: usb_f_eem u_ether libcomposite > > brcmfmac brcmutil mmc_block pwm_lpss_pci pwm_lps > > s snd_sof_pci_intel_tng snd_sof_pci snd_sof_acpi_intel_byt > > snd_sof_intel_ipc snd_sof_acpi snd_sof snd_sof_nocodec > > spi_pxa2xx_platform snd_sof_xtensa_dsp spi_pxa2xx_pci > > extcon_intel_mrfld intel_mrfld_adc sdhci_pci cqhci sdhci m > > mc_core intel_mrfld_pwrbtn intel_soc_pmic_mrfld hci_uart btbcm btintel > > [ 195.835604] CPU: 0 PID: 217 Comm: irq/16-dwc3 Not tainted > > 5.12.0-rc4+ #60 > > [ 195.842403] Hardware name: Intel Corporation Merrifield/BODEGA BAY, > > BIOS 542 2015.01.21:18.19.48 > > [ 195.851191] RIP: 0010:dwc3_send_gadget_ep_cmd+0x4b9/0x670 > > [ 195.856608] Code: cd 00 00 00 44 89 44 24 20 48 89 4c 24 18 e8 ee > > f7 e4 ff 48 8b 4c 24 18 4c 89 f2 48 c7 c7 b9 > > ed 4f a0 48 89 c6 e8 ef 24 43 00 <0f> 0b 41 be ea ff ff ff 44 8b 44 24 > > 20 e9 80 fc ff ff 41 83 fe 92 > > [ 195.875381] RSP: :a53c00373ba8 EFLAGS: 00010086 > > [ 195.880617] RAX: RBX: 1387 RCX: > > dfff > > [ 195.887755] RDX: dfff RSI: ffea RDI: > > > > [ 195.894893] RBP: 9ce8c8f2b028 R08: a0732288 R09: > > 9ffb > > [ 195.902034] R10: e000 R11: 3fff R12: > > 00041006 > > [ 195.909170] R13: a53c00373c24 R14: 9ce8c11dadb0 R15: > > 9ce8c2861700 > > [ 195.916310] FS: () GS:9ce8fe20() > > knlGS: > > [ 195.924409] CS: 0010 DS: ES: CR0: 80050033 > > [ 195.930161] CR2: f7f694a0 CR3: 38e0c000 CR4: > > 001006f0 > > [ 195.937300] Call Trace: > > [ 195.939755] __dwc3_gadget_ep_enable+0x2d4/0x4e0 > > [ 195.944393] ? dwc3_remove_requests.constprop.0+0x86/0x170 > > Odd that this change would affect the USB enablment path, as they were > focused on the pullup disable path. Would you happen to have any > downstream changes on top of v5.12-rc4 we could review to see if they > are still required? (ie where is the dwc3_remove_requests() coming from > during ep enable) > >>> > >>> You may check my branch [1] on GH. Basically you may be interested in > >>> the commit: > >>> 0f86df1294ee7523060cc16eafaf4898c693eab0 REVERTME: usb: dwc3: gadget: > >>> skip endpoints ep[18]{in,out} > >>> Otherwise it's a clean v5.12-rc4 with a revert and another USB PHY > >>> suspend fix (which also shouldn't affect this). > >> > >> Can you link your GH reference? > > > > Oops, sorry. > > Here we are: > > > > [1]: https://github.com/andy-shev/linux/tree/eds-ac
Re: [PATCH v3] usb: dwc3: gadget: Prevent EP queuing while stopping transfers
Hi Andy, On 3/22/2021 2:14 PM, Andy Shevchenko wrote: > On Mon, Mar 22, 2021 at 10:06 PM Wesley Cheng wrote: >> >> Hi Andy, >> >> On 3/22/2021 12:34 PM, Andy Shevchenko wrote: >>> On Mon, Mar 22, 2021 at 8:49 PM Wesley Cheng wrote: Hi Andy, On 3/22/2021 5:48 AM, Andy Shevchenko wrote: > On Fri, Mar 12, 2021 at 2:01 AM Wesley Cheng > wrote: >> >> In the situations where the DWC3 gadget stops active transfers, once >> calling the dwc3_gadget_giveback(), there is a chance where a function >> driver can queue a new USB request in between the time where the dwc3 >> lock has been released and re-aquired. This occurs after we've already >> issued an ENDXFER command. When the stop active transfers continues >> to remove USB requests from all dep lists, the newly added request will >> also be removed, while controller still has an active TRB for it. >> This can lead to the controller accessing an unmapped memory address. >> >> Fix this by ensuring parameters to prevent EP queuing are set before >> calling the stop active transfers API. > > > commit f09ddcfcb8c569675066337adac2ac205113471f > Author: Wesley Cheng > Date: Thu Mar 11 15:59:02 2021 -0800 > >usb: dwc3: gadget: Prevent EP queuing while stopping transfers > > effectively broke my gadget setup. > > The output of the kernel (followed by non responsive state of USB > controller): > > [ 195.228586] using random self ethernet address > [ 195.233104] using random host ethernet address > [ 195.245306] usb0: HOST MAC aa:bb:cc:dd:ee:f2 > [ 195.249732] usb0: MAC aa:bb:cc:dd:ee:f1 > # [ 195.773594] IPv6: ADDRCONF(NETDEV_CHANGE): usb0: link becomes ready > [ 195.780585] [ cut here ] > [ 195.785217] dwc3 dwc3.0.auto: No resource for ep2in > [ 195.790162] WARNING: CPU: 0 PID: 217 at > drivers/usb/dwc3/gadget.c:360 dwc3_send_gadget_ep_cmd+0x4b9/0x670 > [ 195.799760] Modules linked in: usb_f_eem u_ether libcomposite > brcmfmac brcmutil mmc_block pwm_lpss_pci pwm_lps > s snd_sof_pci_intel_tng snd_sof_pci snd_sof_acpi_intel_byt > snd_sof_intel_ipc snd_sof_acpi snd_sof snd_sof_nocodec > spi_pxa2xx_platform snd_sof_xtensa_dsp spi_pxa2xx_pci > extcon_intel_mrfld intel_mrfld_adc sdhci_pci cqhci sdhci m > mc_core intel_mrfld_pwrbtn intel_soc_pmic_mrfld hci_uart btbcm btintel > [ 195.835604] CPU: 0 PID: 217 Comm: irq/16-dwc3 Not tainted 5.12.0-rc4+ > #60 > [ 195.842403] Hardware name: Intel Corporation Merrifield/BODEGA BAY, > BIOS 542 2015.01.21:18.19.48 > [ 195.851191] RIP: 0010:dwc3_send_gadget_ep_cmd+0x4b9/0x670 > [ 195.856608] Code: cd 00 00 00 44 89 44 24 20 48 89 4c 24 18 e8 ee > f7 e4 ff 48 8b 4c 24 18 4c 89 f2 48 c7 c7 b9 > ed 4f a0 48 89 c6 e8 ef 24 43 00 <0f> 0b 41 be ea ff ff ff 44 8b 44 24 > 20 e9 80 fc ff ff 41 83 fe 92 > [ 195.875381] RSP: :a53c00373ba8 EFLAGS: 00010086 > [ 195.880617] RAX: RBX: 1387 RCX: > dfff > [ 195.887755] RDX: dfff RSI: ffea RDI: > > [ 195.894893] RBP: 9ce8c8f2b028 R08: a0732288 R09: > 9ffb > [ 195.902034] R10: e000 R11: 3fff R12: > 00041006 > [ 195.909170] R13: a53c00373c24 R14: 9ce8c11dadb0 R15: > 9ce8c2861700 > [ 195.916310] FS: () GS:9ce8fe20() > knlGS: > [ 195.924409] CS: 0010 DS: ES: CR0: 80050033 > [ 195.930161] CR2: f7f694a0 CR3: 38e0c000 CR4: > 001006f0 > [ 195.937300] Call Trace: > [ 195.939755] __dwc3_gadget_ep_enable+0x2d4/0x4e0 > [ 195.944393] ? dwc3_remove_requests.constprop.0+0x86/0x170 Odd that this change would affect the USB enablment path, as they were focused on the pullup disable path. Would you happen to have any downstream changes on top of v5.12-rc4 we could review to see if they are still required? (ie where is the dwc3_remove_requests() coming from during ep enable) >>> >>> You may check my branch [1] on GH. Basically you may be interested in >>> the commit: >>> 0f86df1294ee7523060cc16eafaf4898c693eab0 REVERTME: usb: dwc3: gadget: >>> skip endpoints ep[18]{in,out} >>> Otherwise it's a clean v5.12-rc4 with a revert and another USB PHY >>> suspend fix (which also shouldn't affect this). >> >> Can you link your GH reference? > > Oops, sorry. > Here we are: > > [1]: https://github.com/andy-shev/linux/tree/eds-acpi > Thanks, I took a look and even tried it on my device running 5.12-rc4, but wasn't able to see the same problem. Could you help collect the ftrace after enabling the tracing KCONFIG and running the below sequence? 1. Mount debugfs 2. Set up tracing instance
Re: [PATCH v3] usb: dwc3: gadget: Prevent EP queuing while stopping transfers
On Mon, Mar 22, 2021 at 10:06 PM Wesley Cheng wrote: > > Hi Andy, > > On 3/22/2021 12:34 PM, Andy Shevchenko wrote: > > On Mon, Mar 22, 2021 at 8:49 PM Wesley Cheng wrote: > >> > >> Hi Andy, > >> > >> On 3/22/2021 5:48 AM, Andy Shevchenko wrote: > >>> On Fri, Mar 12, 2021 at 2:01 AM Wesley Cheng > >>> wrote: > > In the situations where the DWC3 gadget stops active transfers, once > calling the dwc3_gadget_giveback(), there is a chance where a function > driver can queue a new USB request in between the time where the dwc3 > lock has been released and re-aquired. This occurs after we've already > issued an ENDXFER command. When the stop active transfers continues > to remove USB requests from all dep lists, the newly added request will > also be removed, while controller still has an active TRB for it. > This can lead to the controller accessing an unmapped memory address. > > Fix this by ensuring parameters to prevent EP queuing are set before > calling the stop active transfers API. > >>> > >>> > >>> commit f09ddcfcb8c569675066337adac2ac205113471f > >>> Author: Wesley Cheng > >>> Date: Thu Mar 11 15:59:02 2021 -0800 > >>> > >>>usb: dwc3: gadget: Prevent EP queuing while stopping transfers > >>> > >>> effectively broke my gadget setup. > >>> > >>> The output of the kernel (followed by non responsive state of USB > >>> controller): > >>> > >>> [ 195.228586] using random self ethernet address > >>> [ 195.233104] using random host ethernet address > >>> [ 195.245306] usb0: HOST MAC aa:bb:cc:dd:ee:f2 > >>> [ 195.249732] usb0: MAC aa:bb:cc:dd:ee:f1 > >>> # [ 195.773594] IPv6: ADDRCONF(NETDEV_CHANGE): usb0: link becomes ready > >>> [ 195.780585] [ cut here ] > >>> [ 195.785217] dwc3 dwc3.0.auto: No resource for ep2in > >>> [ 195.790162] WARNING: CPU: 0 PID: 217 at > >>> drivers/usb/dwc3/gadget.c:360 dwc3_send_gadget_ep_cmd+0x4b9/0x670 > >>> [ 195.799760] Modules linked in: usb_f_eem u_ether libcomposite > >>> brcmfmac brcmutil mmc_block pwm_lpss_pci pwm_lps > >>> s snd_sof_pci_intel_tng snd_sof_pci snd_sof_acpi_intel_byt > >>> snd_sof_intel_ipc snd_sof_acpi snd_sof snd_sof_nocodec > >>> spi_pxa2xx_platform snd_sof_xtensa_dsp spi_pxa2xx_pci > >>> extcon_intel_mrfld intel_mrfld_adc sdhci_pci cqhci sdhci m > >>> mc_core intel_mrfld_pwrbtn intel_soc_pmic_mrfld hci_uart btbcm btintel > >>> [ 195.835604] CPU: 0 PID: 217 Comm: irq/16-dwc3 Not tainted 5.12.0-rc4+ > >>> #60 > >>> [ 195.842403] Hardware name: Intel Corporation Merrifield/BODEGA BAY, > >>> BIOS 542 2015.01.21:18.19.48 > >>> [ 195.851191] RIP: 0010:dwc3_send_gadget_ep_cmd+0x4b9/0x670 > >>> [ 195.856608] Code: cd 00 00 00 44 89 44 24 20 48 89 4c 24 18 e8 ee > >>> f7 e4 ff 48 8b 4c 24 18 4c 89 f2 48 c7 c7 b9 > >>> ed 4f a0 48 89 c6 e8 ef 24 43 00 <0f> 0b 41 be ea ff ff ff 44 8b 44 24 > >>> 20 e9 80 fc ff ff 41 83 fe 92 > >>> [ 195.875381] RSP: :a53c00373ba8 EFLAGS: 00010086 > >>> [ 195.880617] RAX: RBX: 1387 RCX: > >>> dfff > >>> [ 195.887755] RDX: dfff RSI: ffea RDI: > >>> > >>> [ 195.894893] RBP: 9ce8c8f2b028 R08: a0732288 R09: > >>> 9ffb > >>> [ 195.902034] R10: e000 R11: 3fff R12: > >>> 00041006 > >>> [ 195.909170] R13: a53c00373c24 R14: 9ce8c11dadb0 R15: > >>> 9ce8c2861700 > >>> [ 195.916310] FS: () GS:9ce8fe20() > >>> knlGS: > >>> [ 195.924409] CS: 0010 DS: ES: CR0: 80050033 > >>> [ 195.930161] CR2: f7f694a0 CR3: 38e0c000 CR4: > >>> 001006f0 > >>> [ 195.937300] Call Trace: > >>> [ 195.939755] __dwc3_gadget_ep_enable+0x2d4/0x4e0 > >>> [ 195.944393] ? dwc3_remove_requests.constprop.0+0x86/0x170 > >> > >> Odd that this change would affect the USB enablment path, as they were > >> focused on the pullup disable path. Would you happen to have any > >> downstream changes on top of v5.12-rc4 we could review to see if they > >> are still required? (ie where is the dwc3_remove_requests() coming from > >> during ep enable) > > > > You may check my branch [1] on GH. Basically you may be interested in > > the commit: > > 0f86df1294ee7523060cc16eafaf4898c693eab0 REVERTME: usb: dwc3: gadget: > > skip endpoints ep[18]{in,out} > > Otherwise it's a clean v5.12-rc4 with a revert and another USB PHY > > suspend fix (which also shouldn't affect this). > > Can you link your GH reference? Oops, sorry. Here we are: [1]: https://github.com/andy-shev/linux/tree/eds-acpi > > > > But I don't believe it should have affected this. > > > >>> [ 195.949897] dwc3_gadget_ep_enable+0x5d/0x120 > >>> [ 195.954274] usb_ep_enable+0x27/0x80 > >>> [ 195.957869] gether_connect+0x32/0x1f0 [u_ether] > >>> [ 195.962512] eem_set_alt+0x6d/0x140 [usb_f_eem] > >>> [ 195.967061] composite_setup+0x224/0x1ba0 [libcom
Re: [PATCH v3] usb: dwc3: gadget: Prevent EP queuing while stopping transfers
Hi Andy, On 3/22/2021 12:34 PM, Andy Shevchenko wrote: > On Mon, Mar 22, 2021 at 8:49 PM Wesley Cheng wrote: >> >> Hi Andy, >> >> On 3/22/2021 5:48 AM, Andy Shevchenko wrote: >>> On Fri, Mar 12, 2021 at 2:01 AM Wesley Cheng wrote: In the situations where the DWC3 gadget stops active transfers, once calling the dwc3_gadget_giveback(), there is a chance where a function driver can queue a new USB request in between the time where the dwc3 lock has been released and re-aquired. This occurs after we've already issued an ENDXFER command. When the stop active transfers continues to remove USB requests from all dep lists, the newly added request will also be removed, while controller still has an active TRB for it. This can lead to the controller accessing an unmapped memory address. Fix this by ensuring parameters to prevent EP queuing are set before calling the stop active transfers API. >>> >>> >>> commit f09ddcfcb8c569675066337adac2ac205113471f >>> Author: Wesley Cheng >>> Date: Thu Mar 11 15:59:02 2021 -0800 >>> >>>usb: dwc3: gadget: Prevent EP queuing while stopping transfers >>> >>> effectively broke my gadget setup. >>> >>> The output of the kernel (followed by non responsive state of USB >>> controller): >>> >>> [ 195.228586] using random self ethernet address >>> [ 195.233104] using random host ethernet address >>> [ 195.245306] usb0: HOST MAC aa:bb:cc:dd:ee:f2 >>> [ 195.249732] usb0: MAC aa:bb:cc:dd:ee:f1 >>> # [ 195.773594] IPv6: ADDRCONF(NETDEV_CHANGE): usb0: link becomes ready >>> [ 195.780585] [ cut here ] >>> [ 195.785217] dwc3 dwc3.0.auto: No resource for ep2in >>> [ 195.790162] WARNING: CPU: 0 PID: 217 at >>> drivers/usb/dwc3/gadget.c:360 dwc3_send_gadget_ep_cmd+0x4b9/0x670 >>> [ 195.799760] Modules linked in: usb_f_eem u_ether libcomposite >>> brcmfmac brcmutil mmc_block pwm_lpss_pci pwm_lps >>> s snd_sof_pci_intel_tng snd_sof_pci snd_sof_acpi_intel_byt >>> snd_sof_intel_ipc snd_sof_acpi snd_sof snd_sof_nocodec >>> spi_pxa2xx_platform snd_sof_xtensa_dsp spi_pxa2xx_pci >>> extcon_intel_mrfld intel_mrfld_adc sdhci_pci cqhci sdhci m >>> mc_core intel_mrfld_pwrbtn intel_soc_pmic_mrfld hci_uart btbcm btintel >>> [ 195.835604] CPU: 0 PID: 217 Comm: irq/16-dwc3 Not tainted 5.12.0-rc4+ #60 >>> [ 195.842403] Hardware name: Intel Corporation Merrifield/BODEGA BAY, >>> BIOS 542 2015.01.21:18.19.48 >>> [ 195.851191] RIP: 0010:dwc3_send_gadget_ep_cmd+0x4b9/0x670 >>> [ 195.856608] Code: cd 00 00 00 44 89 44 24 20 48 89 4c 24 18 e8 ee >>> f7 e4 ff 48 8b 4c 24 18 4c 89 f2 48 c7 c7 b9 >>> ed 4f a0 48 89 c6 e8 ef 24 43 00 <0f> 0b 41 be ea ff ff ff 44 8b 44 24 >>> 20 e9 80 fc ff ff 41 83 fe 92 >>> [ 195.875381] RSP: :a53c00373ba8 EFLAGS: 00010086 >>> [ 195.880617] RAX: RBX: 1387 RCX: >>> dfff >>> [ 195.887755] RDX: dfff RSI: ffea RDI: >>> >>> [ 195.894893] RBP: 9ce8c8f2b028 R08: a0732288 R09: >>> 9ffb >>> [ 195.902034] R10: e000 R11: 3fff R12: >>> 00041006 >>> [ 195.909170] R13: a53c00373c24 R14: 9ce8c11dadb0 R15: >>> 9ce8c2861700 >>> [ 195.916310] FS: () GS:9ce8fe20() >>> knlGS: >>> [ 195.924409] CS: 0010 DS: ES: CR0: 80050033 >>> [ 195.930161] CR2: f7f694a0 CR3: 38e0c000 CR4: >>> 001006f0 >>> [ 195.937300] Call Trace: >>> [ 195.939755] __dwc3_gadget_ep_enable+0x2d4/0x4e0 >>> [ 195.944393] ? dwc3_remove_requests.constprop.0+0x86/0x170 >> >> Odd that this change would affect the USB enablment path, as they were >> focused on the pullup disable path. Would you happen to have any >> downstream changes on top of v5.12-rc4 we could review to see if they >> are still required? (ie where is the dwc3_remove_requests() coming from >> during ep enable) > > You may check my branch [1] on GH. Basically you may be interested in > the commit: > 0f86df1294ee7523060cc16eafaf4898c693eab0 REVERTME: usb: dwc3: gadget: > skip endpoints ep[18]{in,out} > Otherwise it's a clean v5.12-rc4 with a revert and another USB PHY > suspend fix (which also shouldn't affect this). Can you link your GH reference? > > But I don't believe it should have affected this. > >>> [ 195.949897] dwc3_gadget_ep_enable+0x5d/0x120 >>> [ 195.954274] usb_ep_enable+0x27/0x80 >>> [ 195.957869] gether_connect+0x32/0x1f0 [u_ether] >>> [ 195.962512] eem_set_alt+0x6d/0x140 [usb_f_eem] >>> [ 195.967061] composite_setup+0x224/0x1ba0 [libcomposite] >>> [ 195.972405] ? debug_dma_unmap_page+0x79/0x80 >>> [ 195.976782] ? configfs_composite_setup+0x6b/0x90 [libcomposite] >>> [ 195.982811] configfs_composite_setup+0x6b/0x90 [libcomposite] >>> [ 195.988668] dwc3_ep0_interrupt+0x459/0xa50 >>> [ 195.992869] dwc3_thread_interrupt+0x8e2/0xee0 >>> [ 195.997327] ? __schedule+0x237/
Re: [PATCH v3] usb: dwc3: gadget: Prevent EP queuing while stopping transfers
On Mon, Mar 22, 2021 at 8:49 PM Wesley Cheng wrote: > > Hi Andy, > > On 3/22/2021 5:48 AM, Andy Shevchenko wrote: > > On Fri, Mar 12, 2021 at 2:01 AM Wesley Cheng wrote: > >> > >> In the situations where the DWC3 gadget stops active transfers, once > >> calling the dwc3_gadget_giveback(), there is a chance where a function > >> driver can queue a new USB request in between the time where the dwc3 > >> lock has been released and re-aquired. This occurs after we've already > >> issued an ENDXFER command. When the stop active transfers continues > >> to remove USB requests from all dep lists, the newly added request will > >> also be removed, while controller still has an active TRB for it. > >> This can lead to the controller accessing an unmapped memory address. > >> > >> Fix this by ensuring parameters to prevent EP queuing are set before > >> calling the stop active transfers API. > > > > > > commit f09ddcfcb8c569675066337adac2ac205113471f > > Author: Wesley Cheng > > Date: Thu Mar 11 15:59:02 2021 -0800 > > > >usb: dwc3: gadget: Prevent EP queuing while stopping transfers > > > > effectively broke my gadget setup. > > > > The output of the kernel (followed by non responsive state of USB > > controller): > > > > [ 195.228586] using random self ethernet address > > [ 195.233104] using random host ethernet address > > [ 195.245306] usb0: HOST MAC aa:bb:cc:dd:ee:f2 > > [ 195.249732] usb0: MAC aa:bb:cc:dd:ee:f1 > > # [ 195.773594] IPv6: ADDRCONF(NETDEV_CHANGE): usb0: link becomes ready > > [ 195.780585] [ cut here ] > > [ 195.785217] dwc3 dwc3.0.auto: No resource for ep2in > > [ 195.790162] WARNING: CPU: 0 PID: 217 at > > drivers/usb/dwc3/gadget.c:360 dwc3_send_gadget_ep_cmd+0x4b9/0x670 > > [ 195.799760] Modules linked in: usb_f_eem u_ether libcomposite > > brcmfmac brcmutil mmc_block pwm_lpss_pci pwm_lps > > s snd_sof_pci_intel_tng snd_sof_pci snd_sof_acpi_intel_byt > > snd_sof_intel_ipc snd_sof_acpi snd_sof snd_sof_nocodec > > spi_pxa2xx_platform snd_sof_xtensa_dsp spi_pxa2xx_pci > > extcon_intel_mrfld intel_mrfld_adc sdhci_pci cqhci sdhci m > > mc_core intel_mrfld_pwrbtn intel_soc_pmic_mrfld hci_uart btbcm btintel > > [ 195.835604] CPU: 0 PID: 217 Comm: irq/16-dwc3 Not tainted 5.12.0-rc4+ #60 > > [ 195.842403] Hardware name: Intel Corporation Merrifield/BODEGA BAY, > > BIOS 542 2015.01.21:18.19.48 > > [ 195.851191] RIP: 0010:dwc3_send_gadget_ep_cmd+0x4b9/0x670 > > [ 195.856608] Code: cd 00 00 00 44 89 44 24 20 48 89 4c 24 18 e8 ee > > f7 e4 ff 48 8b 4c 24 18 4c 89 f2 48 c7 c7 b9 > > ed 4f a0 48 89 c6 e8 ef 24 43 00 <0f> 0b 41 be ea ff ff ff 44 8b 44 24 > > 20 e9 80 fc ff ff 41 83 fe 92 > > [ 195.875381] RSP: :a53c00373ba8 EFLAGS: 00010086 > > [ 195.880617] RAX: RBX: 1387 RCX: > > dfff > > [ 195.887755] RDX: dfff RSI: ffea RDI: > > > > [ 195.894893] RBP: 9ce8c8f2b028 R08: a0732288 R09: > > 9ffb > > [ 195.902034] R10: e000 R11: 3fff R12: > > 00041006 > > [ 195.909170] R13: a53c00373c24 R14: 9ce8c11dadb0 R15: > > 9ce8c2861700 > > [ 195.916310] FS: () GS:9ce8fe20() > > knlGS: > > [ 195.924409] CS: 0010 DS: ES: CR0: 80050033 > > [ 195.930161] CR2: f7f694a0 CR3: 38e0c000 CR4: > > 001006f0 > > [ 195.937300] Call Trace: > > [ 195.939755] __dwc3_gadget_ep_enable+0x2d4/0x4e0 > > [ 195.944393] ? dwc3_remove_requests.constprop.0+0x86/0x170 > > Odd that this change would affect the USB enablment path, as they were > focused on the pullup disable path. Would you happen to have any > downstream changes on top of v5.12-rc4 we could review to see if they > are still required? (ie where is the dwc3_remove_requests() coming from > during ep enable) You may check my branch [1] on GH. Basically you may be interested in the commit: 0f86df1294ee7523060cc16eafaf4898c693eab0 REVERTME: usb: dwc3: gadget: skip endpoints ep[18]{in,out} Otherwise it's a clean v5.12-rc4 with a revert and another USB PHY suspend fix (which also shouldn't affect this). But I don't believe it should have affected this. > > [ 195.949897] dwc3_gadget_ep_enable+0x5d/0x120 > > [ 195.954274] usb_ep_enable+0x27/0x80 > > [ 195.957869] gether_connect+0x32/0x1f0 [u_ether] > > [ 195.962512] eem_set_alt+0x6d/0x140 [usb_f_eem] > > [ 195.967061] composite_setup+0x224/0x1ba0 [libcomposite] > > [ 195.972405] ? debug_dma_unmap_page+0x79/0x80 > > [ 195.976782] ? configfs_composite_setup+0x6b/0x90 [libcomposite] > > [ 195.982811] configfs_composite_setup+0x6b/0x90 [libcomposite] > > [ 195.988668] dwc3_ep0_interrupt+0x459/0xa50 > > [ 195.992869] dwc3_thread_interrupt+0x8e2/0xee0 > > [ 195.997327] ? __schedule+0x237/0x6d0 > > [ 196.001005] ? disable_irq_nosync+0x10/0x10 > > [ 196.005200] irq_thread_fn+0x1b/0x60 > > [ 196.008789] i
Re: [PATCH v3] usb: dwc3: gadget: Prevent EP queuing while stopping transfers
Hi Andy, On 3/22/2021 5:48 AM, Andy Shevchenko wrote: > On Fri, Mar 12, 2021 at 2:01 AM Wesley Cheng wrote: >> >> In the situations where the DWC3 gadget stops active transfers, once >> calling the dwc3_gadget_giveback(), there is a chance where a function >> driver can queue a new USB request in between the time where the dwc3 >> lock has been released and re-aquired. This occurs after we've already >> issued an ENDXFER command. When the stop active transfers continues >> to remove USB requests from all dep lists, the newly added request will >> also be removed, while controller still has an active TRB for it. >> This can lead to the controller accessing an unmapped memory address. >> >> Fix this by ensuring parameters to prevent EP queuing are set before >> calling the stop active transfers API. > > > commit f09ddcfcb8c569675066337adac2ac205113471f > Author: Wesley Cheng > Date: Thu Mar 11 15:59:02 2021 -0800 > >usb: dwc3: gadget: Prevent EP queuing while stopping transfers > > effectively broke my gadget setup. > > The output of the kernel (followed by non responsive state of USB controller): > > [ 195.228586] using random self ethernet address > [ 195.233104] using random host ethernet address > [ 195.245306] usb0: HOST MAC aa:bb:cc:dd:ee:f2 > [ 195.249732] usb0: MAC aa:bb:cc:dd:ee:f1 > # [ 195.773594] IPv6: ADDRCONF(NETDEV_CHANGE): usb0: link becomes ready > [ 195.780585] [ cut here ] > [ 195.785217] dwc3 dwc3.0.auto: No resource for ep2in > [ 195.790162] WARNING: CPU: 0 PID: 217 at > drivers/usb/dwc3/gadget.c:360 dwc3_send_gadget_ep_cmd+0x4b9/0x670 > [ 195.799760] Modules linked in: usb_f_eem u_ether libcomposite > brcmfmac brcmutil mmc_block pwm_lpss_pci pwm_lps > s snd_sof_pci_intel_tng snd_sof_pci snd_sof_acpi_intel_byt > snd_sof_intel_ipc snd_sof_acpi snd_sof snd_sof_nocodec > spi_pxa2xx_platform snd_sof_xtensa_dsp spi_pxa2xx_pci > extcon_intel_mrfld intel_mrfld_adc sdhci_pci cqhci sdhci m > mc_core intel_mrfld_pwrbtn intel_soc_pmic_mrfld hci_uart btbcm btintel > [ 195.835604] CPU: 0 PID: 217 Comm: irq/16-dwc3 Not tainted 5.12.0-rc4+ #60 > [ 195.842403] Hardware name: Intel Corporation Merrifield/BODEGA BAY, > BIOS 542 2015.01.21:18.19.48 > [ 195.851191] RIP: 0010:dwc3_send_gadget_ep_cmd+0x4b9/0x670 > [ 195.856608] Code: cd 00 00 00 44 89 44 24 20 48 89 4c 24 18 e8 ee > f7 e4 ff 48 8b 4c 24 18 4c 89 f2 48 c7 c7 b9 > ed 4f a0 48 89 c6 e8 ef 24 43 00 <0f> 0b 41 be ea ff ff ff 44 8b 44 24 > 20 e9 80 fc ff ff 41 83 fe 92 > [ 195.875381] RSP: :a53c00373ba8 EFLAGS: 00010086 > [ 195.880617] RAX: RBX: 1387 RCX: > dfff > [ 195.887755] RDX: dfff RSI: ffea RDI: > > [ 195.894893] RBP: 9ce8c8f2b028 R08: a0732288 R09: > 9ffb > [ 195.902034] R10: e000 R11: 3fff R12: > 00041006 > [ 195.909170] R13: a53c00373c24 R14: 9ce8c11dadb0 R15: > 9ce8c2861700 > [ 195.916310] FS: () GS:9ce8fe20() > knlGS: > [ 195.924409] CS: 0010 DS: ES: CR0: 80050033 > [ 195.930161] CR2: f7f694a0 CR3: 38e0c000 CR4: > 001006f0 > [ 195.937300] Call Trace: > [ 195.939755] __dwc3_gadget_ep_enable+0x2d4/0x4e0 > [ 195.944393] ? dwc3_remove_requests.constprop.0+0x86/0x170 Odd that this change would affect the USB enablment path, as they were focused on the pullup disable path. Would you happen to have any downstream changes on top of v5.12-rc4 we could review to see if they are still required? (ie where is the dwc3_remove_requests() coming from during ep enable) > [ 195.949897] dwc3_gadget_ep_enable+0x5d/0x120 > [ 195.954274] usb_ep_enable+0x27/0x80 > [ 195.957869] gether_connect+0x32/0x1f0 [u_ether] > [ 195.962512] eem_set_alt+0x6d/0x140 [usb_f_eem] > [ 195.967061] composite_setup+0x224/0x1ba0 [libcomposite] > [ 195.972405] ? debug_dma_unmap_page+0x79/0x80 > [ 195.976782] ? configfs_composite_setup+0x6b/0x90 [libcomposite] > [ 195.982811] configfs_composite_setup+0x6b/0x90 [libcomposite] > [ 195.988668] dwc3_ep0_interrupt+0x459/0xa50 > [ 195.992869] dwc3_thread_interrupt+0x8e2/0xee0 > [ 195.997327] ? __schedule+0x237/0x6d0 > [ 196.001005] ? disable_irq_nosync+0x10/0x10 > [ 196.005200] irq_thread_fn+0x1b/0x60 > [ 196.008789] irq_thread+0xd6/0x170 > [ 196.012202] ? irq_thread_check_affinity+0x70/0x70 > [ 196.017004] ? irq_forced_thread_fn+0x70/0x70 > [ 196.021373] kthread+0x116/0x130 > [ 196.024617] ? kthread_create_worker_on_cpu+0x60/0x60 > [ 196.029680] ret_from_fork+0x22/0x30 > [ 196.033272] ---[ end trace 8dd104a950d8d248 ]--- > > Also, as I mentioned above, the changes should affect the pullup disable path, so when you 'echo "" > UDC' or something similar to that operation, did you see any errors? Can you provide a ftrace output w/ the DWC3 tracing enabled once removing the UDC? >
Re: [PATCH v3] usb: dwc3: gadget: Prevent EP queuing while stopping transfers
On Fri, Mar 12, 2021 at 2:01 AM Wesley Cheng wrote: > > In the situations where the DWC3 gadget stops active transfers, once > calling the dwc3_gadget_giveback(), there is a chance where a function > driver can queue a new USB request in between the time where the dwc3 > lock has been released and re-aquired. This occurs after we've already > issued an ENDXFER command. When the stop active transfers continues > to remove USB requests from all dep lists, the newly added request will > also be removed, while controller still has an active TRB for it. > This can lead to the controller accessing an unmapped memory address. > > Fix this by ensuring parameters to prevent EP queuing are set before > calling the stop active transfers API. commit f09ddcfcb8c569675066337adac2ac205113471f Author: Wesley Cheng Date: Thu Mar 11 15:59:02 2021 -0800 usb: dwc3: gadget: Prevent EP queuing while stopping transfers effectively broke my gadget setup. The output of the kernel (followed by non responsive state of USB controller): [ 195.228586] using random self ethernet address [ 195.233104] using random host ethernet address [ 195.245306] usb0: HOST MAC aa:bb:cc:dd:ee:f2 [ 195.249732] usb0: MAC aa:bb:cc:dd:ee:f1 # [ 195.773594] IPv6: ADDRCONF(NETDEV_CHANGE): usb0: link becomes ready [ 195.780585] [ cut here ] [ 195.785217] dwc3 dwc3.0.auto: No resource for ep2in [ 195.790162] WARNING: CPU: 0 PID: 217 at drivers/usb/dwc3/gadget.c:360 dwc3_send_gadget_ep_cmd+0x4b9/0x670 [ 195.799760] Modules linked in: usb_f_eem u_ether libcomposite brcmfmac brcmutil mmc_block pwm_lpss_pci pwm_lps s snd_sof_pci_intel_tng snd_sof_pci snd_sof_acpi_intel_byt snd_sof_intel_ipc snd_sof_acpi snd_sof snd_sof_nocodec spi_pxa2xx_platform snd_sof_xtensa_dsp spi_pxa2xx_pci extcon_intel_mrfld intel_mrfld_adc sdhci_pci cqhci sdhci m mc_core intel_mrfld_pwrbtn intel_soc_pmic_mrfld hci_uart btbcm btintel [ 195.835604] CPU: 0 PID: 217 Comm: irq/16-dwc3 Not tainted 5.12.0-rc4+ #60 [ 195.842403] Hardware name: Intel Corporation Merrifield/BODEGA BAY, BIOS 542 2015.01.21:18.19.48 [ 195.851191] RIP: 0010:dwc3_send_gadget_ep_cmd+0x4b9/0x670 [ 195.856608] Code: cd 00 00 00 44 89 44 24 20 48 89 4c 24 18 e8 ee f7 e4 ff 48 8b 4c 24 18 4c 89 f2 48 c7 c7 b9 ed 4f a0 48 89 c6 e8 ef 24 43 00 <0f> 0b 41 be ea ff ff ff 44 8b 44 24 20 e9 80 fc ff ff 41 83 fe 92 [ 195.875381] RSP: :a53c00373ba8 EFLAGS: 00010086 [ 195.880617] RAX: RBX: 1387 RCX: dfff [ 195.887755] RDX: dfff RSI: ffea RDI: [ 195.894893] RBP: 9ce8c8f2b028 R08: a0732288 R09: 9ffb [ 195.902034] R10: e000 R11: 3fff R12: 00041006 [ 195.909170] R13: a53c00373c24 R14: 9ce8c11dadb0 R15: 9ce8c2861700 [ 195.916310] FS: () GS:9ce8fe20() knlGS: [ 195.924409] CS: 0010 DS: ES: CR0: 80050033 [ 195.930161] CR2: f7f694a0 CR3: 38e0c000 CR4: 001006f0 [ 195.937300] Call Trace: [ 195.939755] __dwc3_gadget_ep_enable+0x2d4/0x4e0 [ 195.944393] ? dwc3_remove_requests.constprop.0+0x86/0x170 [ 195.949897] dwc3_gadget_ep_enable+0x5d/0x120 [ 195.954274] usb_ep_enable+0x27/0x80 [ 195.957869] gether_connect+0x32/0x1f0 [u_ether] [ 195.962512] eem_set_alt+0x6d/0x140 [usb_f_eem] [ 195.967061] composite_setup+0x224/0x1ba0 [libcomposite] [ 195.972405] ? debug_dma_unmap_page+0x79/0x80 [ 195.976782] ? configfs_composite_setup+0x6b/0x90 [libcomposite] [ 195.982811] configfs_composite_setup+0x6b/0x90 [libcomposite] [ 195.988668] dwc3_ep0_interrupt+0x459/0xa50 [ 195.992869] dwc3_thread_interrupt+0x8e2/0xee0 [ 195.997327] ? __schedule+0x237/0x6d0 [ 196.001005] ? disable_irq_nosync+0x10/0x10 [ 196.005200] irq_thread_fn+0x1b/0x60 [ 196.008789] irq_thread+0xd6/0x170 [ 196.012202] ? irq_thread_check_affinity+0x70/0x70 [ 196.017004] ? irq_forced_thread_fn+0x70/0x70 [ 196.021373] kthread+0x116/0x130 [ 196.024617] ? kthread_create_worker_on_cpu+0x60/0x60 [ 196.029680] ret_from_fork+0x22/0x30 [ 196.033272] ---[ end trace 8dd104a950d8d248 ]--- Revert helps (I'm on v5.12-rc4 now with a revert). The script to enable gadget: #!/bin/sh -efu # Mounting CONFIGFS grep -q -w /sys/kernel/config /proc/mounts || mount -t configfs none /sys/kernel/config # Addresses and files readonly GADGET_BASE_DIR="/sys/kernel/config/usb_gadget/g1" readonly DEV_ETH_ADDR="aa:bb:cc:dd:ee:f1" readonly HOST_ETH_ADDR="aa:bb:cc:dd:ee:f2" readonly USBDISK="/usbdisk.img" # Insert modules modprobe libcomposite # Create directory structure mkdir "${GADGET_BASE_DIR}" cd "${GADGET_BASE_DIR}" mkdir -p configs/c.1/strings/0x409 mkdir -p strings/0x409 # Ethernet device mkdir functions/eem.usb0 echo "${DEV_ETH_ADDR}" > functions/eem.usb0/dev_addr echo "${HOST_ETH_ADDR}" > functions/eem.usb0/host_addr ln -s functions/eem.usb0 configs/c.1/ # Compos