Re: ARCH_DMA_MINALIGN on PA-RISC

[James Bottomley <James.Bottomley@HansenPartnership.com>]

On Thu, 2024-07-25 at 19:27 +0200, Mikulas Patocka wrote:
> 
> 
> On Thu, 25 Jul 2024, John David Anglin wrote:
> 
> > On 2024-07-24 3:25 p.m., James Bottomley wrote:
> > > On Wed, 2024-07-24 at 20:17 +0200, Mikulas Patocka wrote:
> > > > Hi
> > > > 
> > > > Thanks for fixing the cache aliasing issues on PA-RISC in the
> > > > commit 72d95924ee35c8cd16ef52f912483ee938a34d49.
> > > > 
> > > > I think there is still one problem left - and that is
> > > > ARCH_DMA_MINALIGN. Currently, it is 16, which is obviously
> > > > wrong.
> > > I don't think that's obvious, why is it wrong?
> > I see this comment in arch/arm64/include/asm/cache.h:
> > 
> > /*
> >  * Memory returned by kmalloc() may be used for DMA, so we must
> > make
> >  * sure that all such allocations are cache aligned. Otherwise,
> >  * unrelated code may cause parts of the buffer to be read into the
> >  * cache before the transfer is done, causing old data to be seen
> > by
> >  * the CPU.
> >  */

This comment is copied from the same file in arch/arm.  arm is mostly
VIVT caching and has even worse problems than the PA VIPT cache. 
aarch64 has variable cache policy (CTR_EL0 register), but I think most
of them are actually PIPT or VIPT.

> > #define ARCH_DMA_MINALIGN       (128)
> > #define ARCH_KMALLOC_MINALIGN   (8)
> > 
> > L1_CACHE_BYTES is 64 on arm64.
> > 
> > Possibly, the same can occur on parisc.
> > 
> > Dave
> 
> L1_CACHE_BYTES is a performance hint that is used to avoid cache line
> ping-pong when multiple CPUs modify nearby data.
> 
> ARCH_DMA_MINALIGN is the biggest possible cache line size to avoid
> DMA data corruption. As there are some arm64 machines with 128-byte
> cache line, arm64 has to define it to 128.
> 
> James said that the L2 cache on PA8800/8900 is coherent with PCI. So,
> I  think that ARCH_DMA_MINALIGN should be 64 (is that the L1 cache
> line size  on PA8800/8900?).

By default, if unset, ARCH_DMA_MINALIGN defaults to alignof(long long),
so it is already 64 on parisc.

James

> L1_CACHE_BYTES could be 128 to avoid ping-pong between sockets.
> 
> For ARCH_KMALLOC_MINALIGN, there is an arm64 commit 
> 9382bc44b5f58ccee375f08f518e53c0280051dc, it is an optimization, so
> that they can use cache line size probed at startup instead of 128. I
> think we don't have to do this optimization on PA-RISC, the PA-RISC
> machines with 16-byte or 32-byte cache line size are old and slow -
> so that there is no point in trying to optimize kmalloc pools for
> them.
> 
> Mikulas