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

[Nicolas Pomarède <npomarede@xxxxxxxxxxxx>]

Le 24/09/2020 à 22:15, Thomas Huth a écrit :
> 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.

Hi

I tried to fix this ide.c because it was a consequence of an STE demo 
crashing with bus error when blitter accessed the ide memory region and 
because region 0xF0xxxx was always mapped to ide registers, even for 
plain ST/STE with no ide drive (instead of mapping the whole region 
directly to bus error)

As if pulling a thread, trying to fix this bus error region showed more 
side effect than expected in the ide.c code itself ;)

I will try to give another look at the code I changed, it might not 
require that much change to make it work in all cases. Don't have much 
time too at the moment, so if you have any idea, don't hesitate to 
change my commits if you have a working solution :)

Nicolas