linux-kernel - Re: [PATCH] usb: hub: try old enumeration scheme first for high speed devices

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 for Android: free password hash cracker in your pocket

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

Date:   Thu, 16 Aug 2018 10:31:42 +0300
From:   Roger Quadros <rogerq@...com>
To:     "Zengtao (B)" <prime.zeng@...ilicon.com>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "stern@...land.harvard.edu" <stern@...land.harvard.edu>,
        "mathias.nyman@...ux.intel.com" <mathias.nyman@...ux.intel.com>,
        "drinkcat@...omium.org" <drinkcat@...omium.org>,
        "felipe.balbi@...ux.intel.com" <felipe.balbi@...ux.intel.com>,
        "drake@...lessm.com" <drake@...lessm.com>,
        "mike.looijmans@...ic.nl" <mike.looijmans@...ic.nl>,
        "joe@...ches.com" <joe@...ches.com>
CC:     "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] usb: hub: try old enumeration scheme first for high speed
 devices

On 14/08/18 10:33, Zengtao (B) wrote:
> Hi Roger:
> 
> Thank you for review
> 
>> -----Original Message-----
>> From: Roger Quadros [mailto:rogerq@...com]
>> Sent: Friday, August 10, 2018 6:51 PM
>> To: Zengtao (B) <prime.zeng@...ilicon.com>; gregkh@...uxfoundation.org;
>> stern@...land.harvard.edu; mathias.nyman@...ux.intel.com;
>> drinkcat@...omium.org; felipe.balbi@...ux.intel.com; drake@...lessm.com;
>> mike.looijmans@...ic.nl; joe@...ches.com
>> Cc: linux-usb@...r.kernel.org; linux-kernel@...r.kernel.org
>> Subject: Re: [PATCH] usb: hub: try old enumeration scheme first for high speed
>> devices
>>
>> Hi,
>>
>> On 10/08/18 18:01, Zeng Tao wrote:
>>> The new scheme is required just to support legacy low and full-speed
>>> devices. For high speed devices, it will slower the enumeration speed.
>>> So in this patch we try the "old" enumeration scheme first for high
>>> speed devices.
>>
>> How slow does it get? Is it significant?
>> Do we risk breaking existing HS devices that work? I don't think we can be sure
>> till we run this through testing.
>>
> 
> We added the new scheme because this is what the windows did , and mainly for 
> legacy low and full speed devices.
> Now for new windows version(8.1 and later), the second port reset has already been
> removed for high speed devices for better enumeration speed.
> And In this patch if use_both_schemes is true, it will fallback to new scheme if the old
> scheme fails.
> 
> So I think it's reasonable to follow the windows behavior again.
> 
>>>
>>> Signed-off-by: Zeng Tao <prime.zeng@...ilicon.com>
>>> ---
>>>  drivers/usb/core/hub.c | 5 ++++-
>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index
>>> 1fb2668..d265b19 100644
>>> --- a/drivers/usb/core/hub.c
>>> +++ b/drivers/usb/core/hub.c
>>> @@ -2661,10 +2661,13 @@ static bool use_new_scheme(struct usb_device
>> *udev, int retry,
>>>  	int old_scheme_first_port =
>>>  		port_dev->quirks & USB_PORT_QUIRK_OLD_SCHEME;
>>>
>>> +	int quick_enumeration = (udev->speed == USB_SPEED_HIGH);
>>> +
>>>  	if (udev->speed >= USB_SPEED_SUPER)
>>>  		return false;
>>
>> how about replacing the above if with
>>
>> 	if (udev->speed >= USB_SPEED_HIGH)
>> 		return false;
> 
> No, for SS device, only use old scheme, but for speed device, we can fallback to new 
> scheme if the old fails.

You are correct.
> 
>>>
>>> -	return USE_NEW_SCHEME(retry, old_scheme_first_port ||
>> old_scheme_first);
>>> +	return USE_NEW_SCHEME(retry, old_scheme_first_port ||
>> old_scheme_first
>>> +			      || quick_enumeration);
>>>  }
>>
>> Now we no longer respect the "old_scheme_first" parameter for most of the
>> devices.
>>
>> It should be clarified in Documentation/admin/kernel-parameters.txt that
>> "old_scheme_first" is only applicable to LOW/FULL speed devices.
>>
> 
> On the contrary, new scheme is only applicable for LOW/FULL speed devices? 

No. What I meant is that the "old_scheme_first" module parameter is now
only applicable for LOW/FULL speed devices.

As HIGH_SPEED devices will always use old scheme first after this patch.

cheers,
-roger

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki