Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <bf1c79cc-2108-ceb0-4f0a-d83386046f00@linux.ibm.com>
Date:   Tue, 18 Jan 2022 16:52:58 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.4.0
Subject: Re: [RFC PATCH v1 01/10] s390/uaccess: Add storage key checked access
 to user memory
Content-Language: en-US
To:     Sven Schnelle <svens@linux.ibm.com>
Cc:     Heiko Carstens <hca@linux.ibm.com>,
        Vasily Gorbik <gor@linux.ibm.com>,
        Christian Borntraeger <borntraeger@linux.ibm.com>,
        Nico Boehr <nrb@linux.ibm.com>,
        Alexander Gordeev <agordeev@linux.ibm.com>,
        linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20220118095210.1651483-1-scgl@linux.ibm.com>
 <20220118095210.1651483-2-scgl@linux.ibm.com>
 <yt9dmtjtcaws.fsf@linux.ibm.com>
From:   Janis Schoetterl-Glausch <scgl@linux.ibm.com>
In-Reply-To: <yt9dmtjtcaws.fsf@linux.ibm.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
Precedence: bulk

On 1/18/22 16:37, Sven Schnelle wrote:
> Hi Janis,
> 
> Janis Schoetterl-Glausch <scgl@linux.ibm.com> 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@linux.ibm.com>
> 
> 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@linux.ibm.com>

Thanks!