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

[Janosch Frank <frankja@xxxxxxxxxxxxx>]

On 10/20/22 11:25, Claudio Imbrenda wrote:
> On Thu, 20 Oct 2022 09:00:07 +0000
> Janosch Frank <frankja@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx>
> > ---
> >   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