Re: [kvm-unit-tests PATCH 3/4] s390x: lib: sie: Add struct vm (de)initialization functions

Janosch Frank
2021-07-29T14:32:15Z

On 7/29/21 4:21 PM, Claudio Imbrenda wrote:
> On Thu, 29 Jul 2021 13:48:02 +0000
> Janosch Frank <frankja@linux.ibm.com> wrote:
>
>> Before I start copying the same code over and over lets move this into
>> the library.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>> lib/s390x/sie.c | 30 ++++++++++++++++++++++++++++++
>> lib/s390x/sie.h | 3 +++
>> s390x/mvpg-sie.c | 18 ++----------------
>> s390x/sie.c | 19 +++----------------
>> 4 files changed, 38 insertions(+), 32 deletions(-)
>>
>> diff --git a/lib/s390x/sie.c b/lib/s390x/sie.c
>> index 9107519f..ec0c4867 100644
>> --- a/lib/s390x/sie.c
>> +++ b/lib/s390x/sie.c
>> @@ -11,6 +11,9 @@
>> #include <asm/barrier.h>
>> #include <libcflat.h>
>> #include <sie.h>
>> +#include <asm/page.h>
>> +#include <libcflat.h>
>> +#include <alloc_page.h>
>>
>> static bool validity_expected;
>> static uint16_t vir;
>> @@ -39,3 +42,30 @@ void sie_handle_validity(struct vm *vm)
>> report_abort("VALIDITY: %x", vir);
>> validity_expected = false;
>> }
>> +
>> +/* Initializes the struct vm members like the SIE control block. */
>> +void sie_guest_create(struct vm *vm, uint64_t guest_mem, uint64_t
>> guest_mem_len) +{
>> + vm->sblk = alloc_page();
>> + memset(vm->sblk, 0, PAGE_SIZE);
>
> you can skip the memset, the page allocator always zeroes the page,
> unless you explicitly pass FLAG_DONTZERO

Yeah I thought as much but I still like to have that explicitly for ease
of mind.

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

Thanks

>
>> + vm->sblk->cpuflags = CPUSTAT_ZARCH | CPUSTAT_RUNNING;
>> + vm->sblk->ihcpu = 0xffff;
>> + vm->sblk->prefix = 0;
>> +
>> + /* Guest memory chunks are always 1MB */
>> + assert(!(guest_mem_len & ~HPAGE_MASK));
>> + /* Currently MSO/MSL is the easiest option */
>> + vm->sblk->mso = (uint64_t)guest_mem;
>> + vm->sblk->msl = (uint64_t)guest_mem + ((guest_mem_len - 1) &
>> HPAGE_MASK); +
>> + /* CRYCB needs to be in the first 2GB */
>> + vm->crycb = alloc_pages_flags(0, AREA_DMA31);
>> + vm->sblk->crycbd = (uint32_t)(uintptr_t)vm->crycb;
>> +}
>> +
>> +/* Frees the memory that was gathered on initialization */
>> +void sie_guest_destroy(struct vm *vm)
>> +{
>> + free_page(vm->crycb);
>> + free_page(vm->sblk);
>> +}
>> diff --git a/lib/s390x/sie.h b/lib/s390x/sie.h
>> index 7ff98d2d..946bd164 100644
>> --- a/lib/s390x/sie.h
>> +++ b/lib/s390x/sie.h
>> @@ -190,6 +190,7 @@ struct vm_save_area {
>> struct vm {
>> struct kvm_s390_sie_block *sblk;
>> struct vm_save_area save_area;
>> + uint8_t *crycb; /* Crypto
>> Control Block */ /* Ptr to first guest page */
>> uint8_t *guest_mem;
>> };
>> @@ -200,5 +201,7 @@ extern void sie64a(struct kvm_s390_sie_block
>> *sblk, struct vm_save_area *save_ar void sie_expect_validity(void);
>> void sie_check_validity(uint16_t vir_exp);
>> void sie_handle_validity(struct vm *vm);
>> +void sie_guest_create(struct vm *vm, uint64_t guest_mem, uint64_t
>> guest_mem_len); +void sie_guest_destroy(struct vm *vm);
>>
>> #endif /* _S390X_SIE_H_ */
>> diff --git a/s390x/mvpg-sie.c b/s390x/mvpg-sie.c
>> index 2ac91eec..71ae4f88 100644
>> --- a/s390x/mvpg-sie.c
>> +++ b/s390x/mvpg-sie.c
>> @@ -110,22 +110,7 @@ static void setup_guest(void)
>> /* The first two pages are the lowcore */
>> guest_instr = guest + PAGE_SIZE * 2;
>>
>> - vm.sblk = alloc_page();
>> -
>> - vm.sblk->cpuflags = CPUSTAT_ZARCH | CPUSTAT_RUNNING;
>> - vm.sblk->prefix = 0;
>> - /*
>> - * Pageable guest with the same ASCE as the test programm,
>> but
>> - * the guest memory 0x0 is offset to start at the allocated
>> - * guest pages and end after 1MB.
>> - *
>> - * It's not pretty but faster and easier than managing guest
>> ASCEs.
>> - */
>> - vm.sblk->mso = (u64)guest;
>> - vm.sblk->msl = (u64)guest;
>> - vm.sblk->ihcpu = 0xffff;
>> -
>> - vm.sblk->crycbd = (uint64_t)alloc_page();
>> + sie_guest_create(&vm, (uint64_t)guest, HPAGE_SIZE);
>>
>> vm.sblk->gpsw.addr = PAGE_SIZE * 4;
>> vm.sblk->gpsw.mask = 0x0000000180000000ULL;
>> @@ -150,6 +135,7 @@ int main(void)
>> setup_guest();
>> test_mvpg();
>> test_mvpg_pei();
>> + sie_guest_destroy(&vm);
>>
>> done:
>> report_prefix_pop();
>> diff --git a/s390x/sie.c b/s390x/sie.c
>> index 5c798a9e..9cb9b055 100644
>> --- a/s390x/sie.c
>> +++ b/s390x/sie.c
>> @@ -84,22 +84,7 @@ static void setup_guest(void)
>> /* The first two pages are the lowcore */
>> guest_instr = guest + PAGE_SIZE * 2;
>>
>> - vm.sblk = alloc_page();
>> -
>> - vm.sblk->cpuflags = CPUSTAT_ZARCH | CPUSTAT_RUNNING;
>> - vm.sblk->prefix = 0;
>> - /*
>> - * Pageable guest with the same ASCE as the test programm,
>> but
>> - * the guest memory 0x0 is offset to start at the allocated
>> - * guest pages and end after 1MB.
>> - *
>> - * It's not pretty but faster and easier than managing guest
>> ASCEs.
>> - */
>> - vm.sblk->mso = (u64)guest;
>> - vm.sblk->msl = (u64)guest;
>> - vm.sblk->ihcpu = 0xffff;
>> -
>> - vm.sblk->crycbd = (uint64_t)alloc_page();
>> + sie_guest_create(&vm, (uint64_t)guest, HPAGE_SIZE);
>> }
>>
>> int main(void)
>> @@ -112,6 +97,8 @@ int main(void)
>>
>> setup_guest();
>> test_diags();
>> + sie_guest_destroy(&vm);
>> +
>> done:
>> report_prefix_pop();
>> return report_summary();
>