Skip to contentSkip to conversation

memcpy alignment

DISCUSSION Jon Masters 6 participants 9 messages Dec 15, 2015

lore
Share & tools
lore
#0 lore raw

Hi Folks,

What's supposed to happen if the natural alignment of the pointers dst and src is not the same? We don't seem to handle that case today, and instead will base our access data type on the source alignment only.

Jon.

#1 lore raw
Quoted 3 lines · Jon Masters

What's supposed to happen if the natural alignment of the pointers dst and src is not the same? We don't seem to handle that case today, and instead will base our access data type on the source alignment only.

The hardware takes care of the other one. The memcpy behaviour in the kernel is the same as the glibc one (the Cortex Strings library).

Signature · 1 line
Catalin
#2 lore raw
Quoted 7 lines · Catalin Marinas
Quoted 3 lines · Jon Masters

What's supposed to happen if the natural alignment of the pointers dst and src is not the same? We don't seem to handle that case today, and instead will base our access data type on the source alignment only.

The hardware takes care of the other one. The memcpy behaviour in the kernel is the same as the glibc one (the Cortex Strings library).

Not if you're dealing with Device memory. I accept that one could always ensure that there's a non-Device mapping (I got the other replies) but I don't think it's unreasonable to expect a memcpy to work in the presence of misaligned addresses either.

Jon.

#3 lore raw
Quoted 6 lines · Jon Masters
Quoted 5 lines · Catalin Marinas
Quoted 3 lines · Jon Masters

What's supposed to happen if the natural alignment of the pointers dst and src is not the same? We don't seem to handle that case today, and instead will base our access data type on the source alignment only.

Quoted 7 lines
Quoted 2 lines

The hardware takes care of the other one. The memcpy behaviour in the kernel is the same as the glibc one (the Cortex Strings library).

Not if you're dealing with Device memory. I accept that one could always ensure that there's a non-Device mapping (I got the other replies) but I don't think it's unreasonable to expect a memcpy to work in the presence of misaligned addresses either.

Using memcpy() on memory returned from functions which setup IO/MMIO mappings has always been outlawed. It's the whole reason we have things like memcpy_toio(), memcpy_fromio() and memset_io(), which date back a few decades now, and they exist for exactly these kinds of reasons.

If you get an __iomem pointer, then you must respect that it essentially can not be non-dereferenced, and you must use one of the standard kernel accessors (read[bwl]/ioread*/write[bwl]/iowrite*/ memcpy_fromio/memcpy_toio/memset_io) to access it. That's the API contract you implicitly signed up to by using something like ioremap() or other mapping which gives you an iomem mapping.

Signature · 3 lines
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
#4 lore raw
Quoted 21 lines · Russell King - ARM Linux
Quoted 6 lines · Jon Masters
Quoted 5 lines · Catalin Marinas
Quoted 3 lines · Jon Masters

What's supposed to happen if the natural alignment of the pointers dst and src is not the same? We don't seem to handle that case today, and instead will base our access data type on the source alignment only.

Quoted 7 lines
Quoted 2 lines

The hardware takes care of the other one. The memcpy behaviour in the kernel is the same as the glibc one (the Cortex Strings library).

Not if you're dealing with Device memory. I accept that one could always ensure that there's a non-Device mapping (I got the other replies) but I don't think it's unreasonable to expect a memcpy to work in the presence of misaligned addresses either.

Using memcpy() on memory returned from functions which setup IO/MMIO mappings has always been outlawed. It's the whole reason we have things like memcpy_toio(), memcpy_fromio() and memset_io(), which date back a few decades now, and they exist for exactly these kinds of reasons.

Quoted 6 lines

If you get an __iomem pointer, then you must respect that it essentially can not be non-dereferenced, and you must use one of the standard kernel accessors (read[bwl]/ioread*/write[bwl]/iowrite*/ memcpy_fromio/memcpy_toio/memset_io) to access it. That's the API contract you implicitly signed up to by using something like ioremap() or other mapping which gives you an iomem mapping.

Thanks Russell. If it's definitely never allowed then the existing x86 code needs to be fixed to use an IO access function in that case. I get that those accessors are there for this reason, but I wanted to make sure that we don't ever expect to touch Device memory any other way (for example, conflicting mappings between a VM and hypervisor). I am certain there's other non-ACPI code that is going to have this happen :)

Jon.

#5 lore raw
Quoted 29 lines · Jon Masters
Quoted 21 lines · Russell King - ARM Linux
Quoted 6 lines · Jon Masters
Quoted 5 lines · Catalin Marinas
Quoted 3 lines · Jon Masters

What's supposed to happen if the natural alignment of the pointers dst and src is not the same? We don't seem to handle that case today, and instead will base our access data type on the source alignment only.

Quoted 7 lines
Quoted 2 lines

The hardware takes care of the other one. The memcpy behaviour in the kernel is the same as the glibc one (the Cortex Strings library).

Not if you're dealing with Device memory. I accept that one could always ensure that there's a non-Device mapping (I got the other replies) but I don't think it's unreasonable to expect a memcpy to work in the presence of misaligned addresses either.

Using memcpy() on memory returned from functions which setup IO/MMIO mappings has always been outlawed. It's the whole reason we have things like memcpy_toio(), memcpy_fromio() and memset_io(), which date back a few decades now, and they exist for exactly these kinds of reasons.

Quoted 6 lines

If you get an __iomem pointer, then you must respect that it essentially can not be non-dereferenced, and you must use one of the standard kernel accessors (read[bwl]/ioread*/write[bwl]/iowrite*/ memcpy_fromio/memcpy_toio/memset_io) to access it. That's the API contract you implicitly signed up to by using something like ioremap() or other mapping which gives you an iomem mapping.

Quoted 6 lines

Thanks Russell. If it's definitely never allowed then the existing x86 code needs to be fixed to use an IO access function in that case. I get that those accessors are there for this reason, but I wanted to make sure that we don't ever expect to touch Device memory any other way (for example, conflicting mappings between a VM and hypervisor). I am certain there's other non-ACPI code that is going to have this happen :)

It's important to note that much of the ACPI code was written prior to the availability of memremap.

Depending on the case, the better solution may be to use memremap. I would expect this to be the case for any static tables of information, at least.

Thanks,
Mark.

#6 lore raw
In reply to Jon Masters
Quoted 7 lines · Jon Masters
Quoted 6 lines

If you get an __iomem pointer, then you must respect that it essentially can not be non-dereferenced, and you must use one of the standard kernel accessors (read[bwl]/ioread*/write[bwl]/iowrite*/ memcpy_fromio/memcpy_toio/memset_io) to access it. That's the API contract you implicitly signed up to by using something like ioremap() or other mapping which gives you an iomem mapping.

Quoted 6 lines

Thanks Russell. If it's definitely never allowed then the existing x86 code needs to be fixed to use an IO access function in that case. I get that those accessors are there for this reason, but I wanted to make sure that we don't ever expect to touch Device memory any other way (for example, conflicting mappings between a VM and hypervisor). I am certain there's other non-ACPI code that is going to have this happen :)

A lot of code that has never run on anything other than x86 will have such issues.

Tracking the use of page_is_ram() around the kernel, looking at what it does for different architectures, and looking at how its (not formalised) semantics are interpreted can also be quite unsettling.

/
Leif

#7 lore raw
Quoted 15 lines · Leif Lindholm
Quoted 7 lines · Jon Masters
Quoted 6 lines

If you get an __iomem pointer, then you must respect that it essentially can not be non-dereferenced, and you must use one of the standard kernel accessors (read[bwl]/ioread*/write[bwl]/iowrite*/ memcpy_fromio/memcpy_toio/memset_io) to access it. That's the API contract you implicitly signed up to by using something like ioremap() or other mapping which gives you an iomem mapping.

Quoted 6 lines

Thanks Russell. If it's definitely never allowed then the existing x86 code needs to be fixed to use an IO access function in that case. I get that those accessors are there for this reason, but I wanted to make sure that we don't ever expect to touch Device memory any other way (for example, conflicting mappings between a VM and hypervisor). I am certain there's other non-ACPI code that is going to have this happen :)

Quoted 6 lines

A lot of code that has never run on anything other than x86 will have such issues.

Tracking the use of page_is_ram() around the kernel, looking at what it does for different architectures, and looking at how its (not formalised) semantics are interpreted can also be quite unsettling.

Yeah. That was the reason I didn't just change the existing initrd code in the first place (wanted to leave it as is). I *did not know* memcpy to/from Device memory was explicitly banned (and I get why, and I do know one is supposed to map Device memory as such, etc. etc.) for this reason. However it's actually very reasonable to demand correctness going in. If it were hacked/kludged to paper over the situation I describe it would stand even less chance of being fixed.

I would /separately/ note that there's an inefficiency in that the existing code relies upon assumed equal alignment between src/dst so the hardware is probably doing a lot of silent unaligned writes.

Jon.

#8 lore raw
Quoted 23 lines · Jon Masters
Quoted 15 lines · Leif Lindholm
Quoted 7 lines · Jon Masters
Quoted 6 lines

If you get an __iomem pointer, then you must respect that it essentially can not be non-dereferenced, and you must use one of the standard kernel accessors (read[bwl]/ioread*/write[bwl]/iowrite*/ memcpy_fromio/memcpy_toio/memset_io) to access it. That's the API contract you implicitly signed up to by using something like ioremap() or other mapping which gives you an iomem mapping.

Quoted 6 lines

Thanks Russell. If it's definitely never allowed then the existing x86 code needs to be fixed to use an IO access function in that case. I get that those accessors are there for this reason, but I wanted to make sure that we don't ever expect to touch Device memory any other way (for example, conflicting mappings between a VM and hypervisor). I am certain there's other non-ACPI code that is going to have this happen :)

Quoted 6 lines

A lot of code that has never run on anything other than x86 will have such issues.

Tracking the use of page_is_ram() around the kernel, looking at what it does for different architectures, and looking at how its (not formalised) semantics are interpreted can also be quite unsettling.

Quoted 5 lines

Yeah. That was the reason I didn't just change the existing initrd code in the first place (wanted to leave it as is). I *did not know* memcpy to/from Device memory was explicitly banned (and I get why, and I do know one is supposed to map Device memory as such, etc. etc.) for this reason.

Additionally to alignment constraints, IO regions often allow only certain access sizes, and memcpy() doesn't make any promises as to what it might do.

Quoted 3 lines

I would /separately/ note that there's an inefficiency in that the existing code relies upon assumed equal alignment between src/dst so the hardware is probably doing a lot of silent unaligned writes.

This is the most efficient way. Manually shifting things around to get both reads and writes aligned costs more than letting the hardware handle one side. An unaligned store typically costs on average one cycle more than an aligned store. On some hardware (e.g. Cortex-A8), an unaligned store within a cache line is free but one that crosses cache lines needs several extra cycles.

Signature · 1 line
M?ns Rullg?rd