Re: [Qemu-devel] [PATCH v8 4/5] generic-loader: Add a generic loader

[Peter Maydell]

On 13 July 2016 at 18:45, Alistair Francis <address@hidden> wrote:
> On Tue, Jul 12, 2016 at 9:39 AM, Peter Maydell <address@hidden> wrote:
>> On 2 July 2016 at 02:07, Alistair Francis <address@hidden> wrote:
>>> Add a generic loader to QEMU which can be used to load images or set
>>> memory values.
>>>
>>> Signed-off-by: Alistair Francis <address@hidden>
>>> ---

>>> +    if (s->cpu) {
>>> +        CPUClass *cc = CPU_GET_CLASS(s->cpu);
>>> +        cpu_reset(s->cpu);
>>
>> If something else sets the PC later on this gets lost, which is
>> ugly and fragile. It's kinda-sorta OK that we bodge this for things
>> like the hw/arm/boot.c code because all the pieces are inside QEMU,
>> but when you start adding in command line devices I'm a bit
>> nervous. Maybe we should actually figure out our reset order
>> woes properly ?
>
> This is up to the user to specify the commands in the order they want
> them applied. What other method should we be using?

Well, if it works it works, I guess, but one day we
will need to figure out an actual model for reset order
rather than having it work by chance (in this case,
because command line devices get reset after ones in
the board model proper, I think.)

>>> +    qemu_register_reset(generic_loader_reset, dev);
>>
>> What's wrong with a device reset function set via dc->reset ?
>
> This allows setting values from the HMP command line interface once
> the machine is running. The dc->reset isn't applied in that case.

I don't understand this -- could you explain in a bit
more detail, please?

>> Changing that after this goes into the tree would be a command
>> line compatibility break, so this inclines me to saying that this
>> isn't baked enough for 2.7 and we should aim to put it into 2.8.
>
> That's fair, but upsetting.

I'm not really happy about pushing this back yet another
release either, and there are two weeks left til hard
freeze. So it's not impossible that we could fit it in,
but we're still discussing the semantics of the device
at this point, so if it did get into 2.7 I think it
would be ending up going in really fairly late, and I
tend by preference to be conservative about freezes.
Wider review (ie somebody else as well as me) would help
in providing reassurance that the interface is correct.

thanks
-- PMM