[libvirt PATCH 0/5] qemu: Fix defaults for TPM on ARM virt guests
The current default is unfortunately broken, and the user has to manually step in and provide the version number explicitly for the TPM device to work at all. https://bugzilla.redhat.com/show_bug.cgi?id=1970310 Andrea Bolognani (5): docs: Fix information for default TPM version tests: Add aarch64-tpm test to qemuxml2xml qemu: Default to TPM 2.0 for ARM virt guests tests: Test the defaults for TPM on ARM virt guests qemu: Reject TPM 1.2 for ARM virt guests docs/formatdomain.rst | 9 ++++-- src/qemu/qemu_domain.c | 3 +- src/qemu/qemu_validate.c | 6 ++++ .../aarch64-tpm-wrong-model.err | 1 + ...64-tpm.xml => aarch64-tpm-wrong-model.xml} | 2 +- tests/qemuxml2argvdata/aarch64-tpm.xml | 4 +-- tests/qemuxml2argvtest.c | 1 + .../aarch64-tpm.aarch64-latest.xml | 29 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 9 files changed, 49 insertions(+), 7 deletions(-) create mode 100644 tests/qemuxml2argvdata/aarch64-tpm-wrong-model.err copy tests/qemuxml2argvdata/{aarch64-tpm.xml => aarch64-tpm-wrong-model.xml} (88%) create mode 100644 tests/qemuxml2xmloutdata/aarch64-tpm.aarch64-latest.xml -- 2.31.1
The current information is not accurate, because the default is 2.0 instead of 1.2 for the tpm-crb and tpm-spapr models. Any detailed list will surely become obsolete and out of sync with reality over time, so let's just document that the default model depends on a number of factors and avoid getting any more specific than that. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- docs/formatdomain.rst | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index c6dede053f..25e6bf73ba 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -7540,13 +7540,16 @@ Example: usage of the TPM Emulator each QEMU guest requesting access to it. ``version`` - The ``version`` attribute indicates the version of the TPM. By default a TPM - 1.2 is created. This attribute only works with the ``emulator`` backend. The - following versions are supported: + The ``version`` attribute indicates the version of the TPM. This attribute + only works with the ``emulator`` backend. The following versions are + supported: - '1.2' : creates a TPM 1.2 - '2.0' : creates a TPM 2.0 + The default version used depends on the combination of hypervisor, guest + architecture, TPM model and backend. + ``persistent_state`` The ``persistent_state`` attribute indicates whether 'swtpm' TPM state is kept or not when a transient domain is powered off or undefined. This -- 2.31.1
We're going to change the input file later, and having this additional coverage will demonstrate that such a change does not alter the behavior. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- .../aarch64-tpm.aarch64-latest.xml | 29 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 2 files changed, 30 insertions(+) create mode 100644 tests/qemuxml2xmloutdata/aarch64-tpm.aarch64-latest.xml diff --git a/tests/qemuxml2xmloutdata/aarch64-tpm.aarch64-latest.xml b/tests/qemuxml2xmloutdata/aarch64-tpm.aarch64-latest.xml new file mode 100644 index 0000000000..e97f39aec3 --- /dev/null +++ b/tests/qemuxml2xmloutdata/aarch64-tpm.aarch64-latest.xml @@ -0,0 +1,29 @@ +<domain type='qemu'> + <name>aarch64test</name> + <uuid>496d7ea8-9739-544b-4ebd-ef08be936e8b</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='aarch64' machine='virt'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <gic version='2'/> + </features> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>cortex-a15</model> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + <controller type='pci' index='0' model='pcie-root'/> + <tpm model='tpm-tis'> + <backend type='emulator' version='2.0'/> + </tpm> + <audio id='1' type='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 40e027aaa4..8b7538f666 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -783,6 +783,7 @@ mymain(void) DO_TEST_CAPS_LATEST("tpm-emulator-tpm2"); DO_TEST_CAPS_LATEST("tpm-emulator-tpm2-enc"); DO_TEST_CAPS_LATEST("tpm-emulator-tpm2-pstate"); + DO_TEST_CAPS_ARCH_LATEST("aarch64-tpm", "aarch64"); DO_TEST("metadata", NONE); DO_TEST("metadata-duplicate", NONE); -- 2.31.1
The TPM 2.0 specification predates ARM virtualization, and so implementing TPM 1.2 support on ARM was not considered a useful endeavor. This is technically a breaking change, but TPM support on ARM was only introduced fairly recently (libvirt 7.1.0) and the previous default resulted in non working TPM devices; anyone who has a working configuration is not going to be affected. https://bugzilla.redhat.com/show_bug.cgi?id=1970310 Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_domain.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index fc60e15eea..8488f58e09 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4445,7 +4445,8 @@ qemuDomainDefTPMsPostParse(virDomainDef *def) /* TPM 1.2 and 2 are not compatible, so we choose a specific version here */ if (tpm->version == VIR_DOMAIN_TPM_VERSION_DEFAULT) { if (tpm->model == VIR_DOMAIN_TPM_MODEL_SPAPR || - tpm->model == VIR_DOMAIN_TPM_MODEL_CRB) + tpm->model == VIR_DOMAIN_TPM_MODEL_CRB || + qemuDomainIsARMVirt(def)) tpm->version = VIR_DOMAIN_TPM_VERSION_2_0; else tpm->version = VIR_DOMAIN_TPM_VERSION_1_2; -- 2.31.1
That works for me. Thanks. Tested-by: liuyd.fnst@fujitsu.com On 6/25/21 10:27 PM, Andrea Bolognani wrote:
The TPM 2.0 specification predates ARM virtualization, and so implementing TPM 1.2 support on ARM was not considered a useful endeavor.
This is technically a breaking change, but TPM support on ARM was only introduced fairly recently (libvirt 7.1.0) and the previous default resulted in non working TPM devices; anyone who has a working configuration is not going to be affected.
https://bugzilla.redhat.com/show_bug.cgi?id=1970310
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_domain.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index fc60e15eea..8488f58e09 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4445,7 +4445,8 @@ qemuDomainDefTPMsPostParse(virDomainDef *def) /* TPM 1.2 and 2 are not compatible, so we choose a specific version here */ if (tpm->version == VIR_DOMAIN_TPM_VERSION_DEFAULT) { if (tpm->model == VIR_DOMAIN_TPM_MODEL_SPAPR || - tpm->model == VIR_DOMAIN_TPM_MODEL_CRB) + tpm->model == VIR_DOMAIN_TPM_MODEL_CRB || + qemuDomainIsARMVirt(def)) tpm->version = VIR_DOMAIN_TPM_VERSION_2_0; else tpm->version = VIR_DOMAIN_TPM_VERSION_1_2;
-- Best Regards. Yiding Liu
On Tue, Jun 29, 2021 at 02:54:24AM +0000, liuyd.fnst@fujitsu.com wrote:
That works for me. Thanks.
Tested-by: liuyd.fnst@fujitsu.com
Glad to hear that! Can you please provide a full Tested-by tag in the expected format Tested-by: FirstName LastName <email@address.tld> so that it's suitable for inclusion in the git log? Thanks! -- Andrea Bolognani / Red Hat / Virtualization
Sorry for the inconvenience. Tested-by: Liu Yiding <liuyd.fnst@fujitsu.com> Thanks, Liu On 6/29/21 9:08 PM, Andrea Bolognani wrote:
On Tue, Jun 29, 2021 at 02:54:24AM +0000, liuyd.fnst@fujitsu.com wrote:
That works for me. Thanks.
Tested-by: liuyd.fnst@fujitsu.com Glad to hear that! Can you please provide a full Tested-by tag in the expected format
Tested-by: FirstName LastName <email@address.tld>
so that it's suitable for inclusion in the git log? Thanks!
-- Best Regards. Yiding Liu
Instead of providing the configuration explicitly, let libvirt fill in the blanks. After the recent changes, this results in a working configuration without the need for user input. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- tests/qemuxml2argvdata/aarch64-tpm.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/qemuxml2argvdata/aarch64-tpm.xml b/tests/qemuxml2argvdata/aarch64-tpm.xml index d338a20f17..b22dbee71e 100644 --- a/tests/qemuxml2argvdata/aarch64-tpm.xml +++ b/tests/qemuxml2argvdata/aarch64-tpm.xml @@ -8,8 +8,8 @@ </os> <devices> <emulator>/usr/bin/qemu-system-aarch64</emulator> - <tpm model='tpm-tis'> - <backend type='emulator' version='2.0'/> + <tpm> + <backend type='emulator'/> </tpm> </devices> </domain> -- 2.31.1
We already reject TPM 1.2 in a number of scenarios; let's add ARM virt guests to the list. https://bugzilla.redhat.com/show_bug.cgi?id=1970310 Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_validate.c | 6 ++++++ .../qemuxml2argvdata/aarch64-tpm-wrong-model.err | 1 + .../qemuxml2argvdata/aarch64-tpm-wrong-model.xml | 15 +++++++++++++++ tests/qemuxml2argvtest.c | 1 + 4 files changed, 23 insertions(+) create mode 100644 tests/qemuxml2argvdata/aarch64-tpm-wrong-model.err create mode 100644 tests/qemuxml2argvdata/aarch64-tpm-wrong-model.xml diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 382473d03b..b133ce3cd6 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -4517,6 +4517,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")); + return -1; + } break; case VIR_DOMAIN_TPM_VERSION_2_0: case VIR_DOMAIN_TPM_VERSION_DEFAULT: diff --git a/tests/qemuxml2argvdata/aarch64-tpm-wrong-model.err b/tests/qemuxml2argvdata/aarch64-tpm-wrong-model.err new file mode 100644 index 0000000000..a3a82fdcf5 --- /dev/null +++ b/tests/qemuxml2argvdata/aarch64-tpm-wrong-model.err @@ -0,0 +1 @@ +unsupported configuration: TPM 1.2 is not supported on ARM diff --git a/tests/qemuxml2argvdata/aarch64-tpm-wrong-model.xml b/tests/qemuxml2argvdata/aarch64-tpm-wrong-model.xml new file mode 100644 index 0000000000..9441c4d05a --- /dev/null +++ b/tests/qemuxml2argvdata/aarch64-tpm-wrong-model.xml @@ -0,0 +1,15 @@ +<domain type="qemu"> + <name>aarch64test</name> + <uuid>496d7ea8-9739-544b-4ebd-ef08be936e8b</uuid> + <memory>1048576</memory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch="aarch64" machine="virt">hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + <tpm model='tpm-tis'> + <backend type='emulator' version='1.2'/> + </tpm> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 9df28658b9..16236f0331 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2565,6 +2565,7 @@ mymain(void) DO_TEST_CAPS_LATEST("tpm-emulator-tpm2-pstate"); DO_TEST_CAPS_LATEST_PPC64("tpm-emulator-spapr"); DO_TEST_CAPS_ARCH_LATEST("aarch64-tpm", "aarch64"); + DO_TEST_PARSE_ERROR("aarch64-tpm-wrong-model", "aarch64"); DO_TEST_PARSE_ERROR("pci-domain-invalid", NONE); DO_TEST_PARSE_ERROR("pci-bus-invalid", NONE); -- 2.31.1
It works. Thanks. Tested-by: liuyd.fnst@fujitsu.com On 6/25/21 10:27 PM, Andrea Bolognani wrote:
We already reject TPM 1.2 in a number of scenarios; let's add ARM virt guests to the list.
https://bugzilla.redhat.com/show_bug.cgi?id=1970310
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_validate.c | 6 ++++++ .../qemuxml2argvdata/aarch64-tpm-wrong-model.err | 1 + .../qemuxml2argvdata/aarch64-tpm-wrong-model.xml | 15 +++++++++++++++ tests/qemuxml2argvtest.c | 1 + 4 files changed, 23 insertions(+) create mode 100644 tests/qemuxml2argvdata/aarch64-tpm-wrong-model.err create mode 100644 tests/qemuxml2argvdata/aarch64-tpm-wrong-model.xml
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 382473d03b..b133ce3cd6 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -4517,6 +4517,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")); + return -1; + } break; case VIR_DOMAIN_TPM_VERSION_2_0: case VIR_DOMAIN_TPM_VERSION_DEFAULT: diff --git a/tests/qemuxml2argvdata/aarch64-tpm-wrong-model.err b/tests/qemuxml2argvdata/aarch64-tpm-wrong-model.err new file mode 100644 index 0000000000..a3a82fdcf5 --- /dev/null +++ b/tests/qemuxml2argvdata/aarch64-tpm-wrong-model.err @@ -0,0 +1 @@ +unsupported configuration: TPM 1.2 is not supported on ARM diff --git a/tests/qemuxml2argvdata/aarch64-tpm-wrong-model.xml b/tests/qemuxml2argvdata/aarch64-tpm-wrong-model.xml new file mode 100644 index 0000000000..9441c4d05a --- /dev/null +++ b/tests/qemuxml2argvdata/aarch64-tpm-wrong-model.xml @@ -0,0 +1,15 @@ +<domain type="qemu"> + <name>aarch64test</name> + <uuid>496d7ea8-9739-544b-4ebd-ef08be936e8b</uuid> + <memory>1048576</memory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch="aarch64" machine="virt">hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + <tpm model='tpm-tis'> + <backend type='emulator' version='1.2'/> + </tpm> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 9df28658b9..16236f0331 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2565,6 +2565,7 @@ mymain(void) DO_TEST_CAPS_LATEST("tpm-emulator-tpm2-pstate"); DO_TEST_CAPS_LATEST_PPC64("tpm-emulator-spapr"); DO_TEST_CAPS_ARCH_LATEST("aarch64-tpm", "aarch64"); + DO_TEST_PARSE_ERROR("aarch64-tpm-wrong-model", "aarch64");
DO_TEST_PARSE_ERROR("pci-domain-invalid", NONE); DO_TEST_PARSE_ERROR("pci-bus-invalid", NONE);
-- Best Regards. Yiding Liu
On 6/25/21 4:27 PM, Andrea Bolognani wrote:
The current default is unfortunately broken, and the user has to manually step in and provide the version number explicitly for the TPM device to work at all.
https://bugzilla.redhat.com/show_bug.cgi?id=1970310
Andrea Bolognani (5): docs: Fix information for default TPM version tests: Add aarch64-tpm test to qemuxml2xml qemu: Default to TPM 2.0 for ARM virt guests tests: Test the defaults for TPM on ARM virt guests qemu: Reject TPM 1.2 for ARM virt guests
docs/formatdomain.rst | 9 ++++-- src/qemu/qemu_domain.c | 3 +- src/qemu/qemu_validate.c | 6 ++++ .../aarch64-tpm-wrong-model.err | 1 + ...64-tpm.xml => aarch64-tpm-wrong-model.xml} | 2 +- tests/qemuxml2argvdata/aarch64-tpm.xml | 4 +-- tests/qemuxml2argvtest.c | 1 + .../aarch64-tpm.aarch64-latest.xml | 29 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 9 files changed, 49 insertions(+), 7 deletions(-) create mode 100644 tests/qemuxml2argvdata/aarch64-tpm-wrong-model.err copy tests/qemuxml2argvdata/{aarch64-tpm.xml => aarch64-tpm-wrong-model.xml} (88%) create mode 100644 tests/qemuxml2xmloutdata/aarch64-tpm.aarch64-latest.xml
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (3)
-
Andrea Bolognani -
liuyd.fnst＠fujitsu.com -
Michal Prívozník