search

d-iii-s / msim

Light-weight MIPS R4000 and RISC-V system simulator
https://d3s.mff.cuni.cz/software/msim/
GNU General Public License v2.0
5 stars 4 forks source link

`tr` and `ptd` command implementation #67

Closed HanyzPAPU closed 1 week ago

HanyzPAPU commented 1 month ago

Implementation of the tr command which translates an address and describes the process, ptd command which dumps the current pagetable, together with a bug fix (fixes issue #66) and some refactoring.

vhotspur commented 1 month ago

This looks really good, thank you very much!

I will have a more detailed look (plus some testing with some course implementations where this could help) after I return from vacation and then I will merge this (probably time for a new release too).

Would you, please, add a short summary to the CHANGELOG.md under the Unreleased section? I will do the version bump and the release but it would be nice to have the summary part of a single PR. Thanks!

lbulej commented 1 month ago

I wonder if we need to repeat the PPN and RSW everywhere and not make it into column headers somehow?

I would argue that even though the field prefixes make the lines a longer, the values can be always interpreted locally, even after grepping :-) Column headers have the tendency to scroll away...

HanyzPAPU commented 1 month ago

Therefore, I wild idea: can we also have something like sptd and str (or whatever prefix is better than simulated) where the user can supply future value of SATP?

Would dumping/translating using the physical address of the pagetable work? Right now the only data we need to retrieve from SATP for the translation is the ppn of the root pagetable itself. Checking whether SATP mode is BARE or the ASID (imo) wouldn't be really necessary if we want to dump the pagetable or perform a debugging translation. Also when the address is explicitly specified, I would argue we can skip looking into the TLB. And the refactoring wouldn't be to hard, we would just need to extract the logic after the ppn is taken from SATP.

Do you think it would be better to implement this as a separate commands or to add an optional parameter to tr/ptd?

vhotspur commented 1 month ago

Thanks for the fixes in the output of tr and ptd. I really like it more now.

Would dumping/translating using the physical address of the pagetable work?

Yes, that is also an option. The developer needs to know the root entry address anyway so it should be fine.

Also when the address is explicitly specified, I would argue we can skip looking into the TLB.

That is probably okay too. I wonder if things can break here but probably only if users are debugging the code after several address space switches when they do not execute sfence x0, x0 and modify the page tables (or something along those lines). Thus it should be safe as by that time they should have the page table code correct. Fingers crossed.

Do you think it would be better to implement this as a separate commands or to add an optional parameter to tr/ptd?

Maybe a separate command would be easier to use? For cpu tr VIRT it would be natural to append the ROOT as the third argument (tr VIRT [ROOT]) but it makes it cumbersome when you are trying other addresses as you need to navigate to VIRT first (i.e., for usability I feel that having VIRT as last parameter is best). Also having a separate command would allow us to stress the TLB bypass more easily in the documentation? But this is something that is relatively easy to change and we can always modify it after a test run next semester :-). My $0.02, it is up to you, really.

Thanks!

vhotspur commented 1 week ago

@HanyzPAPU Thanks a lot, I am merging this (and will do a release soonish).