We've made similar changes for aarch64 a few years back (see
d8a1c059e0ed and previous commits), and the rationale is the
same: the architecture is new enough that TPM 2.0 predates it,
so TPM 1.2 support was never considered and will just not work.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
src/qemu/qemu_domain.c | 1 +
src/qemu/qemu_validate.c | 10 ++++++----
.../aarch64-tpm-wrong-model.aarch64-latest.err | 2 +-
...4-virt-default-models.riscv64-latest.abi-update.xml | 2 +-
.../riscv64-virt-default-models.riscv64-latest.xml | 2 +-
5 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index bda62f2e5c..6bb18ad5a8 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -6182,6 +6182,7 @@ qemuDomainTPMDefPostParse(virDomainTPMDef *tpm,
tpm->data.emulator.version == VIR_DOMAIN_TPM_VERSION_DEFAULT) {
if (tpm->model == VIR_DOMAIN_TPM_MODEL_SPAPR ||
tpm->model == VIR_DOMAIN_TPM_MODEL_CRB ||
+ qemuDomainIsRISCVVirt(def) ||
qemuDomainIsARMVirt(def))
tpm->data.emulator.version = VIR_DOMAIN_TPM_VERSION_2_0;
else
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index ac1940cb31..7b871be05f 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -4765,10 +4765,12 @@ qemuValidateDomainDeviceDefTPM(virDomainTPMDef *tpm,
_("TPM 1.2 is not supported with the SPAPR device model"));
return -1;
}
- /* TPM 1.2 + ARM does not work */
- if (qemuDomainIsARMVirt(def)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("TPM 1.2 is not supported on ARM"));
+ /* TPM 1.2 does not work on certain modern architectures */
+ if (qemuDomainIsARMVirt(def) ||
+ qemuDomainIsRISCVVirt(def)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("TPM 1.2 is not supported on architecture '%1$s'"),
+ virArchToString(def->os.arch));
return -1;
}
break;
diff --git a/tests/qemuxmlconfdata/aarch64-tpm-wrong-model.aarch64-latest.err b/tests/qemuxmlconfdata/aarch64-tpm-wrong-model.aarch64-latest.err
index a3a82fdcf5..44c6e7372b 100644
--- a/tests/qemuxmlconfdata/aarch64-tpm-wrong-model.aarch64-latest.err
+++ b/tests/qemuxmlconfdata/aarch64-tpm-wrong-model.aarch64-latest.err
@@ -1 +1 @@
-unsupported configuration: TPM 1.2 is not supported on ARM
+unsupported configuration: TPM 1.2 is not supported on architecture 'aarch64'
diff --git a/tests/qemuxmlconfdata/riscv64-virt-default-models.riscv64-latest.abi-update.xml b/tests/qemuxmlconfdata/riscv64-virt-default-models.riscv64-latest.abi-update.xml
index a3a701b8e4..6712c2d831 100644
--- a/tests/qemuxmlconfdata/riscv64-virt-default-models.riscv64-latest.abi-update.xml
+++ b/tests/qemuxmlconfdata/riscv64-virt-default-models.riscv64-latest.abi-update.xml
@@ -59,7 +59,7 @@
<target type='serial' port='0'/>
</console>
<tpm model='tpm-tis'>
- <backend type='emulator' version='1.2'/>
+ <backend type='emulator' version='2.0'/>
</tpm>
<audio id='1' type='none'/>
<video>
diff --git a/tests/qemuxmlconfdata/riscv64-virt-default-models.riscv64-latest.xml b/tests/qemuxmlconfdata/riscv64-virt-default-models.riscv64-latest.xml
index a3a701b8e4..6712c2d831 100644
--- a/tests/qemuxmlconfdata/riscv64-virt-default-models.riscv64-latest.xml
+++ b/tests/qemuxmlconfdata/riscv64-virt-default-models.riscv64-latest.xml
@@ -59,7 +59,7 @@
<target type='serial' port='0'/>
</console>
<tpm model='tpm-tis'>
- <backend type='emulator' version='1.2'/>
+ <backend type='emulator' version='2.0'/>
</tpm>
<audio id='1' type='none'/>
<video>
--
2.45.1On Mon, May 27, 2024 at 07:31:36PM +0200, Andrea Bolognani wrote:
> We've made similar changes for aarch64 a few years back (see
> d8a1c059e0ed and previous commits), and the rationale is the
> same: the architecture is new enough that TPM 2.0 predates it,
> so TPM 1.2 support was never considered and will just not work.
>
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
> src/qemu/qemu_domain.c | 1 +
> src/qemu/qemu_validate.c | 10 ++++++----
> .../aarch64-tpm-wrong-model.aarch64-latest.err | 2 +-
> ...4-virt-default-models.riscv64-latest.abi-update.xml | 2 +-
> .../riscv64-virt-default-models.riscv64-latest.xml | 2 +-
> 5 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index bda62f2e5c..6bb18ad5a8 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -6182,6 +6182,7 @@ qemuDomainTPMDefPostParse(virDomainTPMDef *tpm,
> tpm->data.emulator.version == VIR_DOMAIN_TPM_VERSION_DEFAULT) {
> if (tpm->model == VIR_DOMAIN_TPM_MODEL_SPAPR ||
> tpm->model == VIR_DOMAIN_TPM_MODEL_CRB ||
> + qemuDomainIsRISCVVirt(def) ||
> qemuDomainIsARMVirt(def))
> tpm->data.emulator.version = VIR_DOMAIN_TPM_VERSION_2_0;
> else
> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> index ac1940cb31..7b871be05f 100644
> --- a/src/qemu/qemu_validate.c
> +++ b/src/qemu/qemu_validate.c
> @@ -4765,10 +4765,12 @@ qemuValidateDomainDeviceDefTPM(virDomainTPMDef *tpm,
> _("TPM 1.2 is not supported with the SPAPR device model"));
> return -1;
> }
> - /* TPM 1.2 + ARM does not work */
> - if (qemuDomainIsARMVirt(def)) {
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> - _("TPM 1.2 is not supported on ARM"));
> + /* TPM 1.2 does not work on certain modern architectures */
> + if (qemuDomainIsARMVirt(def) ||
> + qemuDomainIsRISCVVirt(def)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("TPM 1.2 is not supported on architecture '%1$s'"),
> + virArchToString(def->os.arch));
> return -1;
> }
Hmm, what architectures /do/ allow 1.2 ? x86, s390x, ppc ? Should
we consider just doing an "allow list" for arches, given that going
forward nothing new should be allowed.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Mon, Jun 03, 2024 at 10:50:40AM GMT, Daniel P. Berrangé wrote:
> On Mon, May 27, 2024 at 07:31:36PM +0200, Andrea Bolognani wrote:
> > + /* TPM 1.2 does not work on certain modern architectures */
> > + if (qemuDomainIsARMVirt(def) ||
> > + qemuDomainIsRISCVVirt(def)) {
> > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> > + _("TPM 1.2 is not supported on architecture '%1$s'"),
> > + virArchToString(def->os.arch));
> > return -1;
> > }
>
> Hmm, what architectures /do/ allow 1.2 ? x86, s390x, ppc ? Should
> we consider just doing an "allow list" for arches, given that going
> forward nothing new should be allowed.
ppc64 defaults to 2.0 already and s390x doesn't do TPM. Flipping
things around so that 1.2 becomes the special case and is only
allowed for x86 would make sense.
The only remaining question mark is loongarch64. I assume that, just
like riscv64 and aarch64 before it, it wouldn't bother with 1.2 at
all, but I'm not 100% sure. On the other hand, TPM support is
currently compiled out by default in the QEMU system binary for that
architecture, so we could go ahead with the change under that
assumption and revisit things later if necessary. Does that sound
good?
--
Andrea Bolognani / Red Hat / Virtualization
On Mon, Jun 03, 2024 at 08:32:39AM -0400, Andrea Bolognani wrote:
> On Mon, Jun 03, 2024 at 10:50:40AM GMT, Daniel P. Berrangé wrote:
> > On Mon, May 27, 2024 at 07:31:36PM +0200, Andrea Bolognani wrote:
> > > + /* TPM 1.2 does not work on certain modern architectures */
> > > + if (qemuDomainIsARMVirt(def) ||
> > > + qemuDomainIsRISCVVirt(def)) {
> > > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> > > + _("TPM 1.2 is not supported on architecture '%1$s'"),
> > > + virArchToString(def->os.arch));
> > > return -1;
> > > }
> >
> > Hmm, what architectures /do/ allow 1.2 ? x86, s390x, ppc ? Should
> > we consider just doing an "allow list" for arches, given that going
> > forward nothing new should be allowed.
>
> ppc64 defaults to 2.0 already and s390x doesn't do TPM. Flipping
> things around so that 1.2 becomes the special case and is only
> allowed for x86 would make sense.
>
> The only remaining question mark is loongarch64. I assume that, just
> like riscv64 and aarch64 before it, it wouldn't bother with 1.2 at
> all, but I'm not 100% sure. On the other hand, TPM support is
> currently compiled out by default in the QEMU system binary for that
> architecture, so we could go ahead with the change under that
> assumption and revisit things later if necessary. Does that sound
> good?
Yes, lets limit to x86. Better to unlock more valid cases later, than
to be too loose and let things through by mistake, as we'll easily
forget this when adding new arches later.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
© 2016 - 2024 Red Hat, Inc.