mbox series

[v8,00/11] Multi-phase reset mechanism

Message ID 20200123132823.1117486-1-damien.hedde@greensocs.com
Headers show
Series Multi-phase reset mechanism | expand

Message

Damien Hedde Jan. 23, 2020, 1:28 p.m. UTC
Hi all,

The purpose of this series is to split the current reset procedure
into multiple phases. This will help to solve some ordering
difficulties we have during reset.

This is a ready to merge version. I've taken the few remarks of
Philippe about v7 in account. Thanks to him for all the tests he did.

This series adds resettable interface and transitions base Device and
Bus classes (sysbus subclasses are ok too). It provides new reset
functions but does not switch anymore the old functions
(device_reset() and qdev/qbus_reset_all()) to resettable interface.
These functions keep the exact same behavior as before.

The series also transition the main reset handlers registration which
has no impact until devices and buses are transitioned.

The series is organized as follows:
Patch 1 prepare the reset transition. Patch 2 adds some utility trace
events. Patches 3 to 8 adds resettable api in devices and buses. Patch
9 adds some documentation. Patches 10 and 11 transition the call sites
of qemu_register_reset(qdev/qbus_reset_all_fn, ...).

After this series, the plan is then to transition devices, buses and
legacy reset call sites. Devices and buses have to be transitioned
from mother class to daughter classes order but until the final
(daughter) class is transitioned, old monolitic reset behavior will
be kept for this class.

Thanks,
Damien

v8:
  + patch 3: fix trace-events-subdirs += hw/core double entry (Philippe)
  + patch 3&5: ResettableState::count type from uint32_t to unsigned
    (Philippe)

v7:
https://lists.gnu.org/archive/html/qemu-devel/2020-01/msg02923.html
  + patch 1
    - new device_reset occurrence (rebase)
  + patch 3
    - ResettablePhases un-nest (Richard)
    - warnings (Richard)
  + patch 7
    - inline resettable_state_clear (Richard)

v6:
https://lists.gnu.org/archive/html/qemu-devel/2019-12/msg04664.html
  + patch 1
    - update (microvm machine) (rebase)
  + patch 2
    - added more info in trace_qdev_update_parent_bus (Philippe)
  + patch 3
    - typos (Peter)
    - missing gpl header in header (Peter)
    - clarify global entry_in_progress flag (Peter)
  + patch 4
    -  parent/child terminology (Peter)
  + patch 5
    - typos (Peter)
    - comment about global exit_in_progress flag (Peter)
  + patch 6
    -  update list of qdev_set_parent_bus() call sites (rebase)
  + patch 7
    - clear reset state in realize before doing the hotplug reset
  + patch 9
    - added entry in index.rst
    - fix commit message and various improvements (Peter)
    - do parent phase method before child actions (Peter)
    - clarify "Polling the reset state" paragraph 
  + patch 12&13
    - removed (follow-up series to cleanup the raspi sd)

v5:
https://lists.gnu.org/archive/html/qemu-devel/2019-10/msg04730.html
  + various improvement in the resettable interface (regarding
    transition, robustness and several reset types)
  + better handling of transition from legacy reset to resettable
  + device hotplug and parent bus 'hot' change support
  + improved doc with examples and converted to rst format

v4:
https://lists.gnu.org/archive/html/qemu-devel/2019-08/msg04359.html

Damien Hedde (11):
  add device_legacy_reset function to prepare for reset api change
  hw/core/qdev: add trace events to help with resettable transition
  hw/core: create Resettable QOM interface
  hw/core: add Resettable support to BusClass and DeviceClass
  hw/core/resettable: add support for changing parent
  hw/core/qdev: handle parent bus change regarding resettable
  hw/core/qdev: update hotplug reset regarding resettable
  hw/core: deprecate old reset functions and introduce new ones
  docs/devel/reset.rst: add doc about Resettable interface
  vl: replace deprecated qbus_reset_all registration
  hw/s390x/ipl: replace deprecated qdev_reset_all registration

 docs/devel/index.rst     |   1 +
 docs/devel/reset.rst     | 289 +++++++++++++++++++++++++++++++++++++
 include/hw/qdev-core.h   |  58 +++++++-
 include/hw/resettable.h  | 247 ++++++++++++++++++++++++++++++++
 hw/audio/intel-hda.c     |   2 +-
 hw/core/bus.c            | 102 +++++++++++++
 hw/core/qdev.c           | 160 +++++++++++++++++++--
 hw/core/resettable.c     | 301 +++++++++++++++++++++++++++++++++++++++
 hw/hyperv/hyperv.c       |   2 +-
 hw/i386/microvm.c        |   2 +-
 hw/i386/pc.c             |   2 +-
 hw/ide/microdrive.c      |   8 +-
 hw/intc/spapr_xive.c     |   2 +-
 hw/ppc/pnv_psi.c         |   4 +-
 hw/ppc/spapr_pci.c       |   2 +-
 hw/ppc/spapr_vio.c       |   2 +-
 hw/s390x/ipl.c           |  10 +-
 hw/s390x/s390-pci-inst.c |   2 +-
 hw/scsi/vmw_pvscsi.c     |   2 +-
 hw/sd/omap_mmc.c         |   2 +-
 hw/sd/pl181.c            |   2 +-
 vl.c                     |  10 +-
 hw/core/Makefile.objs    |   1 +
 hw/core/trace-events     |  27 ++++
 tests/Makefile.include   |   1 +
 25 files changed, 1210 insertions(+), 31 deletions(-)
 create mode 100644 docs/devel/reset.rst
 create mode 100644 include/hw/resettable.h
 create mode 100644 hw/core/resettable.c

Comments

