| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CALAqxLV=nNuP3-CXrfjR16jpH93TWuEthYJg6TiXj7iC4+4NkA@mail.gmail.com>
Date: Tue, 18 Jul 2017 13:12:07 -0700
From: John Stultz <john.stultz@...aro.org>
To: John Youn <John.Youn@...opsys.com>
Cc: Minas Harutyunyan <Minas.Harutyunyan@...opsys.com>,
lkml <linux-kernel@...r.kernel.org>,
Felipe Balbi <felipe.balbi@...ux.intel.com>
Subject: Re: bug? dwc2: insufficient fifo memory
On Mon, Jun 5, 2017 at 10:50 AM, John Youn <John.Youn@...opsys.com> wrote:
> On 6/5/2017 5:32 AM, Minas Harutyunyan wrote:
>> On 6/2/2017 10:20 PM, John Stultz wrote:
>>> On Mon, Apr 17, 2017 at 3:36 PM, John Stultz <john.stultz@...aro.org> wrote:
>>>> On Fri, Feb 24, 2017 at 2:46 PM, John Stultz <john.stultz@...aro.org> wrote:
>>>>> Hey John,
>>>>> So after the USB tree landed in 4.11-rc, I've been seeing the
>>>>> following warning at bootup.
>>>>>
>>>>> It seems the fifo_mem/total_fifo_size value on hikey is 1920, but I'm
>>>>> seeing the addresses zip upward quickly as the txfsz values are all
>>>>> 2048. Not exactly sure whats wrong here. Things still seem to work
>>>>> properly.
>>>>>
>>>>> thanks
>>>>> -john
>>>>>
>>>>>
>>>>> [ 8.944987] dwc2 f72c0000.usb: bound driver configfs-gadget
>>>>> [ 8.956651] insufficient fifo memory
>>>>> [ 8.956703] ------------[ cut here ]------------
>>>>> [ 8.964906] WARNING: CPU: 7 PID: 1 at drivers/usb/dwc2/gadget.c:330
>>>>> dwc2_hsotg_init_fifo+0x1a8/0x1c8
>>>>
>>>>
>>>> Hey John,
>>>> So I finally got a bit of time to look deeper into this, and it
>>>> seems like this issue was introduced by commit 3c6aea7344c3 ("usb:
>>>> dwc2: gadget: Add checking for g-tx-fifo-size parameter"), as that
>>>> change added the following snippit:
>>>>
>>>> if (hsotg->params.g_tx_fifo_size[fifo] < min ||
>>>> hsotg->params.g_tx_fifo_size[fifo] > dptxfszn) {
>>>> dev_warn(hsotg->dev, "%s: Invalid parameter
>>>> g_tx_fifo_size[%d]=%d\n",
>>>> __func__, fifo,
>>>> hsotg->params.g_tx_fifo_size[fifo]);
>>>> hsotg->params.g_tx_fifo_size[fifo] = dptxfszn;
>>>> }
>>>>
>>>> On HiKey, we have g-tx-fifo-size = <128 128 128 128 128 128> in the
>>>> dtsi, and the fifo_mem value ends up initialized at 1920.
>>>>
>>>> Unfortunately, in the above, it sees other entries in the
>>>> g_tx_fifo_size[] array are initialized to zero, which then causes them
>>>> to be set to the "default" value of dptxfszn which is 2048. So then
>>>> later in dwc2_hsotg_init_fifo() the addr value (which adds the
>>>> fifo_size array value each step) quickly grows beyond the fifo_mem
>>>> value, causing the warning.
>>>>
>>>> Not sure what the right fix is here? Should the min value be used
>>>> instead of the "default" dptxfszn value? Again, I'm not sure I see
>>>> any side-effects from this warning, but wanted to try to figure out
>>>> how to fix it properly.
>>>
>>> Hey John, Minas,
>>> Wanted to follow up on this again. I'm using a patch that looks as
>>> follows (sorry for the copy-paste whitespace damage) in the meantime
>>> to work around this issue:
>>>
>>> diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
>>> index 9cd8722..13a911b 100644
>>> --- a/drivers/usb/dwc2/params.c
>>> +++ b/drivers/usb/dwc2/params.c
>>> @@ -475,7 +475,7 @@ static void dwc2_check_param_tx_fifo_sizes(struct
>>> dwc2_hsotg *hsotg)
>>> dev_warn(hsotg->dev, "%s: Invalid parameter
>>> g_tx_fifo_size[%d]=%d\n",
>>> __func__, fifo,
>>> hsotg->params.g_tx_fifo_size[fifo]);
>>> - hsotg->params.g_tx_fifo_size[fifo] = dptxfszn;
>>> + hsotg->params.g_tx_fifo_size[fifo] = min;
>>> }
>>> }
>>> }
>>>
>>> Any suggestions on what a proper fix would look like?
>>>
>>> thanks
>>> -john
>>>
>>
>> Hi John,
>>
>> The patch series: "[PATCH 0/4] usb: dwc2: gadget: Fix dynamic FIFO
>> initialization flow" from Sevak Arakelyan submitted to LKML at 4/26/2017
>> should resolve this issue.
>>
>> Thanks,
>> Minas
>>
>
> Hi John,
>
> See if this patch helps and let us know.
>
> Unfortunately, the author of this patch (and expert on this part of
> the code) has left Synopsys. So it might take us a bit to look into
> this.
Hey Folks,
I realized I didn't get back to you, as I got pulled out to other
work. I was testing with 4.13-rc, and noticed the same issue, and
figured I should get back to you.
I've applied the 4 patches from the patch series you've pointed me to.
Though with this patchset I still hit the same warning, but this time
from dwc2_hsotg_core_init_disconnected() rather then
dwc2_hsotg_init_fifo().
[ 8.016067] init: processing action (sys.usb.config=adb &&
sys.usb.configfs=1 && sys.usb.ffs.ready=1) from
(/init.usb.configfs.rc:21)
[ 8.029367] dwc2 f72c0000.usb: bound driver configfs-gadget
[ 8.035140] ------------[ cut here ]------------
[ 8.043357] WARNING: CPU: 7 PID: 1 at drivers/usb/dwc2/gadget.c:317
dwc2_hsotg_core_init_disconnected+0x52c/0x560
[ 8.053643] CPU: 7 PID: 1 Comm: init Not tainted
4.13.0-rc1-00038-ga257c7e #3331
[ 8.061042] Hardware name: HiKey Development Board (DT)
[ 8.066285] task: ffffffc005f68000 task.stack: ffffffc005f70000
[ 8.072230] PC is at dwc2_hsotg_core_init_disconnected+0x52c/0x560
[ 8.078415] LR is at dwc2_hsotg_core_init_disconnected+0x52c/0x560
[ 8.084593] pc : [<ffffff80086a65ac>] lr : [<ffffff80086a65ac>]
pstate: 600001c5
[ 8.091987] sp : ffffffc005f73be0
[ 8.095298] x29: ffffffc005f73be0 x28: ffffff8008dc9a18
[ 8.100629] x27: ffffffc074d1907c x26: 000000000000000f
[ 8.105949] x25: 0000000000000007 x24: 000000000000011c
[ 8.111263] x23: 0000000000000580 x22: 0000000000000000
[ 8.116577] x21: 0000000000000007 x20: 0000000008000580
[ 8.121902] x19: ffffffc074d19018 x18: 0000000000000010
[ 8.127229] x17: aaaaaaaaaaaaaaab x16: ffffff80081da680
[ 8.132548] x15: ffffff8088e5b3a7 x14: 0000000000000006
[ 8.137863] x13: ffffff8008e5b3b5 x12: ffffff8008d89118
[ 8.143176] x11: 0000000000000000 x10: ffffffc005f73be0
[ 8.148493] x9 : ffffffc005f73be0 x8 : 79726f6d656d206f
[ 8.153807] x7 : 66696620746e6569 x6 : 0000000000000340
[ 8.159121] x5 : 0000000000000015 x4 : 0000000000000000
[ 8.164439] x3 : 0000000000000002 x2 : 0000000000000002
[ 8.169769] x1 : 0000000000000001 x0 : 0000000000000018
[ 8.175099] Call trace:
[ 8.177552] Exception stack(0xffffffc005f73a10 to 0xffffffc005f73b40)
[ 8.183990] 3a00:
ffffffc074d19018 0000008000000000
[ 8.191829] 3a20: ffffffc005f73be0 ffffff80086a65ac
0000000000000007 000000000000000f
[ 8.199664] 3a40: ffffffc074d1907c ffffff8008dc9a18
ffffffc005f73a90 0000000000000000
[ 8.207512] 3a60: ffffffc005f73be0 ffffffc005f73be0
ffffffc005f73ba0 00000000ffffffc8
[ 8.215362] 3a80: ffffffc005f73ab0 ffffff80081057a4
ffffffc005f73be0 ffffffc005f73be0
[ 8.223210] 3aa0: ffffffc005f73ba0 00000000ffffffc8
0000000000000018 0000000000000001
[ 8.231065] 3ac0: 0000000000000002 0000000000000002
0000000000000000 0000000000000015
[ 8.238919] 3ae0: 0000000000000340 66696620746e6569
79726f6d656d206f ffffffc005f73be0
[ 8.246774] 3b00: ffffffc005f73be0 0000000000000000
ffffff8008d89118 ffffff8008e5b3b5
[ 8.254627] 3b20: 0000000000000006 ffffff8088e5b3a7
ffffff80081da680 aaaaaaaaaaaaaaab
[ 8.262487] [<ffffff80086a65ac>]
dwc2_hsotg_core_init_disconnected+0x52c/0x560
[ 8.269740] [<ffffff80086a7038>] dwc2_hsotg_pullup+0x90/0x100
[ 8.275495] [<ffffff80086cb2a0>] usb_udc_connect_control.isra.0+0x78/0x90
[ 8.282308] [<ffffff80086cb3a4>] udc_bind_to_driver+0xec/0x110
[ 8.288166] [<ffffff80086cc308>] usb_gadget_probe_driver+0xa0/0x150
[ 8.294459] [<ffffff80086ca73c>] gadget_dev_desc_UDC_store+0xe4/0x100
[ 8.300931] [<ffffff8008258af8>] configfs_write_file+0xc0/0x198
[ 8.306881] [<ffffff80081da234>] __vfs_write+0x1c/0x118
[ 8.312133] [<ffffff80081da4d0>] vfs_write+0xa0/0x1b0
[ 8.317201] [<ffffff80081da6c4>] SyS_write+0x44/0xa0
[ 8.322193] [<ffffff8008082f30>] el0_svc_naked+0x24/0x28
[ 8.327508] ---[ end trace c6da4c029bccb75e ]---
The same hack I was using earlier
- hsotg->params.g_tx_fifo_size[fifo] = dptxfszn;
+ hsotg->params.g_tx_fifo_size[fifo] = min;
still avoids the issue.
thanks
-john
Powered by blists - more mailing lists