Re: [kvm-unit-tests PATCH 1/6] s390x: uv-host: Add access checks for donated memory

Nico Boehr
2022-05-16T08:22:30Z

On Fri, 2022-05-13 at 09:50 +0000, Janosch Frank wrote:
> Let's check if the UV really protected all the memory we donated.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>

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

One suggestion below for you to consider.

> ---
>  s390x/uv-host.c | 42 ++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 40 insertions(+), 2 deletions(-)
>
> diff --git a/s390x/uv-host.c b/s390x/uv-host.c
> index a1a6d120..0f0b18a1 100644
> --- a/s390x/uv-host.c
> +++ b/s390x/uv-host.c
> @@ -142,7 +142,8 @@ static void test_cpu_destroy(void)
>  static void test_cpu_create(void)
>  {
>         int rc;
> -       unsigned long tmp;
> +       unsigned long tmp, i;
> +       uint8_t *access_ptr;
>
>         report_prefix_push("csc");
>         uvcb_csc.header.len = sizeof(uvcb_csc);
> @@ -194,6 +195,18 @@ static void test_cpu_create(void)
>         report(rc == 0 && uvcb_csc.header.rc == UVC_RC_EXECUTED &&
>                uvcb_csc.cpu_handle, "success");
>
> +       rc = 1;
> +       for (i = 0; i < uvcb_qui.cpu_stor_len / PAGE_SIZE; i++) {
> +               expect_pgm_int();
> +               access_ptr = (void *)uvcb_csc.stor_origin + PAGE_SIZE
> * i;
> +               *access_ptr = 42;
> +               if (clear_pgm_int() !=
> PGM_INT_CODE_SECURE_STOR_ACCESS) {
> +                       rc = 0;
> +                       break;
> +               }
> +       }
> +       report(rc, "Storage protection");

All of these for loops look pretty similar, would it make sense to move
them to their own function like:

assert_range_write_protected(void *start, size_t len, int
pgm_int_code)?