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

Disallow LPARNUM nn for ARCHLVL S/370 without FORCE option #256

Open
Fish-Git opened this issue Sep 21, 2019 · 6 comments
Open

Disallow LPARNUM nn for ARCHLVL S/370 without FORCE option #256

Fish-Git opened this issue Sep 21, 2019 · 6 comments
Labels
Discussion Developers are invited to discuss a design change or solution to a coding problem. Enhancement This issue does not describe a problem but rather describes a suggested change or improvement.

Comments

@Fish-Git
Copy link
Member

Fish-Git commented Sep 21, 2019

In almost all cases, when a user sets ARCHLVL S/370 they expect the STIDP (Store CPU ID) instruction to store the CPU ID in Basic Mode format (i.e. they expect LPARNUM and CPUIDFMT to be set to BASIC such that the STIDP instruction then stores the CPU ID in the expected format).

This was the reason for the "Limited automatic LPARNUM updating when setting certain architecture modes" change made to v4.1

However, as explained in the above mentioned v4.1 Release Notes:

For the unusual case where users actually do want to run a System/370 Operating System within an LPAR, you will need to manually re-set your LPARNUM and CPUIDFMT values back to their numeric values after setting ARCHLVL S/370. That is to say, one should always follow their ARCHLVL S/370 statement with a LPARNUM n|nn statement (and CPUIDFMT statement too if needed) if LPAR mode for S/370 is truly desired.

This leads to unexpected behavior (introduces a "Surprise!" factor that the "Limited automatic LPARNUM updating when setting certain architecture modes" change was attempting to eliminate (or at least reduce the likelihood of occurring)) when a given configuration file happens to contain a "stray" (accidental) LPARNUM 01 statement (that they forgot to remove) several statements after their ARCHLVL S/370 statement.

Such an unintentional (but quite easy to make) user configuration error can cause much confusion and wasted time and effort trying to discover why their basic-mode (non-LPAR-aware) 370 guest O/S is failing to IPL or otherwise behave correctly.

As explained, the cause of this unexpected behavior is Hercules's need to support LPAR-370 mode, thereby causing Hercules to accept without complaint or warning their unintended LPARNUM 01 statement that happens to follow their ARCHLVL 370 statement.

It is requested that a new FORCE option be introduced to the LPARNUM statement handling that would cause a numeric LPARNUM statement to be rejected if submitted without the new FORCE option for ARCHLVL 370. That is to say, the new FORCE option would only be required when the ARCHLVL is 370 and 370 LPAR mode was truly wanted. If the ARCHLVL is 390 or z/Arch, then a numeric LPARNUM statement would be accepted as-is without the FORCE option, just like today, so the requested change would only impact those rare individuals who actually do want to run in 370 LPAR mode. For the vast majority of the 370 crowd the change would neatly eliminate the unintended/disastrous "Surprise!" factor that could otherwise very easily occur.

@Fish-Git Fish-Git self-assigned this Sep 21, 2019
@Fish-Git Fish-Git added the Enhancement This issue does not describe a problem but rather describes a suggested change or improvement. label Sep 21, 2019
@ivan-w
Copy link
Member

ivan-w commented Sep 25, 2019

Then you break something. System configuration statements should, and always have been, independent of the architecture level. However, in S/370 mode, STIDP should always return a format 0 CPUID (that is, it should return the length of the MCEL whether the LPAR is greater than 16 or not)

@Fish-Git
Copy link
Member Author

Fish-Git commented Oct 1, 2019

Then you break something. System configuration statements should, and always have been, independent of the architecture level.

Then how do you propose to eliminate the described "Surprise!" factor, as well as the other obvious surprise factor that would occur when the user queries the current LPARNUM/CPUIDFMT setting?

That is to say, if the ARCHLVL is set to S/370 but the LPARNUM remains 01 and the CPUIDFMT remains 1, but yet, the STIDP instruction ignores both settings and stores a BASIC mode CPUID instead (completely ignoring the current LPARNUM/CPUIDFMT setting), wouldn't that too lead to a "Surprise!" situation?

More than that, how would someone who purposely wanted to run S/370 in "LPAR mode" accomplish that? That is to say, if a user purposely wanted the STIDP instruction to store a LPAR-mode CPUID (i.e. a non- BASIC mode CPUID), how would they request/accomplish that?

However, in S/370 mode, STIDP should always return a format 0 CPUID (that is, it should return the length of the MCEL whether the LPAR is greater than 16 or not)

I would tend to agree! Mark however (who wrote our LPARNUM/CPUIDFMT logic), might not.

Furthermore, such a change, as explained above, would make it impossible for a user to request the STIDP instruction to store a non-BASIC mode CPUID. Your proposed change would break things too!

So what's the answer? What's the compromise here? How can we achieve both of our goals? What do you suggest?

@Fish-Git Fish-Git added Discussion Developers are invited to discuss a design change or solution to a coding problem. QUESTION... A question was asked but has not been answered yet, -OR- additional feedback is requested. labels Oct 1, 2019
@ivan-w
Copy link
Member

ivan-w commented Oct 2, 2019

NOTE!  Below edited by Fish!  I hope I got it right!

 
Ok, let's run down the various cases.

At run time (past IMPL/POR), we are either in S/370 architecture mode, or in XA or better:

370 only supports Format-0 CPUID format.
(STIDP should NEVER return a Format-1 CPUID!)

  CPUIDFMT 0 is specified, or there is no CPUIDFMT statement:
    STIDP returns a Format-0 CPUID, and ignores the LPARNUM.

  CPUIDFMT 1 is specified:
    Issue warning at IMPL/POR, do as if CPUIDFMT was set to 0.

  CPUIDFMT 1 FORCE:
    Issue a warning at IMPL/POR, STIDP returns a Format-1 CPUID.
    (You're on your own on this one!)

XA or better (S/370 XA, ESA, S/390, z/Arch):

  No CPUIDFMT:
    If LPARNUM is less than 16, STIDP returns a Format-0 CPUID.
    If LPARNUM is greater than 16, a Format-1 CPUID is returned.

  CPUIDFMT 0 is specified:
    STIDP returns a Format-0 CPUID regardless of the LPARNUM specified.

  CPUIDFMT 1 is specified:
    STIDP returns a Format-1 CPUID regardless of the LPARNUM specified.
    If LPARNUM wasn't specified, set it to 0.

 
NOTE!  Above edited by Fish!  I hope I got it right!

@Fish-Git
Copy link
Member Author

Fish-Git commented Oct 2, 2019

Hmmm... So you're proposing a FORCE option on the CPUIDFMT statement, not the LPARNUM statement. Interesting. I think I like that better!

@Fish-Git
Copy link
Member Author

Fish-Git commented Oct 2, 2019

For 370:

CPUIDFMT 1 is specified:
Issue warning at IMPL/POR, do as if CPUIDFMT was set to 0.

I would add that not only should we issuing a warning for the CPUIDFMT statement (letting them know we're ignoring it), but also one for any LPARNUM statement they might also provide (letting them know the same thing: we're ignoring it).

@Fish-Git Fish-Git removed the QUESTION... A question was asked but has not been answered yet, -OR- additional feedback is requested. label Oct 7, 2019
@Fish-Git
Copy link
Member Author

Fish-Git commented Oct 7, 2019

@ivan-w

Ok, let's run down the various cases. [...]

Where in Hercules code should the above checks be made? In each of the ARCHLVL, CPUIDFMT and LPARNUM statement processing routines? Or immediately before IPL? Or someplace else?

I'm thinking maybe we should code a new validate_arch_cpufmt_lparnum_sanity()(?) function that each of the previously mentioned statement processing functions should call after setting the requested value. That is to say, the statement should set the value as-requested and then afterwards, the common "validate" function should be called to make any needed adjustment and issue any warnings, etc, should it decide to ignore the user's request and set a different value instead.

Does that sound reasonable? Or do you have a different/better idea on how to implement this suggested change?

Thanks!

(anyone else is also free to jump in with their own ideas or opinions on this matter too!)

@Fish-Git Fish-Git removed their assignment Oct 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion Developers are invited to discuss a design change or solution to a coding problem. Enhancement This issue does not describe a problem but rather describes a suggested change or improvement.
Projects
None yet
Development

No branches or pull requests

2 participants