Stack overflow while using H.KEYI hook

Page 1/4
| 2 | 3 | 4

By tvalenca

Paladin (747)

tvalenca's picture

01-09-2020, 23:16

Hello everyone, long time no see!

I'm writting some code that needs to be executed on a V9990-generated interrupt, so I figure it should be hooked on H.KEYI hook:

"MSX Wiki" wrote:

Address Name Description
FD9Ah H.KEYI Call: Called at the beginning of the KEYINT interrupt routine (Main-ROM at 0038h).

Usage: Used to test whether the interruption was caused by a device other than the VDP. (RS-232C, MSX-Midi, etc)
Note: The program must back to the interrupt routine if no device has caused the interrupt.

So I wrote the code below, poke'd CD 00 80 at #FD9A and it simply freeze: (yep, I redirected TXTTAB to #9001, but it happens on any memory location)

ROUTINE:
    org #8000
    di
    push af
    in a,(#66)
    bit 1,a
    jr z,SPLIT
    bit 0,a
    jr z,REFRESH
    pop af
    ei
    ret
    
SPLIT:
    ld a,2
    out (#66),a
    ld a,13
    out (#64),a
    ld a,4
    out (#63),a
    pop af
    ei
    ret

REFRESH:
    ld a,1
    out (#66),a
    ld a,13
    out (#64),a
    ld a,0
    out (#63),a
    pop af
    ei
    ret

(for now I am just selecting another palette midscreen and resetting it on refresh)

So I opened the debugger and realized that SP was all the way down to #7CFF in just a few seconds of execution. another few seconds and it probably overflow. I also noted main RAM contents completely corrupted.

For testing purposes I reduced to just this: (with no noticeable changes)

ROUTINE:
    org #8000
    di
    push af
    in a,(#66)
    bit 1,a
    pop af
    ei
    ret

Can anyone point what am I missing?

Login or register to post comments

By Grauw

Ascended (10020)

Grauw's picture

01-09-2020, 23:50

  1. You should not disable and enable interrupts. Interrupts during H.KEYI are already disabled and should remain disabled. If you enable them, any non-V9990 interrupt (e.g. the V9938’s) will retrigger immediately after the ei / ret before the rest of the ISR is processed, causing a stack overflow.
  2. You should move your ISR to page 3 (C000H and up). There is no guarantee about what slot or mapper segment is selected in page 2 at the moment the ISR is called.
  3. You should preserve the old hook and call it at the end of your ISR when the interrupt was different from the one you meant to handle.
  4. You do not need to preserve any registers in H.KEYI, so the push af / pop af is unnecessary. (You do need to preserve AF in H.TIMI.)

Point 1 is the one which caused your issue, but you should also address points 2 and 3 for proper hook handling.

By ducasp

Champion (455)

ducasp's picture

02-09-2020, 00:04

Grauw wrote:
  1. You should not disable and enable interrupts. Interrupts during H.KEYI are already disabled and should remain disabled. If you enable them, any non-V9990 interrupt (e.g. the V9938’s) will retrigger immediately after the ei / ret before the rest of the ISR is processed, causing a stack overflow.
  2. You should move your ISR to page 3 (C000H and up). There is no guarantee about what slot or mapper segment is selected in page 2 at the moment the ISR is called.
  3. You should preserve the old hook and call it at the end of your ISR when the interrupt was different from the one you meant to handle.
  4. You do not need to preserve any registers in H.KEYI, so the push af / pop af is unnecessary. (You do need to preserve AF in H.TIMI.)

Point 1 is the one which caused your issue, but you should also address points 2 and 3 for proper hook handling.

I believe you can use P2 and P1 as long as you make a callslt (or a similar procedure to a mapper segment), I did that on an old version of SM-X UNAPI driver that used interrupts. But it is, for sure, way more efficient to have the routine code at P3 if feasible.

By tvalenca

Paladin (747)

tvalenca's picture

02-09-2020, 00:26

Grauw wrote:
  1. You should not disable and enable interrupts. Interrupts during H.KEYI are already disabled and should remain disabled. If you enable them, any non-V9990 interrupt (e.g. the V9938’s) will retrigger immediately after the ei / ret before the rest of the ISR is processed, causing a stack overflow.
  2. You should move your ISR to page 3 (C000H and up). There is no guarantee about what slot or mapper segment is selected in page 2 at the moment the ISR is called.
  3. You should preserve the old hook and call it at the end of your ISR when the interrupt was different from the one you meant to handle.
  4. You do not need to preserve any registers in H.KEYI, so the push af / pop af is unnecessary. (You do need to preserve AF in H.TIMI.)

Point 1 is the one which caused your issue, but you should also address points 2 and 3 for proper hook handling.

Wow, that's the price you pay for leaving a project 4 years untouched...

ROUTINE:
    org #C000
    in a,(#66)
    bit 1,a
    jr z,SPLIT
    bit 0,a
    jr z,REFRESH
    ret
    
SPLIT:
    ld a,2
    out (#66),a
    ld a,13
    out (#64),a
    ld a,4
    out (#63),a
    ret

REFRESH:
    ld a,1
    out (#66),a
    ld a,13
    out (#64),a
    ld a,0
    out (#63),a
    ret

    
INICIO:
    di
    ld a,10
    out (#64),a
    ld a,32
    out (#63),a
    ld a,11
    out (#64),a
    ld a,0
    out (#63),a
    ld a,9
    out (#64),a
    ld a,2
    out (#63),a
    
    ld hl,#fd9a
    ld (hl),#cd
    inc hl
    ld (hl),#00
    inc hl
    ld (hl),#c0
    ei
    ret

    end INICIO

got a DI;HALT. Will investigate later.

By pgimeno

Champion (299)

pgimeno's picture

02-09-2020, 12:32

tvalenca wrote:
    ...
    ld hl,#fd9a
    ld (hl),#cd
    ...

...
got a DI;HALT. Will investigate later.

Um, that #cd should be a #c3. Grauw's point 3 boils down to copying five bytes from #fd9a to somewhere else in the same page, and jumping to that somewhere else when your routine finishes.

By DarkSchneider

Paladin (941)

DarkSchneider's picture

02-09-2020, 14:24

More specific, you can put your ISR into another pages, but considering the possibilities:

- If working for 64KB system, put the full inter-slot jump at hook.
- For memory mapped systems, put an entry point to a page 3 intermediate caller that sets the slot and segment, call the ISR, and restores them.

This in the case you want to preserve the page 3 memory. Maybe for large ISR that are called eventually in response to some events. No feasible for ISR called often or if performance is required.

By tvalenca

Paladin (747)

tvalenca's picture

02-09-2020, 15:10

pgimeno wrote:

Um, that #cd should be a #c3. Grauw's point 3 boils down to copying five bytes from #fd9a to somewhere else in the same page, and jumping to that somewhere else when your routine finishes.

You're telling me I should use JP instead CALL on this hook?

By tvalenca

Paladin (747)

tvalenca's picture

02-09-2020, 15:26

DarkSchneider wrote:

More specific, you can put your ISR into another pages, but considering the possibilities:

- If working for 64KB system, put the full inter-slot jump at hook.
- For memory mapped systems, put an entry point to a page 3 intermediate caller that sets the slot and segment, call the ISR, and restores them.

This in the case you want to preserve the page 3 memory. Maybe for large ISR that are called eventually in response to some events. No feasible for ISR called often or if performance is required.

Yeah, that was an overlook Running Naked in a Field of Flowers
That specific ISR must be put on highest memory address available to user, because it will perform background animations, and doing so I will be able to use BASIC on this early development stage. But i took it from there (it was originally at #DC00 IIRC) because apparently GBASIC was messing with it (didn't investigated further)

Also, I already knew about interslot calls on hooks (RST30) but it wouldn't be pratical atm (I need to keep it simple for my own sanity)

By pgimeno

Champion (299)

pgimeno's picture

02-09-2020, 15:39

tvalenca wrote:

You're telling me I should use JP instead CALL on this hook?

Yes. Or on any hook, really. If you really want to place a CALL instead of a JP, be prepared to add a RET afterwards, just in case someone else was already using the hook. It's just a waste because it's a tail call, so it can be safely replaced with a JP and save a few cycles and stack space.

It's not likely to be the issue, though, because it's not likely that the hook was in use before you overwrite it, therefore the hook is probably filled with #c9's (RET's), and therefore there's already a RET immediately after your call anyway. But better not leave anything to chance.

By ducasp

Champion (455)

ducasp's picture

02-09-2020, 15:43

Thiago, this code might help you, first to register into HKEY_I hook:

	ramad1		equ	0xF342

	di
	ld		(intr_handler+1), hl	; Replace in intr_handler the address
	;	save hookint 
	ld		hl, #hookint
	ld		de, #backup_hookint
	ld		bc, #5
	ldir
		
	ld		a, #0xF7				; RST_30 (interslot call both dos and bios)
	ld		(hookint), a
	ld		a, (ramad1)
	ld		(hookint+1), a
	ld		hl, #intr_handler
	ld		(hookint+2), hl
	ld		a, #0xc9
	ld		(hookint+4), a
	ei

backup_hookint:
	.ds		5

This might help you, even though it is intended to use interslot call, you can replace it by call to a P3 address if your routine is there, instead of using it, then you won't need ramad1 (slot of ram in page 1, which is the program ram in case of DOS, you might want to change to ramad2 depending on your needs, i.e.: if it is something running in BASIC, it will be running on page 2). It is important to backup the original hook 5 bytes so you can call it after doing your interrupt, you need to chain in this way, otherwise, it won't be nice to any other piece of BIOS/ROM/DRIVER that is hooking there and will no longer get called on interrupts... By the way, this code assumes HL has the address of your HKEY_I routine...

At the end of your program, you can try to get yourself out of the hook by doing this, but only if you are running a program, if you are doing resident stuff (i.e.: allocating a system mapper segment that gets called) this might not work as there is a good chance you are no longer the last to hook:

	di
	ld		hl, #backup_hookint
	ld		de, #hookint
	ld		bc, #5
	ldir
	ei
	ret
intr_handler:
	call	dummy_handler		; This will be replaced by user routine address
dummy_handler:
	jp		backup_hookint		; And continue processing interrupt hooks
	ret

And this is your code executing whenever an interrupt is called... As you can see, this is as generic as can be (this was my proposition, which I had help from DarkSchneider, for functions to register a SDCC function to HKEY_I interrupt). In your case, you might just erase the first LD after DI in the install routine, and below intr_handler put all your interrupt code, just make sure it ends with jp backup_hookint to execute the previous hook and has a ret below, so you will return after the backup hook returns...

This works quite well with SDCC and should work on your code as well. Wink

By ducasp

Champion (455)

ducasp's picture

02-09-2020, 15:44

pgimeno wrote:
tvalenca wrote:

You're telling me I should use JP instead CALL on this hook?

Yes. Or on any hook, really. If you really want to place a CALL instead of a JP, be prepared to add a RET afterwards, just in case someone else was already using the hook. It's just a waste because it's a tail call, so it can be safely replaced with a JP and save a few cycles and stack space.

It's not likely to be the issue, though, because it's not likely that the hook was in use before you overwrite it, therefore the hook is probably filled with #c9's (RET's), and therefore there's already a RET immediately after your call anyway. But better not leave anything to chance.

If the code is not resident (which is most likely the case), it makes sense and save a few cycles Cool

Page 1/4
| 2 | 3 | 4