Re: [Qemu-arm] [Qemu-devel] Running QEMU without default devices / kernel / bios (was: Test devices with all machines, not only with "none")

[Thomas Huth]

On 07.05.2018 21:32, Eduardo Habkost wrote:
> On Mon, May 07, 2018 at 09:13:57PM +0200, Thomas Huth wrote:
>> On 07.05.2018 20:21, Eduardo Habkost wrote:
>>> On Mon, May 07, 2018 at 06:50:35PM +0200, Markus Armbruster wrote:
>>>> Eduardo Habkost <address@hidden> writes:
>>>>
>>>>> On Fri, Apr 27, 2018 at 08:31:58AM +0200, Markus Armbruster wrote:
>>>>>> Eduardo Habkost <address@hidden> writes:
>>>>>>
>>>>>>> On Thu, Apr 26, 2018 at 05:20:25PM +0200, Thomas Huth wrote:
>>>>>>>> On 26.04.2018 13:45, Markus Armbruster wrote:
>>>>>>>>> Thomas Huth <address@hidden> writes:
>>>>>>>> [...]
>>>>>>>>>> @@ -260,6 +263,26 @@ static void test_abstract_interfaces(void)
>>>>>>>>>>      qtest_end();
>>>>>>>>>>  }
>>>>>>>>>>  
>>>>>>>>>> +static void add_machine_test_case(const char *mname)
>>>>>>>>>> +{
>>>>>>>>>> +    char *path, *args;
>>>>>>>>>> +
>>>>>>>>>> +    /* Ignore blacklisted machines */
>>>>>>>>>> +    if (g_str_equal("xenfv", mname) || g_str_equal("xenpv", mname)) 
>>>>>>>>>> {
>>>>>>>>>> +        return;
>>>>>>>>>> +    }
>>>>>>>>>> +
>>>>>>>>>> +    path = 
>>>>>>>>>> g_strdup_printf("device/introspect/concrete-defaults-%s", mname);
>>>>>>>>>> +    args = g_strdup_printf("-machine %s", mname);
>>>>>>>>>> +    qtest_add_data_func(path, args, test_device_intro_concrete);
>>>>>>>>>
>>>>>>>>> This runs test_device_intro_concrete() with "-machine M" for all 
>>>>>>>>> machine
>>>>>>>>> types M, in SPEED=slow mode.
>>>>>>>>>
>>>>>>>>>> +    g_free(path);
>>>>>>>>>> +
>>>>>>>>>> +    path = 
>>>>>>>>>> g_strdup_printf("device/introspect/concrete-nodefaults-%s", mname);
>>>>>>>>>> +    args = g_strdup_printf("-nodefaults -machine %s", mname);
>>>>>>>>>> +    qtest_add_data_func(path, args, test_device_intro_concrete);
>>>>>>>>>
>>>>>>>>> This runs test_device_intro_concrete() with "-nodefaults -machine M" 
>>>>>>>>> for
>>>>>>>>> all machine types M, in SPEED=slow mode.
>>>>>>>>>
>>>>>>>>> Has "without -nodefaults" exposed additional bugs?
>>>>>>>>
>>>>>>>> After testing this with all machines, I had to discover that
>>>>>>>> "-nodefaults" does not work so easily: A lot of the embedded machines
>>>>>>>> (especially the ARM machines) simply refuse to work with "-nodefaults"
>>>>>>>> and exit immediately instead. E.g.:
>>>>>>>>
>>>>>>>> $ arm-softmmu/qemu-system-arm -nodefaults -nographic -M 
>>>>>>>> n810,accel=qtest
>>>>>>>> qemu-system-arm: missing SecureDigital device
>>>>>>
>>>>>> These are all bugs.  --nodefaults is supposed to suppress *optional*
>>>>>> devices, not mandatory ones.
>>>>>
>>>>> I'm not sure I understand the requirements.  What exactly is the
>>>>> definition of "mandatory"?
>>>>>
>>>>> A machine created by "qemu-system-x86_64 -machine pc -nodefaults"
>>>>> is useless because it has no any device to boot from.  How is
>>>>> that different from a n810 machine not booting because there's no
>>>>> SD device?
>>>>
>>>> I propose:
>>>>
>>>> * Stuff that's required for QEMU to run is not suppressed by -nodefaults
>>>>
>>>> * Stuff that a real machine has soldered on is also not suppressed
>>>>
>>>> * Stuff that can be pulled out of a real machine may be suppressed, even
>>>>   when that means the guest won't run
>>>
>>> Makes sense to me.  It looks like the only obstacle for
>>> tests/device-introspect and device-crash-test is the first rule.
>>> "Guest won't boot" isn't a problem, but "QEMU won't run" is.
>>>
>>> The first rule is easily testable, too: running
>>> "$QEMU -machine $MACHINE -nodefaults" and not having a working
>>> QMP monitor should be reported as a bug by automated tests.
>>
>> You mean with "-accel qtest" or without? With "-accel qtest" we should
>> pretty soon be fine, after Peter's current PULL request has been merged
>> (which contains a patch from me for fixing these SD card problems with
>> ARM machines).
>> Without "-accel qtest", things are not that easy, unfortunately. Lots of
>> boards require "-kernel" or "-bios" and refuse to work without. So you
>> can hardly test "-nodefaults" automatically in the normal tcg mode. (But
>> maybe all boards should allow to start QEMU in case you've at least also
>> specified "-S" ? ... in that case we've got plenty of work for
>> BiteSizeTasks ;-) )
> 
> Hmm, maybe it's not a bite-sized task after all.  :)
> 
> Should we do this gradually?
> 
> * Working with -accel qtest is useful, and sounds like an easier goal;

We're pretty much there already. Apart from the SD card problem (and the
xen boards), all machines should work with -nodefaults in qtest mode now.

> * working with -S seems desirable too;

Yes, it could be interesting to load the firmware / OS via HMP or GDB
after QEMU has been started.

Maybe we'd simply need a new function a la:

bool cpu_starts_automatically()
{
    return autostart && !qtest_enabled();
}

And then replace all spots where we exit due to missing -kernel or -bios
parameters, e.g.:

diff --git a/hw/m68k/mcf5208.c b/hw/m68k/mcf5208.c
--- a/hw/m68k/mcf5208.c
+++ b/hw/m68k/mcf5208.c
@@ -286,7 +286,7 @@ static void mcf5208evb_init(MachineState *machine)

     /* Load kernel.  */
     if (!kernel_filename) {
-        if (qtest_enabled()) {
+        if (!cpu_starts_automatically()) {
             return;
         }
         error_report("Kernel image must be specified");

Does that sound like a plan?

> * working without -S (even if the emulated CPU crashes and burns)
>   would be interesting.

Not sure whether we really need this. It's likely better to give the
user a proper error message to use "-kernel" instead of just showing a
crash.

> Related question: what are the use cases where we require
> "-accel qtest" and "-S" wouldn't work?

Maybe there are some boards where you can not load code via HMP or GDB
once you've started QEMU with "-S"? You'd end up with a mostly useless
HMP prompt in that case, which is a little bit ugly, but not fatal.
Apart from that ... I can't think of a case where "-S" would not work at
all once we've introduce something like cpu_starts_automatically().

> Are the requirements and goals of "-accel qtest" documented
> somewhere?  Without documentation, it's hard to say when a given
> qtest_enabled() call in the code is reasonable, or a hack we want
> to get rid of.

There's some few documentation in docs/devel/testing.rst and
https://wiki.qemu.org/Features/QTest - but the quintessence is that
qtest basically rips out the CPU of the emulated machine and replaces it
with the qtest stub. So in this case there is no CPU that can run code
automatically. Thus I think if we abstract this with a function a la
cpu_starts_automatically(), we should be fine.

 Thomas