On Tue, 2024-06-25 at 12:43 +1000, Nicholas Piggin wrote:
> On Fri Jun 21, 2024 at 12:16 AM AEST, 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@xxxxxxxxxxxxx>
> > ---
> > s390x/Makefile | 1 +
> > lib/s390x/asm/arch_def.h | 13 ++++++++
> > lib/s390x/snippet-guest.h | 26 +++++++++++++++
> > lib/s390x/{snippet.h => snippet-host.h} | 10 ++++--
> > 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 +-
> > 13 files changed, 97 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
> >
[...]
> > diff --git a/lib/s390x/snippet-guest.h b/lib/s390x/snippet-guest.h
> > new file mode 100644
> > index 00000000..3cc098e1
> > --- /dev/null
> > +++ b/lib/s390x/snippet-guest.h
> > @@ -0,0 +1,26 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Snippet functionality for the guest.
> > + *
> > + * Copyright IBM Corp. 2023
> > + */
> > +
> > +#ifndef _S390X_SNIPPET_GUEST_H_
> > +#define _S390X_SNIPPET_GUEST_H_
> > +
> > +#include <asm/arch_def.h>
> > +#include <asm/barrier.h>
> > +
> > +static inline void force_exit(void)
> > +{
> > + diag44();
> > + mb(); /* allow host to modify guest memory */
> > +}
> > +
> > +static inline void force_exit_value(uint64_t val)
> > +{
> > + diag9c(val);
> > + mb(); /* allow host to modify guest memory */
> > +}
>
> You have barriers here, but couldn't the diag get moved before a prior
> store by the guest?
Yeah, makes sense to add another before.
>
> Silly question since I don't understand the s390x arch or snippet design
> too well... the diag here causes a guest exit to the host. After the
> host handles that, it may resume guest at the next instruction? If that
> is correct, then the barrier here (I think) is for when the guest
> resumes it would not reorder subsequent loads from guest memory before
> the diag, because the host might have modified it.
[...]
> > diff --git a/lib/s390x/snippet-host.c b/lib/s390x/snippet-host.c
> > new file mode 100644
> > index 00000000..44a60bb9
> > --- /dev/null
> > +++ b/lib/s390x/snippet-host.c
[...]
> > +void snippet_check_force_exit_value(struct vm *vm, uint64_t value_exp)
> > +{
> > + uint64_t value;
> > +
> > + if (snippet_is_force_exit_value(vm)) {
> > + value = snippet_get_force_exit_value(vm);
> > + report(value == value_exp, "guest forced exit with value (0x%lx == 0x%lx)",
> > + value, value_exp);
>
> This is like kvm selftests guest/host synch design, which is quite
> nice and useful.
>
> > + } else {
> > + report_fail("guest forced exit with value");
> > + }
>
> Guest forced exit without value?
It's this way round so the output reads:
FAIL: guest forced exit with value
What's after the colon is what failed and the message
is the same for PASS/FAIL. Indeed a bit confusing.
> And do you also need to check for non-value force
> exit to distinguish from a normal snippet exit?
No, the function does just this check and if you need to handle
more complicated situations you need to do that in the caller.
>
> Thanks,
> Nick
