diff mbox series

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

Message ID d1afced8a92c01367d0aed7c6f82659c9bf79956.1625704981.git.isaku.yamahata@intel.com (mailing list archive)
State New, archived
Headers show
Series TDX support | expand

Commit Message

Isaku Yamahata July 8, 2021, 12:55 a.m. UTC
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(+)

Comments

Connor Kuehl July 22, 2021, 5:54 p.m. UTC | #1
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.
Gerd Hoffmann Aug. 26, 2021, 12:01 p.m. UTC | #2
Hi,

> 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.

Again, better throw an error instead of silently fixing up settings.

take care,
  Gerd
Xiaoyao Li Dec. 10, 2021, 9:54 a.m. UTC | #3
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.
diff mbox series

Patch

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);