TPM devices with model='tpm-tis' are only valid with x86 and aarch64
virt machines. Add a check to qemuValidateDomainDeviceDefTPM() to
ensure VIR_DOMAIN_TPM_MODEL_TIS is only used with these architectures.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---

The conditional is a bit distasteful, but so far I haven't come up with
anything better. I aslo worry about future architectures gaining support
for emulated TPM TIS devices.

 src/qemu/qemu_validate.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index a70737327e..d6ff5e5eef 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -4299,6 +4299,13 @@ qemuValidateDomainDeviceDefTPM(virDomainTPMDef *tpm,
 
     switch (tpm->model) {
     case VIR_DOMAIN_TPM_MODEL_TIS:
+        if (!ARCH_IS_X86(def->os.arch) && (def->os.arch != VIR_ARCH_AARCH64)) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("TPM model %s is only available for "
+                             "x86 and aarch64 guests"),
+                          virDomainTPMModelTypeToString(tpm->model));
+            return -1;
+        }
         flag = QEMU_CAPS_DEVICE_TPM_TIS;
         break;
     case VIR_DOMAIN_TPM_MODEL_CRB:
-- 
2.29.2

On Wed, 2021-02-10 at 10:34 -0700, Jim Fehlig wrote:
> I aslo worry about future architectures gaining support
> for emulated TPM TIS devices.

It's okay to be conservative - we can always relax the check later.

> +        if (!ARCH_IS_X86(def->os.arch) && (def->os.arch != VIR_ARCH_AARCH64)) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("TPM model %s is only available for "
> +                             "x86 and aarch64 guests"),

Please don't split the error message into two separate lines, and
sprinkle some quotes around '%s' while you're at it.

https://libvirt.org/coding-style.html#error-message-format

-- 
Andrea Bolognani / Red Hat / Virtualization

On 2/11/21 3:37 AM, Andrea Bolognani wrote:
> On Wed, 2021-02-10 at 10:34 -0700, Jim Fehlig wrote:
>> I aslo worry about future architectures gaining support
>> for emulated TPM TIS devices.
> 
> It's okay to be conservative - we can always relax the check later.
> 
>> +        if (!ARCH_IS_X86(def->os.arch) && (def->os.arch != VIR_ARCH_AARCH64)) {
>> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> +                           _("TPM model %s is only available for "
>> +                             "x86 and aarch64 guests"),
> 
> Please don't split the error message into two separate lines

I'm surprised that slipped through my copy and paste since I've always preferred 
errors on one line for easy searching. We have a lot of split error messages 
throughout the code. This file is particularly bad. Is there any desire to fix 
existing cases, or just avoid new ones?

> and sprinkle some quotes around '%s' while you're at it.

Will do.

> https://libvirt.org/coding-style.html#error-message-format

The entire page is worth a re-read on occasion :-)

Regards,
Jim

On Thu, 2021-02-11 at 08:44 -0700, Jim Fehlig wrote:
> On 2/11/21 3:37 AM, Andrea Bolognani wrote:
> > On Wed, 2021-02-10 at 10:34 -0700, Jim Fehlig wrote:
> > > +        if (!ARCH_IS_X86(def->os.arch) && (def->os.arch != VIR_ARCH_AARCH64)) {
> > > +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> > > +                           _("TPM model %s is only available for "
> > > +                             "x86 and aarch64 guests"),
> > 
> > Please don't split the error message into two separate lines
> 
> I'm surprised that slipped through my copy and paste since I've always preferred 
> errors on one line for easy searching. We have a lot of split error messages 
> throughout the code. This file is particularly bad. Is there any desire to fix 
> existing cases, or just avoid new ones?

I don't think we're particularly interested in going back and
altering existing messages, and most importantly in the churn doing
so would generate :)

-- 
Andrea Bolognani / Red Hat / Virtualization