On Mon, 23 Jun 2025, Julien Grall wrote: > Hi Stefano, > > On 23/06/2025 20:27, Stefano Stabellini wrote: > > On Mon, 23 Jun 2025, Julien Grall wrote: > > > Hi Stefano, > > > > > > On 22/06/2025 23:15, Stefano Stabellini wrote: > > > > On Thu, 19 Jun 2025, Oleksii Moisieiev wrote: > > > > > On 18/06/2025 02:22, Stefano Stabellini wrote: > > > > > > On Thu, 12 Jun 2025, Oleksii Moisieiev wrote: > > > > > > > [1]:https://git.iliana.fyi/linux/patch/?id=d5141f37c42e0b833863f157ac4cee203b2ba3d2 > > > > > > Keep in mind that [0] refers specifically to access to MMIO regions. > > > > > > I > > > > > > assume that the SCMI shared buffers are on normal memory? Regarding > > > > > > [1], > > > > > > it makes sense if Linux is trying to support shared memory over > > > > > > MMIO. > > > > > > > > > > > > Looking at one of your replies below, I am guessing the memory > > > > > > buffers > > > > > > are actually in normal memory but the issue is that TF-A is mapping > > > > > > them > > > > > > as uncacheable. Is that correct? > > > > > > > > > > > > In that case, I still don't understand why a simple memcpy would not > > > > > > be > > > > > > sufficient. Can you check? > > > > > > > > > > > > If yes, then for now I would just simplify it down to memcpy. When > > > > > > someone adds support for an SCMI server elsewhere we could look into > > > > > > adding a more sophisticated memcpy and we can look at the details at > > > > > > that point in time. Specifically, I am not convinced that > > > > > > memcpy_toio > > > > > > and memcpy_fromio would work if the SCMI server is on a separate > > > > > > non-coherent microcontroller. > > > > > > > > > > > According to the TF-A implementation SCMI memory > > > > > > > > > > is mapped with the flags: MT_DEVICE (like for stm32mp1) or > > > > > MT_NON_CACHEABLE (for rpi3) > > > > > > > > > > So probably you're right. I will check with simple memcpy. > > > > > > > > There is a difference between MT_DEVICE and MT_NON_CACHEABLE: as far as > > > > I know MT_DEVICE requires aligned accesses while MT_NON_CACHEABLE does > > > > not. > > > > > > > > However, as I wrote in the other email, if I am not mistaken the current > > > > implementation of memcpy might work well for us anyway. (To be > > > > confirmed.) > > > > > > I am not entirely sure what exactly you want to confirm. I have already > > > mentioned several time that our memcpy() on arm64 is using unaligned > > > access. > > > So it can't be used for copying data to/from device memory area. > > > > I wrote it more clearly here: > > https://lore.kernel.org/xen-devel/alpine.DEB.2.22.394.2506221438250.8066@ubuntu-linux-20-04-desktop/ > > Ah I missed that e-mail! > > > > > Assuming that the address passed to memcpy is 4K aligned, then it seems > > to me that our memcpy implementation is using only aligned accesses. > > I didn't look at the mempcy() in details. But even if what you say is true, it > seems to be me this will be very fragile because we would assume: > * the addresses passed are always 4KB (I could not easily confirm it) > * the mempcy implementation will not change (I see Linux has updated theirs > in 2020 but we never did it...). > > I can't think of a compiel time check that would help to confirm any > assumptions above will always hold true. > > I also don't see what we would gain with implementing memcpy_toio() with > mempcy(). Maybe you can remind what's your concern with that? > > So right now, I feel Oleksii approach is the best.
OK fair enough :-) I was trying to avoid introducing two functions that seemed unnecessary. If we go with Oleksii's approach, where do you think memcpy_toio() should be added? Oleksii added them to the scmi file, maybe we want to add them in a more generic location?
