| Message ID | 20200115123620.250132-11-damien.hedde@greensocs.com |
|---|---|
| State | New |
| Headers | show |
| Series | Multi-phase reset mechanism | expand |
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 <damien.hedde@greensocs.com> > Reviewed-by: Peter Maydell <peter.maydell@linaro.org> > Reviewed-by: Richard Henderson <richard.henderson@linaro.org> > --- > > 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? Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > vl.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/vl.c b/vl.c > index 751401214c..e5a537d4f9 100644 > --- a/vl.c > +++ b/vl.c > @@ -4362,7 +4362,15 @@ int main(int argc, char **argv, char **envp) > > /* TODO: once all bus devices are qdevified, this should be done > * when bus is created by qdev.c */ > - qemu_register_reset(qbus_reset_all_fn, sysbus_get_default()); > + /* > + * TODO: If we had a main 'reset container' that the whole system > + * lived in, we could reset that using the multi-phase reset > + * APIs. For the moment, we just reset the sysbus, which will cause > + * all devices hanging off it (and all their child buses, recursively) > + * to be reset. Note that this will *not* reset any Device objects > + * which are not attached to some part of the qbus tree! > + */ > + qemu_register_reset(resettable_cold_reset_fn, sysbus_get_default()); > qemu_run_machine_init_done_notifiers(); > > if (rom_check_and_register_reset() != 0) { >
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 <damien.hedde@greensocs.com> >> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> >> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> >> --- >> >> 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
diff --git a/vl.c b/vl.c index 751401214c..e5a537d4f9 100644 --- a/vl.c +++ b/vl.c @@ -4362,7 +4362,15 @@ int main(int argc, char **argv, char **envp) /* TODO: once all bus devices are qdevified, this should be done * when bus is created by qdev.c */ - qemu_register_reset(qbus_reset_all_fn, sysbus_get_default()); + /* + * TODO: If we had a main 'reset container' that the whole system + * lived in, we could reset that using the multi-phase reset + * APIs. For the moment, we just reset the sysbus, which will cause + * all devices hanging off it (and all their child buses, recursively) + * to be reset. Note that this will *not* reset any Device objects + * which are not attached to some part of the qbus tree! + */ + qemu_register_reset(resettable_cold_reset_fn, sysbus_get_default()); qemu_run_machine_init_done_notifiers(); if (rom_check_and_register_reset() != 0) {