1. Patchew
  2. QEMU
  3. View series

[PATCH-for 5.2?] hw/core/ressetable: fix reset count decrement

Damien Hedde posted 1 patch 4 years, 11 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20201202164055.30864-1-damien.hedde@greensocs.com
docs/devel/reset.rst | 6 +++---
hw/core/resettable.c | 3 +--
2 files changed, 4 insertions(+), 5 deletions(-)
[PATCH-for 5.2?] hw/core/ressetable: fix reset count decrement
Posted by Damien Hedde 4 years, 11 months ago 
The reset count was only decremented if the device was in a single
reset.

Also move the decrement before calling the exit phase method to fix
problem of reset state evaluation during that call. Update the doc
accordingly.

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
Fixes: 1905297 ("Zynq7000 UART clock reset initialization", 2020-11-23)
Reported-by: Michael Peter <michael.peter@hensoldt-cyber.com>
--

Hi all,

While looking at the bug reported by Michael and his patch. I found another
bug. Apparently I forgot to decrement the reset count if there was several
reset at the same time.

This patch fixes that.

I also moved the place of the decrement: before calling the exit phase method.
it globally fixes Michael's reported bug, as I think it will avoid some boiler
plate code in every exit phase method we do.

Only other place where the reset state is checked is in the
hw/char/cadence-uart.c so it does not have high impact.

I'm not sure if this meets the condition for 5.2 as it changes a documented
feature. In that case we can just accept Michael solution and I'll fix the
rest later.

Here's the pointer for the bug and michael's patch.
https://lists.nongnu.org/archive/html/qemu-devel/2020-11/msg05786.html
https://lists.nongnu.org/archive/html/qemu-devel/2020-11/msg06105.html

Damien
---
 docs/devel/reset.rst | 6 +++---
 hw/core/resettable.c | 3 +--
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/docs/devel/reset.rst b/docs/devel/reset.rst
index abea1102dc..021a7277a2 100644
--- a/docs/devel/reset.rst
+++ b/docs/devel/reset.rst
@@ -210,9 +210,9 @@ Polling the reset state
 Resettable interface provides the ``resettable_is_in_reset()`` function.
 This function returns true if the object parameter is currently under reset.
 
-An object is under reset from the beginning of the *init* phase to the end of
-the *exit* phase. During all three phases, the function will return that the
-object is in reset.
+An object is under reset from the beginning of the *init* phase to the *exit*
+phase. During *init* and *hold* phase only, the function will return that the
+object is in reset. The state is changed just before calling the *exit* method.
 
 This function may be used if the object behavior has to be adapted
 while in reset state. For example if a device has an irq input,
diff --git a/hw/core/resettable.c b/hw/core/resettable.c
index 96a99ce39e..c3df75c6ba 100644
--- a/hw/core/resettable.c
+++ b/hw/core/resettable.c
@@ -201,12 +201,11 @@ static void resettable_phase_exit(Object *obj, void *opaque, ResetType type)
     resettable_child_foreach(rc, obj, resettable_phase_exit, NULL, type);
 
     assert(s->count > 0);
