ARM: zImage: add DSB and ISB barriers after relocating code
PATCH 1/1 ARM: zImage: add DSB and ISB barriers after relocating code
The zImage loader will relocate the image if it determines that decompression will overwrite its current location. Since the act of relocation is basically a form of code self-modification, we need to ensure that the CPU fetches the updated instruction stream.
Since cache maintenance is skipped during the relocation phase (the MMUs and caches are off), we need to execute both a data sync and instruction sync barrier prior to jumping to the relocated code. Skipping the barriers can result in execution of stale prefetched code, leading to hangs or an UNDEFINED INSTRUCTION exception.
--- arch/arm/boot/compressed/head.S | 8 ++++++++ 1 file changed, 8 insertions(+)
arch/arm/boot/compressed/head.S +8 -0
@@ -400,8 +400,16 @@ dtb_check_done: add sp, sp, r6 #endif + /* + * Perform cache maintenance if caches were enabled earlier. + * Otherwise, execute data and instruction barriers prior to + * jumping to the newly-written code. + */ tst r4, #1 bleq cache_clean_flush + movne r0, #0 + mcrne p15, 0, r0, c7, c10, 4 @ DSB + mcrne p15, 0, r0, c7, c5, 4 @ ISB adr r0, BSYM(restart) add r0, r0, r6
Signature · 1 line
1.8.1.3
PATCH 1/1 ARM: zImage: add DSB and ISB barriers after relocating code
Quoted 7 lines · Marc Carino
The zImage loader will relocate the image if it determines that decompression will overwrite its current location. Since the act of relocation is basically a form of code self-modification, we need to ensure that the CPU fetches the updated instruction stream.
Since cache maintenance is skipped during the relocation phase (the MMUs and caches are off),
Beware... This statement isn't true in most cases.
Quoted 4 lines
we need to execute both a data sync
and instruction sync barrier prior to jumping to the relocated code. Skipping the barriers can result in execution of stale prefetched code, leading to hangs or an UNDEFINED INSTRUCTION exception.
Are you sure of that? When the MMU is off, memory access is strongly ordered. I'm wondering if instruction prefetching applies in that case.
Also, is this a problem that you actually experienced in practice?
Quoted 4 lines
arch/arm/boot/compressed/head.S +0
-0@@ -400,8 +400,16 @@ dtb_check_done: add sp, sp, r6 #endif
arch/arm/boot/compressed/head.S dtb_check_done: · line 412
Quoted 10 lines
arch/arm/boot/compressed/head.S +8
-0+ /* + * Perform cache maintenance if caches were enabled earlier. + * Otherwise, execute data and instruction barriers prior to + * jumping to the newly-written code. + */ tst r4, #1 bleq cache_clean_flush + movne r0, #0 + mcrne p15, 0, r0, c7, c10, 4 @ DSB + mcrne p15, 0, r0, c7, c5, 4 @ ISB
Are you sure those CP15 accesses are fine on all the CPU variants that may execute this code? Officially we still support back to ARMv4 implementations.
Nicolas
PATCH 1/1 ARM: zImage: add DSB and ISB barriers after relocating code
Hi Nicolas,
Quoted 1 line
Beware... This statement isn't true in most cases.
Ah, yes - I do have to reword that bit. (Per the booting document, the I-cache can either be on or off upon entry, correct?)
Quoted 3 lines
Are you sure of that? When the MMU is off, memory access is strongly ordered. I'm wondering if instruction prefetching applies in that case.
I'm unable to find a statement in the ARM architecture reference manual (ARM ARM) supporting the need for the DSB, so upon second glance I think I'll eliminate it.
Anyway, you are correct. While the MMU is off, data accesses are assigned the strongly-ordered memory type. However for instruction fetches, memory is assigned the Normal memory type (B3.2.1). Further, the architecture allows for prefetching of instructions within the current and subsequent 4K page from the currently-executing program counter (B3.2.3).
Furthermore, I've encountered the "self-modifying code" example within the ARM ARM, which implies that a pipeline flush is needed after performing stores memory that is a part of the instruction stream.
Another interesting tidbit is that there id no specification of coherency order between data and instructions for _any_ memory type (A3.9.3). It is expected that the programmer to insert an ISB to ensure instruction fetches are synced-up with the most-recent view of memory.
Quoted 1 line
Also, is this a problem that you actually experienced in practice?
Yes.
Quoted 3 lines
Are you sure those CP15 accesses are fine on all the CPU variants that may execute this code? Officially we still support back to ARMv4 implementations.
No. I suppose I would have to leverage a lookup table scheme as is done with the cache maintenance operations in order for this patch to be portable?
Thank you,
Marc
PATCH 1/1 ARM: zImage: add DSB and ISB barriers after relocating code
Quoted 6 lines · Marc Carino
Hi Nicolas,
Quoted 1 line
Beware... This statement isn't true in most cases.
Ah, yes - I do have to reword that bit. (Per the booting document, the I-cache can either be on or off upon entry, correct?)
Ah, yes.
Quoted 15 lines
Quoted 3 lines
Are you sure of that? When the MMU is off, memory access is strongly ordered. I'm wondering if instruction prefetching applies in that case.
I'm unable to find a statement in the ARM architecture reference manual (ARM ARM) supporting the need for the DSB, so upon second glance I think I'll eliminate it.
Anyway, you are correct. While the MMU is off, data accesses are assigned the strongly-ordered memory type. However for instruction fetches, memory is assigned the Normal memory type (B3.2.1). Further, the architecture allows for prefetching of instructions within the current and subsequent 4K page from the currently-executing program counter (B3.2.3).
Quoted 3 lines
Furthermore, I've encountered the "self-modifying code" example within the ARM ARM, which implies that a pipeline flush is needed after performing stores memory that is a part of the instruction stream.
I assume that invalidating the i-cache would be a good thing to do in all cases.
Quoted 8 lines
Another interesting tidbit is that there id no specification of coherency order between data and instructions for _any_ memory type (A3.9.3). It is expected that the programmer to insert an ISB to ensure instruction fetches are synced-up with the most-recent view of memory.
Quoted 1 line
Also, is this a problem that you actually experienced in practice?
Yes.
Good to know.
I'm curious though: at what address do you load your zImage, and what address does RAM start?
Quoted 7 lines
Quoted 3 lines
Are you sure those CP15 accesses are fine on all the CPU variants that may execute this code? Officially we still support back to ARMv4 implementations.
No. I suppose I would have to leverage a lookup table scheme as is done with the cache maintenance operations in order for this patch to be portable?
I would think so.
Nicolas
PATCH 1/1 ARM: zImage: add DSB and ISB barriers after relocating code
Hello Nicolas,
Quoted 2 lines
I assume that invalidating the i-cache would be a good thing to do in all cases.
Great! It looks like I'll be able to augment the existing cache function lookup table with an additional I-cache invalidate operation.
Quoted 2 lines
I'm curious though: at what address do you load your zImage, and what address does RAM start?
On our platform, we load the kernel zImage at address PA:0x8000. The start of physical memory is at PA:0x0.
Thank you,
Marc
PATCH 1/1 ARM: zImage: add DSB and ISB barriers after relocating code
In reply to Marc CarinoQuoted 7 lines · Marc Carino
Hi Nicolas,
Quoted 1 line
Beware... This statement isn't true in most cases.
Ah, yes - I do have to reword that bit. (Per the booting document, the I-cache can either be on or off upon entry, correct?)
Quoted 7 lines
Quoted 3 lines
Are you sure of that? When the MMU is off, memory access is strongly ordered. I'm wondering if instruction prefetching applies in that case.
I'm unable to find a statement in the ARM architecture reference manual (ARM ARM) supporting the need for the DSB, so upon second glance I think I'll eliminate it.
I believe you won't find a statement in the ARM ARM supporting the view that you _don't_ need a DSB either :)
I believe you need it. DSB is the only operation that can serialise program order against the D-side for abitrary code.
Strongly-ordered memory on the D-side guarantees that D-side operations cannot occur out of order with respect to this CPU, but there is still no guarantee about how that relates to the I-side, even with ISB.
Quoted 20 lines
Anyway, you are correct. While the MMU is off, data accesses are assigned the strongly-ordered memory type. However for instruction fetches, memory is assigned the Normal memory type (B3.2.1). Further, the architecture allows for prefetching of instructions within the current and subsequent 4K page from the currently-executing program counter (B3.2.3).
Furthermore, I've encountered the "self-modifying code" example within the ARM ARM, which implies that a pipeline flush is needed after performing stores memory that is a part of the instruction stream.
Another interesting tidbit is that there id no specification of coherency order between data and instructions for _any_ memory type (A3.9.3). It is expected that the programmer to insert an ISB to ensure instruction fetches are synced-up with the most-recent view of memory.
Quoted 1 line
Also, is this a problem that you actually experienced in practice?
Yes.
Quoted 7 lines
Quoted 3 lines
Are you sure those CP15 accesses are fine on all the CPU variants that may execute this code? Officially we still support back to ARMv4 implementations.
No. I suppose I would have to leverage a lookup table scheme as is done with the cache maintenance operations in order for this patch to be portable?
How is what we're trying to achieve here different from the operation that needs to be done in between decompressing the kernel and jumping to it? Can't we just call cache_clean_flush?
Except that a possibly-redundant flush of the D-cache will be done, this should do exactly the right thing.
This code is hairy to get right ... it would be better not to fragment it unnecessarily.
Cheers
---Dave
PATCH 1/1 ARM: zImage: add DSB and ISB barriers after relocating code
Hi Dave,
My apologies for not addressing this prior to submitting my v2 of the patch.
Quoted 6 lines
How is what we're trying to achieve here different from the operation that needs to be done in between decompressing the kernel and jumping to it? Can't we just call
cache_clean_flush?Except that a possibly-redundant flush of the D-cache will be done, this should do exactly the right thing.
I agree - I don't see why we can't just perform a cache_clean_flush prior to jumping to the relocated code. It seems like a simpler and less error-prone solution.
-Marc
Quoted 67 lines · Dave Martin
Quoted 7 lines · Marc Carino
Hi Nicolas,
Quoted 1 line
Beware... This statement isn't true in most cases.
Ah, yes - I do have to reword that bit. (Per the booting document, the I-cache can either be on or off upon entry, correct?)
Quoted 7 lines
Quoted 3 lines
Are you sure of that? When the MMU is off, memory access is strongly ordered. I'm wondering if instruction prefetching applies in that case.
I'm unable to find a statement in the ARM architecture reference manual (ARM ARM) supporting the need for the DSB, so upon second glance I think I'll eliminate it.
I believe you won't find a statement in the ARM ARM supporting the view that you _don't_ need a DSB either :)
I believe you need it. DSB is the only operation that can serialise program order against the D-side for abitrary code.
Strongly-ordered memory on the D-side guarantees that D-side operations cannot occur out of order with respect to this CPU, but there is still no guarantee about how that relates to the I-side, even with ISB.
Quoted 20 lines
Anyway, you are correct. While the MMU is off, data accesses are assigned the strongly-ordered memory type. However for instruction fetches, memory is assigned the Normal memory type (B3.2.1). Further, the architecture allows for prefetching of instructions within the current and subsequent 4K page from the currently-executing program counter (B3.2.3).
Furthermore, I've encountered the "self-modifying code" example within the ARM ARM, which implies that a pipeline flush is needed after performing stores memory that is a part of the instruction stream.
Another interesting tidbit is that there id no specification of coherency order between data and instructions for _any_ memory type (A3.9.3). It is expected that the programmer to insert an ISB to ensure instruction fetches are synced-up with the most-recent view of memory.
Quoted 1 line
Also, is this a problem that you actually experienced in practice?
Yes.
Quoted 7 lines
Quoted 3 lines
Are you sure those CP15 accesses are fine on all the CPU variants that may execute this code? Officially we still support back to ARMv4 implementations.
No. I suppose I would have to leverage a lookup table scheme as is done with the cache maintenance operations in order for this patch to be portable?
How is what we're trying to achieve here different from the operation that needs to be done in between decompressing the kernel and jumping to it? Can't we just call
cache_clean_flush?Except that a possibly-redundant flush of the D-cache will be done, this should do exactly the right thing.
This code is hairy to get right ... it would be better not to fragment it unnecessarily.
Cheers
---Dave
PATCH 1/1 ARM: zImage: add DSB and ISB barriers after relocating code
Quoted 8 lines · Marc C
Hi Dave,
My apologies for not addressing this prior to submitting my v2 of the patch.
Quoted 4 lines
How is what we're trying to achieve here different from the operation that needs to be done in between decompressing the kernel and jumping to it? Can't we just call
cache_clean_flush?
Quoted 6 lines
Quoted 2 lines
Except that a possibly-redundant flush of the D-cache will be done, this should do exactly the right thing.
I agree - I don't see why we can't just perform a
cache_clean_flushprior to jumping to the relocated code. It seems like a simpler and less error-prone solution.
Agreed.
Nicolas