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?
