Re: [PATCH V5 01/12] cpus: refactor vm_stop

[Fabiano Rosas]

Steven Sistare <steven.sistare@oracle.com> writes:

> On 11/20/2023 8:22 AM, Fabiano Rosas wrote:
>> Steve Sistare <steven.sistare@oracle.com> writes:
>>> Refactor the vm_stop functions into one common subroutine do_vm_stop called
>>> with options.  No functional change.
>>>
>>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>>> ---
>>>  system/cpus.c | 44 +++++++++++++++++---------------------------
>>>  1 file changed, 17 insertions(+), 27 deletions(-)
>>>
>>> diff --git a/system/cpus.c b/system/cpus.c
>>> index 0848e0d..f72c4be 100644
>>> --- a/system/cpus.c
>>> +++ b/system/cpus.c
>>> @@ -252,10 +252,21 @@ void cpu_interrupt(CPUState *cpu, int mask)
>>>      }
>>>  }
>>>  
>>> -static int do_vm_stop(RunState state, bool send_stop)
>>> +static int do_vm_stop(RunState state, bool send_stop, bool force)
>>>  {
>>>      int ret = 0;
>>>  
>>> +    if (qemu_in_vcpu_thread()) {
>>> +        qemu_system_vmstop_request_prepare();
>>> +        qemu_system_vmstop_request(state);
>>> +        /*
>>> +         * FIXME: should not return to device code in case
>>> +         * vm_stop() has been requested.
>>> +         */
>>> +        cpu_stop_current();
>>> +        return 0;
>>> +    }
>> 
>> At vm_stop_force_state(), this block used to be under
>> runstate_is_running(), but now it runs unconditionally.
>
> vm_stop_force_state callers should never be called in a vcpu thread, so this 
> block
> is never executed for them.  I could assert that.
>
>> At vm_shutdown(), this block was not executed at all, but now it is.
>
> vm_shutdown should never be called from a vcpu thread.
> I could assert that.

Yes, this is an assumption that will get lost to time unless we document
it or have code to enforce.