[hatari-devel] Wrong memory init

[Thomas Huth]

 Hi!

Am Sun, 27 Nov 2011 14:23:33 +0100
schrieb Nicolas Pomarède <npomarede at corp.free.fr>:

> I spent some hard hours trying to fix the game Yolanda (on Pompey 
> Pirates 46).

Welcome to the club ;-) I also already spent countless hours with that
game to figure out what goes wrong there. But unlike you I always
failed in the past (maybe just because we did not have that fast-boot
option in Hatari yet)...

> The game freezes after a few second because of a bad random number 
> generator that always returns 0.
> This random number is inititalized with the value in $100. By
> default, this value is 0 under Hatari, because FastBoot is on, which
> bypasses the TOS memory init functions that write data between $8 and
> $200.
> 
> The "culprit" is in stMemory.c. I'm wondering what is this MMU
> reference ? I see this code is present since the initial import of
> tos.c in 2001, but this looks like a bug to me, especially since
> Hatari works fine when fastboot is turned off in System settings.
> 
>          /* Fill in magic numbers, so TOS does not try to reference
> MMU */ if (ConfigureParams.System.bFastBoot
>              || (ConfigureParams.Memory.nMemorySize > 4
> && !bIsEmuTOS)) {
>                  /* Write magic values to sysvars to signal valid
> config */ STMemory_WriteLong(0x420, 0x752019f3);    /* memvalid */
>                  STMemory_WriteLong(0x43a, 0x237698aa);    /* memval2
> */ STMemory_WriteLong(0x51a, 0x5555aaaa);    /* memval3 */
>          }

The three lines of code that fill in these memvalid system variables
originally come from the WinSTon source code (in Tos.cpp there, it has
been later moved to stMemory.c in Hatari). In that source code there is
an additional comment that looks like this:

// As TOS checks hardware for memory size + connected devices on boot-up
// we set these values ourselves and fill in the magic numbers so TOS
// skips these tests which would crash the emulator as the reference the MMU

So I guess in the early days of WinSTon there was a problem when TOS
tried to detect the memory size, so that test got bypassed by patching
the memvalid variables.

I agree that this is not necessary anymore for normal TOS when using
less than 4 MiB of memory...

> So, I would propose to remove this piece of code ; this has nearly no 
> impact on boot time.

... but it makes a difference when using EmuTOS! EmuTOS uses the
memvalid variables to decide whether to display the splashscreen in the
beginning or not.

So I would suggest to change the if-statement like this:

if ((ConfigureParams.System.bFastBoot && bIsEmuTOS)
     || (ConfigureParams.Memory.nMemorySize > 4 && !bIsEmuTOS))

That means patch it when fast boot has been enabled for EmuTOS or patch
it when using > 4 MiB memory when using normal TOS.

What do you think?

> Additionnaly, I think bFastBoot should be false by default, to ensure 
> the system always starts in the maximum compatibility level.

You mean "ConfigureParams.System.bFastBoot = false" in configuration.c ?
... that's fine with me.

> The user can still change it later if he wants(this is used in tos.c, when
> patch has TP_HDIMAGE_OFF ; is it still useful ?)

It makes a big difference with TOS 2.05 and TOS 2.06. That TOS patch is
used to skip the annoying memory test sequence there, so I'd say we
should keep this.

 Thomas