Skip to contentSkip to conversation

arm: enable unaligned access on ARMv7

PATCH New Lucas Stach 11 participants 32 messages Jun 5, 2012 – Jul 20, 2012 1 message not in the archive Arm (32-bit)

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 arm: enable unaligned access on ARMv7

Recent toolchains default to using the hardware feature for unaligned access on ARM v7, rather than doing the software fallback. According to ARM this is safe as all v7 implementations have to support this feature.
(http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0471c/BABJFFAE.html)

To avoid CPU hangs when doing unaligned memory access, we have to turn off alignment checking in our CPU initialisation code.
(http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0491c/CIHCGCFD.html)

CC Albert ARIBAUD <[email protected]>
---
 arch/arm/cpu/armv7/start.S |    2 +-

1 Datei ge?ndert, 1 Zeile hinzugef?gt(+), 1 Zeile entfernt(-)

arch/arm/cpu/armv7/start.S +1 -1
index 261835b..52f7f6e 100644
@@ -316,7 +316,7 @@ 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)
-	orr	r0, r0, #0x00000002	@ set bit 1 (--A-) Align
+	bic	r0, r0, #0x00000002	@ clear bit 1 (--A-) Align
 	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.7.10.2
#1 lore raw

PATCH 1/1 arm: enable unaligned access on ARMv7

Quoted 8 lines · Lucas Stach

Recent toolchains default to using the hardware feature for unaligned access on ARM v7, rather than doing the software fallback. According to ARM this is safe as all v7 implementations have to support this feature.
(http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0471c/BABJFFAE.html)

To avoid CPU hangs when doing unaligned memory access, we have to turn off alignment checking in our CPU initialisation code.
(http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0491c/CIHCGCFD.html)

Does this behavior change trickle down to Linux/... too, or would an OS completely re-initialize this state, and hence not be affected?

#2 lore raw

PATCH 1/1 arm: enable unaligned access on ARMv7

Hi Stephen,

Am Dienstag, den 05.06.2012, 12:42 -0600 schrieb Stephen Warren:

Quoted 6 lines · Lucas Stach
Quoted 5 lines

Recent toolchains default to using the hardware feature for unaligned access on ARM v7, rather than doing the software fallback. According to ARM this is safe as all v7 implementations have to support this feature.
(http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0471c/BABJFFAE.html)

Quoted 7 lines
Quoted 3 lines

To avoid CPU hangs when doing unaligned memory access, we have to turn off alignment checking in our CPU initialisation code.
(http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0491c/CIHCGCFD.html)

Does this behavior change trickle down to Linux/... too, or would an OS completely re-initialize this state, and hence not be affected?

Linux in particular does reinitialize this state and I expect any reasonable OS to do so.

-- Lucas

#3 lore raw

PATCH 1/1 arm: enable unaligned access on ARMv7

Hi Lucas,

Quoted 14 lines · Lucas Stach

Hi Stephen,

Am Dienstag, den 05.06.2012, 12:42 -0600 schrieb Stephen Warren:

Quoted 11 lines · Lucas Stach
Quoted 9 lines

Recent toolchains default to using the hardware feature for unaligned access on ARM v7, rather than doing the software fallback. According to ARM this is safe as all v7 implementations have to support this feature.
(http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0471c/BABJFFAE.html)

To avoid CPU hangs when doing unaligned memory access, we have to turn off alignment checking in our CPU initialisation code.
(http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0491c/CIHCGCFD.html)

Quoted 7 lines
Quoted 4 lines

Does this behavior change trickle down to Linux/... too, or would an OS completely re-initialize this state, and hence not be affected?

Linux in particular does reinitialize this state and I expect any reasonable OS to do so.

Then what is the point of enabling it on U-Boot? Does it fix some issue whereby some mis-aligned piece of data cannot be properly aligned?

Amicalement,

Signature · 1 line
Albert.
#4 lore raw

PATCH 1/1 arm: enable unaligned access on ARMv7

Am Freitag, den 22.06.2012, 11:15 +0200 schrieb Albert ARIBAUD:

Quoted 22 lines

Hi Lucas,

Quoted 19 lines · Lucas Stach

Hi Stephen,

Am Dienstag, den 05.06.2012, 12:42 -0600 schrieb Stephen Warren:

Quoted 15 lines · Lucas Stach
Quoted 9 lines

Recent toolchains default to using the hardware feature for unaligned access on ARM v7, rather than doing the software fallback. According to ARM this is safe as all v7 implementations have to support this feature.
(http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0471c/BABJFFAE.html)

To avoid CPU hangs when doing unaligned memory access, we have to turn off alignment checking in our CPU initialisation code.
(http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0491c/CIHCGCFD.html)

Does this behavior change trickle down to Linux/... too, or would an OS completely re-initialize this state, and hence not be affected?

Quoted 6 lines
Quoted 2 lines

Linux in particular does reinitialize this state and I expect any reasonable OS to do so.

Then what is the point of enabling it on U-Boot? Does it fix some issue whereby some mis-aligned piece of data cannot be properly aligned?

Yes, it fixes U-Boot USB on Tegra, when built with a recent toolchain. Fixing the alignment of some of the structures in the USB code should also be done, but this is a whole lot more invasive and requires some more thought, as the discussion about this on LKML shows. The issue doesn't show for older toolchains, as they by default emit code to work around unaligned accesses.

This patch fixes all unaligned issues, that may appear with recent toolchains. We avoid having to instruct the toolchain to work around unaligned accesses and gain better performance in cases where it is needed.

Thanks,
Lucas

#5 lore raw

PATCH 1/1 arm: enable unaligned access on ARMv7

Hi Lucas,

Quoted 14 lines
Quoted 7 lines
Quoted 2 lines

Linux in particular does reinitialize this state and I expect any reasonable OS to do so.

Then what is the point of enabling it on U-Boot? Does it fix some issue whereby some mis-aligned piece of data cannot be properly aligned?

Yes, it fixes U-Boot USB on Tegra, when built with a recent toolchain. Fixing the alignment of some of the structures in the USB code should also be done, but this is a whole lot more invasive and requires some more thought, as the discussion about this on LKML shows. The issue doesn't show for older toolchains, as they by default emit code to work around unaligned accesses.

Quoted 4 lines

This patch fixes all unaligned issues, that may appear with recent toolchains. We avoid having to instruct the toolchain to work around unaligned accesses and gain better performance in cases where it is needed.

I am not too happy with enabling a lax behavior only to avoid an issue which apparently is diagnosed and could / should be fixed at its root. Can you point me to the relevant LKML thread
so that I get the details and understand what prevents fixing this by 'simply' aligning the USB structures?

Quoted 2 lines

Thanks,
Lucas

Amicalement,

Signature · 1 line
Albert.
#6 lore raw

PATCH 1/1 arm: enable unaligned access on ARMv7

Hi Albert,

Am Freitag, den 22.06.2012, 13:16 +0200 schrieb Albert ARIBAUD:

Quoted 21 lines

Hi Lucas,

Quoted 14 lines
Quoted 7 lines
Quoted 2 lines

Linux in particular does reinitialize this state and I expect any reasonable OS to do so.

Then what is the point of enabling it on U-Boot? Does it fix some issue whereby some mis-aligned piece of data cannot be properly aligned?

Yes, it fixes U-Boot USB on Tegra, when built with a recent toolchain. Fixing the alignment of some of the structures in the USB code should also be done, but this is a whole lot more invasive and requires some more thought, as the discussion about this on LKML shows. The issue doesn't show for older toolchains, as they by default emit code to work around unaligned accesses.

Quoted 4 lines

This patch fixes all unaligned issues, that may appear with recent toolchains. We avoid having to instruct the toolchain to work around unaligned accesses and gain better performance in cases where it is needed.

Quoted 5 lines

