Re: [PATCH v5 12/14] tests: acpi: implement TPM CRB tests for ARM virt

[Stefan Berger <stefanb@linux.ibm.com>]

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 <stefanb@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.