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

Nico Boehr
2022-08-02T07:56:18Z

Quoting Janosch Frank (2022-07-29 10:26:32)
> 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>

Reviewed-by: Nico Boehr <nrb@linux.ibm.com>

with two tiny things for you to consider.

[...]
> diff --git a/lib/s390x/uv.c b/lib/s390x/uv.c
> index 3b4cafa9..99775929 100644
> --- a/lib/s390x/uv.c
> +++ b/lib/s390x/uv.c
> @@ -76,7 +76,8 @@ void uv_init(void)
> int cc;
>
> /* Let's not do this twice */
> - assert(!initialized);
> + if (initialized)
> + return;

Caught me a bit by surprise, maybe you want to point out this change in the commit message.

[...]
> +/*
> + * 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);
> + 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;

I am wondering whether it might make sense to copy over the flags from cr1 so we don't have to worry about keeping them in sync?

But possibly it is sufficiently unlikely that these will ever change...