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

[Alistair Francis]

On Wed, Jul 13, 2016 at 2:09 PM, Peter Maydell <address@hidden> wrote:
> On 13 July 2016 at 21:30, Alistair Francis <address@hidden> wrote:
>> On Wed, Jul 13, 2016 at 12:44 PM, Peter Maydell
>> <address@hidden> wrote:
>>> 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:
>>>>>> +    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?
>>
>> So this loading system is just a device, which means that it can be
>> loaded whenever a normal device would be loaded. The more interesting
>> option is using this via the command line but it is also possible to
>> use the QEMU Monitor once QEMU is running to call this.
>>
>> A user can use the device_add function to add set values once QEMU is
>> running. The problem with that though is that the qdev_device_add()
>> function doesn't connect the reset function. I initially had a patch
>> to do this "qdev-monitor.c: Register reset function if the device has
>> one" but it was decided to just register the reset function in the
>> realise instead.
>
> I'm really surprised that device_add doesn't wire up the device's
> reset method. This sounds to me like it's a bug -- adding a device
> dynamically via the monitor shouldn't behave any differently
> to adding it via the command line.

I thought the same as you, but there was a general consensus that
registering the reset there was not correct as there are issues with
bus-less devices. I think this comes back to the infrastructure not
being ready to connect all devices via the device_add, hopefully in
the future this can be improved on.

>
> If we are doing this to avoid a bug (and it's too risky
> to fix that bug at this point) then we should have a comment
> explaining why we're not using dc->reset, so we can fix this
> when we do fix device_add.

I wouldn't call it a bug, I would call it more of a limitation. I'll
add a comment.

Thanks,

Alistair

>
> thanks
> -- PMM
>