Re: [hatari-devel] EKO System crash

[Thomas Huth <th.huth@xxxxxxxxx>]

Am Sat, 31 Oct 2020 14:22:46 +0100
schrieb Nicolas Pomarède <npomarede@xxxxxxxxxxxx>:

> Le 31/10/2020 à 09:25, Thomas Huth a écrit :  
> > Am Sat, 31 Oct 2020 09:08:04 +0100
> > schrieb Nicolas Pomarède <npomarede@xxxxxxxxxxxx>:
> >     
> >> Le 31/10/2020 à 07:13, Thomas Huth a écrit :    
> >>> Am Thu, 29 Oct 2020 00:11:52 +0200
> >>> schrieb Eero Tamminen <oak@xxxxxxxxxxxxxx>:
> >>>        
> >>>> On 10/27/20 1:12 AM, Eero Tamminen wrote:    
> >>> [...]    
> >>>>>> EKO Systems:  perfectly works (except a little transition in
> >>>>>> the betty boo image).
> >>>>>>          Music plays well until the 3D scifi scene and plays
> >>>>>> well again in the tridi world.    
> >>>>
> >>>> Close to end I got Hatari crash:
> >>>> -----------------------------------
> >>>> WARN : crossbar DMA Play: Illegal buffer size (from 0x0d84e8 to
> >>>> 0x0d84bc) Segmentation fault    
> >>>
> >>> Looking at the crossbar code, I noticed that this could happily
> >>> read from beyond the STRam[] array if a Falcon program like EKO
> >>> system set the DMA start address higher than the end address.
> >>>
> >>> I've changed to code a little bit so that this should hopefully
> >>> not happen anymore, i.e. I hope it also won't crash anymore
> >>> (though I was not able to reproduce the crash here).
> >>>
> >>> As a bonus, the music should now play in the 3D racing scene of
> >>> the demo.
> >>>
> >>> Note: I've also updated the recording function accordingly, but I
> >>> did not test that part. Laurent, could you please try whether AFM
> >>> still works fine for you? Thanks!    
> >>
> >> Hi
> >>
> >> I see you're doing "% STRamEnd" to limit the range ; maybe it would
> >> be better to use "&" to mask out of limit bits instead, as this is
> >> more "hardware like" ?    
> > 
> > Which mask do you suggest? 0x00ffffff? But then we definitely still
> > need an additional check to make sure that we do not go beyond
> > STRamEnd here, to avoid that we access memory beyond the end of
> > STRam[]...
> > 
> > BTW, how's that situation handled in src/dmaSnd.c ? I did not spot
> > any sanity checks there at a very first glance?
> >     
> 
> Hi
> 
> for DMA sound in STE mode, mask is computed using
> DMA_MaskAddressHigh() which will return 0x3f in ST/STE mode because
> sound (or video counter too) is restricted to 4 MB.  

I'm not sure about the Falcon here, whether it caps at 4 MB or 16 ...
that would need some testing with real hardware, certainly...

> dmaSnd.c is also accessing STRam, I know that the demo "amberstar 
> cracktro" is buggy and will set start=end, which will play through
> the whole memory, but it doesn't crash Hatari, maybe by luck and
> because the mask 0x3f restrict it to a smaller region.  

Have you tried with --enable-small-mem ? ... would be interesting to
see whether it crashes there.

> So, dmaSnd.c should also be modified to use get_word().
> 
> We could add a common function DMA_ReadWord() in m68000.c to be
> called by blitter.c / dmaSnd.c / crossbar.c.
> 
> What do you think ?  

Sounds reasonable ... I'm just a little bit afraid that the performance
of Hatari might suffer quite a bit, since dma samples are processed
quite frequently, so always going through the complex get_word
function might slow down things a little bit...?

 Thomas