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

[Markus Armbruster]

Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> +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").
>
> Long term we'd like to decouple the tests/ build from the various
> QEMU configurations, and build the tests once.

This argument applies only to macros from target-specific headers like
$TARGET-config-target.h, not to macros from config-host.h.  #ifdef
CONFIG_TPM should be fine, shouldn't it?