Re: [PATCH v3 04/10] KVM: Implement kvm_put_guest()

From: Steven Price <steven.price@arm.com>
To: Jonathan Cameron <jonathan.cameron@huawei.com>
Cc: "Mark Rutland" <mark.rutland@arm.com>,
	"Radim Krčmář" <rkrcmar@redhat.com>,
	kvm@vger.kernel.org, "Suzuki K Pouloze" <suzuki.poulose@arm.com>,
	"Marc Zyngier" <maz@kernel.org>,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	"Russell King" <linux@armlinux.org.uk>,
	"James Morse" <james.morse@arm.com>,
	linux-arm-kernel@lists.infradead.org,
	"Catalin Marinas" <catalin.marinas@arm.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Will Deacon" <will@kernel.org>,
	kvmarm@lists.cs.columbia.edu,
	"Julien Thierry" <julien.thierry.kdev@gmail.com>
Subject: Re: [PATCH v3 04/10] KVM: Implement kvm_put_guest()
Date: Thu, 22 Aug 2019 11:37:06 +0100	[thread overview]
Message-ID: <003f509f-964b-2af0-d5da-1155fb4ec9d6@arm.com> (raw)
In-Reply-To: <20190822112930.000052db@huawei.com>

On 22/08/2019 11:29, Jonathan Cameron wrote:
> On Wed, 21 Aug 2019 16:36:50 +0100
> Steven Price <steven.price@arm.com> wrote:
> 
>> kvm_put_guest() is analogous to put_user() - it writes a single value to
>> the guest physical address. The implementation is built upon put_user()
>> and so it has the same single copy atomic properties.
>>
>> Signed-off-by: Steven Price <steven.price@arm.com>
>> ---
>>  include/linux/kvm_host.h | 24 ++++++++++++++++++++++++
>>  1 file changed, 24 insertions(+)
>>
>> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
>> index fcb46b3374c6..e154a1897e20 100644
>> --- a/include/linux/kvm_host.h
>> +++ b/include/linux/kvm_host.h
>> @@ -746,6 +746,30 @@ int kvm_write_guest_offset_cached(struct kvm *kvm, struct gfn_to_hva_cache *ghc,
>>  				  unsigned long len);
>>  int kvm_gfn_to_hva_cache_init(struct kvm *kvm, struct gfn_to_hva_cache *ghc,
>>  			      gpa_t gpa, unsigned long len);
>> +
>> +#define __kvm_put_guest(kvm, gfn, offset, value, type)			\
>> +({									\
>> +	unsigned long __addr = gfn_to_hva(kvm, gfn);			\
>> +	type __user *__uaddr = (type __user *)(__addr + offset);	\
>> +	int __ret = 0;							\
> 
> Why initialize __ret?

Good question. Actually looking at this again if I reorder this to be
pessimistic I can make it shorter:

	int __ret = -EFAULT;

	if (!kvm_is_error_hva(__addr))
		__ret = put_user(value, __uaddr);
	if (!__ret)
		mark_page_dirty(kvm, gfn);				
	__ret;

Thanks for taking a look.

Steve

>> +									\
>> +	if (kvm_is_error_hva(__addr))					\
>> +		__ret = -EFAULT;					\
>> +	else								\
>> +		__ret = put_user(value, __uaddr);			\
>> +	if (!__ret)							\
>> +		mark_page_dirty(kvm, gfn);				\
>> +	__ret;								\
>> +})
>> +
>> +#define kvm_put_guest(kvm, gpa, value, type)				\
>> +({									\
>> +	gpa_t __gpa = gpa;						\
>> +	struct kvm *__kvm = kvm;					\
>> +	__kvm_put_guest(__kvm, __gpa >> PAGE_SHIFT,			\
>> +			offset_in_page(__gpa), (value), type);		\
>> +})
>> +
>>  int kvm_clear_guest_page(struct kvm *kvm, gfn_t gfn, int offset, int len);
>>  int kvm_clear_guest(struct kvm *kvm, gpa_t gpa, unsigned long len);
>>  struct kvm_memory_slot *gfn_to_memslot(struct kvm *kvm, gfn_t gfn);
> 
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel