Re: [kvm-unit-tests PATCH v2 08/10] lib: s390x: Introduce snippet helpers

Claudio Imbrenda
2021-12-08T12:57:46Z

On Tue, 7 Dec 2021 16:00:03 +0000
Janosch Frank <frankja@linux.ibm.com> wrote:

> These helpers reduce code duplication for PV snippet tests.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>

Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

although I'd prefer different names for the functions.

snippet_setup_guest sounds like it is actually doing something
snippet-related, whereas it is just preparing an empty guest, the
actual snippet part comes later when you call snippet_init.

Maybe rename snippet_setup_guest to something like
"prepare_guest_for_snippet"? (or maybe something shorter, but you get
what I mean) maybe even "prepare_1m_guest"

> ---
> lib/s390x/snippet.h | 103 ++++++++++++++++++++++++++++++++++++++++++++
> lib/s390x/uv.h | 21 +++++++++
> 2 files changed, 124 insertions(+)
>
> diff --git a/lib/s390x/snippet.h b/lib/s390x/snippet.h
> index 6b77a8a9..b17b2a4c 100644
> --- a/lib/s390x/snippet.h
> +++ b/lib/s390x/snippet.h
> @@ -9,6 +9,10 @@
> #ifndef _S390X_SNIPPET_H_
> #define _S390X_SNIPPET_H_
>
> +#include <sie.h>
> +#include <uv.h>
> +#include <asm/uv.h>
> +
> /* This macro cuts down the length of the pointers to snippets */
> #define SNIPPET_NAME_START(type, file) \
> _binary_s390x_snippets_##type##_##file##_gbin_start
> @@ -26,6 +30,12 @@
> #define SNIPPET_HDR_LEN(type, file) \
> ((uintptr_t)SNIPPET_HDR_END(type, file) - (uintptr_t)SNIPPET_HDR_START(type, file))
>
> +#define SNIPPET_PV_TWEAK0 0x42UL
> +#define SNIPPET_PV_TWEAK1 0UL
> +#define SNIPPET_OFF_C 0
> +#define SNIPPET_OFF_ASM 0x4000
> +
> +
> /*
> * C snippet instructions start at 0x4000 due to the prefix and the
> * stack being before that. ASM snippets don't strictly need a stack
> @@ -38,4 +48,97 @@ static const struct psw snippet_psw = {
> .mask = PSW_MASK_64,
> .addr = SNIPPET_ENTRY_ADDR,
> };
> +
> +/*
> + * Sets up a snippet guest on top of an existing and initialized SIE
> + * vm struct.
> + * Once this function has finished without errors the guest can be started.
> + *
> + * @vm: VM that this function will populated, has to be initialized already
> + * @gbin: Snippet gbin data pointer
> + * @gbin_len: Length of the gbin data
> + * @off: Offset from guest absolute 0x0 where snippet is copied to
> + */
> +static inline void snippet_init(struct vm *vm, const char *gbin,
> + uint64_t gbin_len, uint64_t off)
> +{
> + uint64_t mso = vm->sblk->mso;
> +
> + /* Copy test image to guest memory */
> + memcpy((void *)mso + off, gbin, gbin_len);
> +
> + /* Setup guest PSW */
> + vm->sblk->gpsw = snippet_psw;
> +
> + /*
> + * We want to exit on PGM exceptions so we don't need
> + * exception handlers in the guest.
> + */
> + vm->sblk->ictl = ICTL_OPEREXC | ICTL_PINT;
> +}
> +
> +/*
> + * Sets up a snippet UV/PV guest on top of an existing and initialized
> + * SIE vm struct.
> + * Once this function has finished without errors the guest can be started.
> + *
> + * @vm: VM that this function will populated, has to be initialized already
> + * @gbin: Snippet gbin data pointer
> + * @hdr: Snippet SE header data pointer
> + * @gbin_len: Length of the gbin data
> + * @hdr_len: Length of the hdr data
> + * @off: Offset from guest absolute 0x0 where snippet is copied to
> + */
> +static inline void snippet_pv_init(struct vm *vm, const char *gbin,
> + const char *hdr, uint64_t gbin_len,
> + uint64_t hdr_len, uint64_t off)
> +{
> + uint64_t tweak[2] = {SNIPPET_PV_TWEAK0, SNIPPET_PV_TWEAK1};
> + uint64_t mso = vm->sblk->mso;
> + int i;
> +
> + snippet_init(vm, gbin, gbin_len, off);
> +
> + uv_create_guest(vm);
> + uv_set_se_hdr(vm->uv.vm_handle, (void *)hdr, hdr_len);
> +
> + /* Unpack works on guest addresses so we only need off */
> + uv_unpack(vm, off, gbin_len, tweak[0]);
> + uv_verify_load(vm);
> +
> + /*
> + * Manually import:
> + * - lowcore 0x0 - 0x1000 (asm)
> + * - stack 0x3000 (C)
> + */
> + for (i = 0; i < 4; i++) {
> + uv_import(vm->uv.vm_handle, mso + PAGE_SIZE * i);
> + }
> +}
> +
> +/* Allocates and sets up a snippet based guest */
> +static inline void snippet_setup_guest(struct vm *vm, bool is_pv)
> +{
> + u8 *guest;
> +
> + /* Allocate 1MB as guest memory */
> + guest = alloc_pages(8);
> + memset(guest, 0, HPAGE_SIZE);
> +
> + /* Initialize the vm struct and allocate control blocks */
> + sie_guest_create(vm, (uint64_t)guest, HPAGE_SIZE);
> +
> + if (is_pv) {
> + /* FMT4 needs a ESCA */
> + sie_guest_sca_create(vm);
> +
> + /*
> + * Initialize UV and setup the address spaces needed
> + * to run a PV guest.
> + */
> + uv_init();
> + uv_setup_asces();
> + }
> +}
> +
> #endif
> diff --git a/lib/s390x/uv.h b/lib/s390x/uv.h
> index 6ffe537a..8175d9c6 100644
> --- a/lib/s390x/uv.h
> +++ b/lib/s390x/uv.h
> @@ -3,6 +3,7 @@
> #define _S390X_UV_H_
>
> #include <sie.h>
> +#include <asm/pgtable.h>
>
> bool uv_os_is_guest(void);
> bool uv_os_is_host(void);
> @@ -14,4 +15,24 @@ void uv_destroy_guest(struct vm *vm);
> int uv_unpack(struct vm *vm, uint64_t addr, uint64_t len, uint64_t
> tweak); void uv_verify_load(struct vm *vm);
>
> +/*
> + * To run PV guests we need to setup a few things:
> + * - A valid primary ASCE that contains the guest memory and has the
> P bit set.
> + * - A valid home space ASCE for the UV calls that use home space
> addresses.
> + */
> +static inline void uv_setup_asces(void)
> +{
> + uint64_t asce;
> +
> + /* We need to have a valid primary ASCE to run guests. */
> + setup_vm();
> +
> + /* Set P bit in ASCE as it is required for PV guests */
> + asce = stctg(1) | ASCE_P;
> + lctlg(1, asce);
> +
> + /* Copy ASCE into home space CR */
> + lctlg(13, asce);
> +}
> +
> #endif /* UV_H */