Re: [hatari-devel] IDE IO register range access commit

[ Thread Index | Date Index | More Archives ]

Le 24/09/2020 à 15:29, Nicolas Pomarède a écrit :
Le 24/09/2020 à 15:10, Eero Tamminen a écrit :

On 9/24/20 3:26 PM, Uwe Seimet wrote:
Looks as if there is still an issue with the IDE handling. I don't know
whether this is a new issue or has always been there > In Falcon030 mode Hatari crashes when no IDE images are assigned:

Thread 1 "hatari" received signal SIGSEGV, Segmentation fault.
0x00005555558f7258 in Ide_Mem_bget ()
(gdb) bt
#0  0x00005555558f7258 in Ide_Mem_bget ()
#1  0x00005555559c5a99 in op_4a39_0_ff ()
#2  0x000055555597d092 in m68k_run_2_020 ()
#3  0x00005555559758c2 in m68k_go ()
#4  0x00005555558ba703 in main ()

In TT mode everything is fine, i.e. the IDE interface is not present.

I can reproduce this just with:
     $ hatari --machine falcon --dsp none

Thread 1 "hatari" received signal SIGSEGV, Segmentation fault.
Ide_Mem_bget (addr=15728669) at src/ide.c:107
107            retval = ide_ioport_read(opaque_ide_if, ideport);
(gdb) print opaque_ide_if
$2 = (struct IDEState *) 0x0

It's due to Nicolas' Ide_MmioIsAvailable()
function change claiming that IDE is available
(on Falcon) even when there are no IDE devices

That (previously internal) function is used by IDE
data access functions which chrash when there's no
actual IDE device...


I'm looking at it, but IdeInit() seems strange to me ; it doesn't create the "opaque_ide_if" struct if machine is falcon, only if Ide[0].bUseDevice (else it returns immediately)

But Ide should be enabled on Falcon even if there's no disk on Ide[0].
It seems that sometimes IDE depends on Ide[0].bUseDevice and sometimes it depends on machine==falcon.
I think it should check both in every places.

I pushed a patch to fix this ; I'm not too familiar with ide.c code, but I think the following needed to be changed/fixed :

- In IdeInit, we should not return if machine==falcon, else opaque_ide_if is not malloc'ed and hatari will crash later in Ide_Mem_bget

- Once we don't return in IdeInit() in case of falcon, ide_init2 should not assume that hd_table[n] is initialized, because it can be skipped in the "for" loop in IdeInit if Ide[i].bUseDevice == false. So, I added a check for Ide[i].bUseDevice in ide_init2 too (else there was another crash due to bdrv_get_geometry() doing a division by 0 because "bs->sector_size==0")

Note : not related to these changes, but bdrv_get_geometry() should really check if "bs->sector_size==0" else hatari will crash also

Hopefully, this should now work as before for IDE for Falcon/TT/..., at least it doesn't crash in falcon mode when there's no ide disk inserted.

Can someone test with images in ide disk 0 or 1 if everything is OK ?


Mail converted by MHonArc 2.6.19+