diff mbox

QEMU 0.12.3 and SCSI boot

Message ID 4BB0AE15.6020303@redhat.com
State New
Headers show

Commit Message

Gerd Hoffmann March 29, 2010, 1:41 p.m. UTC
> Tried the same with current git master and it segfaults. This segfault
> was introduced in af12ac98 (lsi: have lsi_request for the whole life
> time of the request):
>
> #0  0x000000000052e2d3 in lsi_command_complete (bus=0xca22f8, reason=1,
> tag=0, arg=512) at /home/kwolf/source/qemu/hw/lsi53c895a.c:690
> #1  0x00000000004416e7 in qcow_aio_read_cb (opaque=0xc813f0, ret=0) at
> block/qcow2.c:480
> #2  0x0000000000433028 in posix_aio_process_queue (opaque=<value
> optimized out>) at posix-aio-compat.c:459
> #3  0x00000000004330cc in posix_aio_read (opaque=0xc4bb60) at
> posix-aio-compat.c:489
> #4  0x000000000040ac60 in main_loop_wait (timeout=0) at
> /home/kwolf/source/qemu/vl.c:3949
> #5  0x000000000040ce85 in main_loop (argc=<value optimized out>,
> argv=<value optimized out>, envp=<value optimized out>)
>      at /home/kwolf/source/qemu/vl.c:4172
> #6  main (argc=<value optimized out>, argv=<value optimized out>,
> envp=<value optimized out>) at /home/kwolf/source/qemu/vl.c:6147
>
> s->current is set to NULL by lsi_queue_command. I don't know the code
> well enough to say if lsi_queue_command is wrong in setting it to NULL
> or if lsi_command_complete shouldn't even try to access it (maybe it
> should search in the queue for the right tag?)

It actually searches the queue in case tag != s->current->tag, and it 
should most likely do the same for s->current == NULL ...

Attached patch makes the rom boot for me.

cheers,
   Gerd
From 4b385e8b5c617f2e14261a609898afdb13c12062 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Mon, 29 Mar 2010 15:31:03 +0200
Subject: [PATCH] lsi: fix segfault in lsi_command_complete


Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/lsi53c895a.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Comments

Kevin Wolf March 29, 2010, 1:51 p.m. UTC | #1
Am 29.03.2010 15:41, schrieb Gerd Hoffmann:
> 
>> Tried the same with current git master and it segfaults. This segfault
>> was introduced in af12ac98 (lsi: have lsi_request for the whole life
>> time of the request):
>>
>> #0  0x000000000052e2d3 in lsi_command_complete (bus=0xca22f8, reason=1,
>> tag=0, arg=512) at /home/kwolf/source/qemu/hw/lsi53c895a.c:690
>> #1  0x00000000004416e7 in qcow_aio_read_cb (opaque=0xc813f0, ret=0) at
>> block/qcow2.c:480
>> #2  0x0000000000433028 in posix_aio_process_queue (opaque=<value
>> optimized out>) at posix-aio-compat.c:459
>> #3  0x00000000004330cc in posix_aio_read (opaque=0xc4bb60) at
>> posix-aio-compat.c:489
>> #4  0x000000000040ac60 in main_loop_wait (timeout=0) at
>> /home/kwolf/source/qemu/vl.c:3949
>> #5  0x000000000040ce85 in main_loop (argc=<value optimized out>,
>> argv=<value optimized out>, envp=<value optimized out>)
>>      at /home/kwolf/source/qemu/vl.c:4172
>> #6  main (argc=<value optimized out>, argv=<value optimized out>,
>> envp=<value optimized out>) at /home/kwolf/source/qemu/vl.c:6147
>>
>> s->current is set to NULL by lsi_queue_command. I don't know the code
>> well enough to say if lsi_queue_command is wrong in setting it to NULL
>> or if lsi_command_complete shouldn't even try to access it (maybe it
>> should search in the queue for the right tag?)
> 
> It actually searches the queue in case tag != s->current->tag, and it 
> should most likely do the same for s->current == NULL ...
> 
> Attached patch makes the rom boot for me.

