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

encoding.h is very far from up-to-date #132

Open
allenjbaum opened this issue Jun 30, 2022 · 8 comments
Open

encoding.h is very far from up-to-date #132

allenjbaum opened this issue Jun 30, 2022 · 8 comments

Comments

@allenjbaum
Copy link

The version of encoding.h is not even cloe to up-to-date. It doesn't even have all the CSRs in the ratified priv spec (e.g. PMPCFGx, PMPADDRx). This needs to be regenerated as part of a C/I script whenever something new is added in its dependencies.

@aswaterman
Copy link
Member

aswaterman commented Jun 30, 2022

This is the canonical version, so it’s up to date by definition.

The file this repo generates is encoding.out.h (which then gets copied by other repos into their encoding.h). That file contains auto-generated macros for all of the CSR numbers.

(We don’t commit the auto-generated version to this repo; you need to check it out and build it.)

@aswaterman
Copy link
Member

aswaterman commented Jun 30, 2022

It probably would’ve been less confusing if the static file in this repo were named encoding.in.h, to make it clear it isn’t the complete file, but rather just one piece of it. It probably wouldn’t be too disruptive to make that change.

cc @neelgala - do you agree?

@allenjbaum
Copy link
Author

allenjbaum commented Jun 30, 2022 via email

@aswaterman
Copy link
Member

That’s a per-repo decision. If you have the time to donate that effort in a way that works for this repo’s maintainers, we will consider it. Don’t forget this essentially is a volunteer effort for us, too.

@Abdulwadoodd
Copy link

Abdulwadoodd commented Jun 30, 2022

Hi @aswaterman, I'm willing to and want to do what needs to be done as @allenjbaum mentioned.

@aswaterman
Copy link
Member

Thanks for volunteering. Can you make a brief proposal for what you'd like to do, so others who use this repo can comment on it?

@neelgala
Copy link
Collaborator

neelgala commented Jul 4, 2022

It probably would’ve been less confusing if the static file in this repo were named encoding.in.h, to make it clear it isn’t the complete file, but rather just one piece of it. It probably wouldn’t be too disruptive to make that change.

cc @neelgala - do you agree?

yes andrew is right here. the encoding.h file here is infact just a template.. and running the make command should generte encoding.out.h which has the information @allenjbaum is looking for and which is what should be consumed by 3rd party tools.

I just want to point out that the encoding.out.h in its current form is primarily targetted towards usage with spike (check the assumptions of this in PR #106 ). While it may work out of the box for other tools - future changes to generation of encoding.out.h will only be done keeping compatibility with spike in mind.

Other tools are free to add their makefile-targets/scripts to this repo to make generation of artificats (specific to their needs) simple for themselves.

Regarding artifacts generation - we first need a CI to version this repo (check #123 comments for this). Each new PR/release then can simply host the outputs of the make command as a zip artifact.

@allenjbaum
Copy link
Author

allenjbaum commented Oct 11, 2022 via email

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

No branches or pull requests

4 participants