[PATCH] conf: Set SPAPR TPM default to 2.0 and prevent 1.2 choice

[Stefan Berger]

On 7/8/20 10:22 AM, Peter Krempa wrote:
> On Wed, Jul 08, 2020 at 10:19:11 -0400, Stefan Berger wrote:
>> On 7/8/20 10:10 AM, Peter Krempa wrote:
>>> On Wed, Jul 08, 2020 at 09:57:56 -0400, Stefan Berger wrote:
>>>> From: Stefan Berger <stefanb at linux.ibm.com>
>>>>
>>>> The firmware (SLOF) on QEMU for ppc64 does not support TPM 1.2, so
>>>> prevent the choice of TPM 1.2 when the SPAPR device model is chosen
>>>> and use a default of '2.0' (TPM 2) for the backend.
>>>>
>>>> Signed-off-by: Stefan Berger <stefanb at linux.ibm.com>
>>>> ---
>>>>    src/qemu/qemu_validate.c | 14 ++++++++++++--
>>>>    1 file changed, 12 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
>>>> index bd7590a00a..f4d71aebf5 100644
>>>> --- a/src/qemu/qemu_validate.c
>>>> +++ b/src/qemu/qemu_validate.c
>>>> @@ -3645,8 +3645,12 @@ qemuValidateDomainDeviceDefTPM(virDomainTPMDef *tpm,
>>>>        virQEMUCapsFlags flag;
>>>>        /* TPM 1.2 and 2 are not compatible, so we choose a specific version here */
>>>> -    if (tpm->version == VIR_DOMAIN_TPM_VERSION_DEFAULT)
>>>> -        tpm->version = VIR_DOMAIN_TPM_VERSION_1_2;
>>>> +    if (tpm->version == VIR_DOMAIN_TPM_VERSION_DEFAULT) {
>>>> +        if (tpm->model == VIR_DOMAIN_TPM_MODEL_SPAPR)
>>>> +            tpm->version = VIR_DOMAIN_TPM_VERSION_2_0;
>>>> +        else
>>>> +            tpm->version = VIR_DOMAIN_TPM_VERSION_1_2;
>>>> +    }
>>> This is the validation callback, which must not modify the config.
>>>
>>> qemuDomainDefTPMsPostParse is the correct place. Beaware that the
>>> function has a short-circuit condition at the beginning.
>>
>> So should I remove the existing code that already set the version to
>> VIR_DOMAIN_TPM_VERSION_1_2 and move this into that other function?
> Ideally yes as a refactor prior to your change. The XML parser shouldn't
> really be setting defaults or validating any form of interdependance of
> values.
>
Well, the issue is that the validation function works on a single device 
(TPM) config but for validating a correct configuration for the SPAPR 
proxy device we would need to see both configs. So this is presumably 
the reason why this was done in the post parse function that has 
visibility into all TPM configs.