On Thu, Nov 20, 2025 at 02:17:47PM +0100, Peter Krempa wrote:
> On Thu, Nov 20, 2025 at 11:57:53 +0000, Daniel P. Berrangé via Devel wrote:
> > From: Daniel P. Berrangé <[email protected]>
> >
> > Querying existence of the 'tdx-guest' type merely tells us whether
> > QEMU has been compiled with TDX support, not whether it is usable
> > on the host. Thus QEMU was incorrectly reporting
> >
> > <tdx supported='yes'/>
> > ...
> > <launchSecurity supported='yes'>
> > <enum name='sectype'>
> > <value>tdx</value>
> > </enum>
> > </launchSecurity>
> >
> > on every platform with new enough QEMU.
> >
> > Unfortunately an earlier patch for a 'query-tdx-capabilities' QMP
> > command in QEMU was dropped, so there is no way to ask QEMU whether
> > it can launch a TDX guest. Libvirt must directly query the KVM
> > device and ask for supported VM types.
> >
> > Signed-off-by: Daniel P. Berrangé <[email protected]>
> > ---
> > src/qemu/qemu_capabilities.c | 60 ++++++++++++++++++++++++++++++++++++
> > src/qemu/qemu_capabilities.h | 3 ++
> > tests/domaincapsmock.c | 6 ++++
> > 3 files changed, 69 insertions(+)
> >
> > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> > index 205bf3d0b8..67fe5d7acf 100644
> > --- a/src/qemu/qemu_capabilities.c
> > +++ b/src/qemu/qemu_capabilities.c
> > @@ -54,11 +54,16 @@
> > # include <sys/types.h>
> > # include <sys/sysctl.h>
> > #endif
> > +#ifdef __linux__
> > +# include <linux/kvm.h>
> > +#endif
> >
> > #define VIR_FROM_THIS VIR_FROM_QEMU
> >
> > VIR_LOG_INIT("qemu.qemu_capabilities");
> >
> > +#define KVM_DEVICE "/dev/kvm"
> > +
> > /* While not public, these strings must not change. They
> > * are used in domain status files which are read on
> > * daemon restarts
> > @@ -3655,6 +3660,59 @@ virQEMUCapsProbeQMPSEVCapabilities(virQEMUCaps
> > *qemuCaps,
> > }
> >
> >
> > +int
> > +virQEMUCapsKVMSupportsVMTypeTDX(void)
> > +{
> > +#if defined(KVM_CAP_VM_TYPES) && defined(KVM_X86_TDX_VM)
> > + int kvmfd = -1;
>
> Preferrably use VIR_AUTOCLOSE here if anyone ever extends this function.
ok
> > + int types;
> > +
> > + if (!virFileExists(KVM_DEVICE))
> > + return 0;
> > +
> > + if ((kvmfd = open(KVM_DEVICE, O_RDONLY)) < 0) {
> > + VIR_DEBUG("Unable to open %s, cannot check TDX", KVM_DEVICE);
> > + return 0;
> > + }
> > +
> > + types = ioctl(kvmfd, KVM_CHECK_EXTENSION, KVM_CAP_VM_TYPES);
> > +
> > + VIR_FORCE_CLOSE(kvmfd);
> > + VIR_DEBUG("KVM VM types: 0x%x", types);
> > +
> > + return !!(types & (1 << KVM_X86_TDX_VM));
>
> I didn't bother looking at what the IOCTL returns but I'd expect a < 0
> check here as bit-checking a negative value could be 'fun'.
KVM_CHECK_EXTENSION will return 0 if any capability
does not exist, so this should be safe in old KVM, but
for safety, lets squash -1 to be 0 explicitly.
> So looking at all branches this only returns 0 or 1 ...
Oh yes, I removed the -1 branch from failure to open
/dev/kvm as I decided that shouldn't be allowed to
break caps probing.
>
> > +#else
> > + VIR_DEBUG("KVM not compiled");
> > + return 0;
> > +#endif
> > +}
> > +
> > +
> > +/* This ought to be virQEMUCapsProbeQMPTDXCapabilities,
> > + * but there is no 'query-tdx-capabilities' command
> > + * available in QEMU currently. If one arrives, rename
> > + * this method & switch to using that on new enough QEMU
> > + */
> > +static int
> > +virQEMUCapsProbeTDXCapabilities(virQEMUCaps *qemuCaps)
> > +{
> > + int rc;
> > +
> > + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_TDX_GUEST))
> > + return 0;
> > +
> > + if ((rc = virQEMUCapsKVMSupportsVMTypeTDX()) < 0)
> > + return -1;
>
> So this branch is dead code. All other branches return 0 so this
> function can be turned into a void function.
Yep.
>
> > +
> > + if (rc == 0) {
> > + virQEMUCapsClear(qemuCaps, QEMU_CAPS_TDX_GUEST);
> > + return 0;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +
> > static int
> > virQEMUCapsProbeQMPSGXCapabilities(virQEMUCaps *qemuCaps,
> > qemuMonitor *mon)
> > @@ -5837,6 +5895,8 @@ virQEMUCapsInitQMPMonitor(virQEMUCaps *qemuCaps,
> > return -1;
> > if (virQEMUCapsProbeQMPSGXCapabilities(qemuCaps, mon) < 0)
> > return -1;
> > + if (virQEMUCapsProbeTDXCapabilities(qemuCaps) < 0)
> > + return -1;
> >
> > virQEMUCapsInitProcessCaps(qemuCaps);
> >
> > diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> > index efbef2acef..64e5c4ff55 100644
> > --- a/src/qemu/qemu_capabilities.h
> > +++ b/src/qemu/qemu_capabilities.h
> > @@ -979,3 +979,6 @@ int
> > virQEMUCapsProbeQMPMachineTypes(virQEMUCaps *qemuCaps,
> > virDomainVirtType virtType,
> > qemuMonitor *mon);
> > +
> > +int
> > +virQEMUCapsKVMSupportsVMTypeTDX(void) ATTRIBUTE_MOCKABLE;
> > diff --git a/tests/domaincapsmock.c b/tests/domaincapsmock.c
> > index cb6e98dbb8..e882c01260 100644
> > --- a/tests/domaincapsmock.c
> > +++ b/tests/domaincapsmock.c
> > @@ -48,6 +48,12 @@ virHostCPUGetPhysAddrSize(const virArch hostArch,
> > }
> >
> > #if WITH_QEMU
> > +int
> > +virQEMUCapsKVMSupportsVMTypeTDX(void)
> > +{
> > + return 1;
> > +}
> > +
> > static bool (*real_virQEMUCapsGetKVMSupportsSecureGuest)(virQEMUCaps
> > *qemuCaps);
> >
> > bool
> > --
> > 2.51.1
> >
>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
