Skip to content

No sound way to set IMMEDIATE_EXIT while we are in VcpuFd::run #373

Description

@olivereanderson

VMMs typically need a way to "kick" a vCPU out of KVM_RUN. The standard way to do this is to send a signal to the desired thread where the vCPU is running. In order to not loose such a signal in the case where the signal is received just before the thread enters KVM_RUN, KVM expects a signal handler (or another thread) to set IMMEDIATE_EXIT = 1 in the kvm_run structure (0). This flag is then read by KVM at the beginning of KVM_RUN and exits if the flag is set.

Unfortunately, VcpuFd::run internally produces a &mut kvm_run reference which then means that the immediate_exit field cannot be read or written to, in any way, elsewhere in the program including a signal handler on the same thread, or from another thread altogether.

Some downstream crates work around this limitation via alias mappings (mapping the same physical memory more than once), but that is also technically unsound. Future Rust compilers may figure out that IMMEDIATE_EXIT cannot be (soundly) set while we are in VcpuFd::run and may start optimizing around this in a way that prevents our programs from behaving as desired.

I thus propose changing VcpuFd::get_kvm_run to return NonNull<kvm_run> instead of &mut kvm_run and also make sure that there is no simple way to obtain &mut kvm_run from KvmRunWrapper. We can still soundly obtain mutable references to any other field of kvm_run by using the raw borrow operator e.g.

let exit_reason_raw: *mut u32 = unsafe {
    &raw mut (*kvm_run_ptr.as_ptr()).exit_reason
};

let exit_reason: &mut u32 = unsafe { &mut *exit_reason_raw };

and we may of course introduce convenience methods for safely obtaining such references from VcpuFd and/or KvmRunWrapper.

If this approach is applied in all internal code and also vCPU threads never produce a &mut kvm_run then it will be easy for applications to soundly update immediate_exit by simply field projecting to immediate_exit and casting the pointer to NonNull<AtomicU8> which we can then write atomically to.

I would be very happy to help out with such a refactor.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions