Re: [Qemu-devel] [PATCH 4/7] tests/qgraph: arm/raspi2 machine node

[Paolo Bonzini]

On 11/07/2018 16:59, Stefan Hajnoczi wrote:
>> +    machine->obj.get_device = raspi2_get_device;
>> +    machine->obj.destructor = raspi2_destroy;
>> +    qos_create_sdhci_mm(&machine->sdhci, 0x3f300000, &(QSDHCIProperties) {
>> +        .version = 3,
>> +        .baseclock = 52,
>> +        .capab.sdma = false,
>> +        .capab.reg = 0x052134b4
>> +    });
>> +    return &machine->obj;
>> +}
>> +
>> +static void raspi2(void)
>> +{
>> +    qos_node_create_machine("arm/raspi2", qos_create_machine_arm_raspi2);
>> +    qos_node_contains("arm/raspi2", "generic-sdhci");
>> +}
>> +
>> +libqos_init(raspi2);
> Hmm...I didn't realize it would be necessary to manually define each
> machine type.  I thought qgraph would use qemu-system-x86_64
> -machine/-device/info qtree to introspect QEMU and instantiate the
> appropriate drivers.

That doesn't provide enough information yet.  However, PCI devices are
already discovered automatically and the next step (next year :)) could
be to use device tree or ACPI to discover embedded devices.

Using QOM properties instead of duplicating things like QSDHCIProperties
is a great idea.  Of course the duplication was already there ("old
sdhci_t structure" in patch 7), so I don't think it should be a blocker,
but indeed there's a better way to do it.

> My concern is that this manual approach of defining machines complicates
> qtests.  This file will need to be modified by multiple people - each of
> them will be interested in a specific driver and not interested in the
> driver that other people have added.

This is exactly the opposite problem that we have now: when you write a
test you are interested typically only in one machine, and the
"if(x86)elseif(arm)" gets in the way.  (Also: it's also difficult to
split the ifs across files, i.e. it scales worse; and there's lots of
duplicated code across tests to do "for" loops of g_test_add, because
the ifs don't lend itself to "automatic" generation of test cases via
path walk).

The advantage is that (just like for OS vs. QEMU) the structure of this
file matches closely the board files in hw/arm/, which is something you
should already be familiar with if you're adding a new board to QEMU.

Paolo