Re: [RFC PATCH v1 01/10] s390/uaccess: Add storage key checked access to user memory

[Janis Schoetterl-Glausch]

On 1/18/22 16:37, Sven Schnelle wrote:
> Hi Janis,
> 
> Janis Schoetterl-Glausch <scgl@xxxxxxxxxxxxx> writes:
> 
>> KVM needs a mechanism to do accesses to guest memory that honor
>> storage key protection.
>> Since the copy_to/from_user implementation makes use of move
>> instructions that support having an additional access key supplied,
>> we can implement __copy_from/to_user_with_key by enhancing the
>> existing implementation.
>>
>> Signed-off-by: Janis Schoetterl-Glausch <scgl@xxxxxxxxxxxxx>
> 
> This doesn't apply to my master branch.

Maybe it's due to the prerequisite patch missing?
https://lore.kernel.org/linux-s390/YeGBmPBJ8NMi0Rkp@osiris/T/#t

> 
>> diff --git a/arch/s390/lib/uaccess.c b/arch/s390/lib/uaccess.c
>> index d3a700385875..ce7a150dd93a 100644
>> --- a/arch/s390/lib/uaccess.c
>> +++ b/arch/s390/lib/uaccess.c
>> @@ -59,11 +59,13 @@ static inline int copy_with_mvcos(void)
>>  #endif
>>  
>>  static inline unsigned long copy_from_user_mvcos(void *x, const void __user *ptr,
>> -						 unsigned long size)
>> +						 unsigned long size, char key)
>>  {
>>  	unsigned long tmp1, tmp2;
>>  	union oac spec = {
>> +		.oac2.key = key,
>>  		.oac2.as = PSW_BITS_AS_SECONDARY,
>> +		.oac2.k = 1,
>>  		.oac2.a = 1,
>>  	};
>>  
>> @@ -94,19 +96,19 @@ static inline unsigned long copy_from_user_mvcos(void *x, const void __user *ptr
>>  }
>>  
>>  static inline unsigned long copy_from_user_mvcp(void *x, const void __user *ptr,
>> -						unsigned long size)
>> +						unsigned long size, char key)
> 
> Any special reason for using 'char' as type for key here? Given the left shift
> below i would prefer 'unsigned char' to avoid having to think about
> whether this can overflow. The end result wouldn't look different,
> so more or less a cosmetic issue.

Will do.

[...]
> 
> With that minor nitpick:
> 
> Reviewed-by: Sven Schnelle <svens@xxxxxxxxxxxxx>

Thanks!