stardot / b-em

An opensource BBC Micro emulator for Win32 and Linux
http://stardot.org.uk/forums/viewtopic.php?f=4&t=10823
GNU General Public License v2.0
112 stars 57 forks source link

Integra-B doesn't respect PRVEN when ACCCON is updated #158

Closed ZornsLemma closed 3 years ago

ZornsLemma commented 3 years ago

Ken Lowe and I have been discussing some Integra-B problems with Ozmoo over on stardot. The issue Ken actually reported is nothing to do with b-em (it's an intermittent crash on real hardware), but in the process of investigation we've found a bug in b-em's Integra-B emulation.

You can reproduce the problem using the .ssd attached to this post: https://stardot.org.uk/forums/viewtopic.php?p=322915#p322915 If you run that on b-em in Integra-B mode with 32K of sideways RAM, it should fail. I believe the problem is that while writing to ROMSEL respects the PRVEN bit (ROMSEL b6) which determines whether ACCCON's PRVS1/4/8 bits take effect, writing to ACCCON causes the PRVS1/4/8 bits to take effect even if PRVEN is not set.

Please don't take this on trust - I always find this stuff a bit mind-bending - but the fix might be to change:

    if (changes & (INTEGRA_PRVS1|INTEGRA_PRVS4|INTEGRA_PRVS8)) {

in write_acccon_integra() to:

    if ((ram_fe30 & INTEGRA_PRVEN) && (changes & (INTEGRA_PRVS1|INTEGRA_PRVS4|INTEGRA_PRVS8))) {

This certainly seems to help, but may still be subtly wrong in a way I haven't noticed yet.

Ken suggested a slightly different way to fix this a few posts down from the one linked above; I haven't tried his suggestion, but I think it would probably work. I'm sure you'll know best how to handle this anyway...

ZornsLemma commented 3 years ago

PS Make sure the Integra-B is in a non-0 OSMODE. "*CONFIGURE OSMODE 4" should be fine.

SteveFosdick commented 3 years ago

This comes hot in the tails of https://github.com/stardot/b-em/issues/156 so it should be the turn of B+ next.

I have for a solution based on the extra test but thanks Ken for mentioning ram[148]k - these were originally internal variables within write_romsel but are now exposed to the debugger as their sole external use. So I have made write_accon_integra set these, hopefully correctly. The version I have pushed passes the test, assuming success is it changes into Mode 128 without crashing.

P.S. 'r ram' in the debugger to look at the these values.

ZornsLemma commented 3 years ago

Thanks, I've just built the fix and it seems to work fine. I was actually referring to the Hollywood_Hijinx.ssd game attached to that post - it would lock up on b-em before this fix, but it's fine now. The "mode 128" program probably was a good test but it was just something I was trying out while trying to investigate Ken's crash on real hardware.

'r ram' looks really useful, I will have to remember that.

SteveFosdick commented 3 years ago

Ok, I have merged the fix to master.