arm: Disable the strict alignment of data on armv7
PATCH 1/1 arm: Disable the strict alignment of data on armv7
From: Nitin Jain <[email protected]>
This patch is used for disable the strict alignment of data to avoid the memory alignment issues.
Also setup this option for Xilinx Zynq.
---
Not sure if there is any side effects but our tests don't show up any issue with disabling this bit.
--- arch/arm/Kconfig | 1 + arch/arm/cpu/armv7/Kconfig | 6 ++++++ arch/arm/cpu/armv7/start.S | 2 ++ 3 files changed, 9 insertions(+)
arch/arm/Kconfig +1 -0
@@ -778,6 +778,7 @@ config ARCH_ZYNQ imply CMD_CLK imply FAT_WRITE imply CMD_SPL + imply ARMV7_MEM_ALIGN_DISABLE config ARCH_ZYNQMP bool "Xilinx ZynqMP based platform"
arch/arm/cpu/armv7/Kconfig +6 -0
@@ -58,4 +58,10 @@ config ARMV7_LPAE Say Y here to use the long descriptor page table format. This is required if U-Boot runs in HYP mode. +config ARMV7_MEM_ALIGN_DISABLE + bool "Disable strict alignment of data" + help + Enabling this option disables strict alignment for armv7 by + setting the alignment bit in system control register of cp15. + endif
arch/arm/cpu/armv7/start.S +2 -0
@@ -150,7 +150,9 @@ ENTRY(cpu_init_cp15) mrc p15, 0, r0, c1, c0, 0 bic r0, r0, #0x00002000 @ clear bits 13 (--V-) bic r0, r0, #0x00000007 @ clear bits 2:0 (-CAM) +#ifndef CONFIG_ARMV7_MEM_ALIGN_DISABLE orr r0, r0, #0x00000002 @ set bit 1 (--A-) Align +#endif orr r0, r0, #0x00000800 @ set bit 11 (Z---) BTB #ifdef CONFIG_SYS_ICACHE_OFF bic r0, r0, #0x00001000 @ clear bit 12 (I) I-cache
Signature · 1 line
1.9.1
PATCH 1/1 arm: Disable the strict alignment of data on armv7
Dear Michal,
Quoted 4 lines · In message <029f7f8f6d89cc77c92e04223a7402376e050f56.1520433579.git.michal.sime…
From: Nitin Jain <[email protected]>
This patch is used for disable the strict alignment of data to avoid the memory alignment issues.
Can you please add some comments what the consequences of this change are? I guess there are advantages, but I also guess these come at a price?
Best regards,
Wolfgang Denk
Signature · 7 lines
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Quote from the Boss after overriding the decision of a task force he created to find a solution: "I'm sorry if I ever gave you the impression your input would have any effect on my decision for the outcome of this project!"
PATCH 1/1 arm: Disable the strict alignment of data on armv7
Hi Wolfgang,
Quoted 11 lines · Wolfgang Denk
Dear Michal,
Quoted 4 lines · In message <029f7f8f6d89cc77c92e04223a7402376e050f56.1520433579.git.michal.sime…
From: Nitin Jain <[email protected]>
This patch is used for disable the strict alignment of data to avoid the memory alignment issues.
Can you please add some comments what the consequences of this change are? I guess there are advantages, but I also guess these come at a price?
That's something what I am expecting from this discussion if there are any corner cases which we are not aware of.
We found this setting based on randomized testing where simply non aligned access for memory read was causing exception.
The same tests were running fine on arm64 where non aligned accesses are permitted.
It is hard to compare performance impact on u-boot but from functionality point of view we are not able to see difference. If there is any performance impact that it is quite low.
Thanks,
Michal
PATCH 1/1 arm: Disable the strict alignment of data on armv7
Dear Michal,
Quoted 10 lines · In message <[email protected]> you wrote:
Quoted 4 lines
Can you please add some comments what the consequences of this change are? I guess there are advantages, but I also guess these come at a price?
That's something what I am expecting from this discussion if there are any corner cases which we are not aware of.
We found this setting based on randomized testing where simply non aligned access for memory read was causing exception.
Hm... One possible position one can take is that unaligned accesses are likely an indication for incorrect or sloppy programming. usually we design data structures, buffers etc. such that no unaligned accesses take place. Most code is explicitly written in such a way that it also runs on architectures where unaligned access simply don't work.
I feel that providing such an option just opens the door for lousy programmers who don't think throroughly about the code they write.
Yes, in some cases this may be conveniendt. But there are also cases where the problem is just papered over, and hits all the harder later like when you also enable caching.
Thi is why I'm asking for the advantages. If it's just is that broken code starts working, then I'd rather see a clear error message that forces the code to be fixed.
Quoted 2 lines
The same tests were running fine on arm64 where non aligned accesses are permitted.
But I guess they still come under a (severe?) performance penalty?
Quoted 3 lines
It is hard to compare performance impact on u-boot but from functionality point of view we are not able to see difference. If there is any performance impact that it is quite low.
Hm.... I have to admit that I don't know ARM/ARM64 that well, but I am pretty sure that even on ARM an aligned 32 bit access on a 32 bit bus will result in a single read cycle on that bus - while I would expect that an unaligned 32 bit access might get broken down into 4 byte accesses?
Also, I wonder if ARM still maintains atomicity for unaligned reads/ writes ? [IIRC, x86 doesn't, while it's guaranteed for aligned accesses.]
Best regards,
Wolfgang Denk
Signature · 5 lines
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
What is mind? No matter. What is matter? Never mind.
-- Thomas Hewitt Key, 1799-1875PATCH 1/1 arm: Disable the strict alignment of data on armv7
Dear Wolfgang,
Quoted 24 lines · Wolfgang Denk
Dear Michal,
Quoted 10 lines · In message <[email protected]> you wrote:
Quoted 4 lines
Can you please add some comments what the consequences of this change are? I guess there are advantages, but I also guess these come at a price?
That's something what I am expecting from this discussion if there are any corner cases which we are not aware of.
We found this setting based on randomized testing where simply non aligned access for memory read was causing exception.
Hm... One possible position one can take is that unaligned accesses are likely an indication for incorrect or sloppy programming. usually we design data structures, buffers etc. such that no unaligned accesses take place. Most code is explicitly written in such a way that it also runs on architectures where unaligned access simply don't work.
I feel that providing such an option just opens the door for lousy programmers who don't think throroughly about the code they write.
Quoted 7 lines
Yes, in some cases this may be conveniendt. But there are also cases where the problem is just papered over, and hits all the harder later like when you also enable caching.
Thi is why I'm asking for the advantages. If it's just is that broken code starts working, then I'd rather see a clear error message that forces the code to be fixed.
We are not seeing any issue from u-boot code itself and I can believe that structures and accesses are aligned properly.
The initial reason was that u-boot allows you to run for example command md 1 and it ends up in panic. Which is something what should be possible to run or code should check that architecture has alignment restriction.
Definitely panic is not proper reaction on command like this. It means we have two options:
1. enable non aligned accesses
2. fix code/commands which enables to pass these non aligned addresses which ends up in panic
The patch is targeting option 1 because on arm64 it is permitted.
Quoted 4 lines
Quoted 2 lines
The same tests were running fine on arm64 where non aligned accesses are permitted.
But I guess they still come under a (severe?) performance penalty?
It is really a question where that penalty is coming from and when it can happen.
Quoted 9 lines
Quoted 3 lines
It is hard to compare performance impact on u-boot but from functionality point of view we are not able to see difference. If there is any performance impact that it is quite low.
Hm.... I have to admit that I don't know ARM/ARM64 that well, but I am pretty sure that even on ARM an aligned 32 bit access on a 32 bit bus will result in a single read cycle on that bus - while I would expect that an unaligned 32 bit access might get broken down into 4 byte accesses?
I would expect that there will be more cycles for read as you mentioned.
But again the code is align and there is not an issue in code that's why this penalty is not happening. And non align accesses are performed when user asks for this.
Quoted 4 lines
Also, I wonder if ARM still maintains atomicity for unaligned reads/ writes ? [IIRC, x86 doesn't, while it's guaranteed for aligned accesses.]
Don't know.
Thanks,
Michal
PATCH 1/1 arm: Disable the strict alignment of data on armv7
Dear Michal,
Quoted 7 lines · In message <[email protected]> you wrote:
We are not seeing any issue from u-boot code itself and I can believe that structures and accesses are aligned properly.
The initial reason was that u-boot allows you to run for example command md 1 and it ends up in panic. Which is something what should be possible to run or code should check that architecture has alignment restriction.
I'm not sure this needs any "fixing". Actually I use this on a regular base as a simple means to find out if an architecutre allos unaligned accesses... U-Boot is just a boot loader. It has no "normal users" - users are usually developers who know what they are doing. I think this is an area where this old quote applies nicely:
"UNIX was not designed to stop you from doing stupid things, because
that would also stop you from doing clever things." - Doug Gwyn
Quoted 1 line
Definitely panic is not proper reaction on command like this.
Why not? Yes, we can blow up the code to handle this in a better way, and add some kB of stricgs for nice and user-friendy warnings not to do this or that. But is it really worth the effort? I consider this a pretty useful feature and would be sad if it went missing. If someone runs into it by accident, he learns a lesson - but htere is no real damage done. Or am I missing anything?
Quoted 4 lines
It means we have two options:
1. enable non aligned accesses
2. fix code/commands which enables to pass these non aligned addresses which ends up in panic
My vorte is for option 3:
3. Leave as is. It's a boot loader, and like other sharp tools it can be dangerous if improperly used.
Quoted 4 lines
Quoted 1 line
But I guess they still come under a (severe?) performance penalty?
It is really a question where that penalty is coming from and when it can happen.
It comes from the memory and cache architecture. Just assume an unaligned read of 2 bytes that happen to cross a cacheline boundary. This access causes a double cache miss; in the worst case this causes two cache lines to be flushed to memory and then two cachelines to be written from memory. Instead of just reading two bytes, we write 128 bytes and read 128 bytes. Yes, this is a pathological case, but you get the idea.
Quoted 3 lines
But again the code is align and there is not an issue in code that's why this penalty is not happening. And non align accesses are performed when user asks for this.
Why not tell the user: don't perform unaligend accesses on this platform then? I can't see the situation where he really _needs_ to do something which is not natively supported by his hardware?
Quoted 5 lines
Quoted 3 lines
Also, I wonder if ARM still maintains atomicity for unaligned reads/ writes ? [IIRC, x86 doesn't, while it's guaranteed for aligned accesses.]
Don't know.
Well, I'm willing to bet a few beer this doesn't work at least in pathological cases like example of crossing cache lines above.
Best regards,
Wolfgang Denk
Signature · 6 lines
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
He'd heard her use that sweet, innocent tone of voice before. It
meant that, pretty soon, there was going to be trouble.
- Terry Pratchett, _Truckers_PATCH 1/1 arm: Disable the strict alignment of data on armv7
Dear Wolfgang,
Quoted 11 lines · Wolfgang Denk
Dear Michal,
Quoted 7 lines · In message <[email protected]> you wrote:
We are not seeing any issue from u-boot code itself and I can believe that structures and accesses are aligned properly.
The initial reason was that u-boot allows you to run for example command md 1 and it ends up in panic. Which is something what should be possible to run or code should check that architecture has alignment restriction.
Quoted 8 lines
I'm not sure this needs any "fixing". Actually I use this on a regular base as a simple means to find out if an architecutre allos unaligned accesses... U-Boot is just a boot loader. It has no "normal users" - users are usually developers who know what they are doing. I think this is an area where this old quote applies nicely:
"UNIX was not designed to stop you from doing stupid things, because
that would also stop you from doing clever things." - Doug Gwyn
I understand your opinion and no problem with it.
Quoted 8 lines
Quoted 1 line
Definitely panic is not proper reaction on command like this.
Why not? Yes, we can blow up the code to handle this in a better way, and add some kB of stricgs for nice and user-friendy warnings not to do this or that. But is it really worth the effort? I consider this a pretty useful feature and would be sad if it went missing. If someone runs into it by accident, he learns a lesson - but htere is no real damage done. Or am I missing anything?
It is not that the patch is removing or changing current behavior. Default option is to use strict alignment and have an option to change this behavior.
Quoted 9 lines
Quoted 4 lines
It means we have two options:
1. enable non aligned accesses
2. fix code/commands which enables to pass these non aligned addresses which ends up in panicMy vorte is for option 3:
3. Leave as is. It's a boot loader, and like other sharp tools it can be dangerous if improperly used.
Sure that's also an option but it seems to me weird that by default armv7 has this requirement and a53 not.
Quoted 5 lines
Quoted 4 lines
Quoted 1 line
But I guess they still come under a (severe?) performance penalty?
It is really a question where that penalty is coming from and when it can happen.
Quoted 7 lines
It comes from the memory and cache architecture. Just assume an unaligned read of 2 bytes that happen to cross a cacheline boundary. This access causes a double cache miss; in the worst case this causes two cache lines to be flushed to memory and then two cachelines to be written from memory. Instead of just reading two bytes, we write 128 bytes and read 128 bytes. Yes, this is a pathological case, but you get the idea.
If this is WB case that write shouldn't be there but you are right that this could happen.
Quoted 7 lines
Quoted 3 lines
But again the code is align and there is not an issue in code that's why this penalty is not happening. And non align accesses are performed when user asks for this.
Why not tell the user: don't perform unaligend accesses on this platform then? I can't see the situation where he really _needs_ to do something which is not natively supported by his hardware?
It is supported by hardware. U-Boot/SW just doing this configuration not to allow it.
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
Also, I wonder if ARM still maintains atomicity for unaligned reads/ writes ? [IIRC, x86 doesn't, while it's guaranteed for aligned accesses.]
Don't know.
Well, I'm willing to bet a few beer this doesn't work at least in pathological cases like example of crossing cache lines above.
:-)
Thanks,
Michal
PATCH 1/1 arm: Disable the strict alignment of data on armv7
In reply to Wolfgang DenkQuoted 24 lines
From: Wolfgang Denk <[email protected]>
Date: Fri, 09 Mar 2018 12:26:01 +0100Dear Michal,
Quoted 10 lines · In message <[email protected]> you wrote:
Quoted 4 lines
Can you please add some comments what the consequences of this change are? I guess there are advantages, but I also guess these come at a price?
That's something what I am expecting from this discussion if there are any corner cases which we are not aware of.
We found this setting based on randomized testing where simply non aligned access for memory read was causing exception.
Hm... One possible position one can take is that unaligned accesses are likely an indication for incorrect or sloppy programming. usually we design data structures, buffers etc. such that no unaligned accesses take place. Most code is explicitly written in such a way that it also runs on architectures where unaligned access simply don't work.
Quoted 6 lines
I feel that providing such an option just opens the door for lousy programmers who don't think throroughly about the code they write.
Yes, in some cases this may be conveniendt. But there are also cases where the problem is just papered over, and hits all the harder later like when you also enable caching.
That is pretty much why we run OpenBSD in strict alignment mode on as much architectures as possible. More strict-alignment architectures means that bugs are found quicker. So as long as U-Boot supports hardware that cares about alignment (e.g. older ARM, MIPS, SuperH) I'd say there is a clear benefit of running armv7 in strict alignment mode.
Also note that some armv7 instructions still require properly aligned data. And that really old ARM hardware silently fetches the wrong data for a misaligned load instruction.
PATCH 1/1 arm: Disable the strict alignment of data on armv7
Quoted 37 lines · Mark Kettenis
Quoted 24 lines
From: Wolfgang Denk <[email protected]>
Date: Fri, 09 Mar 2018 12:26:01 +0100Dear Michal,
Quoted 10 lines · In message <[email protected]> you wrote:
Quoted 4 lines
Can you please add some comments what the consequences of this change are? I guess there are advantages, but I also guess these come at a price?
That's something what I am expecting from this discussion if there are any corner cases which we are not aware of.
We found this setting based on randomized testing where simply non aligned access for memory read was causing exception.
Hm... One possible position one can take is that unaligned accesses are likely an indication for incorrect or sloppy programming. usually we design data structures, buffers etc. such that no unaligned accesses take place. Most code is explicitly written in such a way that it also runs on architectures where unaligned access simply don't work.
Quoted 6 lines
I feel that providing such an option just opens the door for lousy programmers who don't think throroughly about the code they write.
Yes, in some cases this may be conveniendt. But there are also cases where the problem is just papered over, and hits all the harder later like when you also enable caching.
That is pretty much why we run OpenBSD in strict alignment mode on as much architectures as possible. More strict-alignment architectures means that bugs are found quicker. So as long as U-Boot supports hardware that cares about alignment (e.g. older ARM, MIPS, SuperH) I'd say there is a clear benefit of running armv7 in strict alignment mode.
Quoted 4 lines
Also note that some armv7 instructions still require properly aligned data. And that really old ARM hardware silently fetches the wrong data for a misaligned load instruction.
No problem with this at all. I wanted to check that this is intended setup.
Thanks,
Michal