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

page fault when PTE_PBMT bits set and not EXT_SVPBMT #874

Merged
merged 1 commit into from
Dec 5, 2021
Merged

page fault when PTE_PBMT bits set and not EXT_SVPBMT #874

merged 1 commit into from
Dec 5, 2021

Conversation

ingallsj
Copy link
Collaborator

@ingallsj ingallsj commented Dec 5, 2021

when Svpbmt extension for Page-Based Memory Type is not enabled, the PTE_PBMT bits should be behave as PTE_RSVD bits and page fault when set

follow-up to #750 from @daniellustig

@aswaterman
Copy link
Collaborator

aswaterman commented Dec 5, 2021

I think we should enable this one via the --isa string on the command line, rather than using the heavy hammer of a new configure argument. The need to rebuild Spike to support these different options is unfortunate (especially since it leads to combinatorial explosion) and is seemingly avoidable in this case.

(Down the road, I'd even like to remove the configure argument for misaligned ld/st and handle that one via the command line, too. It shouldn't even have a noticeable impact on Spike's perf, since the "do I support misaligned" check can be moved off the critical code path, i.e., after detecting misalignment.)

@ingallsj ingallsj changed the title optional ifndef RISCV_ENABLE_SVPBMT page fault when PTE_PBMT bits set and not EXT_SVPBMT Dec 5, 2021
@aswaterman
Copy link
Collaborator

aswaterman commented Dec 5, 2021

I think this is good to go: in the context of an ISA simulator, I'm not sure what else we'd do besides checking that the extension is implemented then ignoring the PTE bits.

@aswaterman aswaterman merged commit c4fdc8f into riscv-software-src:master Dec 5, 2021
@ingallsj ingallsj deleted the ifndef_pbmt branch December 5, 2021 07:39
@scottj97
Copy link
Collaborator

scottj97 commented Dec 5, 2021

There's also an EXT_SVNAPOT extension flag which doesn't seem to be checked anywhere. Does it have the same issue? It seems to be always enabled?

@ingallsj
Copy link
Collaborator Author

ingallsj commented Dec 5, 2021

Yep, you're right, thanks @scottj97, I for some reason thought Svnapot was fully backwards compatible, but it needs it too. #875

@gfavor
Copy link

gfavor commented Dec 6, 2021

Maybe I'm missing exactly where this thread has gotten to, but ... Svnapot IS backward compatible in that if software writes '0' to the N field, then one should get the exact same behavior on designs that implement Svnapot and designs that don't implement Svnapot. Whereas if software sets the N bit (presumably trying to use this extension), then it will get a page fault if the extension is not actually implemented.

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.

None yet

4 participants