From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marek Vasut Date: Sun, 7 Feb 2021 19:20:14 +0100 Subject: [PATCH] nvme: Fix cache alignment In-Reply-To: <20210204165747.GC10169@bill-the-cat> References: <20210130175340.114209-1-marek.vasut+renesas@gmail.com> <20210202162331.0bde7498@slackpad.fritz.box> <02e88524-e8d2-27e5-cfc8-e6d99bf532fc@gmail.com> <20210203104249.07b8a287@slackpad.fritz.box> <12728498-f69b-4570-725d-79efaa253070@gmail.com> <20210204102636.26a76dfe@slackpad.fritz.box> <20210204165747.GC10169@bill-the-cat> Message-ID: <571fcc35-303e-cedf-48ea-29c24bf09248@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 2/4/21 5:57 PM, Tom Rini wrote: [...] >>>>>>>> +static void nvme_flush_dcache_range(void *start, unsigned long size) >>>>>>>> +{ >>>>>>>> + unsigned long s, e; >>>>>>>> + nvme_align_dcache_range(start, size, &s, &e); >>>>>>>> + flush_dcache_range(s, e); >>>>>> >>>>>> There is no good reason for alignment restrictions when it comes to >>>>>> clean (& invalidate), so there is no need for this wrapper. >>>>> >>>>> Is that on ARM64-specific or is that applicable in general ? The driver >>>>> is expected to work on any CPU. >>>> >>>> Cache clean (actually: cache clean&invalidate) is what happens on evictions >>>> all of the time, at the cache controller's discretion. So there is no >>>> real harm in that operation per se. When an eviction happens on a >>>> *clean* cache line, this is basically just an invalidate, which is also not >>>> harmful. >>>> >>>> There are harmful cases when buffers sharing a cache line are both "just invalidated" >>>> and "cleaned" at different points in time. >>> >>> Is that on ARM64-specific or is that applicable in general ? (the above >>> does not answer that question) >> >> I would say that's a property of *every* write-back cache >> implementation: >> https://en.wikipedia.org/wiki/Cache_(computing)#/media/File:Write-back_with_write-allocation.svg > > I've been reading and digesting the thread as it goes, and the only > thing I do want to chime in on here right now is that yes, U-Boot > does and will continue to support every CPU that someone wants to run it > on, and one of the takeaways I see from this thread is we need some > better documented abstractions around cache, as it's very tricky to get > right all the time. Documenting the u-boot cache function behavior precisely is fine by me, but that is somewhat separate topic from this bugfix. So I will ask a simple question -- is there anything blocking this bugfix from being applied ?