Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
From:   =?UTF-8?B?7KCV7J6s7ZuI?= <jh0801.jung@samsung.com>
To:     "'Thinh Nguyen'" <Thinh.Nguyen@synopsys.com>,
        "'Felipe Balbi'" <balbi@kernel.org>,
        "'Greg Kroah-Hartman'" <gregkh@linuxfoundation.org>
Cc:     "'open list:USB XHCI DRIVER'" <linux-usb@vger.kernel.org>,
        "'open list'" <linux-kernel@vger.kernel.org>,
        "'Seungchull Suh'" <sc.suh@samsung.com>,
        "'Daehwan Jung'" <dh10.jung@samsung.com>, <cpgs@samsung.com>,
        <cpgsproxy5@samsung.com>
In-Reply-To: <99eabe6c-8782-1bc6-e1d7-09b019cee65b@synopsys.com>
Subject: RE: [PATCH] usb: dwc3: Add dwc3 lock for blocking interrupt
 storming
Date:   Fri, 11 Mar 2022 10:29:27 +0900
Message-ID: <016a01d834e7$72b11fd0$58135f70$@samsung.com>
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Language: ko
Thread-Index: AQKjdV3S7gSSYh+T3/lJgRyIlCpbTQJQQbEmAWDN/4GrBVmwIA==
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 102P
DLP-Filter: Pass
References: <CGME20220307052605epcas2p2b84f6db2642863ed61373070f508e200@epcas2p2.samsung.com>
        <1646630679-121585-1-git-send-email-jh0801.jung@samsung.com>
        <99eabe6c-8782-1bc6-e1d7-09b019cee65b@synopsys.com>
Precedence: bulk

Hi.

> -----Original Message-----
> From: Thinh Nguyen =5Bmailto:Thinh.Nguyen=40synopsys.com=5D
> Sent: Thursday, March 10, 2022 11:14 AM
> To: JaeHun Jung; Felipe Balbi; Greg Kroah-Hartman
> Cc: open list:USB XHCI DRIVER; open list; Seungchull Suh; Daehwan Jung
> Subject: Re: =5BPATCH=5D usb: dwc3: Add dwc3 lock for blocking interrupt
> storming
>=20
> Hi,
>=20
> JaeHun Jung wrote:
> > Interrupt Storming occurred with a very low probability of occurrence.
> > The occurrence of the problem is estimated to be caused by a race
> > condition between the top half and bottom half of the interrupt service
> routine.
> > It was confirmed that variables have values that cannot be held when
> > ISR occurs through normal H / W irq.
> > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> > (struct dwc3_event_buffer *) ev_buf =3D 0xFFFFFF88DE6A0380 (
> > 	(void *) buf =3D 0xFFFFFFC01594E000,
> > 	(void *) cache =3D 0xFFFFFF88DDC14080,
> > 	(unsigned int) length =3D 4096,
> > 	(unsigned int) lpos =3D 0,
> > 	(unsigned int) count =3D 0, <<
> > 	(unsigned int) flags =3D 1, <<
> > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> > =22evt->count=3D0=22 and =22evt->flags=3DDWC3_EVENT_PENDING=22 cannot b=
e set at
> > the same time.
> >
> > We estimate that a race condition occurred between dwc3_interrupt()
> > and dwc3_process_event_buf() called by
> > dwc3_gadget_process_pending_events().
> > So I try to block the race condition through spin_lock.
>=20
> This looks like it needs a memory barrier. Would this work for you?
Maybe it could be. But =22evt->count =3D 0;=22 is updated on dwc3_process_e=
vent_buf().
So, I think spin_lock is more clear routine for this issue.

>=20
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index
> c02e239978e0..a96c344b9f17 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> =40=40 -5340,6 +5340,9 =40=40 static irqreturn_t dwc3_check_event_buf(str=
uct
> dwc3_event_buffer *evt)
>                 return IRQ_HANDLED;
>         =7D
>=20
> +       /* Make sure the event flags is updated */
> +       wmb();
> +
>         /*
>          * With PCIe legacy interrupt, test shows that top-half irq handl=
er
> can
>          * be called again after HW interrupt deassertion. Check if botto=
m-
> half
>=20
>=20
> Thanks,
> Thinh