hbios - freertos - scz180 - access to z180 interrupt table about romwbw

[wwarthen]

There is an API in RomWBW HBIOS for interrupt hooking. It is documented (albeit poorly) in the RomWBW Architecture document. It is the SYSINT function ($FC). This is at the very end of the document.

All of the standard Z180 configs are IM2 based. If you look at hbios.asm, search for HBX_IVT. This is the interrupt vector table. Just above it is a comment that documents the interrupt slots assigned to the standard interrupts. In your case, you are going to hook slot 2 (TIM0) which is used for all Z180's as a 50Hz timer.

There is a good example of hooking the vectors in the inttest.asm application included with RomWBW. The app is more generic than you need, but I think you will easily see the simple routines for hooking and unhooking interrupt handlers.

The new interrupt handler must be in the top 32K of memory.

I'm hoping this is enough to get you started. Let me know how you do and ask questions as needed.

Thanks,

Wayne

[wwarthen]

Sure, you can certainly just put entries directly in the HBX_IVT table. Yes, in this scenario, you will be entirely responsible for saving and restoring context, not to mention acknowledging the interrupt and using RETI & EI at the end of the interrupt processing.

The Z180 interrupt address table (HBX_IVT) will always be found at 0xFF00. This does not change, so you are safe to assume it is there. RomWBW programs the Z180 at boot such that the start of the interrupt vectors is at 0xFF00. This fixed and you are safe to assume it is there as long as the processor is using interrupt mode 2 which is the norm for all Z180 platforms. PRT1 will therefore be found at slot 3 of the HBX_IVT.

The IVT and your handler must be located in the top 32K or RAM because an interrupt may occur when various banks are selected in low RAM.

I am a little confused about what you want your ISR to do. You can indeed determine the currently selected bank by examining the value at 0xFFE0. This is the current bank id. However, this is a tricky thing to do because a bank switch can be "in progress" when an interrupt occurs. If you look at HBX_INT, you will see how RomWBW safely handles the selection of the HBIOS bank at the time of an interrupt and restores the original bank at completion. It is easier for your ISR to just set flags/counters in high ram and then return. Then your mainline code would just refer to these flags/counters as needed.

Not sure if I am helping here. I may need to better understand the intended functionality of your ISR.

You may want to reconsider using the API to accomplish this. If you use the API, the context saving is handled for you and the HBIOS bank is guaranteed to be selected in low RAM when your ISR gets control. You can still use PRT1 as desired.

Thanks,

Wayne

[feilipu]

Hmmm... BBR is an 8 bit register. It will have the value 0xE8 when the HBIOS bank is selected. It will have the value 0xF0 when the user bank is selected.

Hmm. I was going with 0xE0 because HBIOS bank was the lowest of 4 banks at the top of RAM. That’s what I think the architecture doc implies. As that is wrong, then it’s clearly the cause of the panic.

I’ll try again avoiding 0xE8 instead.

EDIT just realised they are 32kB Banks, and now I get it. And that’s why it is good to ask.

[wwarthen]

OK, I get it. Give me a little time to work through this.

Thanks, Wayne

[wwarthen]

My (undocumented) rule is that a caller should provide required buffer space and that the bounce buffer is purely for moving the code between banks. So, as long as I stick to my guns, the bounce buffer is fair game.

[wwarthen]

Ah, cool. If it is only NMOS processors, I can live this this!

I will make this change later tonight. Thanks!!!

[wwarthen]

Yes, since you provided code that allows the interrupt state to be maintained across the call, that works for me.

…

On Tue, Mar 3, 2020 at 8:48 PM Phillip Stevens ***@***.***> wrote: Still business hours here. But you decided to go with wrapped rather than moving buffets again? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#87?email_source=notifications&email_token=AATNT2J4GRHDYGIXNJN7VTDRFXMQ3A5CNFSM4K2CCLT2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOENWK3OI#issuecomment-594324921>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AATNT2IMAG7ZNHXEO4CQLM3RFXMQ3ANCNFSM4K2CCLTQ> .

[wwarthen]

Your last comment showed up just after my last check in.

Yes, I will switch PEEK/POKE to use the bounce buffer instead of DI/EI. Better solution as long as PEEK/POKE are never invoked directly, but only through HBIOS API (which is a reasonable assumption).

I will wait for your code example and try to implement SRA mutex. Worried about code size again. Sigh.

[wwarthen]

I also understand @EtchedPixels points. Regardless, the code change implementing the MUTEX is checked in and does not seem to hurt anything for normal CP/M operation. So, I will leave it this way for now.

It does result in an INT stack size of 13 bytes for N8, so N8 will definitely not work. Will worry about that once we clear up the bigger picture.

-Wayne

[wwarthen]

It is entirely possible to hook the existing Z180 timer interrupt in RomWBW via API calls. I will provide some better instructions as soon as I have a bit more time.

[feilipu]

Thanks. I've had a look around in the code. From what I see the Z180 address table entries are all pointing at a HBX_INT with a following code byte to route the interrupt onwards via the HB_IVT. I think the SYSINT function writes to the HB_IVT table which means an interrupt handler will be invoked after the HBIOS partial context save.

I think the best thing for me to do is to use the PRT1 for timing, and then hook the handler directly into the Z180 interrupt address table. This will allow me to set the timer interval at any rate that the user needs. Also, I need to use a full context save including alternate registers and interrupt state because the task switching can be initiated through various mechanisms, not solely the timer interrupt.

So, I need to understand where the Z180 interrupt address table is located, to write the handler address directly. Is that address a supportable (fixed) location?

But, to not break HBIOS, I'll have to ensure that HBIOS functions have not banked the memory when the interrupt is called. So the handler has to look something like this.

prt1_isr() __naked
{
    portSAVE_CONTEXT_IN_ISR();
    xTaskIncrementTick();
    // read some location to confirm we're not banked (in HBIOS)
    // if banked then restore context and return
    // otherwise
    vTaskSwitchContext();
    portRESTORE_CONTEXT_IN_ISR();
}

So, a further question. Is the memory banking address readable generally on all platforms? Or as a better question, is there something else unique that would be preferable / supportable that I can use to determine if a HBIOS function has the 0 bank switched in?

[feilipu]

Sure, you can certainly just put entries directly in the HBX_IVT table. The Z180 interrupt address table (HBX_IVT) will always be found at 0xFF00.

Noted. Excellent!

The IVT and your handler must be located in the top 32K or RAM because an interrupt may occur when various banks are selected in low RAM.

Noted.

I am a little confused about what you want your ISR to do. You can indeed determine the currently selected bank by examining the value at 0xFFE0. This is the current bank id. However, this is a tricky thing to do because a bank switch can be "in progress" when an interrupt occurs.

Yes, I see the HB_CURBNK is set with the currently running bank, and that it is maintained through HBIOS interrupt handling, even when running in the BID_BIOS bank. I guess the only sure fire way to know where the code is running from is to read the Z180_BBR value. That's OK, and I'll just go with that.

If you look at HBX_INT, you will see how RomWBW safely handles the selection of the HBIOS bank at the time of an interrupt and restores the original bank at completion. Not sure if I am helping here. I may need to better understand the intended functionality of your ISR.

The ISR is a part of freeRTOS with preemptive scheduling. But it also needs to handle cooperative scheduling too, where a task can voluntarily yield with interrupts in a specific state, and the same state needs to be provided to the task when it is scheduled back in.

Since the task control block maintains a full CPU state stacked (including interrupt status and alternate registers), it won't be possible to use the HBIOS HBX_INT code. The state stored on the stack in the task control block needs to be identically returned both following a cooperative and a preemptive task swap.

But, the HBX_INT code isn't identical, because it wasn't designed to be identical. It doesn't save IX or any of the alternate registers, for example. I guess it would be possible to store / return the missing registers in the right order, and have the cooperative yield match the same register stacking order. I'll look at possibly doing that.

The issue I see left is that a correct (but fake) interrupt status needs to be pushed onto the stack by the interrupt handler first, (following AF) but the HBX_INT code has already pushed a number of registers onto the stack before the interrupt handler gets invoked.

So my nominal code looks like this. If HBIOS is doing something, and I've interrupted it, then simply increment the tick and exit. HBIOS won't know, and the RTOS doesn't care...

prt1_isr() __naked
{
    portSAVE_CONTEXT_IN_ISR();
    xTaskIncrementTick();
    if( __io_Z180_BBR != BANK_0 )
        vTaskSwitchContext();
    portRESTORE_CONTEXT_IN_ISR();
}

Some more things to think through. But, I think I've got a working direction now.
Probably some questions, when things aren't working.
😄

[wwarthen]

Sounds like a serious challenge. Hope it goes well.

[feilipu]

Just got FreeRTOS running on YAZ180, 👍
So I'm putting HBIOS on the list to finish on the weekend. 😄

[wwarthen]

Excellent! Looking forward to the progress report.

[feilipu]

Ok. Progress report.

TLDR: FreeRTOS now works on SCZ180 with HBIOS. 👍

But, there is still something wrong... because it only runs for about 2 minutes, before I get a panic. But, running for 2 minutes means that the code is mostly OK.

Can I check some assumptions, please?

On not being in the HBIOS bank I do a test: BBR != 0xE000.

If that is true then I assume current execution is not in HBIOS, which is running in the 32k Bank at 0xE0000
Is that a correct assumption?

I think there may be more to this, because this is the only place where a problem could eventuate.

I need to copy the timer_isr() up to somewhere safe. So I've elected to put it at 0xFB00, as that is in the slack space after dbgmon. It is less than about 0x50 Bytes, so could go pretty much anywhere.
Would you suggest another (better) location, where it can be safe?

[wwarthen]

TLDR: FreeRTOS now works on SCZ180 with HBIOS. 👍

Awesome!

But, there is still something wrong... because it only runs for about 2 minutes, before I get a panic. But, running for 2 minutes means that the code is mostly OK.

I assume it is not exactly the same length of time consistently, right?

On not being in the HBIOS bank I do a test: BBR != 0xE000.

If that is true then I assume current execution is not in HBIOS, which is running in the 32k Bank at 0xE0000
Is that a correct assumption?

