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

[Thomas Huth <th.huth@xxxxxxxxx>]

Am Thu, 24 Sep 2020 21:57:15 +0300
schrieb Eero Tamminen <oak@xxxxxxxxxxxxxx>:

> On 9/24/20 4:50 PM, Nicolas Pomarède wrote:
> > Le 24/09/2020 à 15:29, Nicolas Pomarède a écrit :  
> >> 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.  
> 
> Original ide.c code didn't do any checks for
> Falcon, only for IDE drives.
> 
> 
> >> 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  
> 
> Exiting there means that opaque_ide_if structure contains bogus
> (zero) values, and I think that to
> mean IDE register get/put functions not to work
> correctly.
> 
> 
>  > (else there was another crash due to
>  > bdrv_get_geometry() doing a division by 0
>  > because "bs->sector_size==0")  
> 
> I think that's expected when there's no disk...
> 
> 
> What is supposed to happen on Falcon if when there's no disk but IDE 
> registers are accessed?
> 
> (Original code would in that case give bus error in register get/put 
> functions.)

Sorry for the late reply, it's yet another busy week for me...

Original code caused a bus error in Falcon mode, too - but that was
clearly wrong. It happened to work since TOS 4.04 catches the bus
errors and ignores IDE in that case - but some hard disk drivers for
the Falcon crashed (I think it was Cecile IIRC) because they accessed
the registers without checking for bus errors.
The correct behavior is of course that we should always emulate an IDE
controller on the Falcon, which of course should not crash the emulator
if there is no IDE drive attached.
I can maybe have a look at this at the weekend, but if somebody else
currently has some spare time to fix the issues, you're welcome of
course, too.

 Thomas