Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cortex-R Improvement #20594

Closed
22 of 25 tasks
stephanosio opened this issue Nov 12, 2019 · 10 comments
Closed
22 of 25 tasks

Cortex-R Improvement #20594

stephanosio opened this issue Nov 12, 2019 · 10 comments
Assignees
Labels
area: ARM ARM (32-bit) Architecture area: Drivers area: Kernel Meta A collection of features, enhancements or bugs

Comments

@stephanosio
Copy link
Member

stephanosio commented Nov 12, 2019

This is an epic for ARM Cortex-R port improvement, in preparation for the TI Hercules SoC support in #19644.

@stephanosio stephanosio self-assigned this Nov 12, 2019
@stephanosio
Copy link
Member Author

cc @bbolen @ioannisg @carlocaione

@bbolen If, by any chance, you are working on any of the above already, please let me know.

@jhedberg jhedberg added the Enhancement Changes/Updates/Additions to existing features label Jan 2, 2020
@stephanosio stephanosio added the Meta A collection of features, enhancements or bugs label Jan 7, 2020
@bwasim
Copy link
Contributor

bwasim commented May 2, 2020

@stephanosio , I'm planning to start work on Armv7-R MPU and hope to push it into the mainline in the next couple of weeks. Let me know if someone is already working on this, so I don't end up duplicating the effort..

@stephanosio
Copy link
Member Author

stephanosio commented May 2, 2020

@stephanosio , I'm planning to start work on Armv7-R MPU and hope to push it into the mainline in the next couple of weeks. Let me know if someone is already working on this, so I don't end up duplicating the effort..

