Re: [kvm-unit-tests PATCH 2/2] s390x: Remove DAT and add short indication psw bits on diag308 reset

Janosch Frank
2019-11-06T06:58:44Z

[-- Attachment #1.1: Type: text/plain, Size: 3621 bytes --]

On 11/5/19 8:53 PM, David Hildenbrand wrote:
> On 05.11.19 17:28, Janosch Frank wrote:
>
> In the subject "Disable" vs. "Remove" ?
>
>> On a diag308 subcode 0 CRs will be reset, so we need to mask of PSW
>> DAT indication until we restore our CRs.
>>
>> Also we need to set the short psw indication to be compliant with the
>> architecture.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>> lib/s390x/asm-offsets.c | 1 +
>> lib/s390x/asm/arch_def.h | 3 ++-
>> s390x/cstart64.S | 20 ++++++++++++++------
>> 3 files changed, 17 insertions(+), 7 deletions(-)
>>
>> diff --git a/lib/s390x/asm-offsets.c b/lib/s390x/asm-offsets.c
>> index 4b213f8..61d2658 100644
>> --- a/lib/s390x/asm-offsets.c
>> +++ b/lib/s390x/asm-offsets.c
>> @@ -58,6 +58,7 @@ int main(void)
>> OFFSET(GEN_LC_SW_INT_FPRS, lowcore, sw_int_fprs);
>> OFFSET(GEN_LC_SW_INT_FPC, lowcore, sw_int_fpc);
>> OFFSET(GEN_LC_SW_INT_CRS, lowcore, sw_int_crs);
>> + OFFSET(GEN_LC_SW_INT_PSW, lowcore, sw_int_psw);
>> OFFSET(GEN_LC_MCCK_EXT_SA_ADDR, lowcore, mcck_ext_sa_addr);
>> OFFSET(GEN_LC_FPRS_SA, lowcore, fprs_sa);
>> OFFSET(GEN_LC_GRS_SA, lowcore, grs_sa);
>> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
>> index 07d4e5e..7d25e4f 100644
>> --- a/lib/s390x/asm/arch_def.h
>> +++ b/lib/s390x/asm/arch_def.h
>> @@ -79,7 +79,8 @@ struct lowcore {
>> uint32_t sw_int_fpc; /* 0x0300 */
>> uint8_t pad_0x0304[0x0308 - 0x0304]; /* 0x0304 */
>> uint64_t sw_int_crs[16]; /* 0x0308 */
>> - uint8_t pad_0x0310[0x11b0 - 0x0388]; /* 0x0388 */
>> + struct psw sw_int_psw; /* 0x0388 */
>> + uint8_t pad_0x0310[0x11b0 - 0x0390]; /* 0x0390 */
>> uint64_t mcck_ext_sa_addr; /* 0x11b0 */
>> uint8_t pad_0x11b8[0x1200 - 0x11b8]; /* 0x11b8 */
>> uint64_t fprs_sa[16]; /* 0x1200 */
>> diff --git a/s390x/cstart64.S b/s390x/cstart64.S
>> index 0455591..2e0dcf5 100644
>> --- a/s390x/cstart64.S
>> +++ b/s390x/cstart64.S
>> @@ -129,8 +129,15 @@ memsetxc:
>> .globl diag308_load_reset
>> diag308_load_reset:
>> SAVE_REGS
>> - /* Save the first PSW word to the IPL PSW */
>> + /* Backup current PSW */
>
> /*
> * Backup the current PSW MASK, as we have to restore it on
> * success.
> */
>
>> epsw %r0, %r1
>> + st %r0, GEN_LC_SW_INT_PSW
>> + st %r1, GEN_LC_SW_INT_PSW + 4
>
> I was confused at first, but then I realized that you really only store
> the PSW mask here and not also the PSW address ...
>
>
>> + /* Disable DAT as the CRs will be reset too */
>> + nilh %r0, 0xfbff
>> + /* Add psw bit 12 to indicate short psw */
>> + oilh %r0, 0x0008
>
> Why care about the old PSW mask here at all? Wouldn't it be easier to
> just construct a new PSW mask from scratch? (64bit, PSW bit 12 set ...)
>
> Save it somewhere and just load it directly from memory.

Sounds like a good idea, will do

>
>> + /* Save the first PSW word to the IPL PSW */
>> st %r0, 0
>> /* Store the address and the bit for 31 bit addressing */
>> larl %r0, 0f
>> @@ -142,12 +149,13 @@ diag308_load_reset:
>> xgr %r2, %r2
>> br %r14
>> /* Success path */
>> - /* We lost cr0 due to the reset */
>> -0: larl %r1, initial_cr0
>> - lctlg %c0, %c0, 0(%r1)
>> - RESTORE_REGS
>> + /* Switch to z/Architecture mode and 64-bit */
>> +0: RESTORE_REGS
>> lhi %r2, 1
>> - br %r14
>> + larl %r0, 1f
>> + stg %r0, GEN_LC_SW_INT_PSW + 8
>> + lpswe GEN_LC_SW_INT_PSW
>> +1: br %r14
>>
>> .globl smp_cpu_setup_state
>> smp_cpu_setup_state:
>>
>
>

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]