Peter Maydell Jan. 24, 2020, 10:05 a.m. UTC | #1
On Thu, 23 Jan 2020 at 13:28, Damien Hedde <damien.hedde@greensocs.com> wrote:
> v8:
>   + patch 3&5: ResettableState::count type from uint32_t to unsigned
>     (Philippe)

We'll have to change that back if we ever want to migrate
the count (migration insists on fixed-sized types), but
I guess we can do that when we get to it...

-- PMM
Philippe Mathieu-Daudé Jan. 24, 2020, 10:17 a.m. UTC | #2
On 1/24/20 11:05 AM, Peter Maydell wrote:
> On Thu, 23 Jan 2020 at 13:28, Damien Hedde <damien.hedde@greensocs.com> wrote:
>> v8:
>>    + patch 3&5: ResettableState::count type from uint32_t to unsigned
>>      (Philippe)
> 
> We'll have to change that back if we ever want to migrate
> the count (migration insists on fixed-sized types), but
> I guess we can do that when we get to it...

Oh I forgot about migration :( (this was just a suggestion, not a 
requirement).

If you are happy with v7/v8 you can consider to apply v7 instead, the 
only difference is a one-line change in Makefile.objs (which ends no 
modified) and few Tested-by/Reviewed-by tags:

https://patchew.org/QEMU/20200115123620.250132-1-damien.hedde@greensocs.com/diff/20200123132823.1117486-1-damien.hedde@greensocs.com/

Regards,

Phil.
Peter Maydell Jan. 24, 2020, 10:18 a.m. UTC | #3
On Fri, 24 Jan 2020 at 10:17, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> On 1/24/20 11:05 AM, Peter Maydell wrote:
> > On Thu, 23 Jan 2020 at 13:28, Damien Hedde <damien.hedde@greensocs.com> wrote:
> >> v8:
> >>    + patch 3&5: ResettableState::count type from uint32_t to unsigned
> >>      (Philippe)
> >
> > We'll have to change that back if we ever want to migrate
> > the count (migration insists on fixed-sized types), but
> > I guess we can do that when we get to it...
>
> Oh I forgot about migration :( (this was just a suggestion, not a
> requirement).

Migration handling is going to require changes anyway, flipping
the type of the field will just be a minor part of that patch
if/when it arrives. It seems easier to take v8 if it's otherwise OK.

thanks
-- PMM
Damien Hedde Jan. 24, 2020, 10:23 a.m. UTC | #4
On 1/24/20 11:17 AM, Philippe Mathieu-Daudé wrote:
> On 1/24/20 11:05 AM, Peter Maydell wrote:
>> On Thu, 23 Jan 2020 at 13:28, Damien Hedde
>> <damien.hedde@greensocs.com> wrote:
>>> v8:
>>>    + patch 3&5: ResettableState::count type from uint32_t to unsigned
>>>      (Philippe)
>>
>> We'll have to change that back if we ever want to migrate
>> the count (migration insists on fixed-sized types), but
>> I guess we can do that when we get to it...
> 
> Oh I forgot about migration :( (this was just a suggestion, not a
> requirement).

I forgot it too, and I probably put a uint32_t in the first place
because of migration in an earlier version of the patchset...

Damien
Damien Hedde Jan. 27, 2020, 3:20 p.m. UTC | #5
On 1/24/20 11:18 AM, Peter Maydell wrote:
> On Fri, 24 Jan 2020 at 10:17, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>
>> On 1/24/20 11:05 AM, Peter Maydell wrote:
>>> On Thu, 23 Jan 2020 at 13:28, Damien Hedde <damien.hedde@greensocs.com> wrote:
>>>> v8:
>>>>    + patch 3&5: ResettableState::count type from uint32_t to unsigned
>>>>      (Philippe)
>>>
>>> We'll have to change that back if we ever want to migrate
>>> the count (migration insists on fixed-sized types), but
>>> I guess we can do that when we get to it...
>>
>> Oh I forgot about migration :( (this was just a suggestion, not a
>> requirement).
> 
> Migration handling is going to require changes anyway, flipping
> the type of the field will just be a minor part of that patch
> if/when it arrives. It seems easier to take v8 if it's otherwise OK.
> 

Hi Peter,

For me v8 is ok. In which pull queue should it be taken ?

Thanks,
Damien
Peter Maydell Jan. 30, 2020, 2:29 p.m. UTC | #6
On Thu, 23 Jan 2020 at 13:28, Damien Hedde <damien.hedde@greensocs.com> wrote:
>
> Hi all,
>
> The purpose of this series is to split the current reset procedure
> into multiple phases. This will help to solve some ordering
> difficulties we have during reset.
>
> This is a ready to merge version. I've taken the few remarks of
> Philippe about v7 in account. Thanks to him for all the tests he did.
>
> This series adds resettable interface and transitions base Device and
> Bus classes (sysbus subclasses are ok too). It provides new reset
> functions but does not switch anymore the old functions
> (device_reset() and qdev/qbus_reset_all()) to resettable interface.
> These functions keep the exact same behavior as before.
>
> The series also transition the main reset handlers registration which
> has no impact until devices and buses are transitioned.
>
> The series is organized as follows:
> Patch 1 prepare the reset transition. Patch 2 adds some utility trace
> events. Patches 3 to 8 adds resettable api in devices and buses. Patch
> 9 adds some documentation. Patches 10 and 11 transition the call sites
> of qemu_register_reset(qdev/qbus_reset_all_fn, ...).
>
> After this series, the plan is then to transition devices, buses and
> legacy reset call sites. Devices and buses have to be transitioned
> from mother class to daughter classes order but until the final
> (daughter) class is transitioned, old monolitic reset behavior will
> be kept for this class.

Applied to target-arm.next, thanks (it seems the easiest way
to take the series). I made a few minor fixups for textual
conflicts with other patches that have already hit master.

thanks
-- PMM