I am not too happy with enabling a lax behavior only to avoid an issue which apparently is diagnosed and could / should be fixed at its root. Can you point me to the relevant LKML thread
so that I get the details and understand what prevents fixing this by 'simply' aligning the USB structures?

I'm with you that the issue for this particular fault that I ran into should be fixed at it's root and I will do so as soon as I have enough time to do so, i.e. within the next three weeks.
You can find a thread about this here:
https://lkml.org/lkml/2011/4/27/278
The problem here is that simply removing the packed attribute is not the right thing to do for structures that are used for accessing hardware registers. I have to read a bit more of the USB code to come up with the right thing to do for every structure there.

But apart from this, we certainly have situations where we have unaligned accesses that are justified and could not be removed. Activating the aligned access hardware check is crippling a feature of the ARMv7 architecture that's apparently useful enough that all recent toolchains default to using it and not using compiler side workarounds. Furthermore setting the check bit doesn't even deactivate unaligned access (there is also a bit for this, which is forced to 1 by all v7 implementations), but just traps the processor in case you care about such unaligned accesses. Linux for example only sets this check bit if you choose to install a trap handler for this.

I cannot see how enabling a hardware feature can be seen as allowing of lax behaviour. As some of the USB structs are used to access hardware registers, we can not align every struct there. Our options are either:
1. instruct the toolchain to insert workaround code or
2. allow v7 hardware to do the unaligned access on it's own My comment about fixing the USB code applies only to part of the structs used there as some of them generate _unnecessary_ unaligned accesses, the issue that all unaligned accesses fail with current U-Boot built with a recent toolchain has to be fixed either way and is exactly what this patch does.

Thanks,
Lucas

#7 lore raw

PATCH 1/1 arm: enable unaligned access on ARMv7

+Tom

Hi Lucas,

Quoted 51 lines · Lucas Stach

Hi Albert,

Am Freitag, den 22.06.2012, 13:16 +0200 schrieb Albert ARIBAUD:

Quoted 21 lines

Hi Lucas,

Quoted 14 lines
Quoted 7 lines
Quoted 2 lines

Linux in particular does reinitialize this state and I expect any reasonable OS to do so.

Then what is the point of enabling it on U-Boot? Does it fix some issue whereby some mis-aligned piece of data cannot be properly aligned?

Yes, it fixes U-Boot USB on Tegra, when built with a recent toolchain. Fixing the alignment of some of the structures in the USB code should also be done, but this is a whole lot more invasive and requires some more thought, as the discussion about this on LKML shows. The issue doesn't show for older toolchains, as they by default emit code to work around unaligned accesses.

Quoted 4 lines

This patch fixes all unaligned issues, that may appear with recent toolchains. We avoid having to instruct the toolchain to work around unaligned accesses and gain better performance in cases where it is needed.

Quoted 5 lines

I am not too happy with enabling a lax behavior only to avoid an issue which apparently is diagnosed and could / should be fixed at its root. Can you point me to the relevant LKML thread
so that I get the details and understand what prevents fixing this by 'simply' aligning the USB structures?

I'm with you that the issue for this particular fault that I ran into should be fixed at it's root and I will do so as soon as I have enough time to do so, i.e. within the next three weeks.
You can find a thread about this here:
https://lkml.org/lkml/2011/4/27/278
The problem here is that simply removing the packed attribute is not the right thing to do for structures that are used for accessing hardware registers. I have to read a bit more of the USB code to come up with the right thing to do for every structure there.

But apart from this, we certainly have situations where we have unaligned accesses that are justified and could not be removed. Activating the aligned access hardware check is crippling a feature of the ARMv7 architecture that's apparently useful enough that all recent toolchains default to using it and not using compiler side workarounds. Furthermore setting the check bit doesn't even deactivate unaligned access (there is also a bit for this, which is forced to 1 by all v7 implementations), but just traps the processor in case you care about such unaligned accesses. Linux for example only sets this check bit if you choose to install a trap handler for this.

Quoted 10 lines

I cannot see how enabling a hardware feature can be seen as allowing of lax behaviour. As some of the USB structs are used to access hardware registers, we can not align every struct there. Our options are either:
1. instruct the toolchain to insert workaround code or
2. allow v7 hardware to do the unaligned access on it's own My comment about fixing the USB code applies only to part of the structs used there as some of them generate _unnecessary_ unaligned accesses, the issue that all unaligned accesses fail with current U-Boot built with a recent toolchain has to be fixed either way and is exactly what this patch does.

I think this issue was discussed before here(I haven't gone through all the details of your problem, but it looks similar). And I think Tom fixed this by wrapping the problematic accesses with get/set_unaligned().

Please look at this thread, especially starting from my post reporting the OMAP4 regression:
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/113347/

best regards,
Aneesh

#8 lore raw

PATCH 1/1 arm: enable unaligned access on ARMv7

Quoted 67 lines · Aneesh V

+Tom

Hi Lucas,

Quoted 51 lines · Lucas Stach

Hi Albert,

Am Freitag, den 22.06.2012, 13:16 +0200 schrieb Albert ARIBAUD:

Quoted 21 lines

Hi Lucas,

Quoted 14 lines
Quoted 7 lines
Quoted 2 lines

Linux in particular does reinitialize this state and I expect any reasonable OS to do so.

Then what is the point of enabling it on U-Boot? Does it fix some issue whereby some mis-aligned piece of data cannot be properly aligned?

Yes, it fixes U-Boot USB on Tegra, when built with a recent toolchain. Fixing the alignment of some of the structures in the USB code should also be done, but this is a whole lot more invasive and requires some more thought, as the discussion about this on LKML shows. The issue doesn't show for older toolchains, as they by default emit code to work around unaligned accesses.

Quoted 4 lines

This patch fixes all unaligned issues, that may appear with recent toolchains. We avoid having to instruct the toolchain to work around unaligned accesses and gain better performance in cases where it is needed.

Quoted 5 lines

I am not too happy with enabling a lax behavior only to avoid an issue which apparently is diagnosed and could / should be fixed at its root. Can you point me to the relevant LKML thread
so that I get the details and understand what prevents fixing this by 'simply' aligning the USB structures?

I'm with you that the issue for this particular fault that I ran into should be fixed at it's root and I will do so as soon as I have enough time to do so, i.e. within the next three weeks.
You can find a thread about this here:
https://lkml.org/lkml/2011/4/27/278
The problem here is that simply removing the packed attribute is not the right thing to do for structures that are used for accessing hardware registers. I have to read a bit more of the USB code to come up with the right thing to do for every structure there.

But apart from this, we certainly have situations where we have unaligned accesses that are justified and could not be removed. Activating the aligned access hardware check is crippling a feature of the ARMv7 architecture that's apparently useful enough that all recent toolchains default to using it and not using compiler side workarounds. Furthermore setting the check bit doesn't even deactivate unaligned access (there is also a bit for this, which is forced to 1 by all v7 implementations), but just traps the processor in case you care about such unaligned accesses. Linux for example only sets this check bit if you choose to install a trap handler for this.

Quoted 10 lines

I cannot see how enabling a hardware feature can be seen as allowing of lax behaviour. As some of the USB structs are used to access hardware registers, we can not align every struct there. Our options are either:
1. instruct the toolchain to insert workaround code or
2. allow v7 hardware to do the unaligned access on it's own My comment about fixing the USB code applies only to part of the structs used there as some of them generate _unnecessary_ unaligned accesses, the issue that all unaligned accesses fail with current U-Boot built with a recent toolchain has to be fixed either way and is exactly what this patch does.

Quoted 7 lines

I think this issue was discussed before here(I haven't gone through all the details of your problem, but it looks similar). And I think Tom fixed this by wrapping the problematic accesses with get/set_unaligned().

Please look at this thread, especially starting from my post reporting the OMAP4 regression:
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/113347/

BTW, I agree that enabling un-aligned access is not a bad idea.

br,
Aneeesh

#9 lore raw

PATCH 1/1 arm: enable unaligned access on ARMv7

Hi Aneesh,

Quoted 16 lines · Aneesh V
Quoted 15 lines · Aneesh V

+Tom

Hi Lucas,

Quoted 10 lines · Lucas Stach

Hi Albert,

Am Freitag, den 22.06.2012, 13:16 +0200 schrieb Albert ARIBAUD:

Quoted 5 lines

I am not too happy with enabling a lax behavior only to avoid an issue which apparently is diagnosed and could / should be fixed at its root. Can you point me to the relevant LKML thread so that I get the details and understand what prevents fixing this by 'simply' aligning the USB structures?

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

I'm with you that the issue for this particular fault that I ran into should be fixed at it's root and I will do so as soon as I have enough time to do so, i.e. within the next three weeks. You can find a thread about this here:
https://lkml.org/lkml/2011/4/27/278

From what I understand, the issue was not to allow unaligned access at the hardware level, but to modify the attributes of the structure, first by removing the packed attribute, then by reinstating the packed attribute along with align(4).

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines

But apart from this, we certainly have situations where we have unaligned accesses that are justified and could not be removed. [...]
I cannot see how enabling a hardware feature can be seen as allowing of lax behaviour. As some of the USB structs are used to access hardware registers, we can not align every struct there.

If the access is in true RAM, then we can always realign the data; and I don't know of memory-mapped registers which would be unaligned wrt their width. If some USB controller is designed so, then the fix should only and explicitly affect that controller, because we don't know it it will always be used with an ARM implementation that can do unaligned accesses.

Quoted 4 lines
Quoted 4 lines

I think this issue was discussed before here(I haven't gone through all the details of your problem, but it looks similar). And I think Tom fixed this by wrapping the problematic accesses with get/set_unaligned().

Could be a solution if the structures themselves cannot be fixed.

Quoted 3 lines
Quoted 3 lines

Please look at this thread, especially starting from my post reporting the OMAP4 regression:
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/113347/

Thanks for the reference. There seems to have been no confirmation that the structures involved needed packing in the first place, and my general opinion on packing structures is "DO NOT" -- if packing a structure has an effect, it can alway be to de-align some field, which barely makes sense as far as HW prgramming is concerned (I can only see some point in packing a struct when you deal with network layer 7 data in very special cases).

Quoted 1 line

BTW, I agree that enabling un-aligned access is not a bad idea.

Just being "not a bad idea" is not enough for me to accept this. It will have to be the sole sound solution to a problem, and at this point, I do not think it is as far as USB structure mis-alignement issues are concerned.

Quoted 2 lines

br,
Aneeesh

Amicalement,

Signature · 1 line
Albert.
#10 lore raw

PATCH 1/1 arm: enable unaligned access on ARMv7

Hi Albert,

Quoted 63 lines · Albert ARIBAUD

Hi Aneesh,

Quoted 16 lines · Aneesh V
Quoted 15 lines · Aneesh V

+Tom

Hi Lucas,

Quoted 10 lines · Lucas Stach

Hi Albert,

Am Freitag, den 22.06.2012, 13:16 +0200 schrieb Albert ARIBAUD:

Quoted 5 lines

I am not too happy with enabling a lax behavior only to avoid an issue which apparently is diagnosed and could / should be fixed at its root. Can you point me to the relevant LKML thread so that I get the details and understand what prevents fixing this by 'simply' aligning the USB structures?

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

I'm with you that the issue for this particular fault that I ran into should be fixed at it's root and I will do so as soon as I have enough time to do so, i.e. within the next three weeks. You can find a thread about this here:
https://lkml.org/lkml/2011/4/27/278

From what I understand, the issue was not to allow unaligned access at the hardware level, but to modify the attributes of the structure, first by removing the packed attribute, then by reinstating the packed attribute along with align(4).

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines

But apart from this, we certainly have situations where we have unaligned accesses that are justified and could not be removed. [...]
I cannot see how enabling a hardware feature can be seen as allowing of lax behaviour. As some of the USB structs are used to access hardware registers, we can not align every struct there.

If the access is in true RAM, then we can always realign the data; and I don't know of memory-mapped registers which would be unaligned wrt their width. If some USB controller is designed so, then the fix should only and explicitly affect that controller, because we don't know it it will always be used with an ARM implementation that can do unaligned accesses.

Quoted 4 lines
Quoted 4 lines

I think this issue was discussed before here(I haven't gone through all the details of your problem, but it looks similar). And I think Tom fixed this by wrapping the problematic accesses with get/set_unaligned().

Could be a solution if the structures themselves cannot be fixed.

Quoted 3 lines
Quoted 3 lines

Please look at this thread, especially starting from my post reporting the OMAP4 regression:
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/113347/

Thanks for the reference. There seems to have been no confirmation that the structures involved needed packing in the first place, and my general opinion on packing structures is "DO NOT" -- if packing a structure has an effect, it can alway be to de-align some field, which barely makes sense as far as HW prgramming is concerned (I can only see some point in packing a struct when you deal with network layer 7 data in very special cases).

Quoted 6 lines
Quoted 1 line

BTW, I agree that enabling un-aligned access is not a bad idea.

Just being "not a bad idea" is not enough for me to accept this. It will have to be the sole sound solution to a problem, and at this point, I do not think it is as far as USB structure mis-alignement issues are concerned.

My point is that enabling un-aligned accesses in itsown merit is not a bad idea, not as a solution to this problem. I have seen it being enabled in HLOS environment. TI's Symbian port for instance had it enabled for OMAP3. I don't know why Linux too shoudln't take advantage of such hw features.
Perhaps you don't want to take it at this point of time to force the real solution to the USB problem, which is reasonable.

#11 lore raw

PATCH 1/1 arm: enable unaligned access on ARMv7

Hi Aneesh,

Quoted 7 lines
Quoted 6 lines
Quoted 1 line

BTW, I agree that enabling un-aligned access is not a bad idea.

Just being "not a bad idea" is not enough for me to accept this. It will have to be the sole sound solution to a problem, and at this point, I do not think it is as far as USB structure mis-alignement issues are concerned.

Quoted 7 lines

My point is that enabling un-aligned accesses in itsown merit is not a bad idea, not as a solution to this problem. I have seen it being enabled in HLOS environment. TI's Symbian port for instance had it enabled for OMAP3. I don't
know why Linux too shoudln't take advantage of such hw features. Perhaps you don't want to take it at this point of time to force the real solution to the USB problem, which is reasonable.

What is the (non-contrived) problem to which allowing mis-aligned accesses would be a solution?

Amicalement,

Signature · 1 line
Albert.
#12 lore raw

PATCH 1/1 arm: enable unaligned access on ARMv7

Hi Albert,

Quoted 17 lines · Albert ARIBAUD

Hi Aneesh,

Quoted 7 lines
Quoted 6 lines
Quoted 1 line

BTW, I agree that enabling un-aligned access is not a bad idea.

Just being "not a bad idea" is not enough for me to accept this. It will have to be the sole sound solution to a problem, and at this point, I do not think it is as far as USB structure mis-alignement issues are concerned.

Quoted 7 lines

My point is that enabling un-aligned accesses in itsown merit is not a bad idea, not as a solution to this problem. I have seen it being enabled in HLOS environment. TI's Symbian port for instance had it enabled for OMAP3. I don't
know why Linux too shoudln't take advantage of such hw features. Perhaps you don't want to take it at this point of time to force the real solution to the USB problem, which is reasonable.

Quoted 2 lines

What is the (non-contrived) problem to which allowing mis-aligned accesses would be a solution?

memcpy() when there is a mismatch in the alignment of source and destination buffers. Let's say the source buffer is 4 byte aligned but the destination buffer is only 2 byte aligned. I believe relaxed alignment requirements will help in writing an accelerated memcpy routine for this case.

Again, my point is that as a platform software provider I would like to enable such features to make maximum things work on my platform, where as the developer of a generic sw module should probably avoid depending on such features to ensure maximum portability.

br,
Aneesh

#13 lore raw

PATCH 1/1 arm: enable unaligned access on ARMv7

Dear Aneesh V,

Quoted 9 lines · In message <[email protected]> you wrote:
Quoted 3 lines

What is the (non-contrived) problem to which allowing mis-aligned accesses would be a solution?

memcpy() when there is a mismatch in the alignment of source and destination buffers. Let's say the source buffer is 4 byte aligned but the destination buffer is only 2 byte aligned. I believe relaxed alignment requirements will help in writing an accelerated memcpy routine for this case.

If memcpy() has problems with handling such a situation, then clearly memcpy() needs fixing.

Best regards,

Wolfgang Denk

Signature · 7 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
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
"I dislike companies that have a we-are-the-high-priests-of-hardware-
so-you'll-like-what-we-give-you attitude. I like commodity markets in
which iron-and-silicon hawkers know that they exist to  provide  fast
toys for software types like me to play with..."    - Eric S. Raymond
#14 lore raw

PATCH 1/1 arm: enable unaligned access on ARMv7

In reply to Albert ARIBAUD
Quoted 10 lines · Albert ARIBAUD
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

I cannot see how enabling a hardware feature can be seen as allowing of lax behaviour. As some of the USB structs are used to access hardware registers, we can not align every struct there.

If the access is in true RAM, then we can always realign the data; and I don't know of memory-mapped registers which would be unaligned wrt their width. If some USB controller is designed so, then the fix should only and explicitly affect that controller, because we don't know it it will always be used with an ARM implementation that can do unaligned accesses.

The ARM architecture does not permit unaligned accesses to strongly ordered or device memory, so MMIO register accesses are always aligned.

Signature · 2 lines
M?ns Rullg?rd
mans at mansr.com
#15 lore raw

PATCH 1/1 arm: enable unaligned access on ARMv7

In reply to Albert ARIBAUD

Hi Albert,

Am Samstag, den 23.06.2012, 11:01 +0200 schrieb Albert ARIBAUD:
[snip]

Quoted 7 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines

But apart from this, we certainly have situations where we have unaligned accesses that are justified and could not be removed. [...]
I cannot see how enabling a hardware feature can be seen as allowing of lax behaviour. As some of the USB structs are used to access hardware registers, we can not align every struct there.

Quoted 6 lines

If the access is in true RAM, then we can always realign the data; and I don't know of memory-mapped registers which would be unaligned wrt their width. If some USB controller is designed so, then the fix should only and explicitly affect that controller, because we don't know it it will always be used with an ARM implementation that can do unaligned accesses.

My point is: on ARM platforms that can't do unaligned access in hardware the toolchain will automaticly emit helper code to work around this. On an ARMv7 platform hardware assisted unaligned access is mandatory, so toolchains will not emit helper code and rather let the hardware do it's job.
Now the situation is as follows: hardware platforms without the ability to do unaligned access in hardware will just work even though the code is suboptimal, but users of an ARMv7 platform will encounter unexplained system hangs, which is bad imho. This patch just makes behaviour consistent across ARMv5 and ARMv7 platforms.

If we really want to disallow the use of unaligned memory operations in U-Boot we should make all platforms fail at compile time, not make one platform fail randomly at runtime. Do you think this is the way to go? I'm fine either way, I'm just not okay with the current situation where one platform fails while another just works.

Thanks,
Lucas

A message that is not in the archive (sent off-list or not yet received): 20120625221741.3a32790e@lilith

#17 lore raw

PATCH 1/1 arm: enable unaligned access on ARMv7

Hi Albert,

Am Montag, den 25.06.2012, 22:17 +0200 schrieb Albert ARIBAUD:

Quoted 22 lines

Hi Lucas,

Quoted 19 lines · Lucas Stach

Hi Albert,

Am Samstag, den 23.06.2012, 11:01 +0200 schrieb Albert ARIBAUD:
[snip]

Quoted 14 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

But apart from this, we certainly have situations where we have unaligned accesses that are justified and could not be removed. [...]
I cannot see how enabling a hardware feature can be seen as allowing of lax behaviour. As some of the USB structs are used to access hardware registers, we can not align every struct there.

If the access is in true RAM, then we can always realign the data; and I don't know of memory-mapped registers which would be unaligned wrt their width. If some USB controller is designed so, then the fix should only and explicitly affect that controller, because we don't know it it will always be used with an ARM implementation that can do unaligned accesses.

Quoted 9 lines
Quoted 5 lines

My point is: on ARM platforms that can't do unaligned access in hardware the toolchain will automaticly emit helper code to work around this. On an ARMv7 platform hardware assisted unaligned access is mandatory, so toolchains will not emit helper code and rather let the hardware do it's job.

My point is that there should be a reason for doing unaligned access to boot (pun half-intended) and I don't see any.

I see your point here.

Quoted 8 lines
Quoted 5 lines

Now the situation is as follows: hardware platforms without the ability to do unaligned access in hardware will just work even though the code is suboptimal, but users of an ARMv7 platform will encounter unexplained system hangs, which is bad imho. This patch just makes behaviour consistent across ARMv5 and ARMv7 platforms.

Wouldn't aligning data properly make behaviour consistent as well?

Consistent in the way of "it works on all platforms", but not in the way of programming practice. ARMv7 people will have to care about alignment to not encounter runtime errors, while on ARMv5 it just doesn't matter. (Which certainly does not make it the right thing to do on ARMv5, but as the existing USB code shows people just haven't cared about this.)

Quoted 6 lines
Quoted 5 lines

If we really want to disallow the use of unaligned memory operations in U-Boot we should make all platforms fail at compile time, not make one platform fail randomly at runtime. Do you think this is the way to go? I'm fine either way, I'm just not okay with the current situation where one platform fails while another just works.

Quoted 7 lines

I do not want to disallow unaligned accesses, because this assumes they were allowed in the first place; I want to keep not allowing them, just like they were unallowed far, and just like they can keep on being. If a misaligned access causes one platform to fail while another just works, then we should avoid the misaligned access by re-aligning it properly so as to make both platforms work.

The thing I wanted to say is: if we generally consider unaligned access a no-go for ARMv7 (which I'm fine with), we should equally consider it a no-go for ARMv5. So I would rather want to see both platforms fail at compile time if we have unaligned data, to avoid the unintended introduction of such code in the codebase as has happened with the usb code. Sadly I have not found a way to tell gcc to error out if encounters an unaligned access. Currently ARMv5 "allows" unaligned data, as toolchains insert helper code which makes it work.

Also it is really unfortunate that coding errors (which the unjustified use of unaligned data certainly is) manifest as a runtime error, rather than a compiletime failure, which make them a lot harder to spot.

Quoted 3 lines

IOW, things will not change until I am shown some actual situation where a misaligned access is necessary and the core prevents it.

I'm okay with you dropping this patch and will try to come up with something that make programming constraints more consistent across both ARM platforms.

Thanks,
Lucas

#18 lore raw

PATCH 1/1 arm: enable unaligned access on ARMv7

In reply to Albert ARIBAUD
Quoted 22 lines · Albert ARIBAUD

Hi Lucas,

Quoted 19 lines · Lucas Stach

Hi Stephen,

Am Dienstag, den 05.06.2012, 12:42 -0600 schrieb Stephen Warren:

Quoted 15 lines · Lucas Stach
Quoted 9 lines

Recent toolchains default to using the hardware feature for unaligned access on ARM v7, rather than doing the software fallback. According to ARM this is safe as all v7 implementations have to support this feature.
(http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0471c/BABJFFAE.html)

To avoid CPU hangs when doing unaligned memory access, we have to turn off alignment checking in our CPU initialisation code.
(http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0491c/CIHCGCFD.html)

Does this behavior change trickle down to Linux/... too, or would an OS completely re-initialize this state, and hence not be affected?

Quoted 6 lines
Quoted 2 lines

Linux in particular does reinitialize this state and I expect any reasonable OS to do so.

Then what is the point of enabling it on U-Boot? Does it fix some issue whereby some mis-aligned piece of data cannot be properly aligned?

This is a new optimization feature in gcc 4.7 (and backported to some 4.6 versions like the ubuntu 12.04 arm cross compiler (4.6.3)):

http://lists.linaro.org/pipermail/linaro-dev/2012-June/012360.html

http://seabright.co.nz/2012/06/11/kernel-not-booting-with-linaro-gcc/

If you don't want to enable unaligned accesses, then "-mno-unaligned-access" needs to be added.

Regards,
Rob

#19 lore raw

PATCH 1/1 arm: enable unaligned access on ARMv7

Hi Rob and all,

Quoted 28 lines
Quoted 18 lines
Quoted 18 lines

Am Dienstag, den 05.06.2012, 12:42 -0600 schrieb Stephen Warren:

Quoted 15 lines · Lucas Stach
Quoted 9 lines

Recent toolchains default to using the hardware feature for unaligned access on ARM v7, rather than doing the software fallback. According to ARM this is safe as all v7 implementations have to support this feature.
(http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0471c/BABJFFAE.html)

To avoid CPU hangs when doing unaligned memory access, we have to turn off alignment checking in our CPU initialisation code.
(http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0491c/CIHCGCFD.html)

Does this behavior change trickle down to Linux/... too, or would an OS completely re-initialize this state, and hence not be affected?

Quoted 6 lines
Quoted 2 lines

Linux in particular does reinitialize this state and I expect any reasonable OS to do so.

Then what is the point of enabling it on U-Boot? Does it fix some issue whereby some mis-aligned piece of data cannot be properly aligned?

This is a new optimization feature in gcc 4.7 (and backported to some 4.6 versions like the ubuntu 12.04 arm cross compiler (4.6.3)):

Quoted 7 lines

http://lists.linaro.org/pipermail/linaro-dev/2012-June/012360.html

http://seabright.co.nz/2012/06/11/kernel-not-booting-with-linaro-gcc/

If you don't want to enable unaligned accesses, then "-mno-unaligned-access" needs to be added.

I verified it. Option "-mno-unaligned-access" works good.

include/mtd/cfi_flash.h

/* CFI standard query structure */

struct cfi_qry {
	u8	qry[3];
	u16	p_id;  <-- unaligned!
	...
} __attribute__((packed));
$ ${CROSS_COMPILE}gcc --version
arm-none-eabi-gcc (GNU Tools for ARM Embedded Processors) 4.6.2 20110921 (release) [ARM/embedded-4_6-branch revision 182083]
Copyright (C) 2011 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

=====================================================================
Compiled without --mno-unaligned-access

$ ${CROSS_COMPILE}objdump -d -S u-boot 
                info->vendor = le16_to_cpu(qry.p_id);
    cc88:       e3003a1c        movw    r3, #2588       ; 0xa1c
    cc8c:       e1dd11bb        ldrh    r1, [sp, #27]  <-- this is unaligned access
    cc90:       ...
    cc94:       e18410b3        strh    r1, [r4, r3]

=====================================================================
Compiled with --mno-unaligned-access

$ ${CROSS_COMPILE}objdump -d -S u-boot 
                info->vendor = le16_to_cpu(qry.p_id);
    cce8:       e5dd101c        ldrb    r1, [sp, #28]  <--
    ccec:       e5dd301b        ldrb    r3, [sp, #27]  <-- separated 2 byte accesses
    ccf0:       ...
    ccf4:       e1831401        orr     r1, r3, r1, lsl #8
    ccf8:       e3003a1c        movw    r3, #2588       ; 0xa1c
    ccfc:       e18410b3        strh    r1, [r4, r3]
#20 lore raw

PATCH 1/1 arm: armv7: add compile option -mno-unaligned-access if available

Recent compiler generates unaligned memory access in armv7 default. But current U-Boot does not allow unaligned memory access, so it causes data abort exception.
This patch add compile option "-mno-unaligned-access" if it is available.

---
 arch/arm/cpu/armv7/config.mk |    2 ++
 1 file changed, 2 insertions(+)
arch/arm/cpu/armv7/config.mk +2 -0
index 5407cb6..560c084 100644
@@ -26,6 +26,8 @@ PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float
 # supported by more tool-chains
 PF_CPPFLAGS_ARMV7 := $(call cc-option, -march=armv7-a, -march=armv5)
 PLATFORM_CPPFLAGS += $(PF_CPPFLAGS_ARMV7)
+PF_CPPFLAGS_NO_UNALIGNED := $(call cc-option, -mno-unaligned-access,)
+PLATFORM_CPPFLAGS += $(PF_CPPFLAGS_NO_UNALIGNED)
 
 # =========================================================================
 #

-- 1.7.9.5

#21 lore raw

PATCH 1/1 arm: armv7: add compile option -mno-unaligned-access if available

Quoted 4 lines · Tetsuyuki Kobayashi

Recent compiler generates unaligned memory access in armv7 default. But current U-Boot does not allow unaligned memory access, so it causes data abort exception.
This patch add compile option "-mno-unaligned-access" if it is available.

Why not allow unaligned accesses instead?

Signature · 2 lines
M?ns Rullg?rd
mans at mansr.com
#22 lore raw

PATCH 1/1 arm: armv7: add compile option -mno-unaligned-access if available

Am Montag, den 02.07.2012, 10:53 +0100 schrieb M?ns Rullg?rd:

Quoted 9 lines
Quoted 4 lines · Tetsuyuki Kobayashi

Recent compiler generates unaligned memory access in armv7 default. But current U-Boot does not allow unaligned memory access, so it causes data abort exception.
This patch add compile option "-mno-unaligned-access" if it is available.

Why not allow unaligned accesses instead?

IMHO, our recent discussion showed that both ways are wrong. "-mno-unaligned-access" works around misaligned data on the software level, while allowing unaligned access does on the hardware level.

What we really want is no unaligned access in U-Boot at all. Just because "-mno-unaligned-access" is the default on ARMv5, we should not consider it a gold standard.

Thanks,
Lucas

#23 lore raw

PATCH 1/1 arm: armv7: add compile option -mno-unaligned-access if available

Quoted 10 lines · Lucas Stach

Am Montag, den 02.07.2012, 10:53 +0100 schrieb M?ns Rullg?rd:

Quoted 9 lines
Quoted 4 lines · Tetsuyuki Kobayashi

Recent compiler generates unaligned memory access in armv7 default. But current U-Boot does not allow unaligned memory access, so it causes data abort exception.
This patch add compile option "-mno-unaligned-access" if it is available.

Why not allow unaligned accesses instead?

Quoted 7 lines

IMHO, our recent discussion showed that both ways are wrong. "-mno-unaligned-access" works around misaligned data on the software level, while allowing unaligned access does on the hardware level.

What we really want is no unaligned access in U-Boot at all. Just because "-mno-unaligned-access" is the default on ARMv5, we should not consider it a gold standard.

It's slightly more complicated than that. Data can be misaligned for a variety of reasons:

1. Errors in software.
2. Specified by a file format or communication protocol.
3. Deliberately misaligned by the compiler.

Misaligned data of type 1 should of course be fixed properly, not worked around in any way.

Type 2 happens all the time, and has to be dealt with one way or another. If the hardware supports unaligned accesses, this is usually faster than reading a byte at a time.

When targeting ARMv6 and later, recent gcc versions have started issuing deliberate unaligned accesses where previously byte by byte accesses would have been done. This happens with "packed" structs and sometimes to write multiple smaller values at once, typically when zero-initialising things. These unaligned accesses are *good*. They make code smaller and faster.

The real problem here is that u-boot is setting the strict alignment checking flag, invalidating the assumption of the compiler that the system allows unaligned accesses. For ARMv5 and earlier, setting this flag is usually advisable since it makes finding accidental unaligned accesses much easier.

This was debated in the context of the kernel a while ago, ultimately leading to strict alignment being disabled for ARMv6 and up [1].

[1] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=8428e84d42179c2a00f5f6450866e70d802d1d05

Signature · 2 lines
M?ns Rullg?rd
mans at mansr.com
#24 lore raw

PATCH 1/1 arm: armv7: add compile option -mno-unaligned-access if available

Hello, M?ns
Thank you for summarizing.

I am not against you.
I'm OK either "Allow unaligned access in U-Boot setting" or "Specify compiler not to generate unaligned memory access" or other.
I just want to solve hung-up by unaligned access. I follow custodian's decision.

Quoted 52 lines · M?ns Rullg?rd
Quoted 10 lines · Lucas Stach

Am Montag, den 02.07.2012, 10:53 +0100 schrieb M?ns Rullg?rd:

Quoted 9 lines
Quoted 4 lines · Tetsuyuki Kobayashi

Recent compiler generates unaligned memory access in armv7 default. But current U-Boot does not allow unaligned memory access, so it causes data abort exception.
This patch add compile option "-mno-unaligned-access" if it is available.

Why not allow unaligned accesses instead?

Quoted 7 lines

IMHO, our recent discussion showed that both ways are wrong. "-mno-unaligned-access" works around misaligned data on the software level, while allowing unaligned access does on the hardware level.

What we really want is no unaligned access in U-Boot at all. Just because "-mno-unaligned-access" is the default on ARMv5, we should not consider it a gold standard.

It's slightly more complicated than that. Data can be misaligned for a variety of reasons:

1. Errors in software.
2. Specified by a file format or communication protocol.
3. Deliberately misaligned by the compiler.

Misaligned data of type 1 should of course be fixed properly, not worked around in any way.

Type 2 happens all the time, and has to be dealt with one way or another. If the hardware supports unaligned accesses, this is usually faster than reading a byte at a time.

When targeting ARMv6 and later, recent gcc versions have started issuing deliberate unaligned accesses where previously byte by byte accesses would have been done. This happens with "packed" structs and sometimes to write multiple smaller values at once, typically when zero-initialising things. These unaligned accesses are *good*. They make code smaller and faster.

The real problem here is that u-boot is setting the strict alignment checking flag, invalidating the assumption of the compiler that the system allows unaligned accesses. For ARMv5 and earlier, setting this flag is usually advisable since it makes finding accidental unaligned accesses much easier.

This was debated in the context of the kernel a while ago, ultimately leading to strict alignment being disabled for ARMv6 and up [1].

[1] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=8428e84d42179c2a00f5f6450866e70d802d1d05

#25 lore raw

PATCH 1/1 arm: armv7: add compile option -mno-unaligned-access if available

In reply to Måns Rullgård

Hi M?ns,

Quoted 11 lines · M?ns Rullg?rd
Quoted 7 lines

IMHO, our recent discussion showed that both ways are wrong. "-mno-unaligned-access" works around misaligned data on the software level, while allowing unaligned access does on the hardware level.

What we really want is no unaligned access in U-Boot at all. Just because "-mno-unaligned-access" is the default on ARMv5, we should not consider it a gold standard.

It's slightly more complicated than that. Data can be misaligned for a variety of reasons:

Quoted 6 lines

1. Errors in software.
2. Specified by a file format or communication protocol.
3. Deliberately misaligned by the compiler.

Misaligned data of type 1 should of course be fixed properly, not worked around in any way.

Agreed.

Quoted 3 lines

Type 2 happens all the time, and has to be dealt with one way or another. If the hardware supports unaligned accesses, this is usually faster than reading a byte at a time.

Sorry but I don't accept a systemic change as a good solution to a specific issue. The best fix is "don't accept using a protocol or file format which was not designed to avoid native misaligned accesses", and the second best fix is "if you really must deal with a protocol or file format which causes native alignment issues, then the fix should only affect the protocol or file format's issue, not with your system which is not the root cause".

But to be honest, I haven't seen such badly a designed protocol or file format; their designers tend to make sure that (assuming the start of a protocol or file 'content' (frame, record, etc) is aligned, then all fields in it are aligned as well. Can someone point me to an example of a protocol or file format which does present such a misalignment risk ?

Quoted 3 lines

When targeting ARMv6 and later, recent gcc versions have started issuing deliberate unaligned accesses where previously byte by byte accesses would have been done.

Wrongly formulated: "mis-aligning deliberately" seems to imply that GCC did away with the possibility of properly aligning, which they of course did not. What they did is switch its default behavior regarding native misaligned accesses from forbidding to allowing them. The change here is of policy, a change which we may or may not want to follow.

Quoted 4 lines

This happens with "packed" structs
and sometimes to write multiple smaller values at once, typically when zero-initialising things. These unaligned accesses are *good*. They make code smaller and faster.

Again, "good" is a policy, or subjective, statement, not an absolute. Just as "good" is correctly aligning data in the first place (to begin with, in protocols and file formats) and keeping the compiler's native misaligned access policy set to "do not allow".

Quoted 5 lines

The real problem here is that u-boot is setting the strict alignment checking flag, invalidating the assumption of the compiler that the system allows unaligned accesses. For ARMv5 and earlier, setting this flag is usually advisable since it makes finding accidental unaligned accesses much easier.

Just as it is for ARMv6 and up. Again, just because the compiler folks changed their default policy does not mean we should change ours, which is not based on the same goals.

Quoted 5 lines

This was debated in the context of the kernel a while ago, ultimately leading to strict alignment being disabled for ARMv6 and up [1].

[1]
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=8428e84d42179c2a00f5f6450866e70d802d1d05

I'd rather have a link to the rationale than to the commit, but anyway, the kernel folks' decision is theirs and does not necessarily apply to us. I have mailed Catalin Marinas off-list to get details on the rationale and context of the kernel patch; I will report conclusions here.

Meanwhile, our policy regarding misalignment accesses is to only allow them when externally required (by something other than a bad design). Someone please show me such an external requirement for U-Boot, and I will reconsider -- and then, other arch custodians may have a problem with that too.

Regarding the origin of this patch, i.e. a mis-alignment of USB fields, and unless U-Boot USB folks say otherwise, this issue should be fixed by aligning said fields properly.

Amicalement,

Signature · 1 line
Albert.
#26 lore raw

PATCH 1/1 arm: armv7: add compile option -mno-unaligned-access if available

on Thu, 5 Jul 2012 09:57:19 +0200,

Quoted 91 lines · Albert ARIBAUD

Hi M?ns,

Quoted 11 lines · M?ns Rullg?rd
Quoted 7 lines

IMHO, our recent discussion showed that both ways are wrong. "-mno-unaligned-access" works around misaligned data on the software level, while allowing unaligned access does on the hardware level.

What we really want is no unaligned access in U-Boot at all. Just because "-mno-unaligned-access" is the default on ARMv5, we should not consider it a gold standard.

It's slightly more complicated than that. Data can be misaligned for a variety of reasons:

Quoted 6 lines

1. Errors in software.
2. Specified by a file format or communication protocol.
3. Deliberately misaligned by the compiler.

Misaligned data of type 1 should of course be fixed properly, not worked around in any way.

Agreed.

Quoted 3 lines

Type 2 happens all the time, and has to be dealt with one way or another. If the hardware supports unaligned accesses, this is usually faster than reading a byte at a time.

Sorry but I don't accept a systemic change as a good solution to a specific issue. The best fix is "don't accept using a protocol or file format which was not designed to avoid native misaligned accesses", and the second best fix is "if you really must deal with a protocol or file format which causes native alignment issues, then the fix should only affect the protocol or file format's issue, not with your system which is not the root cause".

But to be honest, I haven't seen such badly a designed protocol or file format; their designers tend to make sure that (assuming the start of a protocol or file 'content' (frame, record, etc) is aligned, then all fields in it are aligned as well. Can someone point me to an example of a protocol or file format which does present such a misalignment risk ?

Quoted 3 lines

When targeting ARMv6 and later, recent gcc versions have started issuing deliberate unaligned accesses where previously byte by byte accesses would have been done.

Wrongly formulated: "mis-aligning deliberately" seems to imply that GCC did away with the possibility of properly aligning, which they of course did not. What they did is switch its default behavior regarding native misaligned accesses from forbidding to allowing them. The change here is of policy, a change which we may or may not want to follow.

Quoted 4 lines

This happens with "packed" structs
and sometimes to write multiple smaller values at once, typically when zero-initialising things. These unaligned accesses are *good*. They make code smaller and faster.

Again, "good" is a policy, or subjective, statement, not an absolute. Just as "good" is correctly aligning data in the first place (to begin with, in protocols and file formats) and keeping the compiler's native misaligned access policy set to "do not allow".

Quoted 5 lines

The real problem here is that u-boot is setting the strict alignment checking flag, invalidating the assumption of the compiler that the system allows unaligned accesses. For ARMv5 and earlier, setting this flag is usually advisable since it makes finding accidental unaligned accesses much easier.

Just as it is for ARMv6 and up. Again, just because the compiler folks changed their default policy does not mean we should change ours, which is not based on the same goals.

Quoted 5 lines

This was debated in the context of the kernel a while ago, ultimately leading to strict alignment being disabled for ARMv6 and up [1].

[1]
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=8428e84d42179c2a00f5f6450866e70d802d1d05

I'd rather have a link to the rationale than to the commit, but anyway, the kernel folks' decision is theirs and does not necessarily apply to us. I have mailed Catalin Marinas off-list to get details on the rationale and context of the kernel patch; I will report conclusions here.

Meanwhile, our policy regarding misalignment accesses is to only allow them when externally required (by something other than a bad design). Someone please show me such an external requirement for U-Boot, and I will reconsider -- and then, other arch custodians may have a problem with that too.

Quoted 3 lines

Regarding the origin of this patch, i.e. a mis-alignment of USB fields, and unless U-Boot USB folks say otherwise, this issue should be fixed by aligning said fields properly.

We are nearing the release, and we obviously won't have misalignments fixed and tested in time for it.

So I suspect that if I want the ARM U-Boot release to work I'll have to allow this patch in my master branch to be pulled in for 12.07, so that the compiler keeps behaving as it did before gcc changed the default.

... but only for the upcoming release, i.e. I will revert the patch in my 'next' branch, which will apply right after 12.07 is out. Therefore, before next release, misalignments will have to be fixed at the root.

Amicalement,

Signature · 1 line
Albert.
#27 lore raw

PATCH 1/1 arm: armv7: add compile option -mno-unaligned-access if available

In reply to Albert ARIBAUD
Quoted 5 lines · On Thursday 05 July 2012 03:57:19 Albert ARIBAUD wrote:

But to be honest, I haven't seen such badly a designed protocol or file format; their designers tend to make sure that (assuming the start of a protocol or file 'content' (frame, record, etc) is aligned, then all fields in it are aligned as well. Can someone point me to an example of a protocol or file format which does present such a misalignment risk ?

simply search the kernel for get_unaligned then. there are plenty of examples in there. granted, many apply to stacks that don't show up in u-boot (yet?) such as bluetooth, wireless, and irda, but i'm pretty sure TCP/IPv4 has a few edge cases too.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120719/f157e47d/attachment.pgp>

#28 lore raw

PATCH 1/1 arm: armv7: add compile option -mno-unaligned-access if available

In reply to Måns Rullgård
Quoted 9 lines · On Monday 02 July 2012 12:14:40 M?ns Rullg?rd wrote:

It's slightly more complicated than that. Data can be misaligned for a variety of reasons:

1. Errors in software.
2. Specified by a file format or communication protocol.
3. Deliberately misaligned by the compiler.

Misaligned data of type 1 should of course be fixed properly, not worked around in any way.

it's also a reliability aspect. people don't write bug free software, not bug free protocols, nor bug free compilers. when misalignment does happen in the field, it's a hell of a lot better if the software continued to execute correctly rather than randomly triggered an exception.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120719/aed59442/attachment.pgp>

#29 lore raw

PATCH 1/1 arm: armv7: add compile option -mno-unaligned-access if available

Hi Mike,

Quoted 8 lines · Mike Frysinger
Quoted 7 lines · On Monday 02 July 2012 12:14:40 M?ns Rullg?rd wrote:

It's slightly more complicated than that. Data can be misaligned for a variety of reasons:

1. Errors in software.
2. Specified by a file format or communication protocol.
3. Deliberately misaligned by the compiler.

Quoted 8 lines
Quoted 2 lines

Misaligned data of type 1 should of course be fixed properly, not worked around in any way.

it's also a reliability aspect. people don't write bug free software, not bug free protocols, nor bug free compilers. when misalignment does happen in the field, it's a hell of a lot better if the software continued to execute correctly rather than randomly triggered an exception.
-mike

Nitpick: this is robustness, not reliability.

That being said, yes, this robustness is desirable when you do not control all of the SW running on the product; Linux, for instance, will have to execute processes which were built with any old (or new) compiler settings, thus the Linux folks have to make sure the kernel won't fail running those.

But the only uncontrolled SW U-Boot runs is its payload -- typically the kernel image -- which are usually very cautious in what they assume they can do, thus are unlikely to perform unaligned accesses.

(pasting your other comment)

Quoted 4 lines

simply search the kernel for get_unaligned then. there are plenty of examples in there. granted, many apply to stacks that don't show up in u-boot (yet?) such as bluetooth, wireless, and irda, but i'm pretty sure TCP/IPv4 has a few edge cases too.

I'll have a look, if only to lament that protocol are not what they used to be in the old days. :)

Anyway: as I said: performing *controlled* unaligned accesses for external reasons other than bugs is fine with me. Having our own get_unaligned() in such places would be fine with me.

Amicalement,

Signature · 1 line
Albert.
#30 lore raw

PATCH 1/1 arm: armv7: add compile option -mno-unaligned-access if available

Quoted 12 lines · On Thursday 19 July 2012 02:28:05 Albert ARIBAUD wrote:
Quoted 11 lines · Mike Frysinger
Quoted 9 lines · On Monday 02 July 2012 12:14:40 M?ns Rullg?rd wrote:

It's slightly more complicated than that. Data can be misaligned for a variety of reasons:

1. Errors in software.
2. Specified by a file format or communication protocol.
3. Deliberately misaligned by the compiler.

Misaligned data of type 1 should of course be fixed properly, not worked around in any way.

Quoted 7 lines
Quoted 5 lines

it's also a reliability aspect. people don't write bug free software, not bug free protocols, nor bug free compilers. when misalignment does happen in the field, it's a hell of a lot better if the software continued to execute correctly rather than randomly triggered an exception.

Nitpick: this is robustness, not reliability.

useless pedantry: by increasing robustness, the system is more reliable

Quoted 9 lines

That being said, yes, this robustness is desirable when you do not control all of the SW running on the product; Linux, for instance, will have to execute processes which were built with any old (or new) compiler settings, thus the Linux folks have to make sure the kernel won't fail running those.

But the only uncontrolled SW U-Boot runs is its payload -- typically the kernel image -- which are usually very cautious in what they assume they can do, thus are unlikely to perform unaligned accesses.

it isn't just that. there is no way you can guarantee both the linux kernel and u-boot code bases themselves are perfect. in fact, it's even worse when these are the ones that get tripped up because it means your system resets/hardlocks/kills a kitten. when doing driver development under the linux kernel, we would come across parts of core stacks that lacked alignment checking and would panic the system. sometimes it would always panic, other times it depended on factors that made life worse: the compiler version (newer ones always like to pack/optimize better), the actual data stream, or the execution paths.

Quoted 11 lines
Quoted 4 lines

simply search the kernel for get_unaligned then. there are plenty of examples in there. granted, many apply to stacks that don't show up in u-boot (yet?) such as bluetooth, wireless, and irda, but i'm pretty sure TCP/IPv4 has a few edge cases too.

I'll have a look, if only to lament that protocol are not what they used to be in the old days. :)

Anyway: as I said: performing *controlled* unaligned accesses for external reasons other than bugs is fine with me. Having our own get_unaligned() in such places would be fine with me.

i have no problem adding put/get_unaligned() to all the right places. that makes perfect sense. but, as an orthogonal issue wrt ARMv7, i don't see any problem enabling hardware functionality: it increases robustness (:P), shrinks the code base (all the get/put unaligned macros expand into a single memory access as they no longer have to do alignment fixups in software), and speeds up the runtime (a single unaligned memory access is always faster than address masking/multiple loads/bit shifting/etc... -- obviously this ignores multimedia type code that does alignment adjustment at the start, then lets of memory accesses, then another adjustment at the end, but that's not what we're talking about here).

if you want to tell people that if they found an unaligned access in code they must fix that, then great. make them fix it. then once that bug has been fixed, let's merge the purely optimization patch that allows the hardware to do unaligned accesses.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120719/2ddfe667/attachment.pgp>

#31 lore raw

PATCH 1/1 arm: armv7: add compile option -mno-unaligned-access if available

Hi Mike,

Quoted 19 lines · Mike Frysinger
Quoted 18 lines · On Thursday 19 July 2012 02:28:05 Albert ARIBAUD wrote:
Quoted 16 lines · Mike Frysinger
Quoted 9 lines · On Monday 02 July 2012 12:14:40 M?ns Rullg?rd wrote:

It's slightly more complicated than that. Data can be misaligned for a variety of reasons:

1. Errors in software.
2. Specified by a file format or communication protocol.
3. Deliberately misaligned by the compiler.

Misaligned data of type 1 should of course be fixed properly, not worked around in any way.

it's also a reliability aspect. people don't write bug free software, not bug free protocols, nor bug free compilers. when misalignment does happen in the field, it's a hell of a lot better if the software continued to execute correctly rather than randomly triggered an exception.

Quoted 3 lines
Quoted 1 line

Nitpick: this is robustness, not reliability.

useless pedantry: by increasing robustness, the system is more reliable

Your description confirms that robustness and reliability are not equivalent, thereby goes against your statement about pedantry... :)

Quoted 10 lines
Quoted 9 lines

That being said, yes, this robustness is desirable when you do not control all of the SW running on the product; Linux, for instance, will have to execute processes which were built with any old (or new) compiler settings, thus the Linux folks have to make sure the kernel won't fail running those.

But the only uncontrolled SW U-Boot runs is its payload -- typically the kernel image -- which are usually very cautious in what they assume they can do, thus are unlikely to perform unaligned accesses.

Quoted 9 lines

it isn't just that. there is no way you can guarantee both the linux kernel and u-boot code bases themselves are perfect. in fact, it's even worse when these are the ones that get tripped up because it means your system resets/hardlocks/kills a kitten. when doing driver development under the linux kernel, we would come across parts of core stacks that lacked alignment checking and would panic the system. sometimes it would always panic, other times it depended on factors that made life worse: the compiler version (newer ones always like to pack/optimize better), the actual data stream, or the execution paths.

Correct; here I was considering the requirements / operating conditions for both projects, I was not considering development issues -- bugs during development happen (morethan they do in the field, hopefully).

Do you mean you'd like to catch misalignments as early as possible, and would like to have both -munaligned-access and A=1 during dev?

Quoted 12 lines
Quoted 11 lines
Quoted 4 lines

simply search the kernel for get_unaligned then. there are plenty of examples in there. granted, many apply to stacks that don't show up in u-boot (yet?) such as bluetooth, wireless, and irda, but i'm pretty sure TCP/IPv4 has a few edge cases too.

I'll have a look, if only to lament that protocol are not what they used to be in the old days. :)

Anyway: as I said: performing *controlled* unaligned accesses for external reasons other than bugs is fine with me. Having our own get_unaligned() in such places would be fine with me.

Quoted 10 lines

i have no problem adding put/get_unaligned() to all the right places. that makes perfect sense. but, as an orthogonal issue wrt ARMv7, i don't see any problem enabling hardware functionality: it increases robustness (:P), shrinks the code base (all the get/put unaligned macros expand into a single memory access as they no longer have to do alignment fixups in software), and speeds up the runtime (a single unaligned memory access is always faster than address masking/multiple loads/bit shifting/etc... -- obviously this ignores multimedia type code that does alignment adjustment at the start, then lets of memory accesses, then another adjustment at the end, but that's not what we're talking about here).

I wouldn't care about the ARMv7 implementing explicit unaligned accesses with native instructions if it didn't mean it won't catch unwanted unaligned accesses any more, and thus such unalignments will be found in another context and will take some more time to trace back.

Quoted 4 lines

if you want to tell people that if they found an unaligned access in code they must fix that, then great. make them fix it. then once that bug has been fixed, let's merge the purely optimization patch that allows the hardware to do unaligned accesses.

My problem is that as long as people start configuring their HW and compiler to not care about unaligned accesses, they *won't* find such accesses when accidental, because nothing will tell them.

Here's my suggestion: when building U-Boot as usual, strict aligment policy is enforced, i.e. -mno-unaligned-access and A=1, and for platforms that could benefit from native unaligned accesses, a run-time warning is emitted on the console. However, with a specific command 'PRODUCTION' line option added, the constraint is relaxed, i.e. -munaligned-access and A=0, no run-time message, but a build warning is emitted stating that this build is afoul of the U-Boot strict policy.

This way, you get the robustness you want as you can easily build an ARMv7- efficient binary, and I get the one I want as the build used for development is slightly less MCPS-efficient but will catch potential issues.

Comments welcome.

Quoted 1 line

-mike

Amicalement,

Signature · 1 line
Albert.
#32 lore raw

PATCH 1/1 arm: armv7: add compile option -mno-unaligned-access if available

In reply to Tetsuyuki Kobayashi
Quoted 15 lines · Tetsuyuki Kobayashi

Recent compiler generates unaligned memory access in armv7 default. But current U-Boot does not allow unaligned memory access, so it causes data abort exception.
This patch add compile option "-mno-unaligned-access" if it is available.

---
  arch/arm/cpu/armv7/config.mk |    2 ++
  1 file changed, 2 insertions(+)
arch/arm/cpu/armv7/config.mk +0 -0
index 5407cb6..560c084 100644

arch/arm/cpu/armv7/config.mk PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float

Quoted 9 lines
arch/arm/cpu/armv7/config.mk +2 -1
@@ -26,6 +26,8 @@ PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float
  # supported by more tool-chains
  PF_CPPFLAGS_ARMV7 := $(call cc-option, -march=armv7-a, -march=armv5)
  PLATFORM_CPPFLAGS += $(PF_CPPFLAGS_ARMV7)
+PF_CPPFLAGS_NO_UNALIGNED := $(call cc-option, -mno-unaligned-access,)
+PLATFORM_CPPFLAGS += $(PF_CPPFLAGS_NO_UNALIGNED)
 
  # =========================================================================
  #
-- 1.7.9.5
Signature · 4 lines
------------------------------------------------------------
Gary Thomas                 |  Consulting for the
MLB Associates              |    Embedded world
------------------------------------------------------------