hoglet67 / PiTubeDirect

Bare-metal Raspberry Pi project that attaches to the Acorn TUBE interface and emulates many BBC Micro Co Processors
GNU General Public License v3.0
187 stars 23 forks source link

JIT 6502: Indirect Stores to IO Crash #149

Closed hoglet67 closed 2 years ago

hoglet67 commented 2 years ago

(Just creating an issue for this, rather than leaving it in email)

I tried cherry picking 2cc44fa and 0fa9f08 into hognose-dev

When I did ?&FEF8=65 the system hung, recovered by BREAK.

hoglet67 commented 2 years ago

I don't fully understand the operation of the JIT 6502, but yes that's the goal.

The JITTEDTABLE16 has one ARM instruction per 6502 address, and is normally used to signal if that byte has been jitted.

So normally the table contains one of the following instructions:

Dominic's change replaces the instructions in this table in the range &FEE0 - &FEFF with:

(And it does this correctly - I've checked carefully the contents of the table in Memory)

When STA (ZP), Y executes, the following code fragment is being run:

Bopc_91:
   ldrh reg1,[ram6502,#00]
   strb regA,[reg1,regY,LSR #24]!
   add reg0,jittedtable16ptr, reg1,LSL #2
   BLX reg0

This indirects through the "hook" in the JITTEDTABLE16 table, which normally just returns (or jits), but in the case of an &FEE0-&FEFF it branches to iostoreindirect:

indirectiostore:
   sub     reg0,reg0,jittedtable16ptr
   and     reg1,regA,#0xFF    // This is an assumption that the value is in regA
   mov     reg0,reg0,LSR #2

iostore:
   push    {lr}               // ram6502 (r6) is used as a working register
   mrs     reg4, CPSR         // Save 6502 flags and current FIQ/IRQ bits
   CPSID   if                 // Disable FIQ/IRQ at the same time
   bl      tube_parasite_write_banksel // Call up to C code to handle parasite write
   msr     CPSR, reg4         // Restore 6502 flags and FIQ/IRQ bits
   pop     {pc}               // Restore registers and return

So having tried to explain this, I understand how it's meant to work.

But for some reason it's not working in practice.

And I also checked the banked memory, and that doesn't seem to work in the JIT 6502, which I don't think is a surprise.... But then why is the above code calling the banksel version of tube_parasite_write?

hoglet67 commented 2 years ago

But obviously not carefully enough!

I eventually spotted the mistake:

There is a sign wrong in this expression:

   ldr   temp,=indirectiostore-8-JITTEDTABLE16+(0xFEE0<<2)

It should be:

   ldr   temp,=indirectiostore-8-JITTEDTABLE16-(0xFEE0<<2)

or alternatively there should be more brackets:

   ldr temp,=indirectiostore-8-(JITTEDTABLE16+(0xFEE0<<2))

With this change, ?&FEF8=65 prints a "A"

hoglet67 commented 2 years ago

Fixed in 9242296