Patchwork [03/32] lsi: use qdev_reset_all

login
register
mail settings
Submitter Paolo Bonzini
Date July 27, 2012, 3:02 p.m.
Message ID <1343401379-19495-4-git-send-email-pbonzini@redhat.com>
Download mbox | patch
Permalink /patch/173689/
State New
Headers show

Comments

Paolo Bonzini - July 27, 2012, 3:02 p.m.
By first resetting the devices, lsi_soft_reset will find the queue
already cleared so there is no need to do that forcibly (which may also
leak SCSIRequests, and/or worse due to dangling references to the
lsi_request in the hba_private field).

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/lsi53c895a.c |   15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)
Jan Kiszka - April 23, 2013, 8:47 a.m.
On 2012-07-27 17:02, Paolo Bonzini wrote:
> By first resetting the devices, lsi_soft_reset will find the queue
> already cleared so there is no need to do that forcibly (which may also
> leak SCSIRequests, and/or worse due to dangling references to the
> lsi_request in the hba_private field).
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/lsi53c895a.c |   15 +++------------
>  1 file changed, 3 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
> index 2fe141d..f04cc68 100644
> --- a/hw/lsi53c895a.c
> +++ b/hw/lsi53c895a.c
> @@ -282,8 +282,6 @@ static inline int lsi_irq_on_rsl(LSIState *s)
>  
>  static void lsi_soft_reset(LSIState *s)
>  {
> -    lsi_request *p;
> -
>      DPRINTF("Reset\n");
>      s->carry = 0;
>  
> @@ -350,15 +348,8 @@ static void lsi_soft_reset(LSIState *s)
>      s->sbc = 0;
>      s->csbc = 0;
>      s->sbr = 0;
> -    while (!QTAILQ_EMPTY(&s->queue)) {
> -        p = QTAILQ_FIRST(&s->queue);
> -        QTAILQ_REMOVE(&s->queue, p, next);
> -        g_free(p);
> -    }
> -    if (s->current) {
> -        g_free(s->current);
> -        s->current = NULL;
> -    }
> +    assert(QTAILQ_EMPTY(&s->queue));
> +    assert(!s->current);
>  }
>  
>  static int lsi_dma_40bit(LSIState *s)
> @@ -1738,7 +1729,7 @@ static void lsi_reg_writeb(LSIState *s, int offset, uint8_t val)
>              lsi_execute_script(s);
>          }
>          if (val & LSI_ISTAT0_SRST) {
> -            lsi_soft_reset(s);
> +            qdev_reset_all(&s->dev.qdev);

What should ensure that all device resets are performed before the host
reset? I'm asking as we just ran into the assert(!s->current) over
1.3.1. Or is there some patch from a later version missing (not seeing
anything suspicious for lsi at least)?

Thanks,
Jan
Paolo Bonzini - April 23, 2013, 4:13 p.m.
Il 23/04/2013 10:47, Jan Kiszka ha scritto:
>> >          if (val & LSI_ISTAT0_SRST) {
>> > -            lsi_soft_reset(s);
>> > +            qdev_reset_all(&s->dev.qdev);
> What should ensure that all device resets are performed before the host
> reset? I'm asking as we just ran into the assert(!s->current) over
> 1.3.1. Or is there some patch from a later version missing (not seeing
> anything suspicious for lsi at least)?

qdev should, but it looks like it uses pre-order instead of post-order.

Paolo

Patch

diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index 2fe141d..f04cc68 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -282,8 +282,6 @@  static inline int lsi_irq_on_rsl(LSIState *s)
 
 static void lsi_soft_reset(LSIState *s)
 {
-    lsi_request *p;
-
     DPRINTF("Reset\n");
     s->carry = 0;
 
@@ -350,15 +348,8 @@  static void lsi_soft_reset(LSIState *s)
     s->sbc = 0;
     s->csbc = 0;
     s->sbr = 0;
-    while (!QTAILQ_EMPTY(&s->queue)) {
-        p = QTAILQ_FIRST(&s->queue);
-        QTAILQ_REMOVE(&s->queue, p, next);
-        g_free(p);
-    }
-    if (s->current) {
-        g_free(s->current);
-        s->current = NULL;
-    }
+    assert(QTAILQ_EMPTY(&s->queue));
+    assert(!s->current);
 }
 
 static int lsi_dma_40bit(LSIState *s)
@@ -1738,7 +1729,7 @@  static void lsi_reg_writeb(LSIState *s, int offset, uint8_t val)
             lsi_execute_script(s);
         }
         if (val & LSI_ISTAT0_SRST) {
-            lsi_soft_reset(s);
+            qdev_reset_all(&s->dev.qdev);
         }
         break;
     case 0x16: /* MBOX0 */