From c3c719fce784abf4b2326b3dc12d0f599b0bfc19 Mon Sep 17 00:00:00 2001 From: Benjamin Lamowski Date: Mon, 24 Mar 2025 14:48:28 +0100 Subject: [PATCH] hw: simplify x86 vCPU initialization Initializing a vCPU on base-hw involved a round-trip to the kernel to send the initial startup exit signal and setting up the vCPU on first run of the vCPU thread. Signal the vCPU handler directly from the base libary. This removes the need to call `Vm::run()` during construction of the vCPU, possibly from a remote core. Check that `Vm::run()` and `Vm::pause()` are only called from the local CPU core. Finally, move the initialization of the vCPU from `Vm::proceed()` to `Vm::run()`, so that the `Vcpu_state` can be synced from the VMM directly after initialization. Fixes #5483 --- repos/base-hw/src/core/kernel/vm.h | 10 +- .../core/spec/arm_v7/trustzone/kernel/vm.cc | 15 +-- .../spec/arm_v7/virtualization/kernel/vm.cc | 16 ++-- .../spec/arm_v8/virtualization/kernel/vm.cc | 15 +-- .../core/spec/x86_64/virtualization/board.h | 2 - .../spec/x86_64/virtualization/kernel/vm.cc | 93 ++++++++----------- repos/base-hw/src/lib/base/x86_64/vm.cc | 14 ++- 7 files changed, 79 insertions(+), 86 deletions(-) diff --git a/repos/base-hw/src/core/kernel/vm.h b/repos/base-hw/src/core/kernel/vm.h index 3bb8ecddf3..0b4d7467bb 100644 --- a/repos/base-hw/src/core/kernel/vm.h +++ b/repos/base-hw/src/core/kernel/vm.h @@ -16,6 +16,7 @@ #define _CORE__KERNEL__VM_H_ /* core includes */ +#include #include #include #include @@ -61,8 +62,6 @@ class Kernel::Vm : private Kernel::Object, public Cpu_context Scheduler_state _scheduled = INACTIVE; Board::Vcpu_context _vcpu_context; - void _sync_to_vmm(); - void _sync_from_vmm(); void _pause_vcpu() { if (_scheduled != INACTIVE) @@ -134,12 +133,7 @@ class Kernel::Vm : private Kernel::Object, public Cpu_context void run(); - void pause() - { - _pause_vcpu(); - _sync_to_vmm(); - } - + void pause(); /***************** ** Cpu_context ** diff --git a/repos/base-hw/src/core/spec/arm_v7/trustzone/kernel/vm.cc b/repos/base-hw/src/core/spec/arm_v7/trustzone/kernel/vm.cc index 0c4d190621..34c4664454 100644 --- a/repos/base-hw/src/core/spec/arm_v7/trustzone/kernel/vm.cc +++ b/repos/base-hw/src/core/spec/arm_v7/trustzone/kernel/vm.cc @@ -86,15 +86,16 @@ void Vm::proceed() void Vm::run() { - _sync_from_vmm(); if (_scheduled != ACTIVE) Cpu_context::_activate(); _scheduled = ACTIVE; } -void Vm::_sync_to_vmm() -{} - - -void Vm::_sync_from_vmm() -{} +void Vm::pause() +{ + if (_cpu().id() != Cpu::executing_id()) { + Genode::error("vCPU pause called from remote core."); + return; + } + _pause_vcpu(); +} diff --git a/repos/base-hw/src/core/spec/arm_v7/virtualization/kernel/vm.cc b/repos/base-hw/src/core/spec/arm_v7/virtualization/kernel/vm.cc index 175566561f..9ce0a282c0 100644 --- a/repos/base-hw/src/core/spec/arm_v7/virtualization/kernel/vm.cc +++ b/repos/base-hw/src/core/spec/arm_v7/virtualization/kernel/vm.cc @@ -207,18 +207,18 @@ void Kernel::Vm::proceed() void Vm::run() { - _sync_from_vmm(); if (_scheduled != ACTIVE) Cpu_context::_activate(); _scheduled = ACTIVE; } - -void Vm::_sync_to_vmm() -{} - - -void Vm::_sync_from_vmm() -{} +void Vm::pause() +{ + if (_cpu().id() != Cpu::executing_id()) { + Genode::error("vCPU pause called from remote core."); + return; + } + _pause_vcpu(); +} void Vm::inject_irq(unsigned irq) diff --git a/repos/base-hw/src/core/spec/arm_v8/virtualization/kernel/vm.cc b/repos/base-hw/src/core/spec/arm_v8/virtualization/kernel/vm.cc index 39446108c2..8f8bc7e3de 100644 --- a/repos/base-hw/src/core/spec/arm_v8/virtualization/kernel/vm.cc +++ b/repos/base-hw/src/core/spec/arm_v8/virtualization/kernel/vm.cc @@ -214,18 +214,19 @@ void Vm::proceed() void Vm::run() { - _sync_from_vmm(); if (_scheduled != ACTIVE) Cpu_context::_activate(); _scheduled = ACTIVE; } -void Vm::_sync_to_vmm() -{} - - -void Vm::_sync_from_vmm() -{} +void Vm::pause() +{ + if (_cpu().id() != Cpu::executing_id()) { + Genode::error("vCPU pause called from remote core."); + return; + } + _pause_vcpu(); +} void Vm::inject_irq(unsigned irq) diff --git a/repos/base-hw/src/core/spec/x86_64/virtualization/board.h b/repos/base-hw/src/core/spec/x86_64/virtualization/board.h index a821d15eaa..aa33d09b42 100644 --- a/repos/base-hw/src/core/spec/x86_64/virtualization/board.h +++ b/repos/base-hw/src/core/spec/x86_64/virtualization/board.h @@ -35,7 +35,6 @@ namespace Board { enum Platform_exitcodes : uint64_t { EXIT_NPF = 0xfc, - EXIT_STARTUP = 0xfe, EXIT_PAUSED = 0xff, }; @@ -55,7 +54,6 @@ struct Board::Vcpu_context { enum class Init_state { CREATED, - INITIALIZING, STARTED }; diff --git a/repos/base-hw/src/core/spec/x86_64/virtualization/kernel/vm.cc b/repos/base-hw/src/core/spec/x86_64/virtualization/kernel/vm.cc index 9d67040677..f1d8e09867 100644 --- a/repos/base-hw/src/core/spec/x86_64/virtualization/kernel/vm.cc +++ b/repos/base-hw/src/core/spec/x86_64/virtualization/kernel/vm.cc @@ -55,36 +55,57 @@ Vm::~Vm() void Vm::run() { - if (_vcpu_context.init_state == Board::Vcpu_context::Init_state::CREATED) { - _vcpu_context.exit_reason = Board::EXIT_STARTUP; - _vcpu_context.init_state = Board::Vcpu_context::Init_state::INITIALIZING; - _context.submit(1); + if (_cpu().id() != Cpu::executing_id()) { + error("vCPU run called from remote core."); return; } - _sync_from_vmm(); + /* + * On first start, initialize the vCPU + */ + if (_vcpu_context.init_state == Board::Vcpu_context::Init_state::CREATED) { + _vcpu_context.initialize(_cpu(), + reinterpret_cast(_id.table)); + _vcpu_context.tsc_aux_host = _cpu().id(); + _vcpu_context.init_state = Board::Vcpu_context::Init_state::STARTED; + } + + _vcpu_context.read_vcpu_state(_state); + if (_scheduled != ACTIVE) Cpu_context::_activate(); _scheduled = ACTIVE; } -void Vm::proceed() +void Vm::pause() { - using namespace Board; - - if (_vcpu_context.init_state == Board::Vcpu_context::Init_state::INITIALIZING) { - _vcpu_context.initialize(_cpu(), - reinterpret_cast(_id.table)); - _vcpu_context.tsc_aux_host = _cpu().id(); - _vcpu_context.init_state = Board::Vcpu_context::Init_state::STARTED; - - /* - * Sync the initial state from the VMM that was skipped due to - * the vCPU being uninitialized. - */ - _sync_from_vmm(); + if (_cpu().id() != Cpu::executing_id()) { + Genode::error("vCPU pause called from remote core."); + return; } + /* + * The vCPU isn't initialized yet when the VMM first queries the state. + * Just return so that the VMM gets presented with the default startup + * exit code set at construction. + */ + if (_vcpu_context.init_state != Board::Vcpu_context::Init_state::STARTED) + return; + + _pause_vcpu(); + + _vcpu_context.write_vcpu_state(_state); + + /* + * Set exit code so that if _run() was not called after an exit, the + * next exit due to a signal will be interpreted as PAUSE request. + */ + _vcpu_context.exit_reason = Board::EXIT_PAUSED; +} + + +void Vm::proceed() +{ Cpu::Ia32_tsc_aux::write( (Cpu::Ia32_tsc_aux::access_t)_vcpu_context.tsc_aux_guest); @@ -165,40 +186,6 @@ void Vm::exception(Genode::Cpu_state &state) } -void Vm::_sync_to_vmm() -{ - /* - * If the vCPU isn't initialized, sync instructions such as vmread may fail. - * Just sync the startup exit and skip the rest of the synchronization. - */ - if (_vcpu_context.init_state != Board::Vcpu_context::Init_state::STARTED) { - _state.exit_reason = (unsigned) Board::EXIT_STARTUP; - return; - } - - _vcpu_context.write_vcpu_state(_state); - - /* - * Set exit code so that if _run() was not called after an exit, the - * next exit due to a signal will be interpreted as PAUSE request. - */ - _vcpu_context.exit_reason = Board::EXIT_PAUSED; -} - - -void Vm::_sync_from_vmm() -{ - /* - * Syncing the state to an unitialized vCPU may fail. - * The inial state from the VMM will be synced after vCPU initialization. - */ - if (_vcpu_context.init_state != Board::Vcpu_context::Init_state::STARTED) - return; - - _vcpu_context.read_vcpu_state(_state); -} - - Board::Vcpu_context::Vcpu_context(unsigned id, Vcpu_data &vcpu_data) : regs(1), diff --git a/repos/base-hw/src/lib/base/x86_64/vm.cc b/repos/base-hw/src/lib/base/x86_64/vm.cc index 47f357a2cb..3d9dcc6414 100644 --- a/repos/base-hw/src/lib/base/x86_64/vm.cc +++ b/repos/base-hw/src/lib/base/x86_64/vm.cc @@ -19,8 +19,11 @@ #include #include #include + #include +#include + #include #include @@ -38,6 +41,7 @@ using Exit_config = Vm_connection::Exit_config; struct Hw_vcpu : Rpc_client, Noncopyable { private: + enum Initial_exitcode : unsigned { EXIT_STARTUP = 0xfe }; Attached_dataspace _state; Native_capability _kernel_vcpu { }; @@ -73,6 +77,11 @@ Hw_vcpu::Hw_vcpu(Env &env, Vm_connection &vm, Vcpu_handler_base &handler) _kernel_vcpu = call(); _id = counter++; _ep_handler = reinterpret_cast(&handler.rpc_ep()); + + /* + * Set the startup exit for the initial signal to the VMM's Vcpu_handler + */ + _local_state().exit_reason = EXIT_STARTUP; } @@ -119,5 +128,8 @@ Vm_connection::Vcpu::Vcpu(Vm_connection &vm, Allocator &alloc, : _native_vcpu(*new (alloc) Hw_vcpu(vm._env, vm, handler)) { - static_cast(_native_vcpu).run(); + /* + * Send the initial startup signal to the vCPU handler. + */ + Signal_transmitter(handler.signal_cap()).submit(); }