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

Janosch Frank
2021-12-08T13:56:28Z

On 12/8/21 12:46, Claudio Imbrenda wrote:
> 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"

snippet_setup_guest_shell() ?

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