linux-kernel - Re: [PATCH 1/3] xhci: Ensure link state is U3 after setting USB_SS_PORT_LS

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:   Tue, 14 Jan 2020 17:07:39 +0200
From:   Mathias Nyman <mathias.nyman@...ux.intel.com>
To:     Kai-Heng Feng <kai.heng.feng@...onical.com>
Cc:     Mathias Nyman <mathias.nyman@...el.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Alan Stern <stern@...land.harvard.edu>,
        AceLan Kao <acelan.kao@...onical.com>,
        USB list <linux-usb@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/3] xhci: Ensure link state is U3 after setting
 USB_SS_PORT_LS_U3

On 13.1.2020 11.10, Kai-Heng Feng wrote:
> On Fri, Jan 10, 2020 at 5:33 PM Mathias Nyman
> <mathias.nyman@...ux.intel.com> wrote:
>>> @@ -1316,9 +1317,17 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
>>>                        msleep(20); /* wait device to enter */
>>>                        spin_lock_irqsave(&xhci->lock, flags);
>>>
>>> -                     temp = readl(ports[wIndex]->addr);
>>> -                     if (link_state == USB_SS_PORT_LS_U3)
>>> +                     if (link_state == USB_SS_PORT_LS_U3) {
>>> +                             retval = xhci_handshake(ports[wIndex]->addr, PORT_PLS_MASK, XDEV_U3, 80 * 1000);
>>> +                             if (retval)
>>> +                                     xhci_dbg(xhci, "polling XDEV_U3 on port %d-%d timeout\n", hcd->self.busnum, wIndex + 1);
>>
>> In worst case we are busylooping for 80ms here, keeping the cpu busy.
>> It should be ok to sleep here, so how about just reading the register
>> every 10ms max 10 times, sleeping in between.
> 
> Ok. Is the polling safe outside of spin_lock_irqsave()?
> 

Should be, we only read one 32 bit register, and we anyway used to release
and re-acquire the lock right before this anyway.

-Mathias