On Tue, Apr 21, 2026 at 8:42 PM Peter Krempa <[email protected]> wrote:
> On Tue, Apr 21, 2026 at 20:30:35 +0530, Srihari Parimi wrote: > > On Tue, Apr 21, 2026 at 3:55 PM Peter Krempa <[email protected]> wrote: > > > > > On Tue, Apr 21, 2026 at 11:08:47 +0530, Srihari Parimi via Devel wrote: > > > > Parses vtpm.present from VMX files and converts to libvirt TPM > > > > device with CRB model and emulator backend. VMware vTPM uses > > > > TPM 2.0 as specified in the document below > > > > > > > > > > > > https://techdocs.broadcom.com/us/en/vmware-cis/vsphere/vsphere/8-0/vsphere-security/securing-virtual-machines-with-virtual-trusted-platform-module/vtpm-overview.html > > > > > > > > Default to the CRB interface for TPM 2.0 systems to improve > > > > performance and follow industry standards over legacy TIS. > > > > > > So what this patch does is to parse presence of the vTPM and represent > > > it in the XML. It doesn't set any default or anything, just represents > > > what the VM has configured. > > > > > > > > True that, the patch parses the presence of vTPM, but there is still a > > choice to use one of the following: > > 1. tpm-tis (compatible with both TPM 1.0 and TPM 2.0) - includes legacy > > systems support > > 2. tpm-crb (compatible with TPM 2.0) > > There's no choice actually. What the function does is to transform the > *existing* definition of the hardware used on ESX into the libvirt XML. > Thus the represented type *must* be the one which is actually used by > ESX and thus visible to the guest OS. > > The unfortunate part is that the ESX definition is just one boolean > saying that there is a TPM. Thus we need to infer from what ESX does > what the actual type is and represent that one. > > That is the reason I've asked for docs showing which kind of TPM is > actually used. > Yes - we are left to infer. In the next patch, I will remove the last statement in the commit description to avoid any confusion. > > > > > Beyond references about the advantages of using the CRB interface with > TPM > > 2.0 and what is "accepted in Industry," I did not find any standard > > document which could help > > > > > > > So what is the sentence above trying to state? > > > > > > > > > > > > > > Signed-off-by: Srihari Parimi <[email protected]> > > > > --- > > > > src/vmx/vmx.c | 31 +++++++++++++++++++++++++++++++ > > > > tests/vmx2xmldata/vtpm.vmx | 22 ++++++++++++++++++++++ > > > > tests/vmx2xmldata/vtpm.xml | 32 ++++++++++++++++++++++++++++++++ > > > > tests/vmx2xmltest.c | 2 ++ > > > > 4 files changed, 87 insertions(+) > > > > create mode 100644 tests/vmx2xmldata/vtpm.vmx > > > > create mode 100644 tests/vmx2xmldata/vtpm.xml > > > > > > > > diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c > > > > index 57dfd57cfc..bec985aef3 100644 > > > > --- a/src/vmx/vmx.c > > > > +++ b/src/vmx/vmx.c > > > > @@ -599,6 +599,7 @@ static int virVMXParseSerial(virVMXContext *ctx, > > > virConf *conf, int port, > > > > static int virVMXParseParallel(virVMXContext *ctx, virConf *conf, > int > > > port, > > > > virDomainChrDef **def); > > > > static int virVMXParseSVGA(virConf *conf, virDomainVideoDef **def); > > > > +static int virVMXParseTPM(virConf *conf, virDomainTPMDef **def); > > > > > > > > static int virVMXFormatVNC(virDomainGraphicsDef *def, virBuffer > > > *buffer); > > > > static int virVMXFormatDisk(virVMXContext *ctx, virDomainDiskDef > *def, > > > > @@ -1938,6 +1939,15 @@ virVMXParseConfig(virVMXContext *ctx, > > > > > > > > def->nvideos = 1; > > > > > > > > + /* def:tpms */ > > > > + { > > > > + virDomainTPMDef *tpm = NULL; > > > > + if (virVMXParseTPM(conf, &tpm) < 0) > > > > + goto cleanup; > > > > + if (tpm) > > > > + VIR_APPEND_ELEMENT(def->tpms, def->ntpms, tpm); > > > > + } > > > > + > > > > /* def:sounds */ > > > > /* FIXME */ > > > > > > > > @@ -3367,6 +3377,27 @@ virVMXParseSVGA(virConf *conf, > virDomainVideoDef > > > **def) > > > > return result; > > > > } > > > > > > > > +static int > > > > +virVMXParseTPM(virConf *conf, virDomainTPMDef **def) > > > > +{ > > > > + bool vtpm_present = false; > > > > + > > > > + /* vmx:vtpm.present */ > > > > + if (virVMXGetConfigBoolean(conf, "vtpm.present", &vtpm_present, > > > > + false, true) < 0) { > > > > + return -1; > > > > + } > > > > + > > > > + if (!vtpm_present) > > > > + return 0; > > > > + > > > > + *def = g_new0(virDomainTPMDef, 1); > > > > + (*def)->type = VIR_DOMAIN_TPM_TYPE_EMULATOR; > > > > + (*def)->model = VIR_DOMAIN_TPM_MODEL_CRB; > > > > + (*def)->data.emulator.version = VIR_DOMAIN_TPM_VERSION_2_0; > > > > + > > > > + return 0; > > > > +} > > > > > > > > > > > > > > > > > In other recent patch adding 'nvram' config to vmx I've noticed that > > > also virVMXFormatConfig is changed. > > > > > > I wonder if the VMX driver needs something similar for the vTPM. > > > Although that will require some validation and I don't see that the VMX > > > driver implements the validation callbacks which would make adding the > > > validation much simpler. > > > > > > > > I will need to understand this and decide. > > Note that once you do that the validation you'll have to add will have > to ensure that only the one specific type of tpm accepted since it is > then converted to a boolean flag. > It looks like virVMXFormatConfig must handle vTPM for completeness. While mapping Domain XML to VMX doesn't require a connection with the ESX hypervisor, testing the functionality after mapping (use of keys, encryption of VM and NVRAM, etc.) requires that connection. Given this, I want to push this change and raise another PR which implements vTPM handling in virVMXFormatConfig. Would that be okay?
