On Thu, Nov 20, 2025 at 13:37:34 +0000, Daniel P. Berrangé wrote: > 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.
With the suggested changes (especially for the -1) case: Reviewed-by: Peter Krempa <[email protected]>
