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

[Xiaoyao Li]

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?