On Wed, 13 Dec 2023 13:49:40 +0100
Nina Schoetterl-Glausch <nsg@xxxxxxxxxxxxx> 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 the host's.
you should also mention here that you are splitting snippet.h into a
host-only part and a guest-only part
>
> Signed-off-by: Nina Schoetterl-Glausch <nsg@xxxxxxxxxxxxx>
> ---
> 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} | 9 ++++--
> lib/s390x/sie.c | 28 +++++++++++++++++
> lib/s390x/snippet-host.c | 40 +++++++++++++++++++++++++
> 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, 123 insertions(+), 11 deletions(-)
> create mode 100644 lib/s390x/snippet-guest.h
> rename lib/s390x/{snippet.h => snippet-host.h} (93%)
> create mode 100644 lib/s390x/snippet-host.c
[...]
> diff --git a/lib/s390x/sie.c b/lib/s390x/sie.c
> index 40936bd2..908b0130 100644
> --- a/lib/s390x/sie.c
> +++ b/lib/s390x/sie.c
> @@ -42,6 +42,34 @@ void sie_check_validity(struct vm *vm, uint16_t vir_exp)
> report(vir_exp == vir, "VALIDITY: %x", vir);
> }
>
> +bool sie_is_diag_icpt(struct vm *vm, unsigned int diag)
> +{
> + uint32_t ipb = vm->sblk->ipb;
> + uint64_t code;
uint64_t code = 0;
> + uint16_t displace;
> + uint8_t base;
> + bool ret = true;
bool ret;
> +
> + ret = ret && vm->sblk->icptcode == ICPT_INST;
> + ret = ret && (vm->sblk->ipa & 0xff00) == 0x8300;
ret = vm->sblk->icptcode == ICPT_INST && (vm->sblk->ipa & 0xff00) ==
0x8300;
> + switch (diag) {
> + case 0x44:
> + case 0x9c:
> + ret = ret && !(ipb & 0xffff);
> + ipb >>= 16;
> + displace = ipb & 0xfff;
maybe it's more readable to avoid shifting thigs around all the time:
displace = (ipb >> 16) & 0xfff;
base = (ipb >> 28) & 0xf;
if (base)
code = vm->....[base];
code = (code + displace) & 0xffff;
if (ipb & 0xffff || code != diag)
return false;
> + ipb >>= 12;
> + base = ipb & 0xf;
> + code = base ? vm->save_area.guest.grs[base] + displace : displace;
> + code &= 0xffff;
> + ret = ret && (code == diag);
> + break;
> + default:
> + abort(); /* not implemented */
> + }
> + return ret;
although I have the feeling that this would be more readable if you
would check diag immediately, and avoid using ret
> +}
> +
> void sie_handle_validity(struct vm *vm)
> {
> if (vm->sblk->icptcode != ICPT_VALIDITY)
> diff --git a/lib/s390x/snippet-host.c b/lib/s390x/snippet-host.c
> new file mode 100644
> index 00000000..a829c1d5
> --- /dev/null
> +++ b/lib/s390x/snippet-host.c
> @@ -0,0 +1,40 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Snippet functionality for the host.
> + *
> + * Copyright IBM Corp. 2023
> + */
> +
> +#include <libcflat.h>
> +#include <snippet-host.h>
> +#include <sie.h>
> +
> +bool snippet_check_force_exit(struct vm *vm)
> +{
> + bool r;
> +
> + r = sie_is_diag_icpt(vm, 0x44);
> + report(r, "guest forced exit");
> + return r;
> +}
> +
> +bool snippet_get_force_exit_value(struct vm *vm, uint64_t *value)
> +{
> + struct kvm_s390_sie_block *sblk = vm->sblk;
> +
> + if (sie_is_diag_icpt(vm, 0x9c)) {
> + *value = vm->save_area.guest.grs[(sblk->ipa & 0xf0) >> 4];
> + report_pass("guest forced exit with value: 0x%lx", *value);
> + return true;
> + }
> + report_fail("guest forced exit with value");
> + return false;
> +}
> +
> +void snippet_check_force_exit_value(struct vm *vm, uint64_t value_exp)
> +{
> + uint64_t value;
> +
> + if (snippet_get_force_exit_value(vm, &value))
> + report(value == value_exp, "guest exit value matches 0x%lx", value_exp);
> +}
from a readability and a consistency perspective, it would be better if
the functions would only check stuff and return a bool or a value, and
do the report() in the body of the testcase
[...]
