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

Preliminary support for MMU emulation #438

Merged
merged 2 commits into from
Oct 28, 2024
Merged

Conversation

ChinYikMing
Copy link
Collaborator

@ChinYikMing ChinYikMing commented May 12, 2024

The purpose of this commit is to boot 32-bit RISC-V Linux in the future. The virtual memory scheme to support is Sv32. There are one change to original code base to adapt the MMU: the prototype of riscv_io_t interface needs to be changed. Particularly, add a RISC-V instance(riscv_t) as the first parameter. MMU related callbacks require to access the satp CSR to perform a page table walk during virtual memory translation but satp CSR is stored in RISC-V instance(riscv_t), thus it should have a way to access the satp CSR. The trivial solution is adding RISC-V
instance(riscv_t) to the prototype of riscv_io_t interface.

After this change, we can reuse riscv_io_t for system emulation afterward.

The rest of changes are implementing the Sv32 virtual memory scheme. For every memory access, it has to walk through the page table to get the corresponding PTE. Depends on the retrieval of PTE, there are several page faults to be handled if necessary, so there are three exceptions handlers have been introduced which are insn_pgfault, load_pgfault, and store_pgfault and they are used in MMU_CHECK_FAULT. In this commit, the access fault are not handled well since they are related to PMA and PMP and they might not the must to boot 32-bit RISC-V Linux (tested on semu). More PTE, S-mode, M-mode CSR helper macro are introduced as well.

Related: #310

@ChinYikMing
Copy link
Collaborator Author

This PR is not fully ready to be merged since testing is not yet fully designed. PR earlier to get some feedbacks for further design.

Copy link
Contributor

@jserv jserv left a comment

Choose a reason for hiding this comment

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

Benchmarks

Benchmark suite Current: 6317605 Previous: 79302e2 Ratio
Dhrystone 1422 Average DMIPS over 10 runs 1610 Average DMIPS over 10 runs 1.13
Coremark 1431.449 Average iterations/sec over 10 runs 1414.587 Average iterations/sec over 10 runs 0.99

This comment was automatically generated by workflow using github-action-benchmark.

@ChinYikMing
Copy link
Collaborator Author

The initial design mentioned in here does not fully consider the CSR such as satp CSR needs to be accessed during MMU translation. During implementation, the interface shall be changed to adapt MMU translation.

src/common.h Outdated Show resolved Hide resolved
src/emulate.c Outdated Show resolved Hide resolved
src/emulate.c Outdated Show resolved Hide resolved
src/emulate.c Outdated Show resolved Hide resolved
src/emulate.c Outdated Show resolved Hide resolved
src/emulate.c Outdated Show resolved Hide resolved
src/emulate.c Outdated Show resolved Hide resolved
@jserv jserv requested a review from qwe661234 May 13, 2024 01:48
@jserv jserv changed the title Support emulating memory management unit(MMU) Preliminary support for MMU emulation May 13, 2024
@jserv
Copy link
Contributor

jserv commented May 13, 2024

This PR is not fully ready to be merged since testing is not yet fully designed. PR earlier to get some feedbacks for further design.

How can we test the MMU specific operations?

@ChinYikMing
Copy link
Collaborator Author

ChinYikMing commented May 13, 2024

This PR is not fully ready to be merged since testing is not yet fully designed. PR earlier to get some feedbacks for further design.

How can we test the MMU specific operations?

The testing idea can be break down to following steps:

  1. Creating a simple userspace application and kernel supervisor
  2. Starts executing with the simple kernel supervisor. Read/write CSR register to install exception vector table to specific address for traps and root page table for MMU translation.
  3. After all CSR stuffs are done, switch to user mode and execute userspace application. At this point, I would like to design some scenario to testing all three types of page fault (instruction, load, store page fault). For every userspace memory access, dump the page table could be beneficial for verification or debugging.

If I am at the wrong path, please correct me.

It take times to design this testing. So, I would try to support other peripherals emulation at the same time such as PLIC.

@jserv
Copy link
Contributor

jserv commented May 13, 2024

  1. Creating a simple userspace application and kernel supervisor
  2. Starts executing with the simple kernel supervisor. Read/write CSR register to install exception vector table to specific address for traps and root page table for MMU translation.
  3. After all CSR stuffs are done, switch to user mode and execute userspace application. At this point, I would like to design some scenario to testing all three types of page fault (instruction, load, store page fault). For every userspace memory access, dump the page table could be beneficial for verification or debugging.

The above sound great. I expect the lean and reasonably straightforward approach as following:

ChinYikMing added a commit to ChinYikMing/rv32emu that referenced this pull request May 26, 2024
sret instruction is used for returning from a trap when trap occurs in
S-mode level. Thus, the execution flow will not be sequential. During
basic block translation, the sret instruction should be considered as
can_branch instruction.

Moreover, the existing system instruction decoder does not support
decoding the sret instruction. Thus, the ir->opcode should be set
correctly to support decoding the sret instruction. The implementation
of sret instruction is simply returning false for now, the improved
implementation will be completed and tested in sysprog21#438.
ChinYikMing added a commit to ChinYikMing/rv32emu that referenced this pull request May 26, 2024
sret instruction is used for returning from a trap when trap occurs in
S-mode level. Thus, the execution flow will not be sequential. During
basic block translation, the sret instruction should be considered as
can_branch instruction.

Moreover, the existing system instruction decoder does not support
decoding the sret instruction. Thus, the ir->opcode should be set
correctly to support decoding the sret instruction. The implementation
of sret instruction is simply returning false for now, the improved
implementation will be completed and tested in sysprog21#438 since the sret
instruction involves privilege mode changing.
ChinYikMing added a commit to ChinYikMing/rv32emu that referenced this pull request May 27, 2024
sret instruction is used for returning from a trap when trap occurs in
S-mode level. Thus, the execution flow will not be sequential. During
basic block translation, the sret instruction should be considered as
can_branch instruction.

Moreover, the existing system instruction decoder does not support
decoding the sret instruction. Thus, the ir->opcode should be set
correctly to support decoding the sret instruction. The implementation
of sret instruction is simply returning false for now, the improved
implementation will be completed and tested in sysprog21#438 since the sret
instruction involves privilege mode changing.
src/riscv.h Show resolved Hide resolved
@ChinYikMing
Copy link
Collaborator Author

ChinYikMing commented Jun 3, 2024

During block emulation, I think the instructions are executed sequentially until block ends. As such, when a page fault exception is generated during block emulation, the RISC-V core has to jump to the corresponding exception handler. The potential problem is that even thought the PC could be updated in a exception handler, but the block to emulate is not updated, and this cause the page fault cannot be handled properly.

I have tested calling rv_step after updated the PC to exception handler to resolve the potential problem. It works in gdb but not outside the gdb. Any recommend way for this potential problem?

@jserv
Copy link
Contributor

jserv commented Jun 3, 2024

During block emulation, I think the instructions are executed sequentially until block ends. As such, when a page fault exception is generated during block emulation, the RISC-V core has to jump to the corresponding exception handler. The potential problem is that even thought the PC could be updated in a exception handler, but the block to emulate is not updated, and this cause the page fault cannot be handled properly.

I have tested calling rv_step after updated the PC to exception handler to resolve the potential problem. It works in gdb but not outside the gdb. Any recommend way for this potential problem?

Can you provide a minimal reproducible example so that @qwe661234 can verify if the current block chaining is functioning as expected?

@ChinYikMing
Copy link
Collaborator Author

ChinYikMing commented Jun 3, 2024

Steps to reproduce the VM test:

  1. make ENABLE_SYSTEM=1
  2. Go to the tests/system directory, run make
  3. build/rv32emu tests/system/vm.elf

Some output would look like this:

delegated to supervisor
fault addr: 0x4
new PC: 0x800000b0
next insn addr: 0x4, next insn: 0xfe010113
delegated to supervisor
fault addr: 0x8
new PC: 0x800000b0
next insn addr: 0x8, next insn: 0x112e23
delegated to supervisor
fault addr: 0xc
new PC: 0x800000b0
next insn addr: 0xc, next insn: 0x812c23
delegated to supervisor
fault addr: 0x10
new PC: 0x800000b0
next insn addr: 0x10, next insn: 0x2010413
delegated to supervisor
fault addr: 0x14
new PC: 0x800000b0
next insn addr: 0x14, next insn: 0x6400793
delegated to supervisor
fault addr: 0x18
new PC: 0x800000b0
next insn addr: 0x18, next insn: 0xfef42623
delegated to supervisor
fault addr: 0x1c
new PC: 0x800000b0
next insn addr: 0x1c, next insn: 0xc800793
delegated to supervisor
fault addr: 0x20
new PC: 0x800000b0
next insn addr: 0x20, next insn: 0xfef42423
delegated to supervisor
fault addr: 0x24
new PC: 0x800000b0
next insn addr: 0x24, next insn: 0xfec42703
delegated to supervisor
fault addr: 0x28
new PC: 0x800000b0
next insn addr: 0x28, next insn: 0xfe842783
delegated to supervisor
fault addr: 0x2c
new PC: 0x800000b0
next insn addr: 0x2c, next insn: 0xf707b3
delegated to supervisor
fault addr: 0x30
new PC: 0x800000b0
next insn addr: 0x30, next insn: 0xfef42223
delegated to supervisor
fault addr: 0x34
new PC: 0x800000b0
next insn addr: 0x34, next insn: 0x100513
delegated to supervisor
fault addr: 0x38
new PC: 0x800000b0
next insn addr: 0x38, next insn: 0x80000097
delegated to supervisor
fault addr: 0x3c
new PC: 0x800000b0
next insn addr: 0x3c, next insn: 0x64080e7
next insn addr: 0x8000009c, next insn: 0x5d00893
next insn addr: 0x800000a0, next insn: 0x73
a0: 1
exit syscall called
inferior exit code 1

Notice that the user space code starts at address "0x4" and the address "0x800000b0" is the supervisor exception handler entry.

When instruction fetch fault occurs at address "0x4", the PC is updated to "0x800000b0" but the next instruction is still from address "0x4" and I think it should be from address "0x800000b0". The relevant information is as follows:

delegated to supervisor
fault addr: 0x4
new PC: 0x800000b0
next insn addr: 0x4, next insn: 0xfe010113

The consequent instruction address ( "0x8", "0xc", ... ) face the same problem.

@jserv
Copy link
Contributor

jserv commented Jun 3, 2024

Steps to reproduce the VM test:

  1. make ENABLE_SYSTEM=1
  2. Go to the tests/system directory, run make
  3. build/rv32emu tests/system/vm.elf

At first glance, it appears that the MMU was not set in tests/system/vm.c, and exceptions are delegated to S-mode. Could you show the expected flow for exception handling?

@jserv jserv requested a review from qwe661234 June 3, 2024 09:50
@qwe661234
Copy link
Collaborator

ext insn addr: 0x4

Could you provide your printf format? I want to map the fields in riscv_t.

@ChinYikMing
Copy link
Collaborator Author

ChinYikMing commented Jun 3, 2024

Steps to reproduce the VM test:

  1. make ENABLE_SYSTEM=1
  2. Go to the tests/system directory, run make
  3. build/rv32emu tests/system/vm.elf

At first glance, it appears that the MMU was not set in tests/system/vm.c, and exceptions are delegated to S-mode. Could you show the expected flow for exception handling?

Please ignore the "vm.c" file. The MMU setup is done in "vm_setup.c".

/* Enable paging */
uintptr_t satp_val =((pte_t) &l1pt >> PG_SHIFT) | SV32_MODE;
write_csr(satp, satp_val);

The expected flow for exception handling is that:

  1. Search the correspond PTE in page table via mmu_walk
  2. If the PTE is not found, a correspond page fault exception is generated. In this case, it should be instruction fetch page fault.
  3. RISC-V core tends to check if the exception is delegated to S-mode or not. If yes, then set the PC to stvec else set the PC to mtvec. Base mode or Vectored mode depends on the implementation. In this case, the exception is delegated to S-mode so PC is set to stvec. The sepc CSR saves the next instruction to be executed which used by sret instruction to resume the execution seamlessly when page fault is handled. The page fault handler will map 4KiB data during the handling.
  4. The instruction address "0x8", "0xc", ... should not cause instruction page fault since 4KiB data are mapped.

@ChinYikMing
Copy link
Collaborator Author

ext insn addr: 0x4

Could you provide your printf format? I want to map the fields in riscv_t.

Sure. Here it is: "next insn addr: 0x%x, next insn: 0x%x\n".

@qwe661234
Copy link
Collaborator

ext insn addr: 0x4

Could you provide your printf format? I want to map the fields in riscv_t.

Sure. Here it is: "next insn addr: 0x%x, next insn: 0x%x\n".

Is the variable rv->PC?

@ChinYikMing
Copy link
Collaborator Author

Is it better to rename from tests/system/pgfault to tests/system/mmu since more tests can be integrated?

Agree.

@ChinYikMing ChinYikMing force-pushed the mmu branch 3 times, most recently from d784b88 to b35f744 Compare October 24, 2024 00:12
src/emulate.c Outdated
@@ -103,7 +95,7 @@ static void trap_handler(riscv_t *rv);
* identifier called tval, as both are handled by TRAP_HANDLER_IMPL.
*/
#define TRAP_HANDLER_IMPL(type, code) \
static void rv_trap_##type(riscv_t *rv, uint32_t tval) \
void rv_trap_##type(riscv_t *rv, uint32_t tval) \
Copy link
Contributor

Choose a reason for hiding this comment

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

It is a pity that the helper functions can not be declared as static. Can you find a way for that?

Copy link
Collaborator Author

@ChinYikMing ChinYikMing Oct 24, 2024

Choose a reason for hiding this comment

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

It is a pity that the helper functions can not be declared as static. Can you find a way for that?

We can register the trap_handler with the I/O interface during the initialization of the RISC-V core, keeping the dispatch table concealed. When a page fault occurs, the trap_handler is invoked, which in turn calls _trap_handler, and then _trap_handler calls __trap_handler. This __trap_handler may correspond to a helper function defined in emulate.c. The __trap_handler would represent the original implementation of trap_handler, while _trap_handler may need to inspect the scause CSR to determine which TRAP_HANDLER_IMPL to invoke. This approach ensures that the specific TRAP_HANDLER_IMPL remains hidden.

Copy link
Collaborator Author

@ChinYikMing ChinYikMing Oct 25, 2024

Choose a reason for hiding this comment

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

In detail, trap_handler callback (need to set corresponding scause CSR, in IO interface)
---(call)-----> _trap_handler(check scause CSR) ---(dispatch)-----> TRAP_HANDLER_IMPL (depends on scause CSR, static function in emulate.c)
---(call)-----> __trap_handler(actual trap handler, static function in emulate.c)

src/emulate.c Outdated Show resolved Hide resolved
@ChinYikMing
Copy link
Collaborator Author

ChinYikMing commented Oct 26, 2024

Now, the trap handling flow is as following:

trap_handler callback (requires setting the appropriate scause CSR) ---(invokes)---> _trap_handler (verifies scause CSR) ---(dispatches to)---> TRAP_HANDLER_IMPL (based on scause CSR, implemented as a static function in emulate.c) ---(calls)---> __trap_handler (actual trap handler, also a static function in emulate.c)

@ChinYikMing ChinYikMing force-pushed the mmu branch 2 times, most recently from 6547846 to 99acca3 Compare October 26, 2024 21:13
@vacantron
Copy link
Collaborator

Since m/scause, m/stval has been set in SET_CAUSE_AND_TVAL_THEN_TRAP(), setting tval in

static void rv_trap_##type(riscv_t *rv, uint32_t tval) \
is redundant.

src/emulate.c Outdated
#define TRAP_HANDLER_IMPL(type, code) \
static void rv_trap_##type(riscv_t *rv, uint32_t tval) \
#define TRAP_HANDLER_IMPL(type) \
static void rv_trap_##type(riscv_t *rv) \
Copy link
Collaborator

Choose a reason for hiding this comment

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

We do not need different handler for different trap type anymore. The information for trapping is stored in m/scause, m/stval.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We do not need different handler for different trap type anymore. The information for trapping is stored in m/scause, m/stval.

Could you elaborate more on this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you elaborate more on this?

I meant we could simplify some handlers in _trap_handler() (e.g. illegal_insn, breakpoint, misalligned). They have the same routine of reading m/stval and m/scause to identify the type of traps.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Could you elaborate more on this?

I meant we could simplify some handlers in _trap_handler() (e.g. illegal_insn, breakpoint, misalligned). They have the same routine of reading m/stval and m/scause to identify the type of traps.

It make sense.

@ChinYikMing ChinYikMing force-pushed the mmu branch 3 times, most recently from ac9fa59 to 266e477 Compare October 27, 2024 20:44
@jserv
Copy link
Contributor

jserv commented Oct 28, 2024

@vacantron, after merging this pull request, let's prepare the prebuilt MMU test executable file.

@jserv
Copy link
Contributor

jserv commented Oct 28, 2024

Any reason to expose mmu_ifetch, mmu_read_w, mmu_read_s, etc.? I marked them as static functions, and the SYSTEM configuration still builds.

@ChinYikMing
Copy link
Collaborator Author

Any reason to expose mmu_ifetch, mmu_read_w, mmu_read_s, etc.?

They should be static in system.c and only accessible through riscv_io_t interface. In this way, the declaration of them shall be removed from the system.h header file.

I marked them as static functions, and the SYSTEM configuration still builds.

Do you mark them in system.c?

@jserv
Copy link
Contributor

jserv commented Oct 28, 2024

Any reason to expose mmu_ifetch, mmu_read_w, mmu_read_s, etc.?
They should be static in system.c and only accessible through riscv_io_t interface. In this way, the declaration of them shall be removed from the system.h header file.

Go ahead for avoid disclosing the internal functions.

@ChinYikMing
Copy link
Collaborator Author

Any reason to expose mmu_ifetch, mmu_read_w, mmu_read_s, etc.?
They should be static in system.c and only accessible through riscv_io_t interface. In this way, the declaration of them shall be removed from the system.h header file.

Go ahead for avoid disclosing the internal functions.

'system.h' is removed due to too less LoC, it is not worth to add a new header file. Thus, I moved the original content to 'riscv.h' instead.

To boot a 32-bit RISC-V Linux with MMU, MMU emulation support is
crucial, using the SV32 virtual memory scheme. This commit’s main
changes include implementing the MMU-related riscv_io_t interface and
binding it during RISC-V instance initialization. To enable reuse of
riscv_io_t, its prototype is modified to accept the RISC-V core
instance as the first parameter, allowing MMU-enabled I/O to access
the SATP CSR. A trap_handler callback is also added to the riscv_io_t
interface. This keeps the dispatch_table and _trap_handler static
within emulate.c, aligning the schema with other handlers, like
ebreak_handler and ecall_handler. With m/scause and m/stval set
in SET_CAUSE_AND_TVAL_THEN_TRAP, _trap_handler can immediately
dispatch without rechecking scause, avoiding the need for additional
call of specific type of trap handler, thus no more need of
TRAP_HANDLER_IMPL.

For each memory access, the page table is walked to get the
corresponding PTE. Depending on the PTE retrieval, several page faults
may need handling. Thus, three exception handlers have been introduced:
insn_pgfault, load_pgfault, and store_pgfault, used in MMU_CHECK_FAULT.
This commit does not fully handle access faults since they are related
to PMA and PMP, which may not be necessary for booting 32-bit RISC-V
Linux (possibly supported in the future).

Since Linux has not been booted yet, a test suite is needed to test the
MMU emulation. This commit includes a test suite that implements a
simple kernel space supervisor and a user space application. The
supervisor prepares the page table and then passes control to the user
space application to test the three aforementioned page faults.

Related: sysprog21#310
@jserv jserv merged commit 4599b1d into sysprog21:master Oct 28, 2024
8 checks passed
@jserv
Copy link
Contributor

jserv commented Oct 28, 2024

Thank @ChinYikMing for contributing!

@ChinYikMing
Copy link
Collaborator Author

ChinYikMing commented Oct 28, 2024

Thanks all for reviewing. Ready to bring up Linux kernel on next PR!

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.

5 participants