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

[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.)


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

If disk image was successfully opened, I'm pretty sure it has 
sector_size. :-)


> 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 ?


	- Eero