@bwasim Nobody is working on/planning to upstream ARMv7-R MPU support at the moment as far as I know, though it is possible that Lexmark guys already have it (see https://lists.zephyrproject.org/g/devel/message/6695).

p.s. By the way, ARMv7-R MPU and ARMv7-M MPU are very similar (well, they are both PMSAv7 implementations), so the focus should be on trying to refactor/reuse the Cortex-M MPU code. We should not have two separate implementations of the same thing ...

@bbolen
Copy link
Collaborator

bbolen commented May 2, 2020

@stephanosio , I'm planning to start work on Armv7-R MPU and hope to push it into the mainline in the next couple of weeks. Let me know if someone is already working on this, so I don't end up duplicating the effort..

@bwasim Nobody is working on/planning to upstream ARMv7-R MPU support at the moment as far as I know, though it is possible that Lexmark guys already have it (see https://lists.zephyrproject.org/g/devel/message/6695).

Yes, we do have something working but it is not in merge worthy shape right now. If you can wait until Monday, I can send you a patch of what we have in order to take a look.

@bwasim
Copy link
Contributor

bwasim commented May 2, 2020

@stephanosio , I'm planning to start work on Armv7-R MPU and hope to push it into the mainline in the next couple of weeks. Let me know if someone is already working on this, so I don't end up duplicating the effort..

@bwasim Nobody is working on/planning to upstream ARMv7-R MPU support at the moment as far as I know, though it is possible that Lexmark guys already have it (see https://lists.zephyrproject.org/g/devel/message/6695).

Yes, we do have something working but it is not in merge worthy shape right now. If you can wait until Monday, I can send you a patch of what we have in order to take a look.

That would be great, Thanks..

@bbolen
Copy link
Collaborator

bbolen commented May 4, 2020

@stephanosio , I'm planning to start work on Armv7-R MPU and hope to push it into the mainline in the next couple of weeks. Let me know if someone is already working on this, so I don't end up duplicating the effort..

@bwasim Nobody is working on/planning to upstream ARMv7-R MPU support at the moment as far as I know, though it is possible that Lexmark guys already have it (see https://lists.zephyrproject.org/g/devel/message/6695).

Yes, we do have something working but it is not in merge worthy shape right now. If you can wait until Monday, I can send you a patch of what we have in order to take a look.

That would be great, Thanks..

The top three commits on https://github.com/bbolen/zephyr/commits/cortex_r_mpu are what we have. This is based on v2.2.0. We started working on this on 1.14 and merging has made just grabbing patches difficult so I had to just start clean on top of v2.2. I think I got everything, but if you have problems, let me know.

@wali-ku
Copy link

wali-ku commented Jun 26, 2020

@stephanosio , I'm planning to start work on Armv7-R MPU and hope to push it into the mainline in the next couple of weeks. Let me know if someone is already working on this, so I don't end up duplicating the effort..

@bwasim Nobody is working on/planning to upstream ARMv7-R MPU support at the moment as far as I know, though it is possible that Lexmark guys already have it (see https://lists.zephyrproject.org/g/devel/message/6695).

Yes, we do have something working but it is not in merge worthy shape right now. If you can wait until Monday, I can send you a patch of what we have in order to take a look.

That would be great, Thanks..

The top three commits on https://github.com/bbolen/zephyr/commits/cortex_r_mpu are what we have. This is based on v2.2.0. We started working on this on 1.14 and merging has made just grabbing patches difficult so I had to just start clean on top of v2.2. I think I got everything, but if you have problems, let me know.

@bbolen I used your commits to enable MPU in a Cortex-R5F based board. The MPU requires power-of-two alignment (Reference: ARM Cortex-R5 TRM r1p1 Table 4.34: Region Size) but when I enable CONFIG_MPU_REQUIRES_POWER_OF_TWO_ALIGNMENT in Zephyr, I notice that all the non-static (rw) global data (i.e., the one in the .datas section in include/arch/arm/aarch32/cortex_a_r/scripts/linker.ld) gets initialized to 0xFFFF_FFFF instead of its correct initial value which results in data-abort exception (with alignment fault @ 0xffffffff) during kernel boot.

Have you seen anything similar or can you please give a hint as to where should I look to debug this problem?

If I make the global data read-only (by adding static const before it), then it gets initialized to correct value. So the problem is with the non-static data initialization alone. Also, it is worth noting that the problem occurs without MPU enabled as well i.e., if I just modify the linker file to enforce power-of-two alignment without enabling MPU, the problem still persists.

Thanks in advance!

@bbolen
Copy link
Collaborator

bbolen commented Jul 22, 2020

Sorry I haven't responded until now. We are working to get our MPU code rebased on v2.3 and get it cleaned up for upstreaming consideration. With respect to the problem you mention, I have seen something similar. I got around the problem by using "DISCARD" on the *.got section in the cortex R linker script. The problem is that the VMA and LMA drift apart, but I don't know what the correct thing to do is.

@wali-ku
Copy link

wali-ku commented Jul 22, 2020

I tried to figure out what the problem was and found the following:

In the case of power-of-two alignment, there is extra padding added before the "datas" section due to which the addresses of the initialized global (rw) data change but the code still references the old address; without the padding offset.

image

In this image, the date = 26 is stored in a (rw) global variable; which should go to the "datas" section. With and wo ^2 alignment, the code references the variable at the same address but in case of ^2 alignment, the address contains the padded bytes and the correct value of the variable is present at a different location (highlighted in small red rectangle in RHS image).

I was able to work-around the issue using the following patch:

diff --git a/include/arch/arm/aarch32/cortex_a_r/scripts/linker.ld b/include/arch/arm/aarch32/cortex_a_r/scripts/linker.ld
index 57c73e680a..aec33d0ecc 100644
--- a/include/arch/arm/aarch32/cortex_a_r/scripts/linker.ld
+++ b/include/arch/arm/aarch32/cortex_a_r/scripts/linker.ld
@@ -206,13 +206,12 @@ SECTIONS
          * generally is not an issue, since modern ROM and FLASH memory is
          * usually 4k aligned.
          */
-        . = ALIGN(4);
+	_image_rodata_end = .;
+	MPU_ALIGN(_image_rodata_end -_image_rom_start);
     } GROUP_LINK_IN(ROMABLE_REGION)
 
 #include <linker/cplusplus-rom.ld>
 
-    _image_rodata_end = .;
-    MPU_ALIGN(_image_rodata_end -_image_rom_start);
     _image_rom_end = .;
 
     GROUP_END(ROMABLE_REGION)

I am not exactly sure how this patch resolves the problem; I was experimenting with the linker file to see if I could get the code to generate correct addresses of global vars with ^2 alignment and making this change gave correct results.

I also had to make changes to the syscall trampoline code in the commits you referenced; to make it comply with the Zephyr v2.3 z_arm_svc exception handling code. But I was finally able to create Usermode threads for Cortex-R5 on Zephyr with MPU enabled.

Thank you for posting these patches. It would have taken me much longer to get this done on my own.

@stephanosio stephanosio assigned bbolen and unassigned stephanosio Aug 13, 2021
@nashif nashif removed the Enhancement Changes/Updates/Additions to existing features label Nov 19, 2022
@stephanosio
Copy link
Member Author

Closing since this is more or less complete. Any other remaining issues, if any, should be tracked in separate issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: ARM ARM (32-bit) Architecture area: Drivers area: Kernel Meta A collection of features, enhancements or bugs
Projects
None yet
Development

No branches or pull requests

6 participants