Re: [RFC PATCH v2 32/44] tdx: add kvm_tdx_enabled() accessor for later use

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

On 7/23/2021 1:53 AM, Connor Kuehl wrote:
> On 7/7/21 7:55 PM, isaku.yamahata@gmail.com wrote:
>> From: Isaku Yamahata <isaku.yamahata@intel.com>
>>
>> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
>> ---
>>   include/sysemu/tdx.h  | 1 +
>>   target/i386/kvm/kvm.c | 5 +++++
>>   2 files changed, 6 insertions(+)
>>
>> diff --git a/include/sysemu/tdx.h b/include/sysemu/tdx.h
>> index 70eb01348f..f3eced10f9 100644
>> --- a/include/sysemu/tdx.h
>> +++ b/include/sysemu/tdx.h
>> @@ -6,6 +6,7 @@
>>   #include "hw/i386/pc.h"
>>   bool kvm_has_tdx(KVMState *s);
>> +bool kvm_tdx_enabled(void);
>>   int tdx_system_firmware_init(PCMachineState *pcms, MemoryRegion 
>> *rom_memory);
>>   #endif
>> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
>> index af6b5f350e..76c3ea9fac 100644
>> --- a/target/i386/kvm/kvm.c
>> +++ b/target/i386/kvm/kvm.c
>> @@ -152,6 +152,11 @@ int kvm_set_vm_type(MachineState *ms, int kvm_type)
>>       return -ENOTSUP;
>>   }
>> +bool kvm_tdx_enabled(void)
>> +{
>> +    return vm_type == KVM_X86_TDX_VM;
>> +}
>> +
> 
> Is this the whole story? Does this guarantee that the VM QEMU is
> responsible to bring up is a successfully initialized TD?

No, it just means a TDX guest is requested.

>  From my reading of the series as it unfolded, this looks like the
> function proves that KVM can support TDs and that the user requested
> a TDX kvm-type, not that we have a fully-formed TD.

yes, you are right. We referenced what sev_eanbled() and sev_es_enabled().

If the name is misleading, does it looks better to name it is_tdx_vm()?

> Is it possible to associate this with a more verifiable metric that
> the TD has been or will be created successfully? I.e., once the VM
> has successfully called the TDX INIT ioctl or has finalized setup?
> 
> My question mainly comes from a later patch in the series, where the
> "query-tdx-capabilities" and "query-tdx" QMP commands are added.
> 
> Forgive me if I am misinterpreting the semantics of each of these
> commands:

what you understood is correct.

> "query-tdx-capabilities" sounds like it answers the question of
> "can it run a TD?"
> 
> and "query-tdx" sounds like it answers the question of "is it a TD?"
> 
> Is the assumption with "query-tdx" that anything that's gone wrong
> with developing a TD will have resulted in the QEMU process exiting
> and therefore if we get to a point where we can run "query-tdx" then
> we know the TD was successfully formed?
>