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

[Janosch Frank <frankja@linux.ibm.com>]

On 5/16/22 10:21, Nico Boehr wrote:
> 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>

Thanks

> 
> One suggestion below for you to consider.

Sure, I'll rework the loops

> 
>> ---
>>   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)?