From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S936426Ab2C3V7r (ORCPT ); Fri, 30 Mar 2012 17:59:47 -0400 Received: from mail-pb0-f46.google.com ([209.85.160.46]:34386 "EHLO mail-pb0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965553Ab2C3VRs (ORCPT ); Fri, 30 Mar 2012 17:17:48 -0400 Message-Id: <20120330194828.996333504@linuxfoundation.org> User-Agent: quilt/0.60-19.1 Date: Fri, 30 Mar 2012 12:48:47 -0700 From: Greg KH To: linux-kernel@vger.kernel.org, stable@vger.kernel.org Cc: torvalds@linux-foundation.org, akpm@linux-foundation.org, alan@lxorguk.ukuu.org.uk, Supriya Karanth , Praveena Nadahally , srinidhi kasagar , Felipe Balbi Subject: [ 022/175] usb: musb: Reselect index reg in interrupt context In-Reply-To: <20120330195801.GA31806@kroah.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 3.3-stable review patch. If anyone has any objections, please let me know. ------------------ From: Supriya Karanth commit 39287076e46d2c19aaceaa6f0a44168ae4d257ec upstream. musb INDEX register is getting modified/corrupted during temporary un-locking in a SMP system. Set this register with proper value after re-acquiring the lock Scenario: --------- CPU1 is handling a data transfer completion interrupt received for the CLASS1 EP CPU2 is handling a CLASS2 thread which is queuing data to musb for transfer Below is the error sequence: CPU1 | CPU2 -------------------------------------------------------------------- Data transfer completion inter- | rupt recieved. | | musb INDEX reg set to CLASS1 EP | | musb LOCK is acquired. | | | CLASS2 thread queues data. | | CLASS2 thread tries to acquire musb | LOCK but lock is already taken by | CLASS1, so CLASS2 thread is | spinning. | >>From Interrupt Context musb | giveback function is called | | The giveback function releases | CLASS2 thread now acquires LOCK LOCK | | ClASS1 Request's completion cal-| ClASS2 schedules the data transfer and lback is called | sets the MUSB INDEX to Class2 EP number | Interrupt handler for CLASS1 EP | tries to acquire LOCK and is | spinning | | Interrupt for Class1 EP acquires| Class2 completes the scheduling etc and the MUSB LOCK | releases the musb LOCK | Interrupt for Class1 EP schedul-| es the next data transfer | but musb INDEX register is still| set to CLASS2 EP | Since the MUSB INDEX register is set to a different endpoint, we read and modify the wrong registers. Hence data transfer will not happen properly. This results in unpredictable behavior So, the MUSB INDEX register is set to proper value again when interrupt re-acquires the lock Signed-off-by: Supriya Karanth Signed-off-by: Praveena Nadahally Reviewed-by: srinidhi kasagar Signed-off-by: Felipe Balbi --- drivers/usb/musb/musb_gadget.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) --- a/drivers/usb/musb/musb_gadget.c +++ b/drivers/usb/musb/musb_gadget.c @@ -574,6 +574,15 @@ void musb_g_tx(struct musb *musb, u8 epn if (request->actual == request->length) { musb_g_giveback(musb_ep, request, 0); + /* + * In the giveback function the MUSB lock is + * released and acquired after sometime. During + * this time period the INDEX register could get + * changed by the gadget_queue function especially + * on SMP systems. Reselect the INDEX to be sure + * we are reading/modifying the right registers + */ + musb_ep_select(mbase, epnum); req = musb_ep->desc ? next_request(musb_ep) : NULL; if (!req) { dev_dbg(musb->controller, "%s idle now\n", @@ -983,6 +992,15 @@ void musb_g_rx(struct musb *musb, u8 epn } #endif musb_g_giveback(musb_ep, request, 0); + /* + * In the giveback function the MUSB lock is + * released and acquired after sometime. During + * this time period the INDEX register could get + * changed by the gadget_queue function especially + * on SMP systems. Reselect the INDEX to be sure + * we are reading/modifying the right registers + */ + musb_ep_select(mbase, epnum); req = next_request(musb_ep); if (!req) From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Message-Id: <20120330194828.996333504@linuxfoundation.org> Date: Fri, 30 Mar 2012 12:48:47 -0700 From: Greg KH To: linux-kernel@vger.kernel.org, stable@vger.kernel.org Cc: torvalds@linux-foundation.org, akpm@linux-foundation.org, alan@lxorguk.ukuu.org.uk, Supriya Karanth , Praveena Nadahally , srinidhi kasagar , Felipe Balbi Subject: [ 022/175] usb: musb: Reselect index reg in interrupt context In-Reply-To: <20120330195801.GA31806@kroah.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: 3.3-stable review patch. If anyone has any objections, please let me know. ------------------ From: Supriya Karanth commit 39287076e46d2c19aaceaa6f0a44168ae4d257ec upstream. musb INDEX register is getting modified/corrupted during temporary un-locking in a SMP system. Set this register with proper value after re-acquiring the lock Scenario: --------- CPU1 is handling a data transfer completion interrupt received for the CLASS1 EP CPU2 is handling a CLASS2 thread which is queuing data to musb for transfer Below is the error sequence: CPU1 | CPU2 -------------------------------------------------------------------- Data transfer completion inter- | rupt recieved. | | musb INDEX reg set to CLASS1 EP | | musb LOCK is acquired. | | | CLASS2 thread queues data. | | CLASS2 thread tries to acquire musb | LOCK but lock is already taken by | CLASS1, so CLASS2 thread is | spinning. | >>>From Interrupt Context musb | giveback function is called | | The giveback function releases | CLASS2 thread now acquires LOCK LOCK | | ClASS1 Request's completion cal-| ClASS2 schedules the data transfer and lback is called | sets the MUSB INDEX to Class2 EP number | Interrupt handler for CLASS1 EP | tries to acquire LOCK and is | spinning | | Interrupt for Class1 EP acquires| Class2 completes the scheduling etc and the MUSB LOCK | releases the musb LOCK | Interrupt for Class1 EP schedul-| es the next data transfer | but musb INDEX register is still| set to CLASS2 EP | Since the MUSB INDEX register is set to a different endpoint, we read and modify the wrong registers. Hence data transfer will not happen properly. This results in unpredictable behavior So, the MUSB INDEX register is set to proper value again when interrupt re-acquires the lock Signed-off-by: Supriya Karanth Signed-off-by: Praveena Nadahally Reviewed-by: srinidhi kasagar Signed-off-by: Felipe Balbi --- drivers/usb/musb/musb_gadget.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) --- a/drivers/usb/musb/musb_gadget.c +++ b/drivers/usb/musb/musb_gadget.c @@ -574,6 +574,15 @@ void musb_g_tx(struct musb *musb, u8 epn if (request->actual == request->length) { musb_g_giveback(musb_ep, request, 0); + /* + * In the giveback function the MUSB lock is + * released and acquired after sometime. During + * this time period the INDEX register could get + * changed by the gadget_queue function especially + * on SMP systems. Reselect the INDEX to be sure + * we are reading/modifying the right registers + */ + musb_ep_select(mbase, epnum); req = musb_ep->desc ? next_request(musb_ep) : NULL; if (!req) { dev_dbg(musb->controller, "%s idle now\n", @@ -983,6 +992,15 @@ void musb_g_rx(struct musb *musb, u8 epn } #endif musb_g_giveback(musb_ep, request, 0); + /* + * In the giveback function the MUSB lock is + * released and acquired after sometime. During + * this time period the INDEX register could get + * changed by the gadget_queue function especially + * on SMP systems. Reselect the INDEX to be sure + * we are reading/modifying the right registers + */ + musb_ep_select(mbase, epnum); req = next_request(musb_ep); if (!req)