Re: [Qemu-devel] [PATCH 00/15] qdev: make reset semantics more clear and consistent, reset qbuses under virtio devices

[Anthony Liguori <aliguori@us.ibm.com>]

Paolo Bonzini <pbonzini@redhat.com> writes:

> After discussion with mst on the topic of resetting virtio devices,
> here is a series that hopefully clarifies the semantics of bus and
> device resets.
>
> After this series, there are two kinds of resets:
>
> 1) device-level reset is the kind of reset that you get with a register
> write on the device.  It will clear interrupts and DMAs among other things,
> but not any bus-level state, for example it will not clear PCI BARs and
> other configuration space data.  It is done with qdev_reset_all.
>
> 2) bus-level reset is the kind of reset that you get with a register
> write on the device that exports the bus (including triggering a device-level
> reset on the device that exports the bus).  It will do a device-level
> reset on the child, but also clear bus-level state such as PCI BARs and
> other configuration space data.  It can be triggered for all devices
> on a bus with qbus_reset_all.  There is still no API for a bus-level
> reset of a single device (like PCI FLR), this can be added later.

I don't really understand this dual abstraction.  I suspect it's
overgeneralizing something that's the result of poor modeling.

Very often with PCI devices, you have an otherwise independent chipset
embedded on a PCI card  There's a clear separate between the chipset and
the card.

It may be possible to poke the chipset directly to do a reset and that
may only affect state that's on the chipset but not any card-specific
state.

But this has nothing to do with busses, this is just about
encapsulation.  IOW, what you have is:

E1000 is-a PCIDevice
  has-a E1000 chipset
  PCIDevice::reset()
    -> calls chipset->reset()

But with the right register write, chipset->reset() can be called w/o
PCIDevice::reset().  But again, this has nothing to do with busses.

What I'm missing with this series is what problem are we trying to
solve?  I don't think we model reset correctly today because I don't
think there's a single notion of reset.

I think reset really ought to just be a bus level concept with
individual implementations for each bus.

Regards,

Anthony Liguori

>
> Patches 1-5 are miscellaneous fixes to the reset paths.
>
> Patches 6-8 introduce qbus_reset_all and uses it.
>
> Patches 9-12 switch qdev_reset_all and qbus_reset_all from pre-order
> to post-order, and document the resulting semantics.
>
> Finally, patches 13-15 adjust the virtio implementations to use qdev
> device-level reset more extensively.
>
> Paolo Bonzini (15):
>   qdev: do not reset a device until the parent has been initialized
>   intel-hda: do not reset codecs from intel_hda_reset
>   pci: clean up resetting of IRQs
>   virtio-pci: reset device before PCI layer
>   virtio-s390: add a reset function to virtio-s390 devices
>   qdev: add qbus_reset_all
>   pci: do not export pci_bus_reset
>   lsi: use qbus_reset_all to reset SCSI bus
>   qdev: allow both pre- and post-order vists in qdev walking functions
>   qdev: switch reset to post-order
>   qdev: remove device_reset
>   qdev: document reset semantics
>   virtio-pci: reset all qbuses too when writing to the status field
>   virtio-s390: reset all qbuses too when writing to the status field
>   virtio-serial: do not perform bus reset by hand
>
>  hw/intel-hda.c         |  1 -
>  hw/lsi53c895a.c        |  7 +----
>  hw/pci.c               | 16 ++++------
>  hw/pci.h               |  1 -
>  hw/pci_bridge.c        |  2 +-
>  hw/qdev-core.h         | 83 ++++++++++++++++++++++++++++++++++++++++++--------
>  hw/qdev.c              | 70 +++++++++++++++++++++++++++---------------
>  hw/s390-virtio-bus.c   | 16 +++++++++-
>  hw/virtio-pci.c        | 27 +++++++---------
>  hw/virtio-serial-bus.c | 19 +++---------
>  10 files changed, 156 insertions(+), 86 deletions(-)
>
> -- 
> 1.8.0.2