[Qemu-devel] [RFC PATCH 0/3] acceptance tests: Test firmware checking debug console output

[Philippe Mathieu-Daudé]

Hi Laszlo,

On 28/09/2018 12:51, Laszlo Ersek wrote:
> Hi Phil,
> 
> (+Daniel, +Kashyap)
> 
> On 09/28/18 02:30, Philippe Mathieu-Daudé wrote:
>> Hi,
>>
>> This RFC series add simple acceptance tests which boot SeaBIOS and
>> EDK2 on Q35 and virt/aarch64.
>>
>> It is more of a proof of concept (to motivate the Avocado team ;) ).
>>
>> Regards,
>>
>> Phil.
>>
>> Philippe Mathieu-Daudé (3):
>>   acceptance tests: Add SeaBIOS boot and debug console checking test
>>   acceptance tests: Add EDK2 OVMF boot and debug console checking test
>>   acceptance tests: Add EDK2 AAVMF boot and console checking test
>>
>>  tests/acceptance/boot_firmware.py | 167 ++++++++++++++++++++++++++++++
>>  1 file changed, 167 insertions(+)
>>  create mode 100644 tests/acceptance/boot_firmware.py
>>
> 
> I'm not experienced with Avocado, so I'm basically reading the patches
> as a "story". My comments are made at that level too. :)

You gave the kind of useful comment I was expecting :) This is what RFC
are for after all.

The idea of this series is to provide users the ability to hack QEMU and
tests they didn't break the boot process (interaction with firmwares).

> 
> * In the blurb, you say "Q35". But the first two patches have
> 
>   vm.set_machine('pc')

Oops, mistake. I suppose we want to have both covered.

> 
> * Please don't call the edk2 ArmVirtQemu platform AAVMF in upstream
>   patches :) Call it ArmVirtQemu pls.

OK.

> 
> * Finding the right way to launch  OVMF and/or ArmVirtQemu firmware
>   images is complicated. (The right way is definitely not "-bios"!)>
>   The general idea is that you need three files (and two pflash chips);
>   (a) a firmware executable mapped read-only, and (b) a variable store
>   file, mapped read-write, that was first copied from (c) a read-only
>   variable store *template* that is never itself mapped. And, this is
>   not the whole story.

We definitively need the variable store to run useful tests.

> 
>   Figuring out the options is complicated enough (for management tools
>   as well) that Daniel made us define a metadata schema for describing
>   firmware packages. Please see:
> 
>   docs/interop/firmware.json
> 
>   I'm not necessarily suggesting that Avocado be able to parse the
>   firmware descriptor metafiles that conform to this schema. I'm just
>   pointing out that the QEMU command line will depend on the exact build
>   of the firmware image under test. The pathname
>   "/usr/share/OVMF/OVMF_CODE.fd" and the URL
>   "https://snapshots.linaro.org/.../QEMU_EFI.img.gz"; don't give us
>   enough information.
> 
>   Therefore, if we want to keep the test case simple (= hard-wire the
>   command lines), then we'll have to refer to OVMF and ArmVirtQemu
>   images with precisely known build configs.

To focus on QEMU here, I'd like to not have to build a know EDK2 config
but rather use releases (or snapshots).

scripts/qemu.py should provide utilities to pack a flash image for
further testing.

IMO tests using a stable QEMU to test different EDK2 configs have to go
in another repository.
But this is true there are also linked and usually to test a new feature
in one component, you need to update/build the other.

> * Looking for debug console messages as "vital signs" is brittle. For
>   example, the line "DetectSmbiosVersion: SMBIOS version from QEMU:
>   0x0208" will change if QEMU changes the SMBIOS version number in the
>   SMBIOS anchor that it generates. It's likely better to make the
>   firmware "do" something.
> 
>   The simplest I can imagine is: prepare a virtual disk with a
>   "startup.nsh" UEFI shell script on it. The script can print a known
>   fixed string, and then power down the VM. (See the UEFI Shell
>   Specification for commands; <http://uefi.org/specifications>.)

I didn't want to interact with the console but your suggestion is simple
enough for a PoC.

> 
>   I'm not sure if Avocado provides disk image preparation utilities, but
>   perhaps (a) we could use the vvfat driver (*shudder*) or (b) we could
>   preformat a small image, and track it as a binary file in git.

I guess we have to go with the utilities (for reproducibility).

Thanks for your comments,

Phil.