mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
To: Krishna Kurapati <quic_kriskura@quicinc.com>
Cc: Thinh Nguyen <Thinh.Nguyen@synopsys.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"quic_ppratap@quicinc.com" <quic_ppratap@quicinc.com>,
	"quic_wcheng@quicinc.com" <quic_wcheng@quicinc.com>,
	"quic_jackp@quicinc.com" <quic_jackp@quicinc.com>,
	"quic_ugoswami@quicinc.com" <quic_ugoswami@quicinc.com>
Subject: Re: [RFC] usb: dwc3: gadget: Fix amount of data copied from event buf to cache
Date: Mon, 22 May 2023 21:17:01 +0000	[thread overview]
Message-ID: <20230522211651.rr2r3caz6ni7m6xr@synopsys.com> (raw)
In-Reply-To: <20230521100330.22478-1-quic_kriskura@quicinc.com>

On Sun, May 21, 2023, Krishna Kurapati wrote:
> In the current implementation, the check_event_buf call reads the
> GEVNTCOUNT register to determine the amount of event data generated
> and copies it from ev->buf to ev->cache after masking interrupt.
> 
> During copy if the amount of data to be copied is more than
> (length - lpos), we fill the ev->cache till the end of 4096 byte
> buffer allocated and then start filling from the top (lpos = 0).
> 
> In one instance of SMMU crash it is observed that GEVNTCOUNT register
> reads more than 4096 bytes:
> 
> dwc3_readl   base=0xffffffc0091dc000  offset=50188  value=63488
> 
> (offset = 50188 -> 0xC40C)  -> reads 63488 bytes
> 
> As per crash dump:
> dwc->lpos = 2056
> 
> and evt->buf is at 0xFFFFFFC009185000 and the crash is at
> 0xFFFFFFC009186000. The diff which is exactly 0x1000 bytes.
> 
> We first memcpy 2040 bytes from (buf + lpos) to (buf + 0x1000).
> 
> And then we copy the rest of the data (64388 - 2040) from beginning
> of dwc->ev_buf. While doing so we go beyond bounds as we are trying
> to memcpy 62348 bytes into a 4K buffer resulting in crash.
> 
> Fix this by limiting the total data being copied to ev->length to
> avoid copying data beyond bounds. Moreover this is logical because if
> the controller generated events more than the size of ring buffer,
> some of them might have been overwritten by the controller.
> 
> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
> ---
> Only one instance of this crash was observed so far. As per the
> databook:
> 
> "The controller always leaves one entry free in each Event Buffer.
> When the Event Buffer is almost full, hardware writes the Event
> Buffer Overflow event and the USB eventually gets stalled when
> endpoints start responding NRDY or the link layer stops returning
> credits (in SuperSpeed). This event is an indication to software that
> it is not processing events quickly enough. During this time, events
> are queued up internally. When software frees up Event Buffer space,
> the queued up events are written out and the USB returns to normal
> operation"
> 
> I didn't see any overflow event in the event buffer after parsing
> crash dump. Although this could be some HW issue, I thought we can
> include this fix in software as well to avoid such scenario.
> 

What's the GEVNTSIZ at the point of the crash? That's where the driver
tells the controller how much it allocated for the event buffer.

Check to make sure that it wasn't reset during operation (not cleanup).

BR,
Thinh

  reply	other threads:[~2023-05-22 21:17 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-21 10:03 Krishna Kurapati
2023-05-22 21:17 ` Thinh Nguyen [this message]
2023-05-23  1:25   ` Krishna Kurapati PSSNV
2023-05-23 21:22     ` Thinh Nguyen
2023-05-24  3:25       ` Krishna Kurapati PSSNV
2023-05-25  0:18         ` Thinh Nguyen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230522211651.rr2r3caz6ni7m6xr@synopsys.com \
    --to=thinh.nguyen@synopsys.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=quic_jackp@quicinc.com \
    --cc=quic_kriskura@quicinc.com \
    --cc=quic_ppratap@quicinc.com \
    --cc=quic_ugoswami@quicinc.com \
    --cc=quic_wcheng@quicinc.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®