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

Add Memory scrubbing for ECC #239

Merged
merged 5 commits into from
Apr 26, 2020
Merged

Add Memory scrubbing for ECC #239

merged 5 commits into from
Apr 26, 2020

Conversation

bsousi5
Copy link
Collaborator

@bsousi5 bsousi5 commented Apr 2, 2020

Scrub memory based of what available in the lds files, ie

      PROVIDE( metal_itim_memory_start = ADDR(.itim) );
      PROVIDE( metal_itim_memory_end = ADDR(.itim) + SIZEOF(.itim) );
      PROVIDE( metal_dtim_memory_start = ADDR(.dtim) );
      PROVIDE( metal_dtim_memory_end = ADDR(.dtim) + SIZEOF(.dtim) );
      PROVIDE( metal_main_memory_start = ADDR(.ram) );
      PROVIDE( metal_main_memory_end = ADDR(.ram) + SIZEOF(.ram) );

And have scrub.S implement the __metal_before_start() which call metal_memory_scrub()
User/Customer have the option to override with their own if desire.

The intention is for metal_init() to enable the BEU and disable ECC suppression

*/
.global __metal_before_start
.type __metal_before_start, @function
__metal_before_start:
Copy link
Contributor

Choose a reason for hiding this comment

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

you'll need to save and restore 'ra' across this function.

Copy link
Collaborator Author

@bsousi5 bsousi5 Apr 3, 2020

Choose a reason for hiding this comment

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

This get called using jal though. It is a single instruction to jump and save return address: jump and link (jal)

Copy link
Contributor

@keith-packard keith-packard left a comment

Choose a reason for hiding this comment

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

Your general approach looks good; we'll want to make this optional as it will have a pretty significant impact on startup time.

Copy link
Contributor

@nategraff-sifive nategraff-sifive left a comment

Choose a reason for hiding this comment

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

This looks fine to me.

As far as startup cost is concerned, maybe disabling this could be as simple as the linker script generator setting the start and end symbols equal to each other when it knows this isn't necessary?

@nategraff-sifive nategraff-sifive changed the title Initial stab Initial stab 🗡 Apr 3, 2020
@bsousi5
Copy link
Collaborator Author

bsousi5 commented Apr 3, 2020

Making the start = end would be a quick work-around. I did make the function a weak type, so user may include their empty function. But have not tested that yet

@keith-packard
Copy link
Contributor

This looks fine to me.

As far as startup cost is concerned, maybe disabling this could be as simple as the linker script generator setting the start and end symbols equal to each other when it knows this isn't necessary?

If we can get to compiling freedom-metal for each project, instead of creating a library, then we can put conditional compilation based on project configuration which will make things like this really easy.

bsousi5 pushed a commit that referenced this pull request Apr 8, 2020
Update to use metal-platform.h (bare header)
@NandkumarJoshi NandkumarJoshi marked this pull request as ready for review April 26, 2020 19:53
@bsousi5 bsousi5 merged commit 2e39b11 into master Apr 26, 2020
@NandkumarJoshi NandkumarJoshi changed the title Initial stab 🗡 Add Memory scrubbing for ECC Apr 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants