linux-kernel - Re: [PATCH] usb: dwc2: hcd: call dwc2_is_controller

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] [day] [month] [year] [list]

Date:   Fri, 15 Feb 2019 14:51:43 +0900
From:   Jaehoon Chung <jh80.chung@...sung.com>
To:     Minas Harutyunyan <minas.harutyunyan@...opsys.com>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>
Cc:     "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "m.szyprowski@...sung.com" <m.szyprowski@...sung.com>
Subject: Re: [PATCH] usb: dwc2: hcd: call dwc2_is_controller_alive under
 spinlock

Hi Minas,

On 2/14/19 8:43 PM, Minas Harutyunyan wrote:
> Hi Jaehoon Chung,
> 
> On 2/14/2019 2:04 PM, Jaehoon Chung wrote:
>> This patch is referred to Robert's patch
>> commit cf54772b913b ("usb: dwc2: call dwc2_is_controller_alive() under spinlock")
>>
>> During running sdb with otg mode, the usb is hung sometime.
>>
>> The one of SDB hang issues should be fixed with this patch.
>> After hang, it doesn't never trigger the usb interrupt.
>>
>> Signed-off-by: Jaehoon Chung <jh80.chung@...sung.com>
>> ---
>>   drivers/usb/dwc2/hcd_intr.c | 7 ++++---
>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/usb/dwc2/hcd_intr.c b/drivers/usb/dwc2/hcd_intr.c
>> index 88b5dcf..8d3b155 100644
>> --- a/drivers/usb/dwc2/hcd_intr.c
>> +++ b/drivers/usb/dwc2/hcd_intr.c
>> @@ -2222,13 +2222,13 @@ irqreturn_t dwc2_handle_hcd_intr(struct dwc2_hsotg *hsotg)
>>   	u32 gintsts, dbg_gintsts;
>>   	irqreturn_t retval = IRQ_NONE;
>>   
>> +	spin_lock(&hsotg->lock);
>> +
>>   	if (!dwc2_is_controller_alive(hsotg)) {
>>   		dev_warn(hsotg->dev, "Controller is dead\n");
>> -		return retval;
>> +		goto out;
>>   	}
>>   
>> -	spin_lock(&hsotg->lock);
>> -
>>   	/* Check if HOST Mode */
>>   	if (dwc2_is_host_mode(hsotg)) {
>>   		gintsts = dwc2_read_core_intr(hsotg);
>> @@ -2276,6 +2276,7 @@ irqreturn_t dwc2_handle_hcd_intr(struct dwc2_hsotg *hsotg)
>>   		}
>>   	}
>>   
>> +out:
>>   	spin_unlock(&hsotg->lock);
>>   
>>   	return retval;
>>
> 
> 1. Checking core alive or not was introduced in our internal reference 
> driver for our setups, because sometime AHB-PCI bridge can hung, not 
> core itself. If it happen then only way to overcome it - reboot core 
> setup and PC.
> 2. Actually this issue was fixed on our setups and currently no need to 
> check access via bridge to core.
> 3. Any case if it still happen on some platforms then first need to 
> check and fix HW

I didn't check with latest kernel. we're using v4.4.172.
This issue is occurred on only ARTIK boards.
- During running test about 10~15 hours, suddenly hang.
- It's impossible to fix HW side.

After applied this patch, passed the test case as 100%. (Before applied, it didn't pass the test.)

> 
> On asserted any interrupt for dwc2, handlers call sequence is follow: 
> common->gadget->host handlers. If core alive checked in common handler 
> then checking same stuff in host handler again not needed at all.

Right, it's the duplicated checking according to sequence.

> 
> So, core alive checking in host handler create issue for your test, I 
> suggest to not fix alive checking in host interrupt handler as you 
> suggested and done in common handler, but just remove it.

Will remove it.

Best Regards,
Jaehoon Chung

> 
> Thanks,
> Minas
> 
> 
> 
> 
>