Re: [PATCH v3 0/9] tests: Add test cases for TPM 1.2 ACPI tables

[Stefan Berger]

On 7/12/21 12:26 PM, Philippe Mathieu-Daudé wrote:
> +Markus
>
> On 7/12/21 5:47 PM, Stefan Berger wrote:
> > On 7/12/21 11:29 AM, Philippe Mathieu-Daudé wrote:
> > > Hi Stefan,
> > >
> > > On 7/12/21 5:09 PM, Stefan Berger wrote:
> > > > This series of patches adds test case for TPM 1.2 ACPI tables.
> > > >
> > > >     Stefan
> > > >
> > > > v3:
> > > >     - Define enum TPMVersion for when CONFIG_TPM is not defined
> > > >       affected patches 2 and 6
> > > I think in 11fb99e6f48..e542b71805d we missed an extra patch
> > > for qtests. Probably (untested):
> > Shouldn't we have seen test compilation errors already?
> >
> > I didn't go down this route for the build system (as you show below)
> > because in this series we are testing ACPI tables and I introduce the
> > reference to enum TPMVersion here, which wasn't needed before. The
> > alternative may be to go into 8/9 and eliminate all TPM code if
> > CONFIG_TPM is not set. The introduction of the enum now passes the tests
> > with --enable-tpm and --disable-tpm.
> >
> > Otherwise the BIOS test are skipped due to this here:
> >
> >
> > static void test_acpi_tcg_tpm(const char *machine, const char *tpm_if,
> >                                uint64_t base, enum TPMVersion tpm_version)
> > {
> > #ifdef CONFIG_TPM
> > [...]
> >
> > #else
> >      g_test_skip("TPM disabled");
> > #endif
> > }
> >
> > So I didn't want to clutter this code with more #ifdef CONFIG_TPM but
> > maybe that would be the right solution.
> IMO the "right" solution is to check via QMP if TMP is supported
> or not. This is now doable since commit caff255a546 ("tpm: Return
> QMP error when TPM is disabled in build").

That above g_test_skip() could be moved to the top of the function and 
be preceded by a QMP check for whether TPM is supported.