From mboxrd@z Thu Jan 1 00:00:00 1970 From: Radim =?utf-8?B?S3LEjW3DocWZ?= Subject: Re: [PATCH v2 07/13] KVM: add vcpu-specific functions to read/write/translate GFNs Date: Fri, 5 Jun 2015 17:13:37 +0200 Message-ID: <20150605151337.GA16327@potion.brq.redhat.com> References: <1432746314-50196-1-git-send-email-pbonzini@redhat.com> <1432746314-50196-8-git-send-email-pbonzini@redhat.com> <20150529192346.GA11651@potion.brq.redhat.com> <5571795B.6080204@redhat.com> <20150605121051.GD11957@potion.brq.redhat.com> <55719A2F.3030301@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, guangrong.xiao@linux.intel.com, bdas@redhat.com To: Paolo Bonzini Return-path: Content-Disposition: inline In-Reply-To: <55719A2F.3030301@redhat.com> Sender: linux-kernel-owner@vger.kernel.org List-Id: kvm.vger.kernel.org 2015-06-05 14:46+0200, Paolo Bonzini: > On 05/06/2015 14:10, Radim Kr=C4=8Dm=C3=A1=C5=99 wrote: > > + ret =3D __kvm_write_guest_page(__gfn_to_memslot(slots, gfn), gfn= , > > + data, offset, seg); >=20 > Even better, let's pass memslots to all the __ functions. Yeah, while scoping it, I noticed a bug in the series ... makes me wish that C had a useful type system. A quick fix would be to replace gpa with gfn in calls to __kvm_read_guest_atomic(). I presume you'd prefer a new patch to rebasing, so it's below. --- KVM: fix gpa/gfn mixup in __kvm_read_guest_atomic Refactoring passed gpa instead of gfn to __kvm_read_guest_atomic. While at it, lessen code duplication by extracting slots earlier. =46ixes: 841509f38372 ("KVM: add vcpu-specific functions to read/write/= translate GFNs") Signed-off-by: Radim Kr=C4=8Dm=C3=A1=C5=99 --- virt/kvm/kvm_main.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 10ae7e348dcc..4fa1edc34630 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -1645,11 +1645,14 @@ int kvm_vcpu_read_guest(struct kvm_vcpu *vcpu, = gpa_t gpa, void *data, unsigned l } EXPORT_SYMBOL_GPL(kvm_vcpu_read_guest); =20 -static int __kvm_read_guest_atomic(struct kvm_memory_slot *slot, gfn_t= gfn, - void *data, int offset, unsigned long len) +static int __kvm_read_guest_atomic(struct kvm_memslots *slots, gpa_t g= pa, + void *data, unsigned long len) { int r; unsigned long addr; + gfn_t gfn =3D gpa >> PAGE_SHIFT; + struct kvm_memory_slot *slot =3D __gfn_to_memslot(slots, gfn); + int offset =3D offset_in_page(gpa); =20 addr =3D gfn_to_hva_memslot_prot(slot, gfn, NULL); if (kvm_is_error_hva(addr)) @@ -1665,22 +1668,18 @@ static int __kvm_read_guest_atomic(struct kvm_m= emory_slot *slot, gfn_t gfn, int kvm_read_guest_atomic(struct kvm *kvm, gpa_t gpa, void *data, unsigned long len) { - gfn_t gfn =3D gpa >> PAGE_SHIFT; - struct kvm_memory_slot *slot =3D gfn_to_memslot(kvm, gfn); - int offset =3D offset_in_page(gpa); + struct kvm_memslots *slots =3D kvm_memslots(kvm); =20 - return __kvm_read_guest_atomic(slot, gpa, data, offset, len); + return __kvm_read_guest_atomic(slots, gpa, data, len); } EXPORT_SYMBOL_GPL(kvm_read_guest_atomic); =20 int kvm_vcpu_read_guest_atomic(struct kvm_vcpu *vcpu, gpa_t gpa, void *data, unsigned long len) { - gfn_t gfn =3D gpa >> PAGE_SHIFT; - struct kvm_memory_slot *slot =3D kvm_vcpu_gfn_to_memslot(vcpu, gfn); - int offset =3D offset_in_page(gpa); + struct kvm_memslots *slots =3D kvm_vcpu_memslots(vcpu); =20 - return __kvm_read_guest_atomic(slot, gpa, data, offset, len); + return __kvm_read_guest_atomic(slots, gpa, data, len); } EXPORT_SYMBOL_GPL(kvm_vcpu_read_guest_atomic); =20