Re: [Qemu-ppc] ppc440_pcix_reg warnings with the new sam460ex board

[Thomas Huth]

On 07.03.2018 22:06, BALATON Zoltan wrote:
> Hello,
> 
> On Wed, 7 Mar 2018, Thomas Huth wrote:
>> I was just trying to add a regression qtest for the new sam460ex board,
>> but I noticed that there are currently some ugly warnings showing up
>> when using this board:
>>
>> qemu-system-ppc: ppc440_pcix_reg_write4: unhandled PCI internal
>> register 0x40
>> qemu-system-ppc: ppc440_pcix_reg_read4: invalid PCI internal register
>> 0x44
>> qemu-system-ppc: ppc440_pcix_reg_write4: unhandled PCI internal
>> register 0x44
>>
>> Are you aware of these warning messages? Could they be silenced somehow?
> 
> I don't know what these registers are and how to implement them properly
> but I've sent a patch that adds dummy implementations that silences
> these warnings if that helps.

OK, thanks! ... alternatively, I think it should be fine to simply use
qemu_log_mask(LOG_UNIMP, ...) instead of error_report() here.

> But why are these warnings cause a problem for tests?

Well, you run "make check" to make sure that there is no error in the
current code base. If there are suddenly some warnings / error messages
showing up, people will start to wonder whether they've introduced a new
problem. So the output of "make check" should look just clean.

>> FWIW, here's what I wanted to add to the qtest suite:
>>
>> diff a/tests/boot-serial-test.c b/tests/boot-serial-test.c
>> --- a/tests/boot-serial-test.c
>> +++ b/tests/boot-serial-test.c
>> @@ -79,12 +79,14 @@ static testdef_t tests[] = {
>>     { "ppc", "40p", "-boot d", "Booting from device d" },
>>     { "ppc", "g3beige", "", "PowerPC,750" },
>>     { "ppc", "mac99", "", "PowerPC,G4" },
>> +    { "ppc", "sam460ex", "-m 256", "DRAM:  256 MiB" },
>>     { "ppc64", "ppce500", "", "U-Boot" },
>>     { "ppc64", "prep", "-boot e", "Booting from device e" },
>>     { "ppc64", "40p", "-m 192", "Memory size: 192 MB" },
>>     { "ppc64", "mac99", "", "PowerPC,970FX" },
>>     { "ppc64", "pseries", "", "Open Firmware" },
>>     { "ppc64", "powernv", "-cpu POWER8", "OPAL" },
>> +    { "ppc64", "sam460ex", "-device e1000", "8086  100e" },
> 
> Why are you using random command line options (like -m and -device)
> instead of just running the default config without options and looking
> for something like "Board: Sam460ex" in the output which is printed by
> default? That looks more straight forward to me. Or if you want to test
> most of the devices you could look for "SM502: found" which is one of
> the last lines printed on serial during boot after probing for most
> hardware so that should also catch errors in some device emulation if it
> does not get to that but breaks before.

By using an additional command line option which changes the output of
the firmware, we can test that the interface from the CLI to QEMU to the
emulated firmware is also working fine. So this is (slightly) more
additional test coverage compared to just checking a "static" string
that is always available in the output.

> Also does it make sense to test both with ppc and ppc64 when the two
> should be identical? Like for g3beige (and unlike for mac99) the
> sam460ex is only a 32bit board so probably testing with ppc64 does not
> really make sense and only ppc should be added?

The machine is available in both, qemu-system-ppc and qemu-system-ppc64,
and we've had bugs in the past where something broke in ppc64 but not in
ppc and vice versa, so I think it makes indeed sense to check it in both
setups. Some people also only compile with --target-list=ppc64-softmmu
and they would not notice a problem with sam460ex otherwise.

Not sure anymore why I didn't add g3beige to ppc64, too ... I think we
should simply add it there nowadays, too, for the very same reasons.

 Thomas