Re: [Qemu-devel] [PATCH v3 01/33] Create Resettable QOM interface

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 4446 bytes --]

On Thu, Aug 01, 2019 at 11:35:20AM +0200, Damien Hedde wrote:
> 
> 
> On 7/31/19 7:46 AM, David Gibson wrote:
> > On Tue, Jul 30, 2019 at 04:08:59PM +0200, Damien Hedde wrote:
> >>
> >> On 7/30/19 3:59 PM, Peter Maydell wrote:
> >>> On Tue, 30 Jul 2019 at 14:56, Cornelia Huck <cohuck@redhat.com> wrote:
> >>>>
> >>>> On Tue, 30 Jul 2019 14:44:21 +0100
> >>>> Peter Maydell <peter.maydell@linaro.org> wrote:
> >>>>
> >>>>> On Tue, 30 Jul 2019 at 14:42, Cornelia Huck <cohuck@redhat.com> wrote:
> >>>>>> I'm having a hard time figuring out what a 'cold' or a 'warm' reset is
> >>>>>> supposed to be... can you add a definition/guideline somewhere?
> >>>>>
> >>>>> Generally "cold" reset is "power on" and "warm" is "we were already
> >>>>> powered-on, but somebody flipped a reset line somewhere".
> >>>>
> >>>> Ok, that makes sense... my main concern is to distinguish that in a
> >>>> generic way, as it is a generic interface. What about adding something
> >>>> like:
> >>>>
> >>>> "A 'cold' reset means that the object to be reset is initially reset; a 'warm'
> >>>> reset means that the object to be reset has already been initialized."
> >>>>
> >>>> Or is that again too generic?
> >>>
> >>> I think it doesn't quite capture the idea -- an object can have already
> >>> been reset and then get a 'cold' reset: this is like having a powered-on
> >>> machine and then power-cycling it.
> >>>
> >>> The 'warm' reset is the vaguer one, because the specific behaviour
> >>> is somewhat device-dependent (many devices might not have any
> >>> difference from 'cold' reset, for those that do the exact detail
> >>> of what doesn't get reset on warm-reset will vary). But every
> >>> device should have some kind of "as if you power-cycled it" (or
> >>> for QEMU, "go back to the same state as if you just started QEMU on the
> >>> command line"). Our current "reset" method is really cold-reset.
> >>>
> >>
> >> Exactly. In the following patches, I've tried to replace existing reset
> >> calls by cold or warm reset depending on whether:
> >> + it is called through the main system reset -> cold
> >> + it is called during normal life-time       -> warm
> >>
> >> But I definitely can add some docs/comments to better explain that.
> > 
> > Hrm, that helps, but it still seems pretty vague to me.
> > 
> > It's not really my call, but building the concept of warm versus cold
> > resets into such a generic interface seems pretty dubios to me.  While
> > it's moderately common for things to have a notion of warm versus cold
> > reset it's certainly not universal.  There are many devices where
> > there's no meaningful difference between the two.  There are some
> > devices where there are > 2 different types of reset suitable for
> > various different situations.
> > 
> > Is there any way this could work with it usually just presenting the
> > cold reset (which is the closest to a universal concept), and any warm
> > or additional resets are handled buy a different instance of the
> > Resettable interface?
> > 
> 
> In my current implementation, cold/warm is only a question of setting a
> flag before calling the reset methods. I rely and the reset methods to
> check that flag if necessary. The feature can be added/removed without
> pain (if we stick with device_reset to do a cold one). So if it's
> better, I can put it aside for now.
> 
> IMO handling warm reset with another interface is probably not a good
> idea because it will need another load of methods.

I was thinking of a different instance of the same interface, not a
new interface.  But on consideration that probably means dummy objects
so that's also ugly.


Here's another way to look at things though: I can see why a common
interface for cold resets is useful; we use it during system resets
and bus resets and so forth.  But AIUI, in order to do a warm reset
whatever is initiating it is going to need to know what a warm reset
means for this device - in which case it doesn't really need a uniform
interface for it.

So, would we be better off with helper functions to make implementing
a multi-phase reset easy for devices that do have one or more notions
of a warm reset.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]