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

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(a)linux.ibm.com&gt; > > > > > > > > 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(a)linux.ibm.com&gt; > > > > --- > > > > 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.