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

[Claudio Imbrenda <imbrenda@linux.ibm.com>]

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 */