Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"PER G" register alteration event interruptions do not occur #87

Closed
Fish-Git opened this issue Apr 4, 2018 · 23 comments
Closed

"PER G" register alteration event interruptions do not occur #87

Fish-Git opened this issue Apr 4, 2018 · 23 comments
Assignees
Labels
BUG The issue describes likely incorrect product functionality that likely needs corrected.

Comments

@Fish-Git
Copy link
Member

Fish-Git commented Apr 4, 2018

"PER G" (Program Event Recording) register alteration event interruptions do not occur

(this is a copy of a problem in herules-390/issues/251 which also exists in SDL Hyperion)

Bob Polmanter writes:

When using Program Event Recording (PER) to trap general register alteration events, Hyperion does not provide a program check PER interruption when register alteration is selected in the PER event mask. Other PER events, such as instruction fetch, do provide the proper program check interruptions as expected.

This has implications for VM users. VM uses PER to facilitate tracing and stepping in virtual machines, including VM/370 with the Waterloo PER package installed. That's when I noticed that PER G was not triggering on any register alteration.

To track this down further and eliminate the possibility of a bug in the Waterloo PER code, or in CP, I simply reduced some PER tests to the bare minimum and placed them into short scripts that can be run from the Hercules console to recreate the problem.

The first script is "perif.txt". It loads the control registers for a PER instruction fetch test. If PER triggers, the program check new PSW is loaded with a disabled wait code 1. This works as expected and demonstrates that the emulator does provide PER interruptions, and that I have the script set up correctly, and that the PER Control Registers 9-11 are correct. The script sets you up in single step mode, and after the execution of the fourth instruction, the PER interruption occurs as expected.

The second script is "perg.txt" and this demonstrates the problem. It uses the exact same code as the first script, except that CR 9 is slightly different to instead set up a register alteration PER event instead of instruction fetch. It's an easy verification to check a S/370 yellow card to see that CR 9 is set up correctly in the script (at location 2F0). In any case, PER should trigger a program check interruption after the fourth instruction (when GR 5 is modified by a BALR 5,0), but it does not. Two instructions later R6 is modified and it also does not trigger.

Regards,
Bob

@Fish-Git Fish-Git added the BUG The issue describes likely incorrect product functionality that likely needs corrected. label Apr 4, 2018
@Fish-Git Fish-Git added the HELP! Help is needed from someone more experienced or I'm simply overloaded with too much work right now! label Jun 23, 2018
@Fish-Git
Copy link
Member Author

I've done some preliminary research into this issue and, believe it or not, it appears PER general register alteration support was never coded! (or rather never completed)

It appears the infrastructure for PER_GRA support is already in place in our cpuint.h header file (macros IC_PER_GRA, ON_IC_PER_GRA, OFF_IC_PER_GRA, IS_IC_PER_GRA, EN_IC_PER_GRA, OPEN_IC_PER_GRA, etc), but looking at the actual instructions that actually alter registers (e.g. the 'add_register', 'add', 'add_halfword', 'add_halfword_immediate', 'add_logical_register', 'and_register', ... 'load_address', etc... functions in source file general1.c for example), there doesn't appear to be any code to trigger any type of PER_GRA event!

So to resolve this issue it appears all we need to do is identify all instructions that alter any general register and add some type of macro or function call to trigger a PER_GRA event if it's enabled in CR9.

I'm a little stumped as to how best to do that however, without negatively impacting performance to any great degree. I mean, it's obvious there's going to be some impact to performance, but how best to design/code it so as to keep the impact to an absolute minimum has me stumped at the moment.

If anyone has any ideas I'd love to hear them!

Thanks!

@Peter-J-Jansen
Copy link
Collaborator

Peter-J-Jansen commented Aug 11, 2018

Am I correct to assume that this only applies to ARCHLVL S/370 and was only possible with PER 1? Perhaps that would explain why it never got implemented?

If that is correct, then the other ARCHLVL's can remain unaffected performance wise.

By the way, I don't know how z/VM implements it, but a "CP TRACE G1 DATA 7FFFFFF0" worked flawlessly. (I used it during my DMSFRR161E debugging.)

Cheers,

Peter

@Fish-Git
Copy link
Member Author

Fish-Git commented Aug 11, 2018

Am I correct to assume that this only applies to ARCHLVL S/370 and was only possible with PER 1?

It appears so, yes.

From page 1-27 of SA22-7832-11 zArchitecture Principles of Operation:

  • The program-event-recording facility 2 (PER 2) is an alternative to the original PER facility, which is now named PER 1.
    ...
    PER 2 deletes the ability to monitor for general-register-alteration events.

(SA22-7201-08 ESA-390 Principles of Operation also says the exact same thing, but on page 1-1 in the section Highlights of ESA/390.)

Thus I'm presuming S/390 was what introduced PER 2 and the dropping of support for monitoring of general-register-alteration events, implying that PER 1 (and the ability to monitor for general-register-alteration events) was a System/370-only feature.

Perhaps that would explain why it never got implemented?

Unknown. But it's only important that it never got implemented, not why.

By the way, I don't know how z/VM implements it, but a "CP TRACE G1 DATA 7FFFFFF0" worked flawlessly.

Thanks! That's good to know. But CP PER G (for ARCHLVL 370) should also work.

And the fact that it currently doesn't is an obvious deviation from the published architecture and thus is something that needs to be fixed.

@wably
Copy link
Member

wably commented Aug 16, 2018

Fish,

I looked into this as well about two weeks ago and I also could not find evidence that register alteration events for PER in S370 mode were ever implemented.

Clearly no one is using this particular PER feature or someone would have reported this error long ago. Even now I am wondering if it would be worth the overhead to have it implemented - an overhead price that would have to be paid for something that isn't used. Yet there is also the view that it should be implemented because it is part of the architecture and that is that.

But I think that if this were done in the inner most core where the instruction execution loop runs (cpu.c ??) overhead could be kept to the barest minimum. Basically, if the PER bit is zero in an EC PSW then you can skip all other checks. If someone is using PER then they pay a higher price for additional checks to come but thats the way it is in order to use the facility. The PER register alteration check could be done immediately after the individual instruction routine has executed and returned to the loop before proceeding to fetch the next instruction. Identifying registers that were altered by whatever instruction just executed could be done by keeping a separate internal copy of the general registers and comparing the internal copy against the actual GRs against a selection mask of particular registers enabled for akteration events in CR9. This overhead would only be required when PSW bit 1 is enabled in EC mode, and then the actual register alteration compares would only need be performed if the register alternation event mask bit in CR9 is set.

One downside of having the check done here (in the execution loop) is that even instructions that do not alter a register have at the check performed. But again is is only expensive when PER is enabled. But it seems quite a bit easier to implement it this way rather than modify every register altering instruction in generalx.c and other places as well in order to call a routine for a PER_CHECK, similar to what is done for privileged instructions via PRIV_CHECK( ).

Regards,
Bob

@Fish-Git
Copy link
Member Author

Fish-Git commented Aug 16, 2018

Identifying registers that were altered by whatever instruction just executed could be done by keeping a separate internal copy of the general registers and comparing the internal copy against the actual GRs against a selection mask of particular registers enabled for alteration events in CR9.

 
That unfortunately will not work:

Chapter 4. Control

Program-Event Recording

PER Events
General-Register Alteration:

The contents of a general register are considered to have been altered whenever a new value is placed in the register. Recognition of the event is not contingent on the new value being different from the previous one.

 
Thus, according to the architecture, LR R0,R0 should cause a PER_GRA event, event though the value of the register's contents has not been changed. There is unfortunately no way that I can see to implement this feature without modifying each and every instruction that updates a register.

Now in order to completely eliminate the overhead for non-S/370 architectures, we can devise a macro that calls a function that checks if PER_GRA is enabled (and if so, checks the CR9 mask to see if it is for the specific register the user is interested in) that is only #defined for the 370 build architectures but not for any other non-370 build architecture (i.e. is defined as an empty "do nothing" macro).

That way there is 0% (absolutely zero) overhead for non-370 architectures, and only the minimum amount of overhead for the 370 build architecture.

I do appreciate the feedback though!

@wably
Copy link
Member

wably commented Aug 16, 2018

Ugh, yes I forgot about that little detail. That's a game changer just as you pointed out.

It wouldn't be too bad a job if it could be limited to the S370 instruction set on the yellow reference card. But then there were some instructions added plus 370-XA and 370/ESA which added even more. And then what about all of the instructions that could be executed by using 'ldmod s37x'?

@ivan-w
Copy link
Member

ivan-w commented Aug 16, 2018

How about : Have and use a separate instruction dispatch table for instructions that update registers and have them have the GPR manipulation macros expand to do a check in that case.. That'd be close to implementing a new architecture - tricky to implement but would not incur any performance hit when PER G is not active (not much change in each individual instruction, but this means another self include..) Just food for thought !

@ivan-w
Copy link
Member

ivan-w commented Aug 16, 2018

Never mind... The issue is that GPR updates are not done through macros (or static inline functions)... They should have been from the start - but that'd be a major rework....

@Fish-Git
Copy link
Member Author

@ivan-w wrote:

The issue is that GPR updates are not done through macros (or static inline functions)... They should have been from the start - but that'd be a major rework....

Yes it would, but nevertheless that is what I'm currently looking into doing, because that's the only way that I can see how it could possibly be done.

I'm currently researching how to maybe devise a macro and set of functions designed for updating register values similar to our existing SET_GR_A macro and add_logical et al. functions in source file inline.h that a lot of instructions use (e.g. DEF_INST(add_register) for example).

Once all of the affected instructions (or possibly all instructions for any architecture that updates any GP register) have been updated to use the new ARCH_DEP macro, then it would be a simple(?) matter of adding the code to check for PER.

The function that said macro would call would be an ARCH_DEP function that would simply call the existing e.g. add_logical function to actually update the register, but would then have an #if defined( FEATURE_PER1 ) code section that would of course only be expanded for build architecture 370. That way none of the existing build architectures would be impacted, only 370.

It would also isolate all general register updating logic to a small set of functions too, making it much easier to implement the PER_GRA code for 370 without having to add special PER_GRA code to every instruction that updated a GP register.

Every instruction that updates a GP register would of course still need to be changed of course, but only to call this new function via the new ARCH_DEP macro. The actual PER_GRA logic however would not exist in each individual instruction, but rather would be isolated in a single function which each of the add_logical et al. functions would then call into (but only if FEATURE_PER1 was #defined of course).

Anyway, that's what I'm currently looking at. Does it sound reasonable (or semi-reasonable) to you, Ivan?

@Fish-Git
Copy link
Member Author

@wably wrote:

And then what about all of the instructions that could be executed by using 'ldmod s37x'?

First, the s37x loadable module no longer exists. It has been replaced by the new HERC_370_EXTENSION facility. Please refer to the "Hercules S/370 Instruction Extension Facility" item on the "Release Notes" web page.

Second, in my opinion, since the 370-extension instructions were never a part of the 370 instruction set, PER_GRA should not apply to them.

However..., If done right (i.e. if, as I alluded to in my reply to Ivan, any/all instructions that update a GP register was properly modified to use the new macro and set of functions), then yes, all 370-extension instructions would also be automatically included too.

Which is why I'm looking at doing what I'm thinking of (considering) doing: because from a technical standpoint I believe doing it that way is the correct way of doing it.

I'm just kind of busy with some other things right now, so I haven't had time to work on it.

If you or Ivan want to work on it, then please, by all means, do so! Just let me know if you do so I don't waste my time doing what you're already doing. I can then move on to something else (like getting back to working on CCKD64).

@wably
Copy link
Member

wably commented Aug 17, 2018

I don't suppose there would be any stomach for a partial implementation?

That is, if the requirement to signal a register alteration event for a register modified with the same value (e.g., the LR R0,R0 case) was ignored, then the implementation of the rest of the feature becomes almost easy, by comparison. Some justification:

  • 99.9% of the functionality of PER GRA would be available and working.
  • A person doing debugging with PER G mostly likely wouldn't mind that an LR 0,0 case doesn't generate an interrupt since that person would be interested in register content that actually changed.
  • The exception to the complete implementation could be documented in the aforementioned Release Notes page.
  • No known S370 operating systems require PER GRA in order to function. This is obvious since they are operating correctly without any PER GRA at all. Thus, a partial implementation would not break anything.
  • There are other cases within the Hercules emulator where function is only partially implemented or the emulation is not 100% like real hardware.
  • This is simply an instance of a huge amount of effort being expended in order to bring that last requirement into force, for very little actual benefit (I do understand the purist case however of following the POO exactly). I'm just suggesting it might not be worth it for this case.
  • Finally, full implementation would be detrimental to the readability of the emulator's source code. It's already difficult enough with multiple levels of macros and other indirection. This would necessarily add to the problem.

@Fish-Git
Copy link
Member Author

I don't suppose there would be any stomach for a partial implementation?

Perhaps, but certainly not any type of hard-coded partial implementation, that's for sure!

Having a new configuration file setting that controlled whether the behavior you described should be enabled or not however, is definitely a possibility IMO (especially since I agree with you 100% regarding most people only being interested in register updates that actually change a register's value, and not those that don't).

  • Finally, full implementation would be detrimental to the readability of the emulator's source code. It's already difficult enough with multiple levels of macros and other indirection. This would necessarily add to the problem.

Not if done properly IMHO.

I agree that Hercules is heavily over-macro-ized IMO (and I hope to, over time, correct that), but once you start working with Hercules code, it doesn't take much to familiarize oneself with its various macros which, IMO, in most cases, actually helps the readability of the code, not detract from it!

I mean, once you become familiar with a given macro and what it does, it's a lot easier (IMO) to read that one statement in a thousand different places than it is to have to wade through the various tests, etc, that would otherwise have to be hand coded in those same thousand different places all throughout Hercules if said macro wasn't used. Plus, when you have to hand code said logic everywhere instead of using a macro, you greatly increase the likelihood of a hard-to-find coding mistake being introduced somewhere due to a copy & paste error, etc. (Plus, should the logic that said macro ever need to be updated (changed/fixed), which would you rather do: implement the change/fix in a thousand different places and hope you didn't miss a spot? Or in one place: the macro definition? Think about it.)(*)

Thank you for the good feedback, Bob!


(*) But as I said, macros with a lot of code behind them should, IMHO, really be functions and not macros. Macros unnecessarily chew up valuable cache lines in the processor's instruction cache (and cannot be stepped through with a debugger either), whereas functions are processor instruction cache friendly and can be stepped through with a debugger.

You also cannot set a debugging breakpoint on the statement in the middle of a macro either, whereas you easily could if it were a function. (And you'd be able to catch the condition you're interested in catching without having to set a thousand breakpoints all over the place too!)

Functions good. Macros bad. (**)

(**) Generally speaking anyway, but especially in Hercules's case where it down right abuses the use of macros IMO!

@wably
Copy link
Member

wably commented Aug 17, 2018

To clarify and to ensure that I understand you correctly, your reference to not wanting a "hard coded" solution means anything that is not 100% accurate to the POO and "always on" and active and ready to be invoked (whenever the PSW and CR bits are right of course)?

And then, taking that a step further, again for clarification, the point of the configuration file option would be so that a user could deliberately choose to enable a partial implementation, knowing that it is only partial and does not do the LR 0,0 type of case? If this is correct, then the more centralized solution of comparing an array of internal registers for changes could be written and enabled by this configuration option?

Do I have this right?

Regards,
Bob

@Fish-Git Fish-Git removed the HELP! Help is needed from someone more experienced or I'm simply overloaded with too much work right now! label Aug 31, 2018
@Fish-Git
Copy link
Member Author

(3.5 years later):

If this is correct, then the more centralized solution of comparing an array of internal registers for changes could be written and enabled by this configuration option?

Do I have this right?

I'm not sure, but I think so, yeah.

@Fish-Git Fish-Git self-assigned this Jan 25, 2022
@Fish-Git Fish-Git added the IN PROGRESS... I'm working on it! (Or someone else is!) label Jan 25, 2022
@Fish-Git
Copy link
Member Author

Fish-Git commented Feb 1, 2022

Bob (@wably),

I have a patch ready for you to test:

I have not performed any comprehensive (thorough) testing of every single instruction. I only tested it with the quick/simple test you posted earlier.

Please confirm for me that it works okay for you so I can commit my changes to the repository and close this issue.

Thanks!

ADDITIONAL INFORMATION:

I've implemented it by adding a PER GRA test to each instruction that Appendix B of the two 370 Principle of Operation manuals list with a "R(PER general-register-alteration event) characteristic.(*)

What is interesting to note is that while manual GA22-7000-10 lists the B229 Insert Storage Key Extended (ISKE) instruction with the "R" characteristic, manual SA22-7200-00 does not. I rather suspect this is a documentation bug. The ISKE instruction definitely alters the operand-1 register and does so whether you are running in ESA mode or not. So manual SA22-7200-00 is in error IMO.

QUESTION: Do you think we should bother to code a comprehensive/thorough PER GRA test program that tests each and every instruction to ensure they are each properly generating the expected PER GRA event, or do you feel your original simple test is good enough? It would take a lot of time/effort to code such a test and I am of the opinion that it's probably not worth the effort. What do you think?


(*) See pages B1 and B-10 to B13 of GA22-7000-10 System/370 Principles of Operation, and pages B1 and B15 to B20 of SA22-7200-00 ESA/370 Principles of Operation.

@Fish-Git Fish-Git added QUESTION... A question was asked but has not been answered yet, -OR- additional feedback is requested. Waiting to close... Waiting for user to report back whether problem still exists or not before closing as resolved. and removed IN PROGRESS... I'm working on it! (Or someone else is!) labels Feb 1, 2022
@Fish-Git
Copy link
Member Author

Fish-Git commented Feb 1, 2022

ADDITIONAL NOTE:

While manual SA22-7201-08 ESA/390 Principles of Operation also lists instructions with the "R" characteristic (implying that ESA/390 also supports PER 1 GRA events), I did not bother coding PER 1 GRA support for all of its instructions since Hercules's support for the 390 architecture purposely does not have PER 1 support enabled for it. Apparently Hercules only supports PER 1 for 370, PER 2 for 390 and PER 2 and PER 3 for z/Architecture.

I'm guessing it's because either not all 390 architecture capable machines necessarily supported PER 1, -or- because initial z/Architecture support necessarily also required support for the 390 architecture as well, and since z/Architecture doesn't support PER 1, neither can 390?  (with the latter being the most likely reason IMO. That is to say, I suspect that while some mainframe models that supported 390 (but not z/Architecture) may have supported PER 1, no mainframe model that supported both z/Architecture and 390 likely supported PER 1?)

Thus, while some 390 VMs (e.g. VM/ESA?) may support PER 1 GRA (Does it? I don't know! I didn't bother to check), such support is not provided on Hercules mainframes. Hercules only supports PER 1 for System/370 architecture only.

I hope that's not a problem?

@wably
Copy link
Member

wably commented Feb 1, 2022

Hi Fish,

Sorry, but early look: no worky. Both VM/370 and VM/SP did not trigger a PER interruption on a PER G command.

I did verify that the perg.txt file that I posted when opening the issue does indeed trigger a PER interruption.

Looking closer, I think I may have found the problem: your patch might not be allowing for the possibility that both CR10 and CR11 (the starting and ending range of addresses for the particular PER trap to be in effect) can be zero. Zero in both CRs means the entire addressing range of the machine.

Both of the S/370 VM operating systems use zero for CR10 and CR11.

For further validation of this theory, I changed a copy of the perg.txt test file here to use zero in both CR10 and CR11 (instead of the X'00000230' and X'0000023F' range it was loading in the original). After making that change and rerunning the test, indeed it did NOT trigger a PER interruption. Then for grins, upon changing CR11 to X'00FFFFFF' then PER interruptions started occurring.

It seems that you need to allow for both CR10-CR11 to be zero and mean 'the entire address range' and not restrict the address range to literally location 0 only. I don't know what the Principles say about this specifically, but if both S/370 VMs are doing it this way, then the PER G command will not work at all despite your efforts.

The PER G command also allows a range to be set on the command, but specifying this range DOES NOT alter the values in CR10-CR11; instead, it is simply a mechanism for CP to verify the address range on receipt of every PER interruption; it will pass them on to the virtual machine if in range, ignore them if outside the range. This paragraph is not relevant to your patch; I only mention it here before someone posts that PER G can have a range and claim that is why CR10/11 are zero if no range is specified on the command. Not true. (CP does it this way because a user can set multiple ranges to be active simultaneously, which can't be supported by the hardware with only one set of CRs.)

Regarding PER 1 on a S/390, I am not up to speed on the capabilities of the various levels of PER on S/390 and which came and went away and when. So I can't confirm or deny anything on that, except for one thing: I tried VM/ESA with PER G using SDL 4.4 even before I applied your patch - just out of curiosity since you mentioned it above - and I found that PER G is working just fine under VM/ESA. I don't think you need to do anything in this area.

Regards,
Bob

@wably
Copy link
Member

wably commented Feb 1, 2022

Fish,

More information. In the S/370 Principles of Operation (1987 version GA22-7000-10) it states on page 4-18 under Storage-Area Designation that only the PER instruction-fetch and storage-alteration events are involved in a storage-area designation. Meaning, that a general register alteration event doesn't use the CR10/CR11 pair to limit the range of the events.

I was also incorrect about stating that both CR10 and CR11 being zeros meant the whole address range was to be used. Also on page 4-18 (2nd to last paragraph) it also states "When the starting address is equal to the ending address, only that one location is designated." I apologize for that. Nevertheless, this is a moot point since CR10-CR11 are not apparently used by GRA events.

Regards,
Bob

@Fish-Git
Copy link
Member Author

Fish-Git commented Feb 1, 2022

... a general register alteration event doesn't use the CR10/CR11 pair to limit the range of the events.

Easy fix.

Here's the replacement patch:

Range check removed.

@wably
Copy link
Member

wably commented Feb 1, 2022

Hey Fish,

Your updated patch is working perfectly now.

I dinked around with a few individual instructions, just by random choice, by storing them into virtual storage in a virtual machine and pointing the PSW at them to make sure that CP got the PER interrupt and reflected it back to the user, and it did.

The CP PER G command is working perfectly when selecting an individual register, such as R6 and only interrupting on R6 modifications. Same for a range of registers, e.g., G6-8. And finally, the PER G command is working properly when an address range is specified on the command and only flagging modifications for the specified register(s) inside the range. It all looks good.

However, please note that register modifications that did not change the register contents did not present an interruption. This would be the LR R2,R2 case or a case like SR R1,R1 then LA R1,0. This is in violation of the Principles of Operation which clearly state that a modified register is interrupted even if the register contents does not change. BUT - we discussed this above in this issue long ago and I was in favor of proceeding in this exact manner because the effort to flag modifications where the contents did not change was too much overhead, given that most programmers only care about actual contents changes. You were not particularly in favor of this (because it didn't follow the principles), so I am surprised to see this here. Anyway, I just wanted to bring that to your attention that I identified this behavior in your patch.

Other than that, this patch looks real good and it is working perfectly. I also verified both VM/SP and VM/370, and both with and without ECPS. ECPS does not interfere with PER G. I also quickly verified that VM/ESA + PER G is still working with your patch applied. It is.

Thanks Fish. Good work!

Regards,
Bob

@Fish-Git
Copy link
Member Author

Fish-Git commented Feb 2, 2022

However, please note that register modifications that did not change the register contents did not present an interruption.

It's obviously VM doing that, not the hardware, because the code, as written, does not care whether a register value has changed from its previous value or not. It does not check to see if the value has changed. Therefore it must be VM itself doing it.

You can verify this for yourself via a minor tweak to your original perg.txt standalone test script: just change the NOPR at address 232 from 0700 to 1822 (LR R2,R2), and the CR9 value at 2F0 from 1000FFFF to 10002000:

*************************************
* PER Register Alteration event test
*************************************
numcpu 1
sysclear
archlvl 370
ostailor null
r 000=0008000000000200  # S/370 restart PSW
r 068=000A000000000000  # S/370 pgm new PSW
r 200=B79902F0          # LCTL 9,9,CR9       Load CR9 with PER events masks
r 204=1B66              # SR R6,R6           Clear for register alteration events
r 206=820002E0          # LPSW PERPSW        Enable PER
r 230=0550              # BALR R5,0
r 232=1822              # LR R2,R2       <== SHOULD trigger PER register alteration event
r 234=41606001          # LA R6,1(,R6)
r 238=07F5              # BR 5               Loop back to NOPR at location 232 
r 2E0=400C000000000230  # PERPSW             Enable PER psw
r 2F0=10002000          # CR9                PER events mask = GRA + R2 only
restart
pause .1
r 96.2                  # PER code           1000 = PER GRA event
r 98.4                  # PER Address        00000232 = LR instruction

The LR instruction should cause a PER G event, even though the register value is obviously not being changed. And when you run the test, that is indeed exactly what happens:

17:30:23.249 ******** Power-On Date and Time  =  2022/02/01 at 17:30:23 PM
17:30:23.249 ******** Program to be executed  = "C:/Users/Fish/Documents/Visual Studio 2008/Projects/Hercules/_GIT/_Fish/hyperion-0/msvc.AMD64.bin/Hercules.exe"
17:30:23.254 ******** Control file to be used = "C:/Users/Fish/HercGUI/Configuration Files/coretests-basic.txt"
17:30:23.267 Hercules started; process-id=00000F98
17:30:23.310 HHC00100I Thread id 00002920, prio 5, name 'impl_thread' started
17:30:23.310 HHC00100I Thread id 0000238c, prio 4, name 'logger_thread' started
17:30:23.311 HHC01413I Hercules version 4.5.0.10665-SDL-DEV-g774a1ae9-modified
17:30:23.311 HHC01414I (C) Copyright 1999-2022 by Roger Bowler, Jan Jaeger, and others
17:30:23.311 HHC01417I ** The SoftDevLabs version of Hercules **
17:30:23.312 HHC01415I Build date: Feb  1 2022 at 13:32:02
17:30:23.312 HHC01417I Built with: Microsoft Visual Studio 2008 (MSVC 150030729 1)
17:30:23.312 HHC01417I Build type: Windows MSVC AMD64 host architecture build
17:30:23.312 HHC01417I Modes: S/370 ESA/390 z/Arch
17:30:23.313 HHC01417I Max CPU Engines: 64
17:30:23.313 HHC01417I Using   shared libraries
17:30:23.313 HHC01417I Using   Fish threads Threading Model
17:30:23.313 HHC01417I Using   Error-Checking Mutex Locking Model
17:30:23.313 HHC01417I With    Shared Devices support
17:30:23.313 HHC01417I With    Dynamic loading support
17:30:23.313 HHC01417I With    External GUI support
17:30:23.314 HHC01417I With    Partial TCP keepalive support
17:30:23.314 HHC01417I With    IPV6 support
17:30:23.314 HHC01417I With    HTTP Server support
17:30:23.314 HHC01417I With    sqrtl support
17:30:23.314 HHC01417I With    Signal handling
17:30:23.314 HHC01417I With    Watchdog monitoring
17:30:23.314 HHC01417I With    CCKD BZIP2 support
17:30:23.315 HHC01417I With    HET BZIP2 support
17:30:23.315 HHC01417I With    ZLIB support
17:30:23.315 HHC01417I With    Regular Expressions support
17:30:23.315 HHC01417I With    Object REXX support
17:30:23.315 HHC01417I Without Regina REXX support
17:30:23.315 HHC01417I With    Automatic Operator support
17:30:23.316 HHC01417I Without National Language Support
17:30:23.316 HHC01417I With    CCKD64 Support
17:30:23.316 HHC01417I With    Transactional-Execution Facility support
17:30:23.316 HHC01417I With    "Optimized" instructions
17:30:23.316 HHC01417I With    OPTION_USE_SKAIP_AS_LOCK
17:30:23.316 HHC01417I With    OPTION_SIE2BK_FLD_COPY
17:30:23.316 HHC01417I Machine dependent assists: cmpxchg1 cmpxchg4 cmpxchg8 cmpxchg16 hatomics=msvcIntrinsics
17:30:23.316 HHC01417I Running on: WIN7 (Windows-6.1.7601 Intel(R) x64) LP=8, Cores=8, CPUs=2
17:30:23.316 HHC01417I Built with crypto external package version 1.0.0.50-geed8a86
17:30:23.316 HHC01417I Built with decNumber external package version 3.68.0.100-g208ee4c
17:30:23.316 HHC01417I Built with SoftFloat external package version 3.5.0.103-g9ba90c7
17:30:23.317 HHC01417I Built with telnet external package version 1.0.0.60-g524b6e8
17:30:23.317 HHC00018I Hercules is running in elevated mode
17:30:23.317 HHC02323W This build of Hercules has only partial TCP keepalive support
17:30:23.317 HHC00007I Previous message from function 'impl' at impl.c(1118)
17:30:23.319 HHC00150I Crypto module loaded (C) Copyright 2003-2016 by Bernard van der Helm
17:30:23.319 HHC00151I Activated facility: Message Security Assist
17:30:23.319 HHC00151I Activated facility: Message Security Assist Extension 1, 2, 3 and 4
17:30:23.344 HHC17528I REXX(OORexx) VERSION: REXX-ooRexx_4.2.0(MT)_64-bit 6.04 22 Feb 2014
17:30:23.344 HHC17529I REXX(OORexx) SOURCE:  WindowsNT
17:30:23.344 HHC17525I REXX(OORexx) Rexx has been started/enabled
17:30:23.344 HHC17500I REXX(OORexx) Mode            : Subroutine
17:30:23.344 HHC17500I REXX(OORexx) MsgLevel        : Off
17:30:23.344 HHC17500I REXX(OORexx) MsgPrefix       : Off
17:30:23.345 HHC17500I REXX(OORexx) ErrPrefix       : Off
17:30:23.345 HHC17500I REXX(OORexx) Resolver        : On
17:30:23.345 HHC17500I REXX(OORexx) SysPath    (45) : On
17:30:23.345 HHC17500I REXX(OORexx) RexxPath   ( 0) :
17:30:23.345 HHC17500I REXX(OORexx) Extensions ( 8) : .REXX;.rexx;.REX;.rex;.CMD;.cmd;.RX;.rx
17:30:23.346 HHC00111I Thread CPU Time IS available (_POSIX_THREAD_CPUTIME=1)
17:30:23.347 HHC00100I Thread id 000026e4, prio 2, name 'Processor CP00' started
17:30:23.347 HHC00100I Thread id 000015c4, prio 7, name 'timer_thread' started
17:30:23.347 HHC00811I Processor CP00: architecture mode z/Arch
17:30:23.347 HHC02204I LPARNAME       set to HERCULES
17:30:23.347 HHC02204I LPARNUM        set to BASIC
17:30:23.348 HHC02204I CPUMODEL       set to 0158
17:30:23.348 HHC02204I CPUSERIAL      set to 111111
17:30:23.348 HHC02204I CPUVERID       set to 03
17:30:23.348 HHC02204I MODEL          set to hardware(EMULATOR) capacity(EMULATOR) perm() temp()
17:30:23.348 HHC02204I MANUFACTURER   set to HRC
17:30:23.348 HHC02204I PLANT          set to ZZ
17:30:23.349 HHC02204I MAXCPU         set to 1
17:30:23.349 HHC02204I NUMCPU         set to 1
17:30:23.349 HHC00827I Processor CP00: engine 00 type 0 set: CP
17:30:23.350 HHC17003I MAIN     storage is 2M (mainsize); storage is not locked
17:30:23.361 HHC00811I Processor CP00: architecture mode S/370
17:30:23.371 HHC02204I ARCHLVL        set to S/370
17:30:23.371 HHC02204I AUTO_SCSI_MOUNT set to NO
17:30:23.372 HHC00346I cckd opts: comp=-1,compparm=-1,debug=0,freepend=-1,fsync=0,gcint=10,gcparm=0
17:30:23.372 HHC00346I            linuxnull=0,nosfd=1,nostress=0,ra=2,raq=4,rat=2,trace=64,wr=2
17:30:23.372 HHC02323W This build of Hercules has only partial TCP keepalive support
17:30:23.372 HHC00007I Previous message from function 'conkpalv_cmd' at hsccmd.c(8458)
17:30:23.373 HHC02204I conkpalv       set to (3,1,10)
17:30:23.373 HHC01474I Using internal codepage conversion table 819/1047
17:30:23.373 HHC02204I DIAG8CMD       set to DISABLE  NOECHO
17:30:23.373 HHC02204I ECPSVM         set to disabled
17:30:23.374 HHC02204I PORT           set to port=80 noauth
17:30:23.374 HHC01508I HDL: loadable module directory is 'C:\Users\Fish\HercGUI\Configuration Files'
17:30:23.374 HHC02204I MODPATH        set to C:\Users\Fish\HercGUI\Configuration Files
17:30:23.374 HHC02204I LEGACYSENSEID  set to disabled
17:30:23.374 HHC02204I MOUNTED_TAPE_REINIT set to disabled
17:30:23.375 HHC02204I AUTOINIT       set to ON
17:30:23.375 HHC02204I PANOPT         set to NAMEONLY RATE=25 MSGCOLOR=DARK TITLE=(default)
17:30:23.375 HHC02204I SHCMDOPT       set to DISABLE  NODIAG8
17:30:23.375 HHC02204I TIMERINT       set to 50
17:30:23.375 HHC02204I TRACEOPT       set to REGSFIRST NOCH9OFLOW
17:30:23.375 HHC02204I TZOFFSET       set to -0800
17:30:23.376 HHC17003I EXPANDED storage is 0 (xpndsize); storage is not locked
17:30:23.376 HHC02204I YROFFSET       set to 0
17:30:23.378 HHC02210I 0:000E LOADED index=0 lpi=6 lpp=66 fcb=1:1,2:7,3:13,4:19,5:25,6:31,7:37,8:43,10:49,11:55,12:61,9:63
17:30:23.381 HHC00100I Thread id 0000223c, prio 4, name 'console_connect' started
17:30:23.382 HHC01541I HDL: dyngui.dll initiated
17:30:23.382 HHC01024I Waiting for console connections on port 3270
17:30:30.883 HHC01603I script "C:\Users\Fish\Downloads\#87 PER G\2 perg.txt"
17:30:30.885 HHC02260I Script 1: begin processing file C:/Users/Fish/Downloads/#87 PER G/2 perg.txt
17:30:30.885 HHC01603I *************************************
17:30:30.885 HHC01603I * PER Register Alteration event test
17:30:30.886 HHC01603I *************************************
17:30:30.886 HHC01603I numcpu 1
17:30:30.886 HHC02204I NUMCPU         set to 1
17:30:30.886 HHC01603I sysclear
17:30:30.897 HHC02311I sysclear completed
17:30:30.898 HHC01603I archlvl 370
17:30:30.898 HHC02204I ARCHLVL        set to S/370
17:30:30.898 HHC01603I ostailor null
17:30:30.899 HHC01603I r 000=0008000000000200  # S/370 restart PSW
17:30:30.899 HHC02290I A:0000000000000000  K:00
17:30:30.899 HHC02290I R:00000000  00080000 00000200                    ........
17:30:30.899 HHC01603I r 068=000A000000000000  # S/370 pgm new PSW
17:30:30.899 HHC02290I A:0000000000000000  K:00
17:30:30.899 HHC02290I R:00000060                    000A0000 00000000          ........
17:30:30.900 HHC01603I r 200=B79902F0          # LCTL 9,9,CR9       Load CR9 with PER events masks
17:30:30.900 HHC02290I A:0000000000000000  K:00
17:30:30.900 HHC02290I R:00000200  B79902F0                             .r.0
17:30:30.900 HHC01603I r 204=1B66              # SR R6,R6           Clear for register alteration events
17:30:30.900 HHC02290I A:0000000000000000  K:00
17:30:30.900 HHC02290I R:00000200           1B66                            ..
17:30:30.900 HHC01603I r 206=820002E0          # LPSW PERPSW        Enable PER
17:30:30.900 HHC02290I A:0000000000000000  K:00
17:30:30.900 HHC02290I R:00000200               8200 02E0                     b..\
17:30:30.900 HHC01603I r 230=0550              # BALR R5,0
17:30:30.900 HHC02290I A:0000000000000000  K:00
17:30:30.900 HHC02290I R:00000230  0550                                 .&
17:30:30.901 HHC01603I r 232=1822              # LR R2,R2       <== SHOULD trigger PER register alteration event
17:30:30.901 HHC02290I A:0000000000000000  K:00
17:30:30.901 HHC02290I R:00000230      1822                               ..
17:30:30.901 HHC01603I r 234=41606001          # LA R6,1(,R6)
17:30:30.901 HHC02290I A:0000000000000000  K:00
17:30:30.901 HHC02290I R:00000230           41606001                        .--.
17:30:30.901 HHC01603I r 238=07F5              # BR 5               Loop back to NOPR at location 232
17:30:30.901 HHC02290I A:0000000000000000  K:00
17:30:30.901 HHC02290I R:00000230                    07F5                       .5
17:30:30.901 HHC01603I r 2E0=400C000000000230  # PERPSW             Enable PER psw
17:30:30.901 HHC02290I A:0000000000000000  K:00
17:30:30.902 HHC02290I R:000002E0  400C0000 00000230                     .......
17:30:30.902 HHC01603I r 2F0=10002000          # CR9                PER events mask = GRA + R2 only
17:30:30.902 HHC02290I A:0000000000000000  K:00
17:30:30.902 HHC02290I R:000002F0  10002000                             ....
17:30:30.902 HHC01603I restart
17:30:30.902 HHC02228I restart key pressed
17:30:30.903 HHC02262I Script 1: processing paused for 100 milliseconds...
17:30:30.903 HHC00801I Processor CP00: Unassigned exception code 0080 ilc 2
17:30:30.903 HHC02269I GR00=00000000 GR01=00000000 GR02=00000000 GR03=00000000
17:30:30.903 HHC02269I GR04=00000000 GR05=40000232 GR06=00000000 GR07=00000000
17:30:30.903 HHC02269I GR08=00000000 GR09=00000000 GR10=00000000 GR11=00000000
17:30:30.903 HHC02269I GR12=00000000 GR13=00000000 GR14=00000000 GR15=00000000
17:30:30.903 HHC02324I PSW=400C000000000232 INST=1822         LR    2,2                    load_register
17:30:30.903 HHC00809I Processor CP00: disabled wait state 000A0000 00000000
17:30:31.003 HHC02263I Script 1: processing resumed...
17:30:31.004 HHC01603I r 96.2                  # PER code           1000 = PER GRA event
17:30:31.004 HHC02290I A:0000000000000000  K:06
17:30:31.005 HHC02290I R:00000090               1000                          ..
17:30:31.005 HHC01603I r 98.4                  # PER Address        00000232 = LR instruction
17:30:31.005 HHC02290I A:0000000000000000  K:06
17:30:31.006 HHC02290I R:00000090                    00000232                   ....
17:30:31.006 HHC02264I Script 1: file C:/Users/Fish/Downloads/#87 PER G/2 perg.txt processing ended
17:31:00.277 HHC01603I exit
17:31:00.319 HHC01420I Begin Hercules shutdown
17:31:00.330 HHC01423I Calling termination routines
17:31:00.341 HHC00101I Thread id 000026e4, prio 2, name 'Processor CP00' ended
17:31:00.342 HHC00101I Thread id 0000223c, prio 4, name 'console_connect' ended
17:31:00.343 HHC01427I Main storage released
17:31:00.343 HHC01427I Expanded storage released
17:31:00.343 HHC01422I Configuration released
17:31:00.353 HHC01542I HDL: dyngui.dll terminated

******************** (last line of logfile above) ********************

Hercules x64 Windows GUI -- Version 1.18.2.5736
Copyright (C) 2001-2022 Software Development Laboratories

HercGUI URL:      http://www.softdevlabs.com/HercGUI
HercGUI support:  support@softdevlabs.com

Hercules URL:     http://www.sdl-hercules-390.org/
Hercules support: group@hercules-390.groups.io

So if your test isn't behaving that way, then it must obviously be VM filtering out such PER G interrupts, because as you can see above, the hardware is obviously properly generating the interrupt!

Check VM.

@wably
Copy link
Member

wably commented Feb 2, 2022

Hey Fish,

You were correct, it was VM that was filtering the presentation to the user of a general register alteration event. I verified that the PER GRA event interrupt is triggered as expected even when the register contents did not physically change. CP is getting these interruptions. Later, I found the code in VM/370 where the presentation of the results to the user is skipped in a no-change situation; the code to do this is in DMKPER at label GSETL.

So that's it. There are no other outstanding issues that I know of. From my view, you can commit this work.

Regards,
Bob

Fish-Git added a commit that referenced this issue Feb 2, 2022
@Fish-Git
Copy link
Member Author

Fish-Git commented Feb 2, 2022

Closed by commit cb5c8c1.

@Fish-Git Fish-Git closed this as completed Feb 2, 2022
@Fish-Git Fish-Git removed QUESTION... A question was asked but has not been answered yet, -OR- additional feedback is requested. Waiting to close... Waiting for user to report back whether problem still exists or not before closing as resolved. labels Feb 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BUG The issue describes likely incorrect product functionality that likely needs corrected.
Projects
None yet
Development

No branches or pull requests

4 participants