From ff506b037537bf0d39f6414261d9367b07323c13 Mon Sep 17 00:00:00 2001 From: Alexander Boettcher Date: Wed, 4 Sep 2024 15:41:29 +0200 Subject: [PATCH] vm/x86: support extended fpu state transfer Extend Genode's vCPU FPU state and adjust all users to copy at most FPU data they actually support. Issue #5314 --- repos/base-foc/src/lib/base/x86/vm.cc | 2 ++ repos/base-hw/src/core/spec/x86_64/fpu.h | 1 + .../spec/x86_64/virtualization/kernel/vm.cc | 5 +++-- repos/base-nova/include/nova/syscall-generic.h | 2 +- repos/base-nova/src/lib/base/vm.cc | 5 ++++- repos/base/include/spec/x86/cpu/vcpu_state.h | 17 ++++++++++------- repos/os/src/test/vmm_x86/component.cc | 6 ++++-- repos/ports/src/virtualbox5/spec/nova/vcpu.h | 6 +++--- repos/ports/src/virtualbox6/sup_vcpu.cc | 9 +++++---- 9 files changed, 33 insertions(+), 20 deletions(-) diff --git a/repos/base-foc/src/lib/base/x86/vm.cc b/repos/base-foc/src/lib/base/x86/vm.cc index 58f3d94c86..2c626f41ae 100644 --- a/repos/base-foc/src/lib/base/x86/vm.cc +++ b/repos/base-foc/src/lib/base/x86/vm.cc @@ -400,6 +400,7 @@ struct Foc_vcpu : Thread, Noncopyable if (state.fpu.charged()) { state.fpu.charge([&] (Vcpu_state::Fpu::State &fpu) { asm volatile ("fxrstor %0" : : "m" (fpu) : "memory"); + return 512; }); } else asm volatile ("fxrstor %0" : : "m" (_fpu_vcpu) : "memory"); @@ -412,6 +413,7 @@ struct Foc_vcpu : Thread, Noncopyable state.fpu.charge([&] (Vcpu_state::Fpu::State &fpu) { asm volatile ("fxsave %0" : "=m" (fpu) :: "memory"); asm volatile ("fxsave %0" : "=m" (_fpu_vcpu) :: "memory"); + return 512; }); asm volatile ("fxrstor %0" : : "m" (_fpu_ep) : "memory"); diff --git a/repos/base-hw/src/core/spec/x86_64/fpu.h b/repos/base-hw/src/core/spec/x86_64/fpu.h index 817a26b716..1a610e4c4f 100644 --- a/repos/base-hw/src/core/spec/x86_64/fpu.h +++ b/repos/base-hw/src/core/spec/x86_64/fpu.h @@ -60,6 +60,7 @@ class Genode::Fpu_context } addr_t fpu_context() const { return _fxsave_addr; } + addr_t fpu_size() const { return sizeof(_fxsave_area); } }; #endif /* _CORE__SPEC__X86_64__FPU_H_ */ 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 750d8f2d5f..f6de031093 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 @@ -222,7 +222,7 @@ void Board::Vcpu_context::read_vcpu_state(Vcpu_state &state) if (state.fpu.charged()) { state.fpu.with_state( [&](Vcpu_state::Fpu::State const &fpu) { - memcpy((void *) regs->fpu_context(), &fpu, sizeof(fpu)); + memcpy((void *) regs->fpu_context(), &fpu, regs->fpu_size()); }); } } @@ -233,7 +233,8 @@ void Board::Vcpu_context::write_vcpu_state(Vcpu_state &state) state.exit_reason = (unsigned) exit_reason; state.fpu.charge([&](Vcpu_state::Fpu::State &fpu) { - memcpy(&fpu, (void *) regs->fpu_context(), sizeof(fpu)); + memcpy(&fpu, (void *) regs->fpu_context(), regs->fpu_size()); + return regs->fpu_size(); }); /* SVM will overwrite rax but VMX doesn't. */ diff --git a/repos/base-nova/include/nova/syscall-generic.h b/repos/base-nova/include/nova/syscall-generic.h index 8210e31b7b..0190c1df58 100644 --- a/repos/base-nova/include/nova/syscall-generic.h +++ b/repos/base-nova/include/nova/syscall-generic.h @@ -694,7 +694,7 @@ namespace Nova { } gdtr, idtr; unsigned long long tsc_val, tsc_off, tsc_aux; unsigned long long exit_reason; - uint8_t fpu[512]; + uint8_t fpu[2560]; } __attribute__((packed)); mword_t mr[(4096 - 4 * sizeof(mword_t)) / sizeof(mword_t)]; }; diff --git a/repos/base-nova/src/lib/base/vm.cc b/repos/base-nova/src/lib/base/vm.cc index bd2c407ded..43603e153b 100644 --- a/repos/base-nova/src/lib/base/vm.cc +++ b/repos/base-nova/src/lib/base/vm.cc @@ -177,7 +177,10 @@ void Nova_vcpu::_read_nova_state(Nova::Utcb &utcb) if (utcb.mtd & Nova::Mtd::FPU) { _vcpu_state.fpu.charge([&] (Vcpu_state::Fpu::State &fpu) { - memcpy(&fpu, utcb.fpu, sizeof(fpu)); + auto const fpu_size = unsigned(min(_vcpu_state.fpu.size(), + sizeof(utcb.fpu))); + memcpy(&fpu, utcb.fpu, fpu_size); + return fpu_size; }); } diff --git a/repos/base/include/spec/x86/cpu/vcpu_state.h b/repos/base/include/spec/x86/cpu/vcpu_state.h index 6141e566b1..f943a75283 100644 --- a/repos/base/include/spec/x86/cpu/vcpu_state.h +++ b/repos/base/include/spec/x86/cpu/vcpu_state.h @@ -206,15 +206,16 @@ class Genode::Vcpu_state struct State { - uint8_t _buffer[512] { }; - } __attribute__((aligned(16))); + uint8_t _buffer[2560] { }; + } __attribute__((aligned(64))); private: friend class Vcpu_state; - State _state { }; - bool _charged { false }; + State _state { }; + bool _charged { false }; + uint64_t _state_size { }; /* see comment for Register::operator=() */ Fpu & operator = (Fpu const &) @@ -232,12 +233,14 @@ class Genode::Vcpu_state void charge(auto const &fn) { - _charged = true; - fn(_state); + _charged = true; + _state_size = fn(_state); } + + auto size() const { return _state_size; } }; - Fpu fpu __attribute__((aligned(16))) { }; + Fpu fpu __attribute__((aligned(64))) { }; /* * Registers transfered by hypervisor from guest on VM exit are charged. diff --git a/repos/os/src/test/vmm_x86/component.cc b/repos/os/src/test/vmm_x86/component.cc index c7cfe8c938..3710a75e1e 100644 --- a/repos/os/src/test/vmm_x86/component.cc +++ b/repos/os/src/test/vmm_x86/component.cc @@ -120,8 +120,10 @@ class Vmm::Vcpu void force_fpu_state_transfer(Vcpu_state & state) { /* force FPU-state transfer on next entry */ - state.fpu.charge([] (Vcpu_state::Fpu::State &) { - /* don't change state */ }); + state.fpu.charge([] (Vcpu_state::Fpu::State &state) { + /* don't change state */ + return sizeof(state); + }); } /* diff --git a/repos/ports/src/virtualbox5/spec/nova/vcpu.h b/repos/ports/src/virtualbox5/spec/nova/vcpu.h index 216b91a2fd..38b735ab80 100644 --- a/repos/ports/src/virtualbox5/spec/nova/vcpu.h +++ b/repos/ports/src/virtualbox5/spec/nova/vcpu.h @@ -85,7 +85,7 @@ class Vcpu_handler : public Vmm::Vcpu_dispatcher, private: - X86FXSTATE _emt_fpu_state __attribute__((aligned(0x10))); + X86FXSTATE _emt_fpu_state __attribute__((aligned(64))); pthread _pthread; pthread_cond_t _cond_wait; @@ -968,7 +968,7 @@ class Vcpu_handler : public Vmm::Vcpu_dispatcher, /* save current FPU state */ fpu_save(reinterpret_cast(&_emt_fpu_state)); /* write FPU state from pCtx to utcb */ - memcpy(utcb->fpu, pCtx->pXStateR3, sizeof(utcb->fpu)); + memcpy(utcb->fpu, pCtx->pXStateR3, sizeof(X86FXSTATE)); /* tell kernel to transfer current fpu registers to vCPU */ utcb->mtd |= Mtd::FPU; @@ -984,7 +984,7 @@ class Vcpu_handler : public Vmm::Vcpu_dispatcher, _current_vcpu = 0; /* write FPU state of vCPU in utcb to pCtx */ - static_assert(sizeof(X86FXSTATE) == sizeof(utcb->fpu), "fpu size mismatch"); + static_assert(sizeof(X86FXSTATE) <= sizeof(utcb->fpu), "fpu size mismatch"); Genode::memcpy(pCtx->pXStateR3, utcb->fpu, sizeof(X86FXSTATE)); /* load saved FPU state of EMT thread */ diff --git a/repos/ports/src/virtualbox6/sup_vcpu.cc b/repos/ports/src/virtualbox6/sup_vcpu.cc index cff44179bb..a2e2312e41 100644 --- a/repos/ports/src/virtualbox6/sup_vcpu.cc +++ b/repos/ports/src/virtualbox6/sup_vcpu.cc @@ -266,10 +266,10 @@ template void Sup::Vcpu_impl::_transfer_state_to_vcpu(CPUM } /* export FPU state */ - AssertCompile(sizeof(Vcpu_state::Fpu::State) >= sizeof(X86FXSTATE)); - - _state->ref.fpu.charge([&](Vcpu_state::Fpu::State &fpu) { - ::memcpy(fpu._buffer, ctx.pXStateR3, sizeof(fpu)); + _state->ref.fpu.charge([&](Vcpu_state::Fpu::State &fpu) { + static_assert(sizeof(*ctx.pXStateR3) >= sizeof(fpu._buffer)); + ::memcpy(fpu._buffer, ctx.pXStateR3, sizeof(X86FXSTATE)); + return sizeof(X86FXSTATE); }); { @@ -415,6 +415,7 @@ template void Sup::Vcpu_impl::_transfer_state_to_vbox(CPUM /* import FPU state */ _state->ref.fpu.with_state([&](Vcpu_state::Fpu::State const &fpu) { + static_assert(sizeof(*ctx.pXStateR3) >= sizeof(fpu._buffer)); ::memcpy(ctx.pXStateR3, fpu._buffer, sizeof(X86FXSTATE)); return true; });