-    if (s->count == 1) {
+    if (--s->count == 0) {
         trace_resettable_phase_exit_exec(obj, obj_typename, !!rc->phases.exit);
         if (rc->phases.exit && !resettable_get_tr_func(rc, obj)) {
             rc->phases.exit(obj);
         }
-        s->count = 0;
     }
     s->exit_phase_in_progress = false;
     trace_resettable_phase_exit_end(obj, obj_typename, s->count);
-- 
2.29.2
Re: [PATCH-for 5.2?] hw/core/ressetable: fix reset count decrement
Posted by Philippe Mathieu-Daudé 4 years, 11 months ago 
On 12/2/20 5:40 PM, Damien Hedde wrote:
> The reset count was only decremented if the device was in a single
> reset.
> 
> Also move the decrement before calling the exit phase method to fix
> problem of reset state evaluation during that call. Update the doc
> accordingly.
> 
> Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
> Fixes: 1905297 ("Zynq7000 UART clock reset initialization", 2020-11-23)

$ git show 1905297
fatal: ambiguous argument '1905297': unknown revision or path not in the
working tree.

Beside, typo ressetable -> resettable in subject.

> Reported-by: Michael Peter <michael.peter@hensoldt-cyber.com>
> --
> 
> Hi all,
> 
> While looking at the bug reported by Michael and his patch. I found another
> bug. Apparently I forgot to decrement the reset count if there was several
> reset at the same time.
> 
> This patch fixes that.
> 
> I also moved the place of the decrement: before calling the exit phase method.
> it globally fixes Michael's reported bug, as I think it will avoid some boiler
> plate code in every exit phase method we do.
> 
> Only other place where the reset state is checked is in the
> hw/char/cadence-uart.c so it does not have high impact.
> 
> I'm not sure if this meets the condition for 5.2 as it changes a documented
> feature. In that case we can just accept Michael solution and I'll fix the
> rest later.
> 
> Here's the pointer for the bug and michael's patch.
> https://lists.nongnu.org/archive/html/qemu-devel/2020-11/msg05786.html
> https://lists.nongnu.org/archive/html/qemu-devel/2020-11/msg06105.html
> 
> Damien
Re: [PATCH-for 5.2?] hw/core/ressetable: fix reset count decrement
Posted by Damien Hedde 4 years, 11 months ago 

On 12/2/20 6:10 PM, Philippe Mathieu-Daudé wrote:
> On 12/2/20 5:40 PM, Damien Hedde wrote:
>> The reset count was only decremented if the device was in a single
>> reset.
>>
>> Also move the decrement before calling the exit phase method to fix
>> problem of reset state evaluation during that call. Update the doc
>> accordingly.
>>
>> Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
>> Fixes: 1905297 ("Zynq7000 UART clock reset initialization", 2020-11-23)
> 

> $ git show 1905297
> fatal: ambiguous argument '1905297': unknown revision or path not in the
> working tree.

I put Michael's bug number there. Should I put the incriminated commit
instead ?

> 
> Beside, typo ressetable -> resettable in subject.

Thanks,
Damien

Cc'ing michael new address too

> 
>> Reported-by: Michael Peter <michael.peter@hensoldt-cyber.com>
>> --
>>
>> Hi all,
>>
>> While looking at the bug reported by Michael and his patch. I found another
>> bug. Apparently I forgot to decrement the reset count if there was several
>> reset at the same time.
>>
>> This patch fixes that.
>>
>> I also moved the place of the decrement: before calling the exit phase method.
>> it globally fixes Michael's reported bug, as I think it will avoid some boiler
>> plate code in every exit phase method we do.
>>
>> Only other place where the reset state is checked is in the
>> hw/char/cadence-uart.c so it does not have high impact.
>>
>> I'm not sure if this meets the condition for 5.2 as it changes a documented
>> feature. In that case we can just accept Michael solution and I'll fix the
>> rest later.
>>
>> Here's the pointer for the bug and michael's patch.
>> https://lists.nongnu.org/archive/html/qemu-devel/2020-11/msg05786.html
>> https://lists.nongnu.org/archive/html/qemu-devel/2020-11/msg06105.html
>>
>> Damien
Re: [PATCH-for 5.2?] hw/core/ressetable: fix reset count decrement
Posted by Philippe Mathieu-Daudé 4 years, 11 months ago 
On 12/3/20 9:58 AM, Damien Hedde wrote:
> On 12/2/20 6:10 PM, Philippe Mathieu-Daudé wrote:
>> On 12/2/20 5:40 PM, Damien Hedde wrote:
>>> The reset count was only decremented if the device was in a single
>>> reset.
>>>
>>> Also move the decrement before calling the exit phase method to fix
>>> problem of reset state evaluation during that call. Update the doc
>>> accordingly.
>>>
>>> Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
>>> Fixes: 1905297 ("Zynq7000 UART clock reset initialization", 2020-11-23)
>>
> 
>> $ git show 1905297
>> fatal: ambiguous argument '1905297': unknown revision or path not in the
>> working tree.
> 
> I put Michael's bug number there. Should I put the incriminated commit
> instead ?

In that case you want:
Buglink: https://bugs.launchpad.net/qemu/+bug/1905297

> 
>>
>> Beside, typo ressetable -> resettable in subject.
> 
> Thanks,
> Damien
> 
> Cc'ing michael new address too
> 
>>
>>> Reported-by: Michael Peter <michael.peter@hensoldt-cyber.com>
>>> --
>>>
>>> Hi all,
>>>
>>> While looking at the bug reported by Michael and his patch. I found another
>>> bug. Apparently I forgot to decrement the reset count if there was several
>>> reset at the same time.
>>>
>>> This patch fixes that.
>>>
>>> I also moved the place of the decrement: before calling the exit phase method.
>>> it globally fixes Michael's reported bug, as I think it will avoid some boiler
>>> plate code in every exit phase method we do.
>>>
>>> Only other place where the reset state is checked is in the
>>> hw/char/cadence-uart.c so it does not have high impact.
>>>
>>> I'm not sure if this meets the condition for 5.2 as it changes a documented
>>> feature. In that case we can just accept Michael solution and I'll fix the
>>> rest later.
>>>
>>> Here's the pointer for the bug and michael's patch.
>>> https://lists.nongnu.org/archive/html/qemu-devel/2020-11/msg05786.html
>>> https://lists.nongnu.org/archive/html/qemu-devel/2020-11/msg06105.html
>>>
>>> Damien
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.