linux-kernel - Re: [PATCH v2 3/4] usb: dwc3: add dual-role support

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 3 Apr 2017 05:31:39 +0000
From:   John Youn <John.Youn@...opsys.com>
To:     Felipe Balbi <felipe.balbi@...ux.intel.com>,
        Roger Quadros <rogerq@...com>,
        Mathias Nyman <mathias.nyman@...ux.intel.com>,
        John Youn <John.Youn@...opsys.com>
CC:     "vivek.gautam@...eaurora.org" <vivek.gautam@...eaurora.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 3/4] usb: dwc3: add dual-role support

On 03/30/2017 02:27 AM, Felipe Balbi wrote:
>
> Hi
>
> Roger Quadros <rogerq@...com> writes:
>>>>> For something that simple, we wouldn't even need to use OTG FSM layer
>>>>> because that brings no benefit for such a simple requirement.
>>>>
>>>> no no. I think you got it wrong. I'm not using the OTG FSM layer at all :).
>>>
>>> what are all the otg_fsm mentions then? Also you have:
>>>
>>>  +	struct usb_otg		otg;
>>>  +	struct otg_fsm		otg_fsm;
>>>
>>
>> I'll get rid of them. They aren't really needed.
>> Now I see why you thought I was using the otg_fsm layer. :)
>
> fair enough
>
>>>>> Can you either confirm or refute the statement above?
>>>>
>>>> The real problem was that if host adapter was removed during a system suspend
>>>> then while resuming XHCI was locking up like below. This is probably because
>>>> we're trying to remove/Halt the HCD in the otg_irq_handler before XHCI driver has resumed?
>>>>
>>>> How can we ensure that we call dwc3_host_exit() only *after* XHCI driver has resumed?
>>>
>>> Well, xHCI is our child, so driver model should *already* be
>>> guaranteeing that, no? Specially since you're calling this from
>>> ->complete() which happens after ->resume() has been called for the
>>> entire tree. It's true, however, that dwc3's ->complete() will be called
>>> before xhci's ->complete(). Is this the problem you're pointing at? Or
>>> do you mean xHCI is runtime suspended (or runtime resuming) while you
>>> call dwc3_host_exit()? If that's the case, then there's a bug in xHCI
>>> itself.
>>
>> Yes dwc3->complete() being called before xhci->complete(), and so HCD being
>> removed before xhci->complete() causes the lockup.
>>
>> It could be a bug in xHCI driver as well.
>
> I see...
>
>>>> We need a way to mask the OTG events without loosing them while they are masked.
>>>> Do you know how that could be achieved?
>>>
>>> masking doesn't clear events. It just masks them. Look at gadget.c for
>>> how we do it. Basically, the code we have here is racy, really racy and
>>> will cause hard-to-debug lockups. Your code can only work with
>>> IRQF_ONESHOT, which we don't want to add back.
>>>
>>> In any case, to mask events, you set BIT 31 in GEVNTSIZ register. Just
>>> copy it from gadget.c ;-)
>>
>> Isn't GEVNTSIZ specific to device side? We're talking about the OTG block here.
>
> that's true, sorry.
>
>> Are you sure that setting bit 31 of GEVNTSIZ will mask OTG_irq? I don't think so.
>>
>> Note that OTG_IRQ is a separate IRQ line than PERIPHERAL_IRQ.
>
> man, there's really no way to mask OTG events. This can be bad :-)
>
> John, can you confirm if there's really no way of masking OTG events
> without loosing them?

Not sure. I'll have to verify.

If the IP version is 3.00a or higher you could experiment with using
interrupt moderation as a global mask.

John