Hmmm... BBR is an 8 bit register. It will have the value 0xE8 when the HBIOS bank is selected. It will have the value 0xF0 when the user bank is selected. It seems like testing for BBR != 0xE8 should work.

Your ISR must be in upper 32K of CPU address space at all times. HBIOS switches to a small temporary stack before selecting the HBIOS bank. If your ISR hits at this point, you could overrun the stack.

I need to copy the timer_isr() up to somewhere safe. So I've elected to put it at 0xFB00, as that is in the slack space after dbgmon. It is less than about 0x50 Bytes, so could go pretty much anywhere.
Would you suggest another (better) location, where it can be safe?

That seems good to me.

[feilipu]

BBR is an 8 bit register. It will have the value 0xE8 when the HBIOS bank is selected. It will have the value 0xF0 when the user bank is selected. It seems like testing for BBR != 0xE8 should work.

I decided that it was much better to test for the presence of the TPA at 0xF0, rather than absence of HBIOS and since the vTaskSwitchContext() is only present in the TPA.

So, now it works 100% FreeRTOS is available for SCZ180 HBIOS.
EDIT 99%. Job nearly done.

Other HBIOS platforms can be added by simply choosing a Timer and configuring it, and adjusting the memory mapping for the TPA (only if needed). All done here.

But, one final question. If I was going to place a 0x50 Byte ISR when CP/M/HBIOS was running, where would be a logical place? I'm pretty sure that 0xFB00 won't work. Is there a slack space somewhere above 0x8000 that you can recommend?

[feilipu]

Oops. An update. Leave it running long enough, and it will eventually crash.

I think it is caused by a conflict in using the Serial ISR (if that needs to go into HBIOS bank). Not totally sure of why this is an issue (very occasionally), because the Timer ISR should back off if it detects it has triggered during a non TPA bank presence.

It would be easy to work around using a semaphore (in FreeRTOS) to protect the serial port. But, that shouldn't be necessary (and it should ideally "just work").

[wwarthen]

Hmmm... Is FreeRTOS doing any bank switching on it's own? If it does a bank switch and a serial int occurs, the serial ISR will restore the wrong bank at exit. If FreeRTOS is leaving the CPU memory space alone, I'm not sure what is happening. Interrupts are turned off throughout the entire time the HBIOS bank is swapped in during ISR processing.

[feilipu]

I'm pretty sure I know the cause. I forgot that xTaskIncrementTick() is also not above 0x8000, so it needs to be protected by a BBR check too.

Fortunately, there's a variable xPendedTicks exactly for this situation, so that if I can't process a tick increment, I simply need to increment this variable, and carry on in HBIOS.

EDIT using xPendedTicks was not necessary after stack adjustments.

Working on it tonight.

[feilipu]

Spent quite a few hours on this over the weekend, and still no definitive fix.

Did realise that pxCurrentTCB needs to be moved be uninitialised (in BSS which is above 0x8000), so that the ISR context save is being done correctly. Fixing this takes the average run time up to 15 minutes. But it will still crash in the end.

After thinking about it, I'm getting the feeling that it is a stack overrun. The context save needs 22 Bytes, so if the PRT1 interrupt handler is called when this stack is not available, then at this time there will be a problem. I need to look at HBIOS code some more to see if there is any time when interrupts are enabled, but the stack is not able to accept this context save.

[wwarthen]

After thinking about it, I'm getting the feeling that it is a stack overrun. The context save needs 22 Bytes, so if the PRT1 interrupt handler is called when this stack is not available, then at this time there will be a problem. I need to look at HBIOS code some more to see if there is any time when interrupts are enabled, but the stack is not able to accept this context save.

Quite possible. When an HBIOS function is called (e.g., read serial port), the upper memory proxy code switches to a small (20 byte) temporary stack (HBX_TMPSTK) just long enough to swap in the HBIOS bank. After that, the stack is switched to a much bigger stack in the HBIOS bank for the remainder of the HBIOS processing. The HBX_TMPSTK is also used briefly during the transition back from the HBIOS function call.

When HBIOS handles an interrupt, the first thing it does is switch to a private stack for interrupt handling, so HBX_TMPSTK size is not an issue. It sounds like your ISR needs 22 bytes. So, if the interrupt hits inside of a few instructions inside HBX_INVOKE, you are probably screwed.

You could probably increase the size of HBX_TMPSTK by 4-6 bytes without causing any issues.

Good luck!

-Wayne

[feilipu]

When an HBIOS function is called (e.g., read serial port), the upper memory proxy code switches to a small (20 byte) temporary stack (HBX_TMPSTK) just long enough to swap in the HBIOS bank. The HBX_TMPSTK is also used briefly during the transition back from the HBIOS function call.

Not sure this is my issue, but just looking at the HBIOS code, I'm wondering whether slightly reorganising the use of the various stacks would help to simplify any issues going forward?

Current state for the discussion:

There's 20 Bytes then HBX_TMPSTK then (in my build for SCZ180) 38 Bytes free HBIOS PROXY STACK space: 38 bytes which is used as stack by interrupts only as HBX_STK.