Yes, works for me. And it seems to work reliably, unlike the 0.12.x
version. Maybe we should include the lsi patches in stable-0.12?

Kevin
Gerd Hoffmann March 29, 2010, 2:27 p.m. UTC | #2
On 03/29/10 15:51, Kevin Wolf wrote:
>> It actually searches the queue in case tag != s->current->tag, and it
>> should most likely do the same for s->current == NULL ...
>>
>> Attached patch makes the rom boot for me.
>
> Yes, works for me. And it seems to work reliably, unlike the 0.12.x
> version.

Oh.  The lsi cleanup patches where supposed to be a no-op.
Looks like I fixed bugs by accident ;)

Seriously:  Could be that stable code silently does something wong when 
reaching the point where master segfaults due to the NULL pointer 
dereference.

> Maybe we should include the lsi patches in stable-0.12?

Probably much easier than brewing a different version of the fix for 0.12.

cheers
   Gerd
Gerhard Wiesinger March 31, 2010, 4:37 a.m. UTC | #3
On Mon, 29 Mar 2010, Gerd Hoffmann wrote:

> On 03/29/10 15:51, Kevin Wolf wrote:
>>> It actually searches the queue in case tag != s->current->tag, and it
>>> should most likely do the same for s->current == NULL ...
>>> 
>>> Attached patch makes the rom boot for me.
>> 
>> Yes, works for me. And it seems to work reliably, unlike the 0.12.x
>> version.
>

I'm also interested in a backport to the 0.12.x version that I can test. 
Can this be easily done?

Ciao,
Gerhard

--
http://www.wiesinger.com/
Gerd Hoffmann March 31, 2010, 7:17 a.m. UTC | #4
On 03/31/10 06:37, Gerhard Wiesinger wrote:
> On Mon, 29 Mar 2010, Gerd Hoffmann wrote:
>
>> On 03/29/10 15:51, Kevin Wolf wrote:
>>>> It actually searches the queue in case tag != s->current->tag, and it
>>>> should most likely do the same for s->current == NULL ...
>>>>
>>>> Attached patch makes the rom boot for me.
>>>
>>> Yes, works for me. And it seems to work reliably, unlike the 0.12.x
>>> version.
>>
>
> I'm also interested in a backport to the 0.12.x version that I can test.
> Can this be easily done?

http://git.savannah.gnu.org/cgit/qemu.git/log/hw/lsi53c895a.c

Pick the five most recent patches (committed jan 11th) and apply the 
patch posted in this thread on top of it.

cheers,
   Gerd
Gerhard Wiesinger April 1, 2010, 7:51 p.m. UTC | #5
On Wed, 31 Mar 2010, Gerd Hoffmann wrote:
>> 
>> I'm also interested in a backport to the 0.12.x version that I can test.
>> Can this be easily done?
>
> http://git.savannah.gnu.org/cgit/qemu.git/log/hw/lsi53c895a.c
>
> Pick the five most recent patches (committed jan 11th) and apply the patch 
> posted in this thread on top of it.
>

Ok, DOS works for me, too.

Thanx for the fix.

Ciao,
Gerhard

BTW: With SCSI also DOS with QEMM 8.03 (licensed) works well. With IDE I 
had the problem, that QEMM hangs on boot (I guess reason is IDE 
busmastering, is it possible to disable it?)

--
http://www.wiesinger.com/
diff mbox

Patch

diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index a332401..525f3ca 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -679,7 +679,7 @@  static void lsi_command_complete(SCSIBus *bus, int reason, uint32_t tag,
         return;
     }
 
-    if (s->waiting == 1 || tag != s->current->tag ||
+    if (s->waiting == 1 || !s->current || tag != s->current->tag ||
         (lsi_irq_on_rsl(s) && !(s->scntl1 & LSI_SCNTL1_CON))) {
         if (lsi_queue_tag(s, tag, arg))
             return;