hercules-390 / hyperion

Hercules 390
Other
248 stars 67 forks source link

LPSW in Z architecture incorrectly requires 31 bit ESA PSW #180

Closed jphartmann closed 7 years ago

jphartmann commented 7 years ago

This should not program check. It is a perfectly valid ESA disabled wait

HHC02324I PSW=0000000180000000 0000000000000224 INST=8200F068     LPSW  104(15)                load_program_status_word
HHC02326I R:0000000000000068:K:06=00820000 00000000 00000000 00000000  .b..............
HHC02269I R0=0000000000000000 R1=0000000000000200 R2=0000000000000000 R3=0000000000000000
HHC02269I R4=0000000000000000 R5=0000000000000000 R6=0000000000000000 R7=0000000000000000
HHC02269I R8=0000000000000000 R9=0000000000000000 RA=0000000000000000 RB=0000000000000000
HHC02269I RC=0000000000000000 RD=0000000000000000 RE=0000000000000000 RF=0000000000000000
HHC00801I Processor CP00: Specification exception code 0006  ilc 0
HHC02324I PSW=0082000000000000 0000000000000000 INST=0000         ????? ,                      ?
HHC02269I R0=0000000000000000 R1=0000000000000200 R2=0000000000000000 R3=0000000000000000
HHC02269I R4=0000000000000000 R5=0000000000000000 R6=0000000000000000 R7=0000000000000000
HHC02269I R8=0000000000000000 R9=0000000000000000 RA=0000000000000000 RB=0000000000000000
HHC02269I RC=0000000000000000 RD=0000000000000000 RE=0000000000000000 RF=0000000000000000
HHC00800I Processor CP00: loaded wait state PSW 0002000180000000 00000000DEADDEAD             

This comment and code in control.c confuses LPSW with set architecture mode. Or with something else.

    /* Restore the amode64 flag setting and set the actual correct
       AMASK value according to it (if we need to) since we had to
       force non-amode64 further above to get the 's390_load_psw'
       function to work right without erroneously program-checking.
    */
    if((regs->psw.amode64 = amode64))
    {
        regs->psw.AMASK = AMASK64;

        /* amode31 bit must be set when amode64 is set */
        if(!regs->psw.amode)
        {
            regs->psw.zeroilc = 1;
            ARCH_DEP(program_interrupt) (regs, PGM_SPECIFICATION_EXCEPTION);
        }
    }                                               
ivan-w commented 7 years ago

John,

No it's not a valid ESA PSW ! Bit 12 (E) is 0 - it should be 1.

--Ivan

On 1/6/2017 5:41 PM, John P. Hartmann wrote:

This should not program check. It is a perfectly valid ESA disabled wait

|HHC02324I PSW=0000000180000000 0000000000000224 INST=8200F068 LPSW 104(15) load_program_status_word HHC02326I R:0000000000000068:K:06=00820000 00000000 00000000 00000000 .b.............. HHC02269I R0=0000000000000000 R1=0000000000000200 R2=0000000000000000 R3=0000000000000000 HHC02269I R4=0000000000000000 R5=0000000000000000 R6=0000000000000000 R7=0000000000000000 HHC02269I R8=0000000000000000 R9=0000000000000000 RA=0000000000000000 RB=0000000000000000 HHC02269I RC=0000000000000000 RD=0000000000000000 RE=0000000000000000 RF=0000000000000000 HHC00801I Processor CP00: Specification exception code 0006 ilc 0 HHC02324I PSW=0082000000000000 0000000000000000 INST=0000 ????? , ? HHC02269I R0=0000000000000000 R1=0000000000000200 R2=0000000000000000 R3=0000000000000000 HHC02269I R4=0000000000000000 R5=0000000000000000 R6=0000000000000000 R7=0000000000000000 HHC02269I R8=0000000000000000 R9=0000000000000000 RA=0000000000000000 RB=0000000000000000 HHC02269I RC=0000000000000000 RD=0000000000000000 RE=0000000000000000 RF=0000000000000000 HHC00800I Processor CP00: loaded wait state PSW 0002000180000000 00000000DEADDEAD |

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/hercules-390/hyperion/issues/180, or mute the thread https://github.com/notifications/unsubscribe-auth/ABjMWxMoqCf6QdtOM_GEVqrRUDZkOiQ6ks5rPm61gaJpZM4Lc4i1.

ivan-w commented 7 years ago

To be more precise :

At X'68', the location contains 00820000 00000000 (which is NOT a valid ESA PSW since bit 12 is 0 - and it should be 1)

However... Some versions of the z/Arch POP (for example SA22-7832-10) define as model dependent whether the check is enforced :

Page 10-53, LPSW, 2nd paragraph :

Bit 12 of the doubleword must be one; otherwise, a specification exception may be recognized, depending on the model.

The original z/Arch behavior was to invert bit 12 on LPSW... Now, it's just a check - which may or may not be enforced.

So whether this should be enforced or not is up to us (but it does explain the PIC 6). So to me it's not a bug, it's just a feature

--Ivan

