Re: [kvm-unit-tests PATCH v2 3/5] s390x: Add library functions for exiting from snippet

[Janosch Frank <frankja@linux.ibm.com>]

On 1/8/24 13:58, Nina Schoetterl-Glausch wrote:
> On Mon, 2024-01-08 at 13:47 +0100, Janosch Frank wrote:
>> On 1/5/24 23:54, Nina Schoetterl-Glausch wrote:
>>> It is useful to be able to force an exit to the host from the snippet,
>>> as well as do so while returning a value.
>>> Add this functionality, also add helper functions for the host to check
>>> for an exit and get or check the value.
>>> Use diag 0x44 and 0x9c for this.
>>> Add a guest specific snippet header file and rename snippet.h to reflect
>>> that it is host specific.
>>>
>>> Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
>>> ---
>>>    s390x/Makefile                          |  1 +
>>>    lib/s390x/asm/arch_def.h                | 13 ++++++++
>>>    lib/s390x/sie.h                         |  1 +
>>>    lib/s390x/snippet-guest.h               | 26 +++++++++++++++
>>>    lib/s390x/{snippet.h => snippet-host.h} | 10 ++++--
>>>    lib/s390x/sie.c                         | 31 ++++++++++++++++++
>>>    lib/s390x/snippet-host.c                | 42 +++++++++++++++++++++++++
>>>    lib/s390x/uv.c                          |  2 +-
>>>    s390x/mvpg-sie.c                        |  2 +-
>>>    s390x/pv-diags.c                        |  2 +-
>>>    s390x/pv-icptcode.c                     |  2 +-
>>>    s390x/pv-ipl.c                          |  2 +-
>>>    s390x/sie-dat.c                         |  2 +-
>>>    s390x/spec_ex-sie.c                     |  2 +-
>>>    s390x/uv-host.c                         |  2 +-
>>>    15 files changed, 129 insertions(+), 11 deletions(-)
>>>    create mode 100644 lib/s390x/snippet-guest.h
>>>    rename lib/s390x/{snippet.h => snippet-host.h} (92%)
>>>    create mode 100644 lib/s390x/snippet-host.c
> 
> [..]
>   
>>> +bool sie_is_diag_icpt(struct vm *vm, unsigned int diag)
>>> +{
>>> +	union {
>>> +		struct {
>>> +			uint64_t     : 16;
>>> +			uint64_t ipa : 16;
>>> +			uint64_t ipb : 32;
>>> +		};
>>> +		struct {
>>> +			uint64_t          : 16;
>>> +			uint64_t opcode   :  8;
>>> +			uint64_t r_1      :  4;
>>> +			uint64_t r_2      :  4;
>>> +			uint64_t r_base   :  4;
>>> +			uint64_t displace : 12;
>>> +			uint64_t zero     : 16;
>>> +		};
>>> +	} instr = { .ipa = vm->sblk->ipa, .ipb = vm->sblk->ipb };
>>> +	uint64_t code;
>>> +
>>> +	assert(diag == 0x44 || diag == 0x9c);
>>
>> You're calling it is_diag_icpt and only allow two.
>> Do you have a reason for clamping this down?
> 
> I should have left the comment.
> They're just "not implemented".
> The PoP doesn't specify how diags are generally interpreted,
> so I intended that if any other diags are needed whoever needs them
> just checks if the existing logic works or if changes are required.
>>

Right, but 288, 308 and 500 are being used in the PV sie tests and could 
be integrated.