Hi Peter,

Thank you for your comments.  A question
1. Here is one code snippet where you indicated the Formatting is broken. If I understood correctly,  I must not call virReportError() - but simply goto the cleanup  like the last condition where you seem to be fine ?

__start
if (def->ntpms > 1) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("VMware only supports a single TPM device"));
goto cleanup;
}
if (virtualHW_version < 14) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("vTPM requires virtual hardware version 14 or higher"));
goto cleanup;
}
if (def->os.firmware != VIR_DOMAIN_OS_DEF_FIRMWARE_EFI) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("vTPM requires EFI firmware"));
goto cleanup;
}
__end

On Wed, Apr 29, 2026 at 1:56 PM Peter Krempa <pkrempa@redhat.com> wrote:
On Thu, Apr 23, 2026 at 17:48:17 +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
>
> implement support for formatting TPM devices in virVMXFormatConfig
>
> Signed-off-by: Srihari Parimi <sparimi@redhat.com>
> ---
>  src/vmx/vmx.c              | 74 ++++++++++++++++++++++++++++++++++++++
>  tests/vmx2xmldata/vtpm.vmx | 22 ++++++++++++
>  tests/vmx2xmldata/vtpm.xml | 32 +++++++++++++++++
>  tests/vmx2xmltest.c        |  2 ++
>  4 files changed, 130 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..977fd3b346 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,
> @@ -609,6 +610,7 @@ static int virVMXFormatFileSystem(virDomainFSDef *def, int number,
>                                    virBuffer *buffer);
>  static int virVMXFormatEthernet(virDomainNetDef *def, int controller,
>                                  virBuffer *buffer, int virtualHW_version);
> +static int virVMXFormatTPM(virDomainTPMDef *def, virBuffer *buffer);
>  static int virVMXFormatSerial(virVMXContext *ctx, virDomainChrDef *def,
>                                virBuffer *buffer);
>  static int virVMXFormatParallel(virVMXContext *ctx, virDomainChrDef *def,
> @@ -1938,6 +1940,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 +3378,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;
> +}


>  /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
> @@ -3701,6 +3733,31 @@ virVMXFormatConfig(virVMXContext *ctx, virDomainXMLOption *xmlopt, virDomainDef
>              goto cleanup;
>      }

> +    /* def:vTPM */
> +    if (def->ntpms > 0) {
> +        /* Validate TPM requirements */
> +        if (def->ntpms > 1) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                _("VMware only supports a single TPM device"));

Formatting is broken here ....

> +            goto cleanup;
> +        }
> +
> +        if (virtualHW_version < 14) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                _("vTPM requires virtual hardware version 14 or higher"));

... here ...

> +            goto cleanup;
> +        }
> +
> +        if (def->os.firmware != VIR_DOMAIN_OS_DEF_FIRMWARE_EFI) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                _("vTPM requires EFI firmware"));

.... here.

Also

> +            goto cleanup;
> +        }
> +
> +        if (virVMXFormatTPM(def->tpms[0], &buffer) < 0)
> +            goto cleanup;
> +    }
> +
>      /* def:inputs */
>      /* FIXME */

> @@ -4231,6 +4288,23 @@ virVMXFormatEthernet(virDomainNetDef *def, int controller,
>  }


> +static int virVMXFormatTPM(virDomainTPMDef *def, virBuffer *buffer)

The coding style is off here too ...

> +{
> +    /* VMware vTPM specifically requires TPM 2.0 */
> +    if (def->model != VIR_DOMAIN_TPM_MODEL_CRB ||
> +        def->type != VIR_DOMAIN_TPM_TYPE_EMULATOR ||
> +        def->data.emulator.version != VIR_DOMAIN_TPM_VERSION_2_0) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                       _("VMware driver only supports TPM 2.0 with the CRB model"));
> +        return -1;
> +    }
> +
> +    /* VMX parameters for vTPM */
> +    virBufferAddLit(buffer, "vtpm.present = \"TRUE\"\n");
> +
> +    return 0;
> +}
> +

>  static int
>  virVMXFormatSerial(virVMXContext *ctx, virDomainChrDef *def,

... ^^^.

I also don't quite understand why the validation is split partly between
virVMXFormatConfig and virVMXFormatTPM. E.g. if you have most of checks
in virVMXFormatConfig adding the last check and the formatting wouldn't
make it much worse. Same way you could put all the checks into
virVMXFormatTPM instead.

With the coding style fixed and all validation moved into
virVMXFormatTPM:

Reviewed-by: Peter Krempa <pkrempa@redhat.com>