| Message ID | 20191018150630.31099-6-damien.hedde@greensocs.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Multi-phase reset mechanism | expand |
On Fri, 18 Oct 2019 at 16:07, Damien Hedde <damien.hedde@greensocs.com> wrote: > > Add a function resettable_change_parent() to do the required > plumbing when changing the parent a of Resettable object. > > We need to make sure that the reset state of the object remains > coherent with the reset state of the new parent. > > We make the 2 following hypothesis: > + when an object is put in a parent under reset, the object goes in > reset. > + when an object is removed from a parent under reset, the object > leaves reset. > > The added function avoid any glitch if both old and new parent are > already in reset. > > Signed-off-by: Damien Hedde <damien.hedde@greensocs.com> > --- > hw/core/resettable.c | 54 +++++++++++++++++++++++++++++++++++++++++ > hw/core/trace-events | 1 + > include/hw/resettable.h | 16 ++++++++++++ > 3 files changed, 71 insertions(+) > > diff --git a/hw/core/resettable.c b/hw/core/resettable.c > index c5e11cff4f..60d4285fcc 100644 > --- a/hw/core/resettable.c > +++ b/hw/core/resettable.c > @@ -32,6 +32,14 @@ static void resettable_phase_exit(Object *obj, void *opaque, ResetType type); > */ > static bool enter_phase_in_progress; > > +/** > + * exit_phase_in_progress: > + * Flag telling whether we are currently in an enter phase where side > + * effects are forbidden. This flag allows us to catch if > + * resettable_change_parent() is called during exit phase. > + */ > +static unsigned exit_phase_in_progress; This is another global that I don't think we should have. Is it also just for asserts ? > + > void resettable_reset(Object *obj, ResetType type) > { > trace_resettable_reset(obj, type); > @@ -58,7 +66,9 @@ void resettable_release_reset(Object *obj, ResetType type) > /* TODO: change that assert when adding support for other reset types */ > assert(type == RESET_TYPE_COLD); > trace_resettable_reset_release_begin(obj, type); > + exit_phase_in_progress += 1; > resettable_phase_exit(obj, NULL, type); > + exit_phase_in_progress -= 1; > trace_resettable_reset_release_end(obj); > } > > @@ -198,6 +208,50 @@ static void resettable_phase_exit(Object *obj, void *opaque, ResetType type) > trace_resettable_phase_exit_end(obj, obj_typename, s->count); > } > > +/* > + * resettable_get_count: > + * Get the count of the Resettable object @obj. Return 0 if @obj is NULL. > + */ > +static uint32_t resettable_get_count(Object *obj) > +{ > + if (obj) { > + ResettableClass *rc = RESETTABLE_GET_CLASS(obj); > + return rc->get_state(obj)->count; > + } > + return 0; > +} > + > +void resettable_change_parent(Object *obj, Object *newp, Object *oldp) > +{ > + ResettableClass *rc = RESETTABLE_GET_CLASS(obj); > + ResettableState *s = rc->get_state(obj); > + uint32_t newp_count = resettable_get_count(newp); > + uint32_t oldp_count = resettable_get_count(oldp); > + > + assert(!enter_phase_in_progress && !exit_phase_in_progress); > + trace_resettable_change_parent(obj, oldp, oldp_count, newp, newp_count); > + > + /* > + * At most one of the two 'for' loop will be executed below "loops" > + * in order to cope with the diff between the two count. "difference". "counts". > + */ > + /* if newp is more reset than oldp */ > + for (uint32_t i = oldp_count; i < newp_count; i++) { > + resettable_assert_reset(obj, RESET_TYPE_COLD); > + } > + /* > + * if obj is leaving a bus under reset, we need to ensure > + * hold phase is not pending. > + */ > + if (oldp_count && s->hold_phase_pending) { > + resettable_phase_hold(obj, NULL, RESET_TYPE_COLD); > + } > + /* if oldp is more reset than newp */ > + for (uint32_t i = newp_count; i < oldp_count; i++) { > + resettable_release_reset(obj, RESET_TYPE_COLD); > + } > +} > + > void resettable_class_set_parent_phases(ResettableClass *rc, > ResettableEnterPhase enter, > ResettableHoldPhase hold, > diff --git a/hw/core/trace-events b/hw/core/trace-events > index 66081d0fb4..6d03ef7ff9 100644 > --- a/hw/core/trace-events > +++ b/hw/core/trace-events > @@ -16,6 +16,7 @@ resettable_reset_assert_begin(void *obj, int cold) "obj=%p cold=%d" > resettable_reset_assert_end(void *obj) "obj=%p" > resettable_reset_release_begin(void *obj, int cold) "obj=%p cold=%d" > resettable_reset_release_end(void *obj) "obj=%p" > +resettable_change_parent(void *obj, void *o, uint32_t oc, void *n, uint32_t nc) "obj=%p from=%p(%" PRIu32 ") to=%p(%" PRIu32 ")" > resettable_phase_enter_begin(void *obj, const char *objtype, uint32_t count, int type) "obj=%p(%s) count=%" PRIu32 " type=%d" > resettable_phase_enter_exec(void *obj, const char *objtype, int type, int has_method) "obj=%p(%s) type=%d method=%d" > resettable_phase_enter_end(void *obj, const char *objtype, uint32_t count) "obj=%p(%s) count=%" PRIu32 > diff --git a/include/hw/resettable.h b/include/hw/resettable.h > index 6b24e1633b..f6d379669f 100644 > --- a/include/hw/resettable.h > +++ b/include/hw/resettable.h > @@ -182,6 +182,22 @@ void resettable_release_reset(Object *obj, ResetType type); > */ > bool resettable_is_in_reset(Object *obj); > > +/** > + * resettable_change_parent: > + * Indicate that the parent of Ressettable @obj change from @oldp to @newp. "is changing from" > + * All 3 objects must implements resettable interface. @oldp or @newp may be "implement" > + * NULL. > + * > + * This function will adapt the reset state of @obj so that it is coherent > + * with the reset state of @newp. It may trigger @resettable_assert_reset() > + * or @resettable_release_reset(). It will do such things only if the reset > + * state of @newp and @oldp are different. > + * > + * When using this function during reset, it must only be called during > + * an hold phase method. Calling this during enter or exit phase is an error. "a hold phase" > + */ > +void resettable_change_parent(Object *obj, Object *newp, Object *oldp); > + thanks -- PMM
On 11/29/19 7:38 PM, Peter Maydell wrote: > On Fri, 18 Oct 2019 at 16:07, Damien Hedde <damien.hedde@greensocs.com> wrote: >> >> Add a function resettable_change_parent() to do the required >> plumbing when changing the parent a of Resettable object. >> >> We need to make sure that the reset state of the object remains >> coherent with the reset state of the new parent. >> >> We make the 2 following hypothesis: >> + when an object is put in a parent under reset, the object goes in >> reset. >> + when an object is removed from a parent under reset, the object >> leaves reset. >> >> The added function avoid any glitch if both old and new parent are >> already in reset. >> >> Signed-off-by: Damien Hedde <damien.hedde@greensocs.com> >> --- >> hw/core/resettable.c | 54 +++++++++++++++++++++++++++++++++++++++++ >> hw/core/trace-events | 1 + >> include/hw/resettable.h | 16 ++++++++++++ >> 3 files changed, 71 insertions(+) >> >> diff --git a/hw/core/resettable.c b/hw/core/resettable.c >> index c5e11cff4f..60d4285fcc 100644 >> --- a/hw/core/resettable.c >> +++ b/hw/core/resettable.c >> @@ -32,6 +32,14 @@ static void resettable_phase_exit(Object *obj, void *opaque, ResetType type); >> */ >> static bool enter_phase_in_progress; >> >> +/** >> + * exit_phase_in_progress: >> + * Flag telling whether we are currently in an enter phase where side >> + * effects are forbidden. This flag allows us to catch if >> + * resettable_change_parent() is called during exit phase. >> + */ >> +static unsigned exit_phase_in_progress; > > This is another global that I don't think we should have. > Is it also just for asserts ? Yes. It's only to ensure we don't miss-use the api. -- Damien
diff --git a/hw/core/resettable.c b/hw/core/resettable.c index c5e11cff4f..60d4285fcc 100644 --- a/hw/core/resettable.c +++ b/hw/core/resettable.c @@ -32,6 +32,14 @@ static void resettable_phase_exit(Object *obj, void *opaque, ResetType type); */ static bool enter_phase_in_progress; +/** + * exit_phase_in_progress: + * Flag telling whether we are currently in an enter phase where side + * effects are forbidden. This flag allows us to catch if + * resettable_change_parent() is called during exit phase. + */ +static unsigned exit_phase_in_progress; + void resettable_reset(Object *obj, ResetType type) { trace_resettable_reset(obj, type); @@ -58,7 +66,9 @@ void resettable_release_reset(Object *obj, ResetType type) /* TODO: change that assert when adding support for other reset types */ assert(type == RESET_TYPE_COLD); trace_resettable_reset_release_begin(obj, type); + exit_phase_in_progress += 1; resettable_phase_exit(obj, NULL, type); + exit_phase_in_progress -= 1; trace_resettable_reset_release_end(obj); } @@ -198,6 +208,50 @@ static void resettable_phase_exit(Object *obj, void *opaque, ResetType type) trace_resettable_phase_exit_end(obj, obj_typename, s->count); } +/* + * resettable_get_count: + * Get the count of the Resettable object @obj. Return 0 if @obj is NULL. + */ +static uint32_t resettable_get_count(Object *obj) +{ + if (obj) { + ResettableClass *rc = RESETTABLE_GET_CLASS(obj); + return rc->get_state(obj)->count; + } + return 0; +} + +void resettable_change_parent(Object *obj, Object *newp, Object *oldp) +{ + ResettableClass *rc = RESETTABLE_GET_CLASS(obj); + ResettableState *s = rc->get_state(obj); + uint32_t newp_count = resettable_get_count(newp); + uint32_t oldp_count = resettable_get_count(oldp); + + assert(!enter_phase_in_progress && !exit_phase_in_progress); + trace_resettable_change_parent(obj, oldp, oldp_count, newp, newp_count); + + /* + * At most one of the two 'for' loop will be executed below + * in order to cope with the diff between the two count. + */ + /* if newp is more reset than oldp */ + for (uint32_t i = oldp_count; i < newp_count; i++) { + resettable_assert_reset(obj, RESET_TYPE_COLD); + } + /* + * if obj is leaving a bus under reset, we need to ensure + * hold phase is not pending. + */ + if (oldp_count && s->hold_phase_pending) { + resettable_phase_hold(obj, NULL, RESET_TYPE_COLD); + } + /* if oldp is more reset than newp */ + for (uint32_t i = newp_count; i < oldp_count; i++) { + resettable_release_reset(obj, RESET_TYPE_COLD); + } +} + void resettable_class_set_parent_phases(ResettableClass *rc, ResettableEnterPhase enter, ResettableHoldPhase hold, diff --git a/hw/core/trace-events b/hw/core/trace-events index 66081d0fb4..6d03ef7ff9 100644 --- a/hw/core/trace-events +++ b/hw/core/trace-events @@ -16,6 +16,7 @@ resettable_reset_assert_begin(void *obj, int cold) "obj=%p cold=%d" resettable_reset_assert_end(void *obj) "obj=%p" resettable_reset_release_begin(void *obj, int cold) "obj=%p cold=%d" resettable_reset_release_end(void *obj) "obj=%p" +resettable_change_parent(void *obj, void *o, uint32_t oc, void *n, uint32_t nc) "obj=%p from=%p(%" PRIu32 ") to=%p(%" PRIu32 ")" resettable_phase_enter_begin(void *obj, const char *objtype, uint32_t count, int type) "obj=%p(%s) count=%" PRIu32 " type=%d" resettable_phase_enter_exec(void *obj, const char *objtype, int type, int has_method) "obj=%p(%s) type=%d method=%d" resettable_phase_enter_end(void *obj, const char *objtype, uint32_t count) "obj=%p(%s) count=%" PRIu32 diff --git a/include/hw/resettable.h b/include/hw/resettable.h index 6b24e1633b..f6d379669f 100644 --- a/include/hw/resettable.h +++ b/include/hw/resettable.h @@ -182,6 +182,22 @@ void resettable_release_reset(Object *obj, ResetType type); */ bool resettable_is_in_reset(Object *obj); +/** + * resettable_change_parent: + * Indicate that the parent of Ressettable @obj change from @oldp to @newp. + * All 3 objects must implements resettable interface. @oldp or @newp may be + * NULL. + * + * This function will adapt the reset state of @obj so that it is coherent + * with the reset state of @newp. It may trigger @resettable_assert_reset() + * or @resettable_release_reset(). It will do such things only if the reset + * state of @newp and @oldp are different. + * + * When using this function during reset, it must only be called during + * an hold phase method. Calling this during enter or exit phase is an error. + */ +void resettable_change_parent(Object *obj, Object *newp, Object *oldp); + /** * resettable_class_set_parent_phases: *
Add a function resettable_change_parent() to do the required plumbing when changing the parent a of Resettable object. We need to make sure that the reset state of the object remains coherent with the reset state of the new parent. We make the 2 following hypothesis: + when an object is put in a parent under reset, the object goes in reset. + when an object is removed from a parent under reset, the object leaves reset. The added function avoid any glitch if both old and new parent are already in reset. Signed-off-by: Damien Hedde <damien.hedde@greensocs.com> --- hw/core/resettable.c | 54 +++++++++++++++++++++++++++++++++++++++++ hw/core/trace-events | 1 + include/hw/resettable.h | 16 ++++++++++++ 3 files changed, 71 insertions(+)