Re: [PATCH v7 10/11] vl: replace deprecated qbus_reset_all registration

[Damien Hedde]

On 1/16/20 12:44 AM, Philippe Mathieu-Daudé wrote:
> On 1/15/20 1:36 PM, Damien Hedde wrote:
>> Replace deprecated qbus_reset_all by resettable_cold_reset_fn for
>> the sysbus reset registration.
>>
>> Apart for the raspi machines, this does not impact the behavior
>> because:
>> + at this point resettable just calls the old reset methods of devices
>>    and buses in the same order as qdev/qbus.
>> + resettable handlers registered with qemu_register_reset are
>>    serialized; there is no interleaving.
>> + eventual explicit calls to legacy reset API (device_reset or
>>    qdev/qbus_reset) inside this reset handler will not be masked out
>>    by resettable mechanism; they do not go through resettable api.
>>
>> For the raspi machines, during the sysbus reset the sd-card is not
>> reset twice anymore but only once. This is a consequence of switching
>> both sysbus reset and changing parent to resettable; it detects the
>> second reset is not needed. This has no impact on the state after
>> reset; the sd-card reset method only reset local state and query
>> information from the block backend.
>>
>> Signed-off-by: Damien Hedde <address@hidden>
>> Reviewed-by: Peter Maydell <address@hidden>
>> Reviewed-by: Richard Henderson <address@hidden>
>> ---
>>
>> The raspi reset change can be observed by using the following command
>> (reset will occurs, then do Ctrl-C to end qemu; no firmware is
>> given here).
>> qemu-system-aarch64 -M raspi3 \
>>      -trace resettable_phase_hold_exec \
>>      -trace qdev_update_parent_bus \
>>      -trace resettable_change_parent \
>>      -trace qdev_reset -trace qbus_reset
>>
>> Before the patch, the qdev/qbus_reset traces show when reset method are
>> called. After the patch, the resettable_phase_hold_exec show when reset
>> method are called.
>>
>> The traced reset order of the raspi3 is listed below. I've added empty
>> lines and the tree structure.
>>
>>   +->bcm2835-peripherals reset
>>   |
>>   |       +->sd-card reset
>>   |   +->sd-bus reset
>>   +->bcm2835_gpio reset
>>   |      -> dev_update_parent_bus (move the sd-card on the sdhci-bus)
>>   |      -> resettable_change_parent
>>   |
>>   +->bcm2835-dma reset
>>   |
>>   |   +->bcm2835-sdhost-bus reset
>>   +->bcm2835-sdhost reset
>>   |
>>   |       +->sd-card (reset ONLY BEFORE BEFORE THE PATCH)
>>   |   +->sdhci-bus reset
>>   +->generic-sdhci reset
>>   |
>>   +->bcm2835-rng reset
>>   +->bcm2835-property reset
>>   +->bcm2835-fb reset
>>   +->bcm2835-mbox reset
>>   +->bcm2835-aux reset
>>   +->pl011 reset
>>   +->bcm2835-ic reset
>>   +->bcm2836-control reset
>> System reset
>>
>> In both case, the sd-card is reset (being on bcm2835_gpio/sd-bus) then
>> moved
>> to generic-sdhci/sdhci-bus by the bcm2835_gpio reset method.
>>
>> Before the patch, it is then reset again being part of
>> generic-sdhci/sdhci-bus.
>> After the patch, it considered again for reset but its reset method is
>> not
>> called because it is already flagged as reset.
> 
> I find this information helpful, have you considered including it in the
> description?

I wasn't sure, I'll add it since I've to respin anyway to fix patch 3.

Thanks,
Damien