Re: [kvm-unit-tests PATCH v2 3/3] s390x: Load reset psw on diag308 reset

[Thomas Huth <thuth@redhat.com>]

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

On 13/11/2019 11.04, Janosch Frank wrote:
> On 11/12/19 5:17 PM, Thomas Huth wrote:
>> On 12/11/2019 14.42, Janosch Frank wrote:
>>> On 11/12/19 1:09 PM, David Hildenbrand wrote:
>>>> On 11.11.19 16:33, Janosch Frank wrote:
>>>>> On a diag308 subcode 0 CRs will be reset, so we need a PSW mask
>>>>> without DAT. Also we need to set the short psw indication to be
>>>>> compliant with the architecture.
>>>>>
>>>>> Let's therefore define a reset PSW mask with 64 bit addressing and
>>>>> short PSW indication that is compliant with architecture and use it.
>>>>>
>>>>> 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         | 24 +++++++++++++++++-------
>>>>>  3 files changed, 20 insertions(+), 8 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 */
>> [...]
>>>> This patch breaks the smp test under TCG (no clue and no time to look
>>>> into the details :) ):
>>>
>>> I forgot to fixup the offset calculation at the top of the patch once
>>> again...
>>
>> Maybe add a
>>
>> _Static_assert(sizeof(struct lowcore) == xyz)
>>
>> after the struct definitions, to avoid that this happens again?
>>
>>  Thomas
>>
> 
> How about this?
> Or do we want to extend the struct to 8K and test for that?
> 
> diff --git i/lib/s390x/asm/arch_def.h w/lib/s390x/asm/arch_def.h
> index 5f034a7..cf6e1ca 100644
> --- i/lib/s390x/asm/arch_def.h
> +++ w/lib/s390x/asm/arch_def.h
> @@ -99,6 +99,7 @@ struct lowcore {
>         uint8_t         pad_0x1400[0x1800 - 0x1400];    /* 0x1400 */
>         uint8_t         pgm_int_tdb[0x1900 - 0x1800];   /* 0x1800 */
>  } __attribute__ ((__packed__));
> +_Static_assert(sizeof(struct lowcore) == 0x1900, "Lowcore size");

Fine for me either way (either checking for 0x1900 or extending the
struct to 8192).
Hmm, maybe we should go with 0x1900 for now, and extend the struct to
8192 bytes later if there is a reason to do it.

 Thomas


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