ioremap_wc on arm64

Quoted 28 lines · Jayachandran C

Quoted 26 lines · Will Deacon

Quoted 14 lines · Jayachandran C

Quoted 6 lines · Catalin Marinas

Quoted 5 lines · Jayachandran C

From its definition, the device "gather" attribute seems to be a better fit for implementing write combining mapping in ioremap_wc(). And on ThunderX2, Device GRE mapping has optimizations that makes it much faster than normal uncached mapping.

Quoted 6 lines

Quoted 3 lines

I am not sure of the reasoning behind the original decision to make ioremap_wc use "Normal Non-Cached" attribute, since all the other variants of ioremap use device attributes, and ioremap_wc looks like an exception.

The reason we kept it as Normal NC is that Device_GRE does not allow unaligned accesses.

Quoted 7 lines

There does not to be an expectation to have unaligned access on __iomem pointers. I also see that memremap can call ioremap_wc (which is a Normal mapping) or ioremap_wt(which is a Device mapping), so that is inconsistent as well.

Was this added for a specific use case? Also, do you think this patchset is acceptable?

I think it was used for framebuffers. Note that using normal-NC also aligns with arch/arm/ and, since normal-NC is strictly a more relaxed memory type than Device-GRE, I'm really not keen making on this change.

Quoted 3 lines

If there is a hardware requirement that Normal-NC has to be implemented as more relaxed attribute than Device GRE, I can go back to the hardware team on this. I did not see any text in the ARM ARM requiring this.

It depends what you mean by "relaxed". Normal NC allows unaligned accesses and also allows speculative loads (you don't get any ordering guarantees of the Device memory). If you mean skipping some transparent cache snooping, it may break other use-cases.

Quoted 5 lines

Even if that is the case, having Normal attribute for ioremap just for ioremap_wc is inconsistent. The Device Gathering attribute by its definition is exactly what write combining requires. The memremap() API looks like a better way to expose Normal-NC mapping (with additional features like speculation etc.) if implemented correctly.

I agree, memremap() is a better way but this was merged in 4.3 while ioremap_wc() had been around for a long time. If you want to change its semantics, you'd need to go through all its uses in the kernel and make sure they *only* use I/O accessors with such memory. At a quick grep, there are several places where the __iomem pointer attribute is dropped shortly after ioremap() and the pointer is accessed directly. That's where things will break with Device GRE memory since the compiler doesn't guarantee aligned accesses.

Quoted 1 line

Also, patch 1/2 should be useful anyway - can that be picked up?

The patch is harmless but are we going to have a user of Device_GRE?