Skip to content

ioremap_wc on arm64

Jayachandran C linux-arm-kernel 1 version New

Get series with b4
b4 am 20170524131054.GA5647@localhost

Versions newest first

  1. v1 Latest0 of 2 patches 0 repliesNew

Patches in v1 No applies recorded yet

None of the 2 patches the cover letter announces are in this archive.

Reviews trailers per patch

No Reviewed-by, Acked-by or Tested-by trailers for v1 yet.

Read the v1 thread

Cover letter [PATCH 0/2] ioremap_wc on arm64
Quoted 23 lines · Catalin Marinas
Quoted 22 lines · Jayachandran C
Quoted 20 lines · Catalin Marinas
Quoted 10 lines · Jayachandran C
Quoted 3 lines · Will Deacon

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.

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 6 lines
Quoted 4 lines

There is a difference in behavior with regard to unaligned access between ioremap_wc and ioremap already. I don't follow why unaligned access has to supported for ioremap_wc but not for ioremap - it would be much better to be consistent here.

Please re-read my paragraph above.

I had the same feeling after seeing your reply :)

Quoted 3 lines

It's nice if both ioremap_wc() and
ioremap() returned Device memory as long as you fix the ioremap_wc() misuses throughout the kernel.

ioremap and ioremap_wc is been used and abused in multiple places after dropping the __iomem annotation, and it is not an easy task to figure out where unaligned access is done on the pointer returned. I am told specifically that in ioremap_wc needs to support unaligned access as opposed to ioremap.

I am trying to figure out if there are specific cases you expect it to fail. Since arm64 is a fairly new architecture and it will be easy to get this correct now, rather than to let the status quo continue.

There is no "Gather" attribute on arm, so using "Normal uncached" for ioremap_wc is understandable. On arm64, given that there is an attribute specifically for write combining - you have decided to go with Normal uncached. I had hoped for a proper reason for that and not a vague "maybe framebuffer, go grep the kernel and figure out" response.

Quoted 4 lines
Quoted 4 lines
Quoted 3 lines
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?

Quoted 8 lines
Quoted 4 lines

This patch would make the attribute in MAIR and the index MT_DEVICE_GRE useful for __ioremap. Otherwise I don't see the use in having the attribute at all. I can at least give this to people who want Device GRE mapping, which is the fastest way to access device memory on ThunderX2.

Is this a driver that's going to be used on arm64 kernels only? I'm not that keen on __ioremap() since it's not a standard driver API, so you end up with #ifdef CONFIG_ARM64 in the driver code.

FWIW, __ioremap() is EXPORT_SYMBOL. Having the attribute will be useful.

JC.

Read the cover letter in its thread