On 12/5/2023 11:50 AM, Peter Xu wrote: > On Mon, Dec 04, 2023 at 05:23:57PM -0500, Steven Sistare wrote: >> The V6 code does not pass a state to vm_start, and knowledge of >> vm_was_suspended >> is confined to the global_state and cpus functions. IMO this is a more >> modular >> and robust solution, as multiple sites may call vm_start(), and the right >> thing >> happens. Look at patch 6. The changes are minimal because vm_start "just >> works". > > Oh I think I see what you meant. Sounds good then. > > Shall we hide that into vm_prepare_start()? It seems three's still one > more call sites that always pass in RUNNING (gdb_continue_partial). > > If with above, vm_prepare_start() will go into either RUNNING, SUSPENDED, > or an error. It returns 0 only if RUNNING, -1 for all the rest. Maybe we > can already also touch up the retval of vm_prepare_start() to be a boolean, > reflecting "whether vcpu needs to be started".
Yes, that is even nicer, thanks. /** * Prepare for (re)starting the VM. * Returns 0 if the vCPUs should be restarted, -1 on an error condition, * and 1 otherwise. */ int vm_prepare_start(bool step_pending) { int ret = vm_was_suspended ? 1 : 0; RunState state = vm_was_suspended ? RUN_STATE_SUSPENDED : RUN_STATE_RUNNING; ... vm_was_suspended = false; return ret; } void vm_start(void) { if (!vm_prepare_start(false)) { resume_all_vcpus(); } } - Steve