hercules-390 / hyperion

Hercules 390
Other
248 stars 67 forks source link

Virtual interval timer incorrectly updated with ECPS:VM active #187

Closed wably closed 7 years ago

wably commented 7 years ago

Ivan,

I believe that I have found the cause of the interval timer problems that occur when ECPSVM is YES. I took me about 3 days to track it down but I think I have it. The solution is simple, but I would like to run it by you since you were the author of the ECPS support to see if you agree with my conclusions.

Briefly, a little background. Around three years or so ago, a member of the VM group posted that he could not get STIMER to work correctly in CMS when running ECPS, and of course it worked fine when ECPS was turned off. That issue is easy to recreate and I was able to validate that user's claims. The cause was that with ECPS active, the real machine's interval timer value was being stored in the virtual interval timer for the CMS user. This means that CP's time slice value in the real timer was being imposed upon the virtual machine user, regardless of what value the user wanted to store in his own timer for the purposes of his STIMER application. (In this case, the user wanted to wait for a few seconds). No matter what a CMS application stored in the virtual timer, ECPS overrode it with the real timer's value.

Of course that should not occur. The virtual timer should be decremented at the same rate as the real timer, but the contents of the virtual timer should not be reset to the real value.

The cause appears to be in the Hercules source in clock.c. A small snippet of code is pasted below for reference. The last statement shown is the one in question.

if defined(FEATURE_INTERVAL_TIMER)

static INLINE void ARCH_DEP(_store_int_timer_2) (REGS *regs,int getlock) { S32 itimer; . . itimer=int_timer(regs); STORE_FW(regs->psa->inttimer, itimer);

if defined(FEATURE_ECPSVM)

if(regs->ecps_vtmrpt) { vtimer=ecps_vtimer(regs); STORE_FW(regs->ecps_vtmrpt, itimer);

What is happening here is that the current real interval timer value is obtained and stored in PSA, and then if ECPS is active, the current virtual timer value is obtained but the real timer value is stored in the virtual timer location.

The problems are resolved if I change the last statement to STORE_FW(regs->ecps_vtmrpt, vtimer);

It is that simple. All of your timer calculations elsewhere and in ecpsvm.c appear to be done correctly. I can find no errors in processing after this change, and the STIMER example works perfectly and with whatever proper interval I specify. The results are now the same with ECPS on or off.

I'm pretty certain of my facts and findings here, but I always have a bit of doubt because I am not sure of your intentions at the time and there are no comments in the code to help clarify. If you could weigh in and say whether you agree with this change or not it would be very helpful.

Regards, Bob

wably commented 7 years ago

On 1/15/2017 ivan-w wrote:

Bob,

This seems like I had a BIG finger check when I wrote this. Your solution looks to be perfectly sensible to me.

Interval Timer management is a bit complicated in hercules because of the various optimization steps it went through - basically :

Inserting the ECPS:VM logic inside of that was always a bit tricky (hence the convoluted code). Your solution looks sensible and if your tests validate it, I would suggested it be implemented

It's a bit outside the scope of the origin SPKA problem... So maybe open a new issue, and provide the solution you suggest ;)

I'll test it with other VM versions.

--Ivan

wably commented 7 years ago

closing; fixed by commit of 3/4/2017