Re: [kvm-unit-tests PATCH v2 5/7] lib: s390x: Use a new asce for each PV guest

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

On Thu, 20 Oct 2022 13:27:22 +0200
Janosch Frank <frankja@linux.ibm.com> wrote:

> On 10/20/22 11:25, Claudio Imbrenda wrote:
> > On Thu, 20 Oct 2022 09:00:07 +0000
> > Janosch Frank <frankja@linux.ibm.com> wrote:
> >   
> >> Every PV guest needs its own ASCE so let's copy the topmost table
> >> designated by CR1 to create a new ASCE for the PV guest. Before and
> >> after SIE we now need to switch ASCEs to and from the PV guest / test
> >> ASCE. The SIE assembly function does that automatically.
> >>
> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> >> ---
> >>   lib/s390x/asm-offsets.c |  2 ++
> >>   lib/s390x/sie.c         |  2 ++
> >>   lib/s390x/sie.h         |  2 ++
> >>   lib/s390x/uv.c          | 24 +++++++++++++++++++++++-
> >>   lib/s390x/uv.h          |  5 ++---
> >>   s390x/cpu.S             |  6 ++++++
> >>   6 files changed, 37 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/lib/s390x/asm-offsets.c b/lib/s390x/asm-offsets.c
> >> index fbea3278..f612f327 100644
> >> --- a/lib/s390x/asm-offsets.c
> >> +++ b/lib/s390x/asm-offsets.c
> >> @@ -75,9 +75,11 @@ int main(void)
> >>   	OFFSET(SIE_SAVEAREA_HOST_GRS, vm_save_area, host.grs[0]);
> >>   	OFFSET(SIE_SAVEAREA_HOST_FPRS, vm_save_area, host.fprs[0]);
> >>   	OFFSET(SIE_SAVEAREA_HOST_FPC, vm_save_area, host.fpc);
> >> +	OFFSET(SIE_SAVEAREA_HOST_ASCE, vm_save_area, host.asce);
> >>   	OFFSET(SIE_SAVEAREA_GUEST_GRS, vm_save_area, guest.grs[0]);
> >>   	OFFSET(SIE_SAVEAREA_GUEST_FPRS, vm_save_area, guest.fprs[0]);
> >>   	OFFSET(SIE_SAVEAREA_GUEST_FPC, vm_save_area, guest.fpc);
> >> +	OFFSET(SIE_SAVEAREA_GUEST_ASCE, vm_save_area, guest.asce);
> >>   	OFFSET(STACK_FRAME_INT_BACKCHAIN, stack_frame_int, back_chain);
> >>   	OFFSET(STACK_FRAME_INT_FPC, stack_frame_int, fpc);
> >>   	OFFSET(STACK_FRAME_INT_FPRS, stack_frame_int, fprs);
> >> diff --git a/lib/s390x/sie.c b/lib/s390x/sie.c
> >> index 3fee3def..6efad965 100644
> >> --- a/lib/s390x/sie.c
> >> +++ b/lib/s390x/sie.c
> >> @@ -85,6 +85,8 @@ void sie_guest_create(struct vm *vm, uint64_t guest_mem, uint64_t guest_mem_len)
> >>   
> >>   	/* Guest memory chunks are always 1MB */
> >>   	assert(!(guest_mem_len & ~HPAGE_MASK));
> >> +	/* For non-PV guests we re-use the host's ASCE for ease of use */
> >> +	vm->save_area.guest.asce = stctg(1);
> >>   	/* 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);
> >> diff --git a/lib/s390x/sie.h b/lib/s390x/sie.h
> >> index 320c4218..3e3605c9 100644
> >> --- a/lib/s390x/sie.h
> >> +++ b/lib/s390x/sie.h
> >> @@ -205,12 +205,14 @@ union {
> >>   struct vm_uv {
> >>   	uint64_t vm_handle;
> >>   	uint64_t vcpu_handle;
> >> +	uint64_t asce;
> >>   	void *conf_base_stor;
> >>   	void *conf_var_stor;
> >>   	void *cpu_stor;
> >>   };
> >>   
> >>   struct vm_save_regs {
> >> +	uint64_t asce;
> >>   	uint64_t grs[16];
> >>   	uint64_t fprs[16];
> >>   	uint32_t fpc;
> >> diff --git a/lib/s390x/uv.c b/lib/s390x/uv.c
> >> index 3b4cafa9..0b6eb843 100644
> >> --- a/lib/s390x/uv.c
> >> +++ b/lib/s390x/uv.c
> >> @@ -90,6 +90,25 @@ void uv_init(void)
> >>   	initialized = true;
> >>   }
> >>   
> >> +/*
> >> + * Create a new ASCE for the UV config because they can't be shared
> >> + * for security reasons. We just simply copy the top most table into a
> >> + * fresh set of allocated pages and use those pages as the asce.
> >> + */
> >> +static uint64_t create_asce(void)
> >> +{
> >> +	void *pgd_new, *pgd_old;
> >> +	uint64_t asce = stctg(1);
> >> +
> >> +	pgd_new = memalign_pages_flags(PAGE_SIZE, PAGE_SIZE * 4, 0);  
> > 
> > here you can use memalign_pages, since you are not using the flags  
> 
> Sure
> 
> >   
> >> +	pgd_old = (void *)(asce & PAGE_MASK);
> >> +
> >> +	memcpy(pgd_new, pgd_old, PAGE_SIZE * 4);
> >> +
> >> +	asce = __pa(pgd_new) | ASCE_DT_REGION1 | REGION_TABLE_LENGTH | ASCE_P;  
> > 
> > why not taking the flags from the old ASCE? what if we choose to use a
> > different type of table?
> > 
> > something like this:
> > 
> > asce = _pa(pgd_new) | ASCE_P | (asce & ~PAGE_MASK);  
> 
> I should at least preserve DT and TL but I'd opt to not copy over the 
> other bits. If someone wants to do funky ASCE stuff they now have the 
> possibility to simply change vm->save_area.guest.asce

that's ok