Then later (above 0xFF00) there's the HBX_INTFILL which for my compile is HBIOS INT space remaining: 20 bytes, followed by a 64 Byte HBX_BUF inter-bank copy buffer.

Suggestion for @feilipu:

I should use the HBX_STK stack specifically for my interrupt, rather than relying on the stack to be somewhere safe. This space would be unchallenged (as two interrupts shouldn't fire at the same time), and is large enough (38 Bytes) not to cause problems. This would add a configuration as this specific stack location would be different depending on the specific implementation. But, not a big deal.

Suggestion below would make the HBX_STK space even more usable at 58 Bytes.

EDIT further thought reveals that this was an uninformed suggestion. It isn't possible to proceed with a context switch if the context isn't pushed onto the exiting Task's stack.

Suggestion for @wwarthen:

Would it be useful to consider moving the HBX_TMPSTK to the 20 Bytes before HBX_BUF?

That is not a space needed for growth or flexibility, and it would provide the HBX_STK and the HBIOS proxy with more uncontested space. By that I mean if any interrupt processing needs a bigger stack than 38 Bytes, and it happened during processing of a HBIOS API call, then it would overwrite the HBX_TMPSTK. Boom.

HBX_TMPSTK doesn't look like it needs the whole 20 Bytes, as the only functions called are HBX_BNKSEL, and HBX_PEEK, and HBX_POKE which don't have much stack usage (except CALL RET).

Also, the size of HBX_STK is not a fixed amount as it is sized by a remaining space calculation. A change in the HBIOS proxy code would consume free stack, possibly leading to something unrelated blowing up, and be potentially difficult to triage. Adding 20 Bytes to it doesn't fix this issue, but does add some more headroom.

[feilipu]

A situation where the more thought is applied, the more nuanced it gets.

I'm reminded that the real reason that the timer interrupt saves the entire CPU context, and then a reference to it in pxCurrentTCB, is that its job is to switch between multiple task contexts. So saving references to a temporary stack not associated with (any) task is going to lead to a crash.

Then, as well as confirming that the BBR is correct, I also need to confirm that the current stack is not inside the HBIOS memory stubs (above 0xFE00) or, more robustly, is outside the configured application stack and heap.

But, because the context handling has to be identical for cooperative and preemptive context switch, I need to make sure that any stack I'm using is more than 22 Bytes plus stack for handling the context switch decision.

Which means that even the HBX_TMPSTK stack has to be large enough not to overrun. Or, alternatively disable interrupts for the short period while the bank is being switched and HBX_TMPSTK is being used. I think that might be the best way given there's not enough space available...

So with the HB_DI HB_EI protection around the HB_INVOKE temporary stack usage, #96, the problem is solved. FreeRTOS is now running stably.

I've still some work to do to ensure that I don't lose ticks when the HBIOS Bank is selected, but that's no longer a HBIOS issue.

[wwarthen]

Well, it is great to hear that you got it stable. There is a downside to using EI/DI inside of HB_INVOKE. It means that code using HBIOS functions cannot keep interrupts disabled across HBIOS calls. I think there are already a few places where this would be an issue.

I guess the RTOS timer ISR must be using more than the 20 byte stack available at HBX_TMPSTK. Is there any way the RTOS timer ISR could switch to it's own stack during interrupt processing? If not, I will review the implications of using DI/EI in HB_INVOKE, but would take a little bit of time.

[feilipu]

There is a downside to using EI/DI inside of HB_INVOKE. It means that code using HBIOS functions cannot keep interrupts disabled across HBIOS calls.

I originally tried to write an interrupt preserving patch but, long thought process - short, the only way I could see to make it work was to use the AF' register to cache the function return whilst restoring the interrupt state, and that might have had other implications that I wasn't aware of.

So to check that this really was a solution (and it certainly seems to be), I just did it simply with DI/EI.

I guess the RTOS timer ISR must be using more than the 20 byte stack available at HBX_TMPSTK.

I'm not sure I'm the best person to illuminate the mechanics of freeRTOS, but here goes... When running the Scheduler assumes that each Task is running with its own stack (or context). The only way that the stack and context changes is if a) the Timer ISR initiates a Yield preemptively, or b) if the Task itself initiates a Yield cooperatively.

So during the context swap the entire state of the CPU must be captured. This includes the state of the IFF2 register, and all alternate registers. I make that to be 22 Bytes, plus the ISR return address pushed by the interrupt. Lets say 24 Bytes. All of that is assumed to be stored on the bottom of the stack of the currently running Task, and this stack pointer is written finally to the currently running Task Control Block (TCB) pointer.

After the context swap (which is a function which also uses stack from the running Task until it returns), with a new TCB pointer containing the new stack pointer, which is then used to unroll the CPU state and return from the ISR / Yield to the newly restored Task.

The key point is that there is already one SP change during the Timer ISR. The context save and restore are assumed to be from different Task stacks. And also the restore within the preemptive Timer ISR may have been originated by a cooperative Yield. So the context stack has to be identical for both mechanisms.

Is there any way the RTOS timer ISR could switch to it's own stack during interrupt processing?

Its complicated. I spent some time thinking about how to do that. I wasn't able to think though a clean solution. You'd have to be aware which Task was running when the ISR was triggered, and move the CPU state over to that Task's stack, all while trying to be responsive inside an ISR.

The "best" solution I have is to go into the ISR, then identify the Bank and if it is wrong (in HBIOS) back off without doing the Tick increment (a function using stack), and without doing the Yield.

[Later I need to correctly increment the missed Tick counter, as the normal Tick increment function shuffles the waiting Task list to put pended Tasks in the ready to run list, and organises the Event list too. It can also process missed Ticks to jump the Tick counter forward. This way "real time" doesn't slip.]

If not, I will review the implications of using DI/EI in HB_INVOKE, but would take a little bit of time.

The other way, which may be just as hard, is to ensure that there is always enough space on the currently enabled stack. Then the Tick ISR can understand where it is, and if the Bank or Stack is wrong it can then back off (after incrementing the missed Tick counter), unroll its context, and return.

I couldn't find enough free space in the HBIOS memory layout to do this properly, so I abandoned that thought. I also thought about perhaps one of the buffers could be used? But again I don't know HBIOS well enough to know what could be changed.

Anyway. Two workable solutions being; DI/EI over the small temporary stack, or make the temporary stack (say) 32 Bytes. Either would work.

[wwarthen]

OK, how about 64 bytes? :-)

I just checked in a change that reuses the proxy bounce buffer as the temporary stack. The uses are mutually exclusive so it is a win-win. Brief testing looks good for me.

-Wayne

[feilipu]

Yes. that will work. 👍

EDIT Didn't work. 😞

I wasn't sure that they were mutually exclusive, as I guessed the bounce buffer might be used to pass info on the way out of the invoked function.

As it isn't; great solution.

Removes the TMPSTK which gives the interrupt stack more space. And the bounce buffer has also space below it that can be used if the new temporary stack happens to grow larger than 64 Bytes.

[feilipu]

I've done some testing, and the instability is back. I'm getting between 5000 Ticks and 100,000+ Ticks before the crash, but it comes inevitably.

Before any more HBIOS modifications, let me rule out the xIncrementTick() function blowing the whole 60 Bytes available to it (after the context save, and using the slack space) first.

Wait! Actually, I know what the problem will be. Attempting to Yield when using the TMPSTK stack will be the issue (now that it is not simply blowing up on the stack, and the stack is unguarded). I'll work on that this evening.

[wwarthen]

Wait! Actually, I know what the problem will be. Attempting to Yield when using the TMPSTK will be the issue (now that it is not simply blowing up on the stack, and the stack is unguarded). I'll work on that this evening.

I hope that is it because I don't think I can find any more stack space! :-)

[feilipu]

Excellent. FreeRTOS is now 100% reliable.

With the additional stack available I don't have to take special care with xIncrementTick(), other than make sure the bank is set to user TPA, which is a big help.

Simply ensuring the stack is not in the 0XFFnn page, along with the user TPA bank check, is sufficient to ensure that vTaskSwitchContext() only happens from within the running Task.

For interested readers the configurations specific to the SCZ180 target (SC126, SC130, and upcoming SC131), are found from in FreeRTOSBoardDefs.h .

I've opened #99 to move all the HIBOS temporary stacks up into 0xFF00 memory.

This is partially to clean up the use of the interrupt stack. But, my ulterior motive is to make testing for their usage easy because they will all be in the same page of RAM, so I'll only have to test for 0xFFnn.

I'm OK to force an upgrade to latest RomWBW version before using FreeRTOS, and this will be the trigger for that requirement.

I think we're done here. ;-)

[wwarthen]

I have merged #99. I did check-in a further update that changes the naming conventions somewhat with the hope of making everything a little easier to follow (not sure if I did or not).

Once you confirm my latest check-in to hbios.asm is OK, I will close this issue.

Thanks!

Wayne

[feilipu]

I had a quick look at your further hbios.asm update, and thanks for including the note about freeRTOS.

In #99 I was unsure whether PEEK and POKE (and for that matter BNKCPY) were invoked through the HBIOS API at all. So, I moved their stacks into the same buffer as INVOKE as a precaution (although for BNKCPY could not be so).

If PEEK, POKE, and BNKCPY are only used by DBGMON or during the initial loading process (not by the API) then there won't be an issue. And its all good. 👍

[wwarthen]

So... PEEK, POKE, and BNKCPY can all be invoked through the HBIOS API. However, I am not quite following your concern though.

For example, if PEEK is invoked through the HBIOS API, then HB_INVOKE will use TMPSTK briefly at the start and end of the routine while switching the bank. TMPSTK is not used during the actual HBIOS processing though (HBIOS has it's own low memory stack in it's bank).

Here's what I think should happen:

    - HBX_INVOKE uses and releases TMPSTK to activate HBIOS bank
    - HBX_INVOKE switches to HB_STACK (a low memory HBIOS stack) to run the PEEK API
        - PEEK API calls HBX_PEEK in high memory
            - HBX_PEEK uses and releases TMPSTK during processing
        - PEEK API returns to HBX_INVOKE
    - HBX_INVOKE uses and releases TMPSTK to restore callers bank
    - HBX_INVOKE restores callers stack
    - HBX_INVOKE returns to caller

I don't think there is any overlap in the use of TMPSTK.

Do you think I am missing something here? I'm worried I have not understood you.

Thanks!

Wayne

[wwarthen]

Oops, correction. HBX_INVOKE now uses HBX_BUF instead of TMPSTK for temporary stack.

    - HBX_INVOKE uses and releases HBX_BUF to activate HBIOS bank
    - HBX_INVOKE switches to HB_STACK (a low memory HBIOS stack) to run the PEEK API
        - PEEK API calls HBX_PEEK in high memory
            - HBX_PEEK uses and releases TMPSTK during processing
        - PEEK API returns to HBX_INVOKE
    - HBX_INVOKE uses and releases HBX_BUF to restore callers bank
    - HBX_INVOKE restores callers stack
    - HBX_INVOKE returns to caller

[feilipu]

If the user calls the HBIOS API, while freeRTOS is running, we have to assume that the Tick interrupt will happen anytime (at the worst possible time). I would prefer not to caveat the HBIOS API to users, and I'm sure you feel the same.

This means that the active stack can never be too small to handle a full context save and xIncrementTick() otherwise-> crash.

So I guess PEEK and POKE need to use the big stack.

[feilipu]

Just thinking a bit further... perhaps PEEK, POKE, and BNKCPY could use DI/EI to protect their use of the small stack (inside the API call?

HBX_PEEK, HBX_POKE, HBX_BNKCPY, could preserve the interrupt state on the HB_STACK, disable interrupts and use the small stack, then recover the former interrupt state off the HB_STACK when they return to INVOKE.

[wwarthen]

Ahhh... now I understand. I honestly didn't think that such calls would be occurring under FreeRTOS, but that was stupid -- why wouldn't a FreeRTOS app use them.

PEEK and POKE are not an issue (they can use use HBX_BUF). However, BNKCPY is a big problem because it cannot use HBX_BUF for stack. I think this was also an issue in your last update as well because you were using HBX_BNKSTK (20 bytes), which is not enough for FreeRTOS, right?

-Wayne

[wwarthen]

Just thinking a bit further... perhaps PEEK, POKE, and BNKCPY could use DI/EI to protect their use of the small stack (inside the API call?

HBX_PEEK, HBX_POKE, HBX_BNKCPY, could preserve the interrupt state on the HB_STACK, disable interrupts and use the small stack, then recover the former interrupt state off the HB_STACK when they return to INVOKE.

Yeah, I was just thinking about that too. That would be a pretty clean solution. I have this dim memory that there is a scenario where reading the current int state can fail. I need to refresh my memory on this. Maybe you know what I am talking about?

[feilipu]

Yes. I didn't have the HBX_BNKCPY fixed in #99. But, I didn't really know whether it was a problem (now you've confirmed it is).

My suggestion: just to get HBX_PEEK, and HBX_POKE to use the buffer as stack. And wrap only HBX_BNKCPY in a state preserving DI/EI solution. Best not to add useless bytes for interrupt management where they're not needed.

[feilipu]

On reading the interrupt state. There is a bug which only affects NMOS Z80 devices. z88dk has a CPU type flag that switches different solutions depending on the target (because for z88dk, the target may be 40 years old).

The normal CMOS Z80 solution looks like this (which I use in freeRTOS critical sections)

ld a,i
di
push af
...
code here running with interrupts disabled
...
pop af
di      ; < - optional, only needed if somehow interrupts could become re-enabled (via NMI).
jp PO,$+4 ; < - recovers the IFF2 state. PE if interrupt status was enabled.
ei
        ; < - normally ret, but interrupts enabled AFTER any instruction.

[wwarthen]

OK, I just checked in the interrupt protection for PEEK, POKE, and BNKCPY functions. It is a little hard to test, but seems to be working in general.

As far as I know, this should complete the FreeRTOS needs.

Thanks!

Wayne

[wwarthen]

@feilipu, if you think we are ready, I will bump the RomWBW version numbers and issue a full pre-release. This will allow you to tell folks exactly which pre-release they will need in order to run FreeRTOS. Just let me know if you are ready for this.

-Wayne

[feilipu]

You decided to go with wrapped rather than moving buffers again?
It looks good. Let me test tonight.

[feilipu]

I've just checked the hbios.com code, and I think there are two further HBX_BNKCPY instances that need to be protected to enable safe access to the HBIOS DIO API.

In HB_DSKWRITE here.
In HB_DSKREAD here.

The final instance is in the startup code, so I can't see that being a problem. 👍

Reading deeper, the other issue I see is HB_BNKCPY is used in quite a few places through the code base. Rather than wrapping every call of HB_BNKCPY, it might be easier to preserve the interrupt status within HBX_BNKCPY itself. Since A register is not used for its call, it doesn't need to be preserved.

Did a little PR #100 to address this. I'll continue testing on this fix. Updates in a few hours.

[feilipu]

OK. I've uploaded a little video, showing the results of our small endeavours...

I'm hammering the DIO API with the SD (also tested RAM disk). No errors in reading or writing. That is all happening whilst being interrupted 256 times per second, each time with a Tick increment and a Yield decision.

Simultaneously, there's a small Task running (making the total of 3 Tasks including the IdleTask that is run when no other Task is on the ReadyList). It won't interrupt the diskio because it is running at a lower priority. You can see the second Task running when the diskio testing is paused between runs.

Very occasionally are still some glitches. I don't know what's left to fix. They might even be caused by glitching from the FTDI interface if the USB interface drops.

Anyway, I think with #100 we're done, done. Ship it! 🍾

EDIT Nope... not done. But getting close.

[wrljet]

Ah, cool. If it is only NMOS processors, I can live this this!

After that comment and info about the interrupt difference/bug, I spent two hours on the phone in bed reading up on all that stuff on the web. :-)

[feilipu]

I am going to move the DI/EI protection for PEEK and POKE back into the main HBIOS code.

If space is an issue for N8, then don’t do the DI/Ei. Rather just use the bounce buffer as stack, like INVOKE.

Secondly, I want to confirm how you are checking that HBIOS is active. The first thing that HBX_INVOKE does is save the callers SP. If an interrupt fires immediately after this and RTOS does a context switch, the new context could call HBX_INVOKE and clobber the SP value stored by the original context, right?

I missed that issue. You’re right. Explains the last few glitches.

A flag won’t be sufficient. We need a SRA mutex. Same, but different. Code example to follow.

[wwarthen]

Ah, cool. If it is only NMOS processors, I can live this this!

After that comment and info about the interrupt difference/bug, I spent two hours on the phone in bed reading up on all that stuff on the web. :-)

Hi Bill!

Yeah, we have had some serious fun working through this FreeRTOS adaptation. Phillip has taught me several nice new tricks in the process. It has brought to light some serious deficiencies in RomWBW that I am happy to have ferreted out. I think we are close to something final. :-)

[wrljet]

Wayne,
Earlier in this exercise I was thinking that just making stacks bigger -hoping- they work is not a way to ensure success with this stuff. But I see now things are really much improved with the new layout.
Bill

[feilipu]

The first thing that HBX_INVOKE does is save the callers SP. If an interrupt fires immediately after this and RTOS does a context switch, the new context could call HBX_INVOKE and clobber the SP value stored by the original context, right?

Yes. You're right! But a flag won’t be sufficient. We need a SRA mutex. Same, but different.

Since we've got multiple Tasks all trying it use HBIOS and it is not reentrant, we need to ensure that only one Task is actively in a HBIOS API call at any one time. And the tricky thing is to avoid the risk of the scheduler swapping contexts between reading a flag, and setting it busy. The atomic mutex is the way to do this.

Some background for those reading along. A MUTual EXclusion semaphore (used to protect a resource) has to do two things; 1) be uniquely testable, and 2) record a new state, and it has to do this atomically (indivisibly). With the z80 processor the SRA (HL) instruction does this perfectly. A quick search will turn up further reading.

The mutex needs to be set as free initially, and this is done by setting 0xFE in a memory location. When competing threads or Tasks wish to use the resource they issue a SRA (HL) on the location.

So in the HBX_INVOKE function, we have to "take" and "give" its mutex to ensure that only one Task is is using the HBIOS API at any time. Doing the SRA (HL) instruction gives us a testable value in Carry, and atomically takes the mutex to prevent others using it.

Perhaps the HBX_LOCK byte can go here?

HB_LOCK  .DB  $FE ; <- the mutex is stored somewhere in high memory

HBX_INVOKE:
    push hl   ; <- assuming HL is needed here, otherwise no push/pop.
    ld hl,HB_LOCK
    sra (hl)   ; get HBIOS mutex
    jr C,$-2   ; just keep trying the sra (hl) until it is free. (I think it is '$-2', but I don't have an assembler here to check
    pop hl
   ;  ...
   ;  all the normal INVOKE code
   ; ...
    ld hl,HB_LOCK  ; give HBIOS mutex ; <- assuming we don't need to return HL here.
    ld (hl),$FE
    ret

Doing this ensures that multiple Tasks all competing for HBIOS will just spin waiting until they can get in.

It is a pity that it is such a "big lock" on all of the HBIOS API. It would be nice to break this down into little locks on each of the resources. But, that would take a redesign of HBIOS invocation process and is out of scope.

IIRC, there was the time that Linux also moved from a "big lock" to "little locks" on individual resources to enhance responsiveness, but @EtchedPixels is qualified to comment on that story, not me.

[wwarthen]

Hi Phillip,

Got it -- makes sense. OK, so naturally there are a couple issues. First is that this is going to use too much space and reduce the interrupt stack space to an inadequate size for some platforms. The second is the performance overhead.

The mutex is only needed for multi-processing environments. I am wondering if it would make sense to "hook" the HBIOS invocation vector at 0xFFF0?

Currently, 0xFFF0 contains a JP instruction that targets HBX_INVOKE. Optionally, the OS can also setup RST 08 to also point to HBX_INVOKE. I am wondering if FreeRTOS can replace the JP at 0xFFF0 to point to it's own code that wraps the call to HBX_INVOKE with the mutex. If RST 08 is setup, then that would need to be updated also. Obviously, this routine would need to be in high memory.

Thoughts?

-Wayne

[feilipu]

This is going to use too much space and reduce the interrupt stack space to an inadequate size for some platforms. I am wondering if FreeRTOS can replace the JP at 0xFFF0 to point to it's own code that wraps the call to HBX_INVOKE with the mutex?

Yes, certainly possible to do that.

But, what about (for expediency) just wrapping this HBX_LOCK code by #IF (MEMMGR == MM_Z180), and keeping this for the Z180 systems that have a simple/small HBX_BNKSEL implementation?

IIRC, moving the HBX_TMPSTK created an extra 20 bytes of interrupt stack space, and for the SCZ180 platform there's 58 Bytes free in my builds.

We can revisit this decision later for other platforms, (but probably not soon).

[wwarthen]

The N8 is actually a Z180. It just has a bizarre bank selection mechanism that provides RAM beyond the normal 1MB addressing space of the Z180.

Regardless, the code is simple, so I am going to go ahead and add it and see what we wind up with. I will make it a config setting, so easy to modify for now. Hang tight, should have this in an hour or so...

Thanks!

-Wayne

[feilipu]

A further thought... from reading this comment

; HBX_INVOKE IS NOT RE-ENTRANT!  HB_INVBNK CAN BE USED GLOBALLY TO DETERMINE
; IF HBIOS IS ALREADY ACTIVE.  HB_INVBNK WILL HAVE A VALUE != $FF WHEN HBIOS
; IS ACTIVE.  ON RETURN, HB_INVBNK IS SET TO $FF TO INDICATE HBIOS IS NOT
; ACTIVE.

Would it be possible to modify this code to be the mutex? It would save double code. It would look like this...

EDIT No, not easily.

[wwarthen]

Yeah, I think that would be a little messy. Let me put the straight-forward solution in and we can go from there.

[EtchedPixels]

The Linux locks are about multi-processing and different.

You don't want to be spinning or you'll deadlock with an RTOS. I would suggest that you do the wrapping in the RTOS because you don't want to spin, you want to yield in the RTOS so you immediately make progress on something else and you need your locking to be at the RTOS level so it can handle priority inversion and yielding.

None of this works at all if you make an HBIOS call that blocks of course!

[feilipu]

I would suggest that you do the wrapping in the RTOS because you don't want to spin, you want to yield in the RTOS so you immediately make progress on something else.

Good point. I think that HBIOS still needs to do the mutex check though, because it is the arbiter of whether it is being used or not. And, HBIOS doesn't know about the RTOS.

What I can do is check the mutex byte before doing a Yield in the Timer interrupt, which will keep the Task using HBIOS active.

(I need to think further about this though... it has some complexities that I'm not sure about. Your point about inversion is relevant to this too.)

[feilipu]

Ok. Thanks I'll do some testing on configSWITCH_CONTEXT using the stack pointer location vs. testing the mutex byte tonight.

Although this is not the end of the story, because it should be possible to Yield to a Task that isn't using HBIOS API, while another Task is using the API. But the complexity is that Yielding a Task using the API will keep it locked for others. This is outside my comfort zone 😱

Initial thought is that since a Yield will cycle through available ready Tasks, yielding at every opportunity (as it stands today) will be the right thing to do, irrespective of the HBIOS API mutex.

Probably the "hospital pass" here is to push the problem to the application writer to manage how they use the HBIOS API. FreeRTOS has Task Notification which works like a counting semaphore. This could be used to manage access to HBIOS API, and provide the best application performance.

I'd see our job is to keep pressing on regardless, or simply not crash.

[wwarthen]

Initial thought is that since a Yield will cycle through available ready Tasks, yielding at every opportunity (as it stands today) will be the right thing to do, irrespective of the HBIOS API mutex.

That's what I was sort of thinking. Clearly, not ideal, but there's no way HBIOS is going to become reentrant in the near future.

[feilipu]

I've done some testing using different Task priorities. TBH, acting maliciously I can get the system to crash, by driving the DIO API with low priority and pushing a high priority CIO API task in every few Ticks. But for normal reasonable usage, I'd say that we've covered everything possible at this stage.

So, whenever you're ready for a point release... and then I'll write it up for UK time zone.
Thanks so much for driving to get the right outcome. I'd have probably have given up. 💯

Phillip

[wwarthen]

So, for the actual point release, I need to do something with the conditional compilation of the MUTEX. It must be removed for the N8 due to space restrictions. Since most people will not be using FreeRTOS, my inclination is to default the use of the MUTEX to FALSE for all configs. This would mean using FreeRTOS requires setting that config variable to TRUE.

Are you OK with that?

Thanks,

Wayne

[feilipu]

I’m sure if you move it to your standard config process it won’t be a problem to leave off by default.

It would be helpful to leave it enabled by default for SCZ180 platforms, since there’s sufficient interrupt stack available. And today I can’t test other platforms. I think there are 32 bytes free.

EDIT Although it doesn’t matter to leave it off either. Since anyone using a point release would need to roll-their-own anyway.