From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Glass Date: Mon, 3 Apr 2017 14:10:40 -0600 Subject: [U-Boot] [PATCH v5 02/19] usb: dwc2: Use separate input and output buffers In-Reply-To: <1636070.mxeDJnAFIs@sbruens-linux> References: <20170401180556.2416-1-sjg@chromium.org> <1839232.Ocurkfb8WW@sbruens-linux> <1636070.mxeDJnAFIs@sbruens-linux> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: u-boot@lists.denx.de Hi Stefan, On 3 April 2017 at 12:24, Brüns, Stefan wrote: > On Montag, 3. April 2017 20:18:42 CEST Brüns, Stefan wrote: >> On Montag, 3. April 2017 17:38:40 CEST you wrote: >> > >> > What am I missing? >> >> The following is a gross oversimplification, but might explain it: >> >> 1. Assume all of the 64kB of the aligned_buffer is dirty. (Likely, if the >> buffer is calloced.) >> 2. We are doing some transfers. All transfers are small, e.g. 64 byte. >> 3. In accordance with the two cases you mentioned above, the first 64 byte >> are no longer dirty, as the last out transfer has flushed the cacheline. 4. >> We are doing our first large in transfer (i.e. larger than 64 byte). 5. Bad >> Things (TM) may happen to any data at aligned_buffer[64] and beyond. >> >> If this holds, a single invalidate_dcache_range(aligned_buffer, >> aligned_buffer +65536,...) during the initialization of the controller >> would suffice. > > I just read "[U-Boot] [PATCH] usb: dwc3: gadget: make cache-maintenance on > event buffers more robust" from Philipp Tomsich, which mentions the same > problem: > > 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') Yes that explains why the cache is dirty when the driver starts. I hadn't thought of that. IMO changing the init sequence is a better solution. Regards, Simon