qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Joelle van Dyne
Subject: Re: [PATCH v5 12/14] tests: acpi: implement TPM CRB tests for ARM virt
Date: Thu, 23 Nov 2023 16:56:21 -0800

On Tue, Nov 14, 2023 at 4:12 PM Stefan Berger <stefanb@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 <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.
> >
> >
>
> 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?



reply via email to

[Prev in Thread] Current Thread [Next in Thread]