11:34 a.m.
These patches are fallout from a discussion about TPM devices and aarch64
https://listman.redhat.com/archives/libvir-list/2021-February/msg00526.html
Jim Fehlig (2):
qemu: Fix swtpm device with aarch64
qemu: Validate TPM TIS device
src/qemu/qemu_command.c | 3 ++
src/qemu/qemu_validate.c | 7 ++++
.../aarch64-tpm.aarch64-latest.args | 37 +++++++++++++++++++
tests/qemuxml2argvdata/aarch64-tpm.xml | 15 ++++++++
tests/qemuxml2argvtest.c | 1 +
5 files changed, 63 insertions(+)
create mode 100644 tests/qemuxml2argvdata/aarch64-tpm.aarch64-latest.args
create mode 100644 tests/qemuxml2argvdata/aarch64-tpm.xml
--
2.29.2
11:34 a.m.
New subject: [PATCH 1/2] qemu: Fix swtpm device with aarch64
Starting a VM with swtpm device fails with qemu-system-aarch64.
E.g. with TPM device config
<tpm model='tpm-tis'>
<backend type='emulator' version='2.0'/>
</tpm>
QEMU reports the following error
error: internal error: process exited while connecting to monitor:
2021-02-07T05:15:35.378927Z qemu-system-aarch64: -device
tpm-tis,tpmdev=tpm-tpm0,id=tpm0: 'tpm-tis' is not a valid device model name
Indeed the TPM device name is 'tpm-tis-device' [1][2] for aarch64,
versus the shorter 'tpm-tis' for x86. The devices are the same from
a functional POV, i.e. they both emulate a TPM device conforming to
the TIS specification. Account for the unfortunate name difference
when building the TPM device option in qemuBuildTPMDevStr(). Also
include a test case for 'tpm-tis-device'.
[1] https://qemu.readthedocs.io/en/latest/specs/tpm.html
[2] https://github.com/qemu/qemu/commit/c294ac327ca99342b90bd3a83d2cef9b447afaa7
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/qemu/qemu_command.c | 3 ++
.../aarch64-tpm.aarch64-latest.args | 37 +++++++++++++++++++
tests/qemuxml2argvdata/aarch64-tpm.xml | 15 ++++++++
tests/qemuxml2argvtest.c | 1 +
4 files changed, 56 insertions(+)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f0333d4f1a..96b956d7ae 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9203,6 +9203,9 @@ qemuBuildTPMDevStr(const virDomainDef *def,
g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
const char *model = virDomainTPMModelTypeToString(tpm->model);
+ if (tpm->model == VIR_DOMAIN_TPM_MODEL_TIS && def->os.arch ==
VIR_ARCH_AARCH64)
+ model = "tpm-tis-device";
+
virBufferAsprintf(&buf, "%s,tpmdev=tpm-%s,id=%s",
model, tpm->info.alias, tpm->info.alias);
diff --git a/tests/qemuxml2argvdata/aarch64-tpm.aarch64-latest.args
b/tests/qemuxml2argvdata/aarch64-tpm.aarch64-latest.args
new file mode 100644
index 0000000000..94a083d816
--- /dev/null
+++ b/tests/qemuxml2argvdata/aarch64-tpm.aarch64-latest.args
@@ -0,0 +1,37 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-aarch64test \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-aarch64test/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-aarch64test/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-aarch64test/.config \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-aarch64 \
+-name guest=aarch64test,debug-threads=on \
+-S \
+-object secret,id=masterKey0,format=raw,\
+file=/tmp/lib/domain--1-aarch64test/master-key.aes \
+-machine virt,accel=tcg,usb=off,dump-guest-core=off,gic-version=2,\
+memory-backend=mach-virt.ram \
+-cpu cortex-a15 \
+-m 1024 \
+-object memory-backend-ram,id=mach-virt.ram,size=1073741824 \
+-overcommit mem-lock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid 496d7ea8-9739-544b-4ebd-ef08be936e8b \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,fd=1729,server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-no-acpi \
+-boot strict=on \
+-tpmdev emulator,id=tpm-tpm0,chardev=chrtpm \
+-chardev socket,id=chrtpm,path=/dev/test \
+-device tpm-tis-device,tpmdev=tpm-tpm0,id=tpm0 \
+-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
+resourcecontrol=deny \
+-msg timestamp=on
diff --git a/tests/qemuxml2argvdata/aarch64-tpm.xml
b/tests/qemuxml2argvdata/aarch64-tpm.xml
new file mode 100644
index 0000000000..d338a20f17
--- /dev/null
+++ b/tests/qemuxml2argvdata/aarch64-tpm.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='2.0'/>
+ </tpm>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index faa71a7a16..d09db77d8c 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2478,6 +2478,7 @@ mymain(void)
DO_TEST_CAPS_LATEST("tpm-emulator-tpm2-enc");
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("pci-domain-invalid", NONE);
DO_TEST_PARSE_ERROR("pci-bus-invalid", NONE);
--
2.29.2
11:34 a.m.
New subject: [PATCH 2/2] qemu: Validate TPM TIS device
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(a)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
4:37 a.m.
New subject: [PATCH 2/2] qemu: Validate TPM TIS device
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
9:44 a.m.
New subject: [PATCH 2/2] qemu: Validate TPM TIS device
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.
The entire page is worth a re-read on occasion :-)
Regards,
Jim
10:23 a.m.
New subject: [PATCH 2/2] qemu: Validate TPM TIS device
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
4:37 a.m.
On Wed, 2021-02-10 at 10:34 -0700, Jim Fehlig wrote:
These patches are fallout from a discussion about TPM devices and
aarch64
https://listman.redhat.com/archives/libvir-list/2021-February/msg00526.html
Jim Fehlig (2):
qemu: Fix swtpm device with aarch64
qemu: Validate TPM TIS device
With the nit I pointed out in 2/2 addressed,
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
1371
days inactive
1372
days old
6 comments
2 participants
participants (2)
-
Andrea Bolognani -
Jim Fehlig