All of lore.kernel.org
 help / color / mirror / Atom feed
From: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] usb: dwc3: gadget: make cache-maintenance on event buffers more robust
Date: Mon,  3 Apr 2017 19:49:58 +0200	[thread overview]
Message-ID: <1491241798-37084-1-git-send-email-philipp.tomsich@theobroma-systems.com> (raw)

Merely using dma_alloc_coherent does not ensure that there is no stale
data left in the caches for the allocated DMA buffer (i.e. that the
affected cacheline may still be dirty).

The original code was doing the following (on AArch64, which
translates a 'flush' into a 'clean + invalidate'):
  # during initialisation:
      1. allocate buffers via memalign
      	 => buffers may still be modified (cached, dirty)
  # during interrupt processing
      2. clean + invalidate buffers
      	 => may commit stale data from a modified cacheline
      3. read from buffers

This could lead to garbage info being written to buffers before
reading them during even-processing.

To make the event processing more robust, we use the following sequence
for the cache-maintenance:
  # during initialisation:
      1. allocate buffers via memalign
      2. clean + invalidate buffers
      	 (we only need the 'invalidate' part, but dwc3_flush_cache()
	  always performs a 'clean + invalidate')
  # during interrupt processing
      3. read the buffers
      	 (we know these lines are not cached, due to the previous
	  invalidation and no other code touching them in-between)
      4. clean + invalidate buffers
      	 => writes back any modification we may have made during event
	    processing and ensures that the lines are not in the cache
	    the next time we enter interrupt processing

Note that with the original sequence, we observe reproducible
(depending on the cache state: i.e. running dhcp/usb start before will
upset caches to get us around this) issues in the event processing (a
fatal synchronous abort in dwc3_gadget_uboot_handle_interrupt on the
first time interrupt handling is invoked) when running USB mass
storage emulation on our RK3399-Q7 with data-caches on.

Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>

---

 drivers/usb/dwc3/core.c   | 2 ++
 drivers/usb/dwc3/gadget.c | 5 +++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index b2c7eb1..f58c7ba 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -125,6 +125,8 @@ static struct dwc3_event_buffer *dwc3_alloc_one_event_buffer(struct dwc3 *dwc,
 	if (!evt->buf)
 		return ERR_PTR(-ENOMEM);
 
+	dwc3_flush_cache((long)evt->buf, evt->length);
+
 	return evt;
 }
 
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 1156662..61af71b 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2668,11 +2668,12 @@ void dwc3_gadget_uboot_handle_interrupt(struct dwc3 *dwc)
 		int i;
 		struct dwc3_event_buffer *evt;
 
+		dwc3_thread_interrupt(0, dwc);
+
+		/* Clean + Invalidate the buffers after touching them */
 		for (i = 0; i < dwc->num_event_buffers; i++) {
 			evt = dwc->ev_buffs[i];
 			dwc3_flush_cache((long)evt->buf, evt->length);
 		}
-
-		dwc3_thread_interrupt(0, dwc);
 	}
 }
-- 
1.9.1

             reply	other threads:[~2017-04-03 17:49 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-03 17:49 Philipp Tomsich [this message]
2017-04-04 16:15 ` [U-Boot] [PATCH] usb: dwc3: gadget: make cache-maintenance on event buffers more robust Marek Vasut
2017-04-04 17:46   ` Dr. Philipp Tomsich
2017-04-04 19:01     ` Marek Vasut
2017-04-04 19:56       ` Dr. Philipp Tomsich
2017-04-04 20:09         ` Marek Vasut
2017-04-04 20:26           ` Dr. Philipp Tomsich
2017-04-05 10:25             ` Marek Vasut
2017-04-05 10:57               ` Dr. Philipp Tomsich
2017-04-05 11:25                 ` Marek Vasut
2017-04-05  8:18       ` Felipe Balbi
2017-04-05  8:33         ` Dr. Philipp Tomsich
2017-04-05  9:49           ` Felipe Balbi
2017-04-05  8:43         ` Marek Vasut
2017-04-05  8:15   ` Felipe Balbi
2017-04-05  8:43     ` Marek Vasut
2017-04-08  6:09 mohammadjannati04

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=1491241798-37084-1-git-send-email-philipp.tomsich@theobroma-systems.com \
    --to=philipp.tomsich@theobroma-systems.com \
    --cc=u-boot@lists.denx.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.