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

CMSIS: add ARMv8-A AARCH64 support #39

Closed
wants to merge 1 commit into from

Conversation

JiafeiPan
Copy link

Currently RTOS can run Cortex-A AARCH64 platforms, so add ARMv8-A AARCH64 support in CMSIS, it includes Core header file support for Cortex-A53 and Cortex-A55, cache driver, MMU driver and GICv3 driver.

This patches are merged from a patch set with the contributors list in the following Signed-off-by list.

Currently RTOS can run Cortex-A AARCH64 platforms, so add ARMv8-A AARCH64
support in CMSIS, it includes Core header file support for Cortex-A53 and
Cortex-A55, cache driver, MMU driver and GICv3 driver.

This patches are merged from a patch set with the contributors list in the
following Signed-off-by list.

Signed-off-by: Stephane Viau <[email protected]>
Signed-off-by: Hou Zhiqiang <[email protected]>
Signed-off-by: Jiafei Pan <[email protected]>
Signed-off-by: Aziz Sellami <[email protected]>
Signed-off-by: Rui Sousa <[email protected]>
Signed-off-by: Fabrice Goucem <[email protected]>
Signed-off-by: Peng Fan <[email protected]>
@JonatanAntoni
Copy link
Member

Thanks for contributing this. May I ask you to align your PR with #38, please?
And, ideally, can you enhance CoreValidation test suite to run some tests for the additions?

@JonatanAntoni JonatanAntoni requested a review from a user August 9, 2023 09:08
@JonatanAntoni JonatanAntoni added the enhancement New feature or request label Aug 9, 2023
@JiafeiPan
Copy link
Author

Thanks for contributing this. May I ask you to align your PR with #38, please? And, ideally, can you enhance CoreValidation test suite to run some tests for the additions?

Sure, I will update my PR after PR #38 is merged.

@JonatanAntoni
Copy link
Member

The PR #38 is merged, now. Please find the restructured layout of CMSIS/Core/. The layout should allow you to integrate your changes into the r-profile folder.

We need to discuss how to reuse stuff like GIC, which is shared with a-profile. We could just include the gic.h from a-profile in any of the core_rNN.h files. Or we could have a redirecting file in r-profile/gic.h which just included a-profile/gic.h.

Any thoughts, @Masmiseim36?

@rpmsousa
Copy link

rpmsousa commented Aug 9, 2023

Hi Jonatan,

Notice these changes concern a-profile but v8 (and possibly above), not r-profile. We had explicitly avoided using Core_A directory in the past, because there are a number of incompatibilities.
Maybe:
Core/
a-profile/
aarch32/... => current code
aarch64/... => our changes

?

@JonatanAntoni
Copy link
Member

Hi @rpmsousa,

Ah, sorry, got confused.
Can you elaborate on the incompatibilites?

The current effort it to push duplicated code up the folder hierarchy to reduce copy&paste. I am not keen on introducing new duplication for aarch64.

Cheers,
Jonatan

@Masmiseim36
Copy link
Contributor

Hello @JiafeiPan
we have discussed the future architecture of the CMSIS in the PR: https://github.com/ARM-software/CMSIS_6/pull/26/files.
I had added ARM64 support there for illustration. Please feel free to compare this implementation with yours.

@JiafeiPan
Copy link
Author

hello, @JonatanAntoni and @Masmiseim36,

This PR is also trying to reduce the code duplications, we defined cache_armv8a.h mmu_armv8a.h and timer_armv8a.h, so that all ARMv8a CPU Core can reuse these code. and in order to adapt to new source files layout, I think we can put these common files in a-profile, maybe need to add aarch64 suffix? And this PR have been verified on silicons with Cortex-A53 and Cortex-A55 CPU Cores.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check the Coding Rules of the CMSIS. Function names should be written in PascalCase and comments should be doxygen compliant. (compare https://arm-software.github.io/CMSIS_5/develop/General/html/index.html).
May I also suggest to align the names of the functions with the existing cache functionality. For example:

icache_invalidate_range       --> InvalidateICache_by_Addr
icache_invalidate_all         -->  InvalidateICache
dcache_clean_range            --> CleanDCache_by_Addr
dcache_invalidate_range       --> InvalidateDCache_by_Addr
dcache_clean_invalidate_range --> CleanInvalidateDCache_by_Addr
dcache_clean_all              --> CleanDCache
dcache_invalidate_all         --> InvalidateDCache
dcache_clean_invalidate_all   --> CleanInvalidateDCache

You cold also add a prefix to each function like L1C_
@JonatanAntoni do you agree?

* CMSIS definitions
******************************************************************************/

#define __CORTEX_Axx (55U) /*!< Cortex-Axx Core */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be called __CORTEX_A?

@JonatanAntoni
Copy link
Member

@JiafeiPan, are you going to work on the review feedback given above?

@zhugengyu
Copy link

Any plan for this PR, sounds pretty good @JonatanAntoni @JiafeiPan

@JonatanAntoni
Copy link
Member

JonatanAntoni commented May 6, 2024

@zhugengyu, this PR is outdated and needs some work. It needs to be aligned/synched with #45 which adds CA53 and CA55 as well.

@JonatanAntoni
Copy link
Member

Superseded by #45.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants