feat(core/remio): add Remote I/O infrastructure #176

joaopeixoto13 · 2024-09-19T14:08:50Z

This PR introduces the foundational support for VirtIO by implementing a mechanism to forward VirtIO requests bidirectionally between the guest VM (or frontend VM) and its associated backend VM.

Validation

Platforms
- Armv8-A AArch64
  - Xilinx Zynq UltraScale+ MPSoC ZCU102/4
  - NVIDIA Jetson TX2
  - QEMU virt
- RISC-V RV64
  - QEMU virt
System configuration
- Multiple VirtIO frontend drivers on multiple guest VMs, each running on multiple vCPUs
- Multiple VirtIO backend devices on multiple backend VMs, each running on multiple vCPUs
Backend devices
- Rust-VMM backends (VirtIO-Block, VirtIO-Net, VirtIO-Console)
- QEMU (VirtIO-Block, VirtIO-Net, VirtIO-Console)
- Vhost-user backends (vhost-user-fs, vhost-user-vsock)
- Vhost backends (vhost-vsock)

DavidMCerdeira

Generic comment: the namespace for this module can be shortened. I suggest rmio, remio, rm_io, rem_io

some of the comments may be a bit subjective. @josecm feel free to wage in

src/core/remote_io.c

src/core/inc/remote_io.h

src/core/remote_io.c

joaopeixoto13 · 2024-09-20T10:28:11Z

This new version includes a refactor of the code following @DavidMCerdeira's suggestions.
The key changes are as follows:

Introduced helper functions as part of a new API, such as remote_io_create_request, remote_io_get_request, remote_io_push_request_event, remote_io_pop_request_event, etc.
Decoupled the large remote_io_hypercall function into smaller, modular functions, each corresponding to a specific hypercall event.
Added the REMOTE_IO_DEV_TYPE enum for better type safety and code clarity.
Introduced a new type, remote_io_id_t, to manage the Remote I/O ID more effectively.

This refactoring improves the structure and maintainability of the code.

joaopeixoto13 · 2024-09-20T10:45:35Z

Generic comment: the namespace for this module can be shortened. I suggest rmio, remio, rm_io, rem_io

some of the comments may be a bit subjective. @josecm feel free to wage in

@josecm What are your thoughts on this? From @DavidMCerdeira's list of suggestions, remio seems like the most appropriate name. However, I'm unsure if remote_io is still considered too lengthy. For this reason, I've retained the current name in the latest version.

src/core/inc/remote_io.h

src/core/remote_io.c

josecm · 2024-09-20T12:32:50Z

Generic comment: the namespace for this module can be shortened. I suggest rmio, remio, rm_io, rem_io
some of the comments may be a bit subjective. @josecm feel free to wage in

@josecm What are your thoughts on this? From @DavidMCerdeira's list of suggestions, remio seems like the most appropriate name. However, I'm unsure if remote_io is still considered too lengthy. For this reason, I've retained the current name in the latest version.

My suggestion would be either remio or simply rio.

joaopeixoto13 · 2024-09-20T13:16:01Z

Generic comment: the namespace for this module can be shortened. I suggest rmio, remio, rm_io, rem_io
some of the comments may be a bit subjective. @josecm feel free to wage in

@josecm What are your thoughts on this? From @DavidMCerdeira's list of suggestions, remio seems like the most appropriate name. However, I'm unsure if remote_io is still considered too lengthy. For this reason, I've retained the current name in the latest version.

My suggestion would be either remio or simply rio.

Update: 01c2cbe

DavidMCerdeira · 2024-09-20T13:27:08Z

Generic comment: the namespace for this module can be shortened. I suggest rmio, remio, rm_io, rem_io
some of the comments may be a bit subjective. @josecm feel free to wage in

@josecm What are your thoughts on this? From @DavidMCerdeira's list of suggestions, remio seems like the most appropriate name. However, I'm unsure if remote_io is still considered too lengthy. For this reason, I've retained the current name in the latest version.

My suggestion would be either remio or simply rio.

Update: 01c2cbe

The namespace change should be reflected in the function, variable names, enums, etc

joaopeixoto13 · 2024-09-20T13:47:36Z

Generic comment: the namespace for this module can be shortened. I suggest rmio, remio, rm_io, rem_io
some of the comments may be a bit subjective. @josecm feel free to wage in

@josecm What are your thoughts on this? From @DavidMCerdeira's list of suggestions, remio seems like the most appropriate name. However, I'm unsure if remote_io is still considered too lengthy. For this reason, I've retained the current name in the latest version.

My suggestion would be either remio or simply rio.

Update: 01c2cbe

The namespace change should be reflected in the function, variable names, enums, etc

Done 00b5acd

src/core/remio.c

src/core/vm.c

src/core/remio.c

src/core/vm.c

src/core/remio.c

DavidMCerdeira

I'd still like to test this before approving, but for now it looks good! 👍
You can squash the commits into a single one now

joaopeixoto13 · 2024-09-21T09:12:45Z

I'd still like to test this before approving, but for now it looks good! 👍 You can squash the commits into a single one now

Thanks, @DavidMCerdeira ! Also, please take a look at this commit, where I temporarily resolved the issue by replacing the psci_power_down call with a WFI instruction. However, I believe we need to find a more appropriate long-term solution. I haven't reviewed the current guest reboot PR yet, but it may offer some insight or help in addressing this issue.

DavidMCerdeira · 2024-09-21T09:20:33Z

I'd still like to test this before approving, but for now it looks good! 👍 You can squash the commits into a single one now

Thanks, @DavidMCerdeira ! Also, please take a look at this commit, where I temporarily resolved the issue by replacing the psci_power_down call with a WFI instruction. However, I believe we need to find a more appropriate long-term solution. I haven't reviewed the current guest reboot PR yet, but it may offer some insight or help in addressing this issue.

I think for now it is fine. Actually we are aware of at least two platforms that require this behavior to work with bao currently. We should eventually allow platforms to have specific code to handle this sort of platform specific quirks

josecm

Despite the very (!) good work, I think we still need a bit of work on this PR. This is my first pass on it. I think we will need one or two more.

src/arch/armv8/armv8-a/cpu.c

src/arch/riscv/sbi.c

src/core/inc/remio.h

src/core/remio.c

src/arch/riscv/sbi.c

src/core/remio.c

DavidMCerdeira

Only nitpicks
Otherwise looks good to me!

src/core/remio.c

src/core/inc/config_defs.h

josecm · 2024-11-02T12:17:00Z

@joaopeixoto13 rebase on main and resolve conflicts please

joaopeixoto13 · 2024-11-02T12:26:11Z

@joaopeixoto13 rebase on main and resolve conflicts please

Done. If everything looks good, I can proceed to merge all the fix commits into the main parent commit that introduced the feature.

josecm · 2024-11-02T15:51:54Z

Done. If everything looks good, I can proceed to merge all the fix commits into the main parent commit that introduced the feature.

Please go ahead with that.

joaopeixoto13 · 2024-11-02T17:21:56Z

Done. If everything looks good, I can proceed to merge all the fix commits into the main parent commit that introduced the feature.

Please go ahead with that.

Done ✅

src/core/inc/hypercall.h

src/core/remio.c

This commit renames the first argument of the hypercall to make the code independent of the specific hypercall type. Signed-off-by: João Peixoto <[email protected]>

This commit introduces a separation of input and output conventions for hypercalls, addressing architecture-specific differences between ARM and RISC-V. On ARM, the x0 register is used to pass both the extid, fid, and the return value, while additional parameters (hypercalls arguments) are passed via registers x1 to xn. On RISC-V, while the return value is passed in a0, the extid and fid are handled by the a7 and a6 registers, respectively. This means that the hypercall input arguments on RISC-V should be passed through registers a0 to a5, while output hypercall arguments are passed via a2 and onward (because all registers except a0 and a1 must be preserved across an SBI call by the callee). To accommodate these differences and provide a uniform interface, two macros were introduced to abstract the handling of input and output hypercall arguments across architectures. Signed-off-by: João Peixoto <[email protected]>

This commit introduces two methods for setting and retrieving hypercall arguments, allowing each hypercall implementation to flexibly manage its specific arguments as needed. Signed-off-by: João Peixoto <[email protected]>

Refactored the vCPU program counter incrementation by introducing the architecture-agnostic method vcpu_writepc, aligning the implementation with the approach used in the ARM architecture. Signed-off-by: João Peixoto <[email protected]>

Implemented support for abort and exception handlers to suspend the current vCPU based on the vCPU active flag. Signed-off-by: João Peixoto <[email protected]>

This commit introduces two new methods to the objpool API. The first method, objpool_alloc_with_id, enables the allocation of an object within a pool and returns the index where the object was allocated. The second method, objpool_get_by_id, allows retrieving a previously allocated object using the index of the pool where it was initially allocated. Signed-off-by: João Peixoto <[email protected]>

This commit introduces the foundational support for VirtIO by implementing a mechanism to forward VirtIO requests bidirectionally between the guest VM (or frontend VM) and its associated backend VM. Signed-off-by: João Peixoto <[email protected]>

joaopeixoto13 requested review from josecm, ninolomata and DavidMCerdeira as code owners September 19, 2024 14:08

joaopeixoto13 force-pushed the feat/remote_io branch from 0afe0bf to 38d7db2 Compare September 19, 2024 14:13

josecm self-assigned this Sep 19, 2024

joaopeixoto13 force-pushed the feat/remote_io branch from 38d7db2 to 01019b2 Compare September 19, 2024 15:37

DavidMCerdeira requested changes Sep 19, 2024

View reviewed changes

joaopeixoto13 force-pushed the feat/remote_io branch from 01019b2 to b999298 Compare September 20, 2024 10:15

DavidMCerdeira self-requested a review September 20, 2024 10:30

DavidMCerdeira requested changes Sep 20, 2024

View reviewed changes

joaopeixoto13 changed the title ~~feat(core/remote_io): add Remote I/O infrastructure~~ feat(core/remio): add Remote I/O infrastructure Sep 20, 2024

DavidMCerdeira self-requested a review September 20, 2024 14:02

DavidMCerdeira requested changes Sep 20, 2024

View reviewed changes

src/core/remio.c Outdated Show resolved Hide resolved

src/core/remio.c Show resolved Hide resolved

src/core/remio.c Outdated Show resolved Hide resolved

src/core/vm.c Outdated Show resolved Hide resolved

src/core/remio.c Outdated Show resolved Hide resolved

DavidMCerdeira requested changes Sep 20, 2024

View reviewed changes

src/core/vm.c Outdated Show resolved Hide resolved

src/core/remio.c Outdated Show resolved Hide resolved

src/core/remio.c Outdated Show resolved Hide resolved

src/core/remio.c Outdated Show resolved Hide resolved

joaopeixoto13 force-pushed the feat/remote_io branch from 93e33be to 0c544f9 Compare September 21, 2024 08:34

DavidMCerdeira reviewed Sep 21, 2024

View reviewed changes

joaopeixoto13 force-pushed the feat/remote_io branch 2 times, most recently from 9c31e3e to 7c277bf Compare September 27, 2024 12:17

josecm requested changes Sep 29, 2024

View reviewed changes

josecm requested changes Oct 7, 2024

View reviewed changes

src/arch/riscv/sbi.c Outdated Show resolved Hide resolved

src/core/remio.c Outdated Show resolved Hide resolved

joaopeixoto13 force-pushed the feat/remote_io branch from f0a2ce4 to 93e0e01 Compare October 22, 2024 16:18

joaopeixoto13 requested a review from AfonsoSantos96 as a code owner October 22, 2024 16:18

joaopeixoto13 force-pushed the feat/remote_io branch 2 times, most recently from 29988f8 to 26a8a46 Compare November 1, 2024 11:42

DavidMCerdeira previously approved these changes Nov 1, 2024

View reviewed changes

src/core/remio.c Outdated Show resolved Hide resolved

src/core/inc/config_defs.h Show resolved Hide resolved

joaopeixoto13 dismissed DavidMCerdeira’s stale review via 6a55d0e November 2, 2024 12:24

joaopeixoto13 force-pushed the feat/remote_io branch from 26a8a46 to 6a55d0e Compare November 2, 2024 12:24

joaopeixoto13 force-pushed the feat/remote_io branch from 6a55d0e to d56ef75 Compare November 2, 2024 17:21

DavidMCerdeira self-requested a review November 2, 2024 18:27

DavidMCerdeira previously approved these changes Nov 2, 2024

View reviewed changes

joaopeixoto13 dismissed DavidMCerdeira’s stale review via 77ff8e8 November 5, 2024 11:03

joaopeixoto13 force-pushed the feat/remote_io branch from d56ef75 to 77ff8e8 Compare November 5, 2024 11:03

josecm reviewed Nov 5, 2024

View reviewed changes

src/core/inc/hypercall.h Outdated Show resolved Hide resolved

josecm reviewed Nov 5, 2024

View reviewed changes

src/core/inc/hypercall.h Outdated Show resolved Hide resolved

josecm requested changes Nov 5, 2024

View reviewed changes

joaopeixoto13 added 6 commits November 5, 2024 14:01

ref(core/hypercall): rename first argument

e13ddb6

This commit renames the first argument of the hypercall to make the code independent of the specific hypercall type. Signed-off-by: João Peixoto <[email protected]>

feat(arch/exception_handler): enable exception handlers to suspend vCPU

cd6f46b

Implemented support for abort and exception handlers to suspend the current vCPU based on the vCPU active flag. Signed-off-by: João Peixoto <[email protected]>

joaopeixoto13 force-pushed the feat/remote_io branch from 77ff8e8 to 583b36f Compare November 5, 2024 14:01

josecm approved these changes Nov 5, 2024

View reviewed changes

joaopeixoto13 force-pushed the feat/remote_io branch from 583b36f to cfcec7c Compare November 5, 2024 14:05

DavidMCerdeira self-requested a review November 5, 2024 16:49

DavidMCerdeira approved these changes Nov 5, 2024

View reviewed changes

josecm merged commit 8f219e9 into bao-project:main Nov 5, 2024
31 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(core/remio): add Remote I/O infrastructure #176

feat(core/remio): add Remote I/O infrastructure #176

joaopeixoto13 commented Sep 19, 2024 •

edited

Loading

DavidMCerdeira left a comment

joaopeixoto13 commented Sep 20, 2024

joaopeixoto13 commented Sep 20, 2024

josecm commented Sep 20, 2024

joaopeixoto13 commented Sep 20, 2024

DavidMCerdeira commented Sep 20, 2024

joaopeixoto13 commented Sep 20, 2024

DavidMCerdeira left a comment •

edited

Loading

joaopeixoto13 commented Sep 21, 2024

DavidMCerdeira commented Sep 21, 2024

josecm left a comment

DavidMCerdeira left a comment

josecm commented Nov 2, 2024

joaopeixoto13 commented Nov 2, 2024 •

edited

Loading

josecm commented Nov 2, 2024

joaopeixoto13 commented Nov 2, 2024 •

edited

Loading

feat(core/remio): add Remote I/O infrastructure #176

feat(core/remio): add Remote I/O infrastructure #176

Conversation

joaopeixoto13 commented Sep 19, 2024 • edited Loading

DavidMCerdeira left a comment

Choose a reason for hiding this comment

joaopeixoto13 commented Sep 20, 2024

joaopeixoto13 commented Sep 20, 2024

josecm commented Sep 20, 2024

joaopeixoto13 commented Sep 20, 2024

DavidMCerdeira commented Sep 20, 2024

joaopeixoto13 commented Sep 20, 2024

DavidMCerdeira left a comment • edited Loading

Choose a reason for hiding this comment

joaopeixoto13 commented Sep 21, 2024

DavidMCerdeira commented Sep 21, 2024

josecm left a comment

Choose a reason for hiding this comment

DavidMCerdeira left a comment

Choose a reason for hiding this comment

josecm commented Nov 2, 2024

joaopeixoto13 commented Nov 2, 2024 • edited Loading

josecm commented Nov 2, 2024

joaopeixoto13 commented Nov 2, 2024 • edited Loading

joaopeixoto13 commented Sep 19, 2024 •

edited

Loading

DavidMCerdeira left a comment •

edited

Loading

joaopeixoto13 commented Nov 2, 2024 •

edited

Loading

joaopeixoto13 commented Nov 2, 2024 •

edited

Loading