Re: [Qemu-devel] [RFC PATCH] arm boot: added QOM device definition

[Alexander Graf]

On 08.02.2012, at 14:04, Peter Crosthwaite wrote:

On Wed, Feb 8, 2012 at 10:41 PM, Alexander Graf <address@hidden> wrote:

On 08.02.2012, at 13:27, Paul Brook wrote:

>> 2012/2/8 Paul Brook <address@hidden>
>>
>>>>> I suspect we want to replace the arm_load_kernel call with an
>>>>> arm_linux_loader device with appropriate properties.
>>>>
>>>> Ok, so does this mean the machine model would still explicitly
>>>> instantiate the bootloader device?
>>>
>>> Yes.  Bootloaders inherently have machine specific knowledge.  They need
>>> to know ram location, board ID, secondary CPU boot protocols, etc.
>>> Requiring the user specify all these things separately from the rest of
>>> the machine description is IMO not acceptable.
>>
>> So what im suggesting here is that machines export these properties to a
>> globally accessible location. Perhaps via the machine opts mechanism? Then
>> we are in a best of both worls situation where machine models do not need
>> bootloader awareness yet bootloaders can still query qemu for ram_size,
>> smp#, board_id and friends.
>
> Hmm, I suppose this might work.  I'm not sure what you think the benefit of
> this is though.  Fact is the machine needs to have bootloader awareness,
> whether it be instantating an object or setting magic variables.
> Having devices rummage around in global state feels messy.  I'd much rather
> use actual properties on the device.  IMO changing the bootloader is similar
> complexity to (say) changing a UART. i.e. it's a board-level change not an
> end-user level change.  Board-level changes are something that will happen
> after QOM conversion, i.e. when we replace machine->init with a board config
> file.

Yeah, basically the variable flow goes:

 vl.c -> machine_opts -> machine_init() -> device properties -> device_init()
 

So that the machine init function that creates the bootloader device enumerates the machine_opts (just like is done in Peter's patches) and then passes those on to the bootloader device as device properties.

So in patch 4/4 in Peters series where he adds a new bootloader feature (the -dtb switch) its done slightly differently, the machine model does not handle the machine_opts at all, i.e. The machine model has no awareness of this dtb argument. Instead the arm boot loader directly queries the machine_opts API itself:

@@ -251,6 +317,9 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info)
        exit(1);
    }

+    info->dtb_filename = qemu_opt_get(qemu_opts_find(qemu_find_opts("machine"),
+                                                     0), "dtb");
+

There is no path through the machine_init for this particular property.

Ah, I see. So he derived from the original proposal, oh well :).

What I am suggesting is that a similar approach is take for machine model set properties (such as ram_size), but instead of the command line setting the props its done by the machine model. The machine model qemu_opt_set() the ram_size = whatever. Then the bootloader qemu_opt_get()s it. If you did this for the key properties related to boot then you would remove the need for machines to have awareness of their boot process.

But that's exactly what we want. We want the machine to be aware of its boot process, because that's the one place that needs to instantiate the boot device, no?

Alex