On 11/24/23 21:39, Joelle van Dyne wrote:
On Fri, Nov 24, 2023 at 8:26 AM Stefan Berger <stef...@linux.ibm.com> wrote:
On 11/24/23 11:21, Joelle van Dyne wrote:
On Fri, Nov 24, 2023 at 8:17 AM Stefan Berger <stef...@linux.ibm.com> wrote:
On 11/23/23 19:56, Joelle van Dyne wrote:
On Tue, Nov 14, 2023 at 4:12 PM Stefan Berger <stef...@linux.ibm.com> wrote:
On 11/14/23 16:05, Stefan Berger wrote:
On 11/14/23 13:03, Stefan Berger wrote:
On 11/14/23 04:36, Marc-André Lureau wrote:
Hi
On Tue, Nov 14, 2023 at 6:12 AM Joelle van Dyne <j...@getutm.app> wrote:
Signed-off-by: Joelle van Dyne <j...@getutm.app>
Reviewed-by: Stefan Berger <stef...@linux.ibm.com>
nit: you also added tests for x86, could be a different patch?
For arm, the test fails until next patch with:
# starting QEMU: exec ./qemu-system-aarch64 -qtest
unix:/tmp/qtest-991279.sock -qtest-log /dev/null -chardev
socket,path=/tmp/qtest-991279.qmp,id=char0 -mon
chardev=char0,mode=control -display none -audio none -machine virt
-accel tcg -nodefaults -nographic -drive
if=pflash,format=raw,file=pc-bios/edk2-aarch64-code.fd,readonly=on
-drive if=pflash,format=raw,file=pc-bios/edk2-arm-vars.fd,snapshot=on
-cdrom tests/data/uefi-boot-images/bios-tables-test.aarch64.iso.qcow2
-cpu cortex-a57 -chardev
socket,id=chr,path=/tmp/qemu-test_acpi_virt_tcg_crb-device.KZ3GE2/sock
-tpmdev emulator,id=dev,chardev=chr -device tpm-crb-device,tpmdev=dev
-accel qtest
Warning! zero length expected file
'tests/data/acpi/virt/TPM2.crb-device.tpm2'
Warning! zero length expected file
'tests/data/acpi/virt/DSDT.crb-device.tpm2'
acpi-test: Warning! binary file mismatch. Actual
[aml:/tmp/aml-GO4ME2], Expected
[aml:tests/data/acpi/virt/TPM2.crb-device.tpm2].
See source file tests/qtest/bios-tables-test.c for instructions on how
to update expected files.
acpi-test: Warning! binary file mismatch. Actual
[aml:/tmp/aml-6N4ME2], Expected
[aml:tests/data/acpi/virt/DSDT.crb-device.tpm2].
See source file tests/qtest/bios-tables-test.c for instructions on how
to update expected files.
to see ASL diff between mismatched files install IASL, rebuild QEMU
from scratch and re-run tests with V=1 environment variable set**
ERROR:../tests/qtest/bios-tables-test.c:538:test_acpi_asl: assertion
failed: (all_tables_match)
not ok /aarch64/acpi/virt/tpm2-crb -
ERROR:../tests/qtest/bios-tables-test.c:538:test_acpi_asl: assertion
failed: (all_tables_match)
Bail out!
qemu-system-aarch64: tpm-emulator: Could not cleanly shutdown the TPM:
Resource temporarily unavailable
Unexpected error in qio_channel_socket_writev() at
../io/channel-socket.c:622:
/home/elmarco/src/qemu/buildall/tests/qtest/bios-tables-test: Unable
to write to socket: Bad file descriptor
Travis testing on s390x I see the following failures for this patchset
(search for 'ERROR'):
https://app.travis-ci.com/github/stefanberger/qemu-tpm/builds/267230363
Summary of Failures:
134/320 qemu:qtest+qtest-aarch64 / qtest-aarch64/tpm-crb-device-test
ERROR 0.70s killed by signal 6 SIGABRT
219/320 qemu:qtest+qtest-x86_64 / qtest-x86_64/tpm-crb-test
ERROR 0.88s killed by signal 6 SIGABRT
Summary of Failures:
271/537 qemu:qtest+qtest-i386 / qtest-i386/tpm-crb-test
ERROR 0.59s killed by signal 6 SIGABRT
My guess is it's an endianess issue on big endian machines due to
reading from the ROM device where we lost the .endianess:
+const MemoryRegionOps tpm_crb_memory_ops = {
+ .read = tpm_crb_mmio_read,
+ .write = tpm_crb_mmio_write,
+ .endianness = DEVICE_LITTLE_ENDIAN,
+ .valid = {
+ .min_access_size = 1,
+ .max_access_size = 4,
+ },
+};
I think we need a 2nd set of registers to support the endianess
conversion. It's not exactly nice, though. Basically the saved_regs
could be used for this directly, even though I did not do that but
introduced n_regs:
https://github.com/stefanberger/qemu-tpm/commit/90f6b21c0dd93dbb13d9e80a628f5b631fd07d91
This patch allows the tests on s390x to run farther but the execution of
the command doesn't seem to work maybe due to command data that were
also written in wrong endianess. I don't know. I would have to get
access to a big endian / s390 machine to be able to fix it.
The latest version now passes on Travis s390x:
https://app.travis-ci.com/github/stefanberger/qemu-tpm/builds/267245220
Are the tests failing on S390X due to the added code or are they
failing because previously it was untested? I don't think the original
code took account of endianness and that should be fixed, but feels
like it should be in a separate patch set?
They are failing because something like the topmost one or two patches
as in this branch here are missing for a big endian host:
https://github.com/stefanberger/qemu-tpm/tree/joelle.v5%2B2nd_registers
Right, but is this issue new due to the patchset? i.e. if just the
Yes, it is due to this patchset. The reason is that CRB switched to a
ROMD interface where the fact that the MMIO registers are little endian
got lost for existing x86_64 support.
tests were added without the other patches, would they still fail? If
so, I think the fixes should be in another patchset while we disable
them in this patchset.
I see, how do you want to best integrate your changes? Do you want me
to squash your changes into the patch that introduces the code? Or do
you want them to be separate commits?
I think squashing them in would be the right thing to do. You may want
to append _LE to the macros in the 2nd patch if you take them.
Stefan
