Re: [RFC PATCH v2 34/44] target/i386/tdx: set reboot action to shutdown when tdx

[Xiaoyao Li <xiaoyao.li@intel.com>]

On 7/23/2021 1:54 AM, Connor Kuehl wrote:
> On 7/7/21 7:55 PM, isaku.yamahata@gmail.com wrote:
>> From: Isaku Yamahata <isaku.yamahata@intel.com>
>>
>> In TDX CPU state is also protected, thus vcpu state can't be reset by 
>> VMM.
>> It assumes -action reboot=shutdown instead of silently ignoring vcpu 
>> reset.
>>
>> TDX module spec version 344425-002US doesn't support vcpu reset by 
>> VMM.  VM
>> needs to be destroyed and created again to emulate REBOOT_ACTION_RESET.
>> For simplicity, put its responsibility to management system like libvirt
>> because it's difficult for the current qemu implementation to destroy and
>> re-create KVM VM resources with keeping other resources.
>>
>> If management system wants reboot behavior for its users, it needs to
>>   - set reboot_action to REBOOT_ACTION_SHUTDOWN,
>>   - set shutdown_action to SHUTDOWN_ACTION_PAUSE optionally and,
>>   - subscribe VM state change and on reboot, (destroy qemu if
>>     SHUTDOWN_ACTION_PAUSE and) start new qemu.
>>
>> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
>> ---
>>   target/i386/kvm/tdx.c | 14 ++++++++++++++
>>   1 file changed, 14 insertions(+)
>>
>> diff --git a/target/i386/kvm/tdx.c b/target/i386/kvm/tdx.c
>> index 1316d95209..0621317b0a 100644
>> --- a/target/i386/kvm/tdx.c
>> +++ b/target/i386/kvm/tdx.c
>> @@ -25,6 +25,7 @@
>>   #include "qapi/qapi-types-misc-target.h"
>>   #include "standard-headers/asm-x86/kvm_para.h"
>>   #include "sysemu/sysemu.h"
>> +#include "sysemu/runstate-action.h"
>>   #include "sysemu/kvm.h"
>>   #include "sysemu/kvm_int.h"
>>   #include "sysemu/tdx.h"
>> @@ -363,6 +364,19 @@ static void tdx_guest_init(Object *obj)
>>       qemu_mutex_init(&tdx->lock);
>> +    /*
>> +     * TDX module spec version 344425-002US doesn't support reset of 
>> vcpu by
>> +     * VMM.  VM needs to be destroyed and created again to emulate
>> +     * REBOOT_ACTION_RESET.  For simplicity, put its responsibility to
>> +     * management system like libvirt.
>> +     *
>> +     * Management system should
>> +     *  - set reboot_action to REBOOT_ACTION_SHUTDOWN
>> +     *  - set shutdown_action to SHUTDOWN_ACTION_PAUSE
>> +     *  - subscribe VM state and on reboot, destroy qemu and start 
>> new qemu
>> +     */
>> +    reboot_action = REBOOT_ACTION_SHUTDOWN;
>> +
>>       tdx->debug = false;
>>       object_property_add_bool(obj, "debug", tdx_guest_get_debug,
>>                                tdx_guest_set_debug);
>>
> 
> I think the same effect could be accomplished with modifying
> kvm_arch_cpu_check_are_resettable.
> 

Yes. Thanks for pointing it out. We will take this approach.