On 1/6/2017 5:41 PM, John P. Hartmann wrote:

This should not program check. It is a perfectly valid ESA disabled wait

|HHC02324I PSW=0000000180000000 0000000000000224 INST=8200F068 LPSW 104(15) load_program_status_word HHC02326I R:0000000000000068:K:06=00820000 00000000 00000000 00000000 .b.............. HHC02269I R0=0000000000000000 R1=0000000000000200 R2=0000000000000000 R3=0000000000000000 HHC02269I R4=0000000000000000 R5=0000000000000000 R6=0000000000000000 R7=0000000000000000 HHC02269I R8=0000000000000000 R9=0000000000000000 RA=0000000000000000 RB=0000000000000000 HHC02269I RC=0000000000000000 RD=0000000000000000 RE=0000000000000000 RF=0000000000000000 HHC00801I Processor CP00: Specification exception code 0006 ilc 0 HHC02324I PSW=0082000000000000 0000000000000000 INST=0000 ????? , ? HHC02269I R0=0000000000000000 R1=0000000000000200 R2=0000000000000000 R3=0000000000000000 HHC02269I R4=0000000000000000 R5=0000000000000000 R6=0000000000000000 R7=0000000000000000 HHC02269I R8=0000000000000000 R9=0000000000000000 RA=0000000000000000 RB=0000000000000000 HHC02269I RC=0000000000000000 RD=0000000000000000 RE=0000000000000000 RF=0000000000000000 HHC00800I Processor CP00: loaded wait state PSW 0002000180000000 00000000DEADDEAD |

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/hercules-390/hyperion/issues/180, or mute the thread https://github.com/notifications/unsubscribe-auth/ABjMWxMoqCf6QdtOM_GEVqrRUDZkOiQ6ks5rPm61gaJpZM4Lc4i1.

ivan-w commented 7 years ago

Now...

I think it was a just a finger check...

You probably meant X'68' to contain 000A0000 00000000 instead of 00820000 00000000

--Ivan

On 1/6/2017 5:41 PM, John P. Hartmann wrote:

This should not program check. It is a perfectly valid ESA disabled wait

|HHC02324I PSW=0000000180000000 0000000000000224 INST=8200F068 LPSW 104(15) load_program_status_word HHC02326I R:0000000000000068:K:06=00820000 00000000 00000000 00000000 .b.............. HHC02269I R0=0000000000000000 R1=0000000000000200 R2=0000000000000000 R3=0000000000000000 HHC02269I R4=0000000000000000 R5=0000000000000000 R6=0000000000000000 R7=0000000000000000 HHC02269I R8=0000000000000000 R9=0000000000000000 RA=0000000000000000 RB=0000000000000000 HHC02269I RC=0000000000000000 RD=0000000000000000 RE=0000000000000000 RF=0000000000000000 HHC00801I Processor CP00: Specification exception code 0006 ilc 0 HHC02324I PSW=0082000000000000 0000000000000000 INST=0000 ????? , ? HHC02269I R0=0000000000000000 R1=0000000000000200 R2=0000000000000000 R3=0000000000000000 HHC02269I R4=0000000000000000 R5=0000000000000000 R6=0000000000000000 R7=0000000000000000 HHC02269I R8=0000000000000000 R9=0000000000000000 RA=0000000000000000 RB=0000000000000000 HHC02269I RC=0000000000000000 RD=0000000000000000 RE=0000000000000000 RF=0000000000000000 HHC00800I Processor CP00: loaded wait state PSW 0002000180000000 00000000DEADDEAD |

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/hercules-390/hyperion/issues/180, or mute the thread https://github.com/notifications/unsubscribe-auth/ABjMWxMoqCf6QdtOM_GEVqrRUDZkOiQ6ks5rPm61gaJpZM4Lc4i1.

ivan-w commented 7 years ago

As for the code snippet you mentioned, it is only to check the z/Arch PSW is valid as far as amode is concerned... A z/Arch PSW with bit 31 set to 1 but bit 32 set to 0 is NOT a valid z/Arch PSW. 31=0, 32=0 : 24 Bit mode 31=0, 32=1 : 31 bit mode 31=1, 32=1 : 64bit mode 31=1, 32=0 : Invalid (PIC 6)

jphartmann commented 7 years ago

Thanks, Ivan. Yes, I set the key, not the ECmode bit. Perhaps it is time to hang up the gloves.

ivan-w commented 7 years ago

I closed the issue, but it ain't over yet.. You may have uncovered something related to LPSW in Z/Arch mode.

This should NOT go as an Early Specification Exception (ILC 0, PSW changed)... The Specification Exception should have been on the LPSW per : 10-53 LPSW :

The PSW fields which are to be loaded by the instruction are not checked for validity before they are loaded, except for the optional checking of bit 12.

But it's a bit of a different issue.... So we should either not raise the PIC 6, or we should have a PIC 6, with the PSW passed the LPSW instruction and the ILC for the LPSW instruction.

But I'd rather open a new issue on that one !

jphartmann commented 7 years ago

New issue is fine by me, Ivan. I was about to make the same observation.