From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Huth Date: Mon, 09 May 2016 08:02:25 +0000 Subject: Re: [PATCH] KVM: PPC: Fix emulated MMIO sign-extension Message-Id: <57304411.2000009@redhat.com> List-Id: References: <20160505061710.GA5559@oak.ozlabs.ibm.com> In-Reply-To: <20160505061710.GA5559@oak.ozlabs.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Paul Mackerras , kvm@vger.kernel.org, kvm-ppc@vger.kernel.org Cc: Bin Lu On 05.05.2016 08:17, Paul Mackerras wrote: > When the guest does a sign-extending load instruction (such as lha > or lwa) to an emulated MMIO location, it results in a call to > kvmppc_handle_loads() in the host. That function sets the > vcpu->arch.mmio_sign_extend flag and calls kvmppc_handle_load() > to do the rest of the work. However, kvmppc_handle_load() sets > the mmio_sign_extend flag to 0 unconditionally, so the sign > extension never gets done. > > To fix this, we rename kvmppc_handle_load to __kvmppc_handle_load > and add an explicit parameter to indicate whether sign extension > is required. kvmppc_handle_load() and kvmppc_handle_loads() then > become 1-line functions that just call __kvmppc_handle_load() > with the extra parameter. > > Reported-by: Bin Lu > Signed-off-by: Paul Mackerras > --- > arch/powerpc/kvm/powerpc.c | 22 ++++++++++++---------- > 1 file changed, 12 insertions(+), 10 deletions(-) > > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c > index 6a68730..02416fe 100644 > --- a/arch/powerpc/kvm/powerpc.c > +++ b/arch/powerpc/kvm/powerpc.c > @@ -800,9 +800,9 @@ static void kvmppc_complete_mmio_load(struct kvm_vcpu *vcpu, > } > } > > -int kvmppc_handle_load(struct kvm_run *run, struct kvm_vcpu *vcpu, > - unsigned int rt, unsigned int bytes, > - int is_default_endian) > +static int __kvmppc_handle_load(struct kvm_run *run, struct kvm_vcpu *vcpu, > + unsigned int rt, unsigned int bytes, > + int is_default_endian, int sign_extend) > { > int idx, ret; > bool host_swabbed; > @@ -827,7 +827,7 @@ int kvmppc_handle_load(struct kvm_run *run, struct kvm_vcpu *vcpu, > vcpu->arch.mmio_host_swabbed = host_swabbed; > vcpu->mmio_needed = 1; > vcpu->mmio_is_write = 0; > - vcpu->arch.mmio_sign_extend = 0; > + vcpu->arch.mmio_sign_extend = sign_extend; > > idx = srcu_read_lock(&vcpu->kvm->srcu); > > @@ -844,6 +844,13 @@ int kvmppc_handle_load(struct kvm_run *run, struct kvm_vcpu *vcpu, > > return EMULATE_DO_MMIO; > } > + > +int kvmppc_handle_load(struct kvm_run *run, struct kvm_vcpu *vcpu, > + unsigned int rt, unsigned int bytes, > + int is_default_endian) > +{ > + return __kvmppc_handle_load(run, vcpu, rt, bytes, is_default_endian, 0); > +} > EXPORT_SYMBOL_GPL(kvmppc_handle_load); > > /* Same as above, but sign extends */ > @@ -851,12 +858,7 @@ int kvmppc_handle_loads(struct kvm_run *run, struct kvm_vcpu *vcpu, > unsigned int rt, unsigned int bytes, > int is_default_endian) > { > - int r; > - > - vcpu->arch.mmio_sign_extend = 1; > - r = kvmppc_handle_load(run, vcpu, rt, bytes, is_default_endian); > - > - return r; > + return __kvmppc_handle_load(run, vcpu, rt, bytes, is_default_endian, 1); > } > > int kvmppc_handle_store(struct kvm_run *run, struct kvm_vcpu *vcpu, > Reviewed-by: Thomas Huth From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Huth Subject: Re: [PATCH] KVM: PPC: Fix emulated MMIO sign-extension Date: Mon, 9 May 2016 10:02:25 +0200 Message-ID: <57304411.2000009@redhat.com> References: <20160505061710.GA5559@oak.ozlabs.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: Bin Lu To: Paul Mackerras , kvm@vger.kernel.org, kvm-ppc@vger.kernel.org Return-path: Received: from mx1.redhat.com ([209.132.183.28]:59546 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751047AbcEIICa (ORCPT ); Mon, 9 May 2016 04:02:30 -0400 In-Reply-To: <20160505061710.GA5559@oak.ozlabs.ibm.com> Sender: kvm-owner@vger.kernel.org List-ID: On 05.05.2016 08:17, Paul Mackerras wrote: > When the guest does a sign-extending load instruction (such as lha > or lwa) to an emulated MMIO location, it results in a call to > kvmppc_handle_loads() in the host. That function sets the > vcpu->arch.mmio_sign_extend flag and calls kvmppc_handle_load() > to do the rest of the work. However, kvmppc_handle_load() sets > the mmio_sign_extend flag to 0 unconditionally, so the sign > extension never gets done. > > To fix this, we rename kvmppc_handle_load to __kvmppc_handle_load > and add an explicit parameter to indicate whether sign extension > is required. kvmppc_handle_load() and kvmppc_handle_loads() then > become 1-line functions that just call __kvmppc_handle_load() > with the extra parameter. > > Reported-by: Bin Lu > Signed-off-by: Paul Mackerras > --- > arch/powerpc/kvm/powerpc.c | 22 ++++++++++++---------- > 1 file changed, 12 insertions(+), 10 deletions(-) > > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c > index 6a68730..02416fe 100644 > --- a/arch/powerpc/kvm/powerpc.c > +++ b/arch/powerpc/kvm/powerpc.c > @@ -800,9 +800,9 @@ static void kvmppc_complete_mmio_load(struct kvm_vcpu *vcpu, > } > } > > -int kvmppc_handle_load(struct kvm_run *run, struct kvm_vcpu *vcpu, > - unsigned int rt, unsigned int bytes, > - int is_default_endian) > +static int __kvmppc_handle_load(struct kvm_run *run, struct kvm_vcpu *vcpu, > + unsigned int rt, unsigned int bytes, > + int is_default_endian, int sign_extend) > { > int idx, ret; > bool host_swabbed; > @@ -827,7 +827,7 @@ int kvmppc_handle_load(struct kvm_run *run, struct kvm_vcpu *vcpu, > vcpu->arch.mmio_host_swabbed = host_swabbed; > vcpu->mmio_needed = 1; > vcpu->mmio_is_write = 0; > - vcpu->arch.mmio_sign_extend = 0; > + vcpu->arch.mmio_sign_extend = sign_extend; > > idx = srcu_read_lock(&vcpu->kvm->srcu); > > @@ -844,6 +844,13 @@ int kvmppc_handle_load(struct kvm_run *run, struct kvm_vcpu *vcpu, > > return EMULATE_DO_MMIO; > } > + > +int kvmppc_handle_load(struct kvm_run *run, struct kvm_vcpu *vcpu, > + unsigned int rt, unsigned int bytes, > + int is_default_endian) > +{ > + return __kvmppc_handle_load(run, vcpu, rt, bytes, is_default_endian, 0); > +} > EXPORT_SYMBOL_GPL(kvmppc_handle_load); > > /* Same as above, but sign extends */ > @@ -851,12 +858,7 @@ int kvmppc_handle_loads(struct kvm_run *run, struct kvm_vcpu *vcpu, > unsigned int rt, unsigned int bytes, > int is_default_endian) > { > - int r; > - > - vcpu->arch.mmio_sign_extend = 1; > - r = kvmppc_handle_load(run, vcpu, rt, bytes, is_default_endian); > - > - return r; > + return __kvmppc_handle_load(run, vcpu, rt, bytes, is_default_endian, 1); > } > > int kvmppc_handle_store(struct kvm_run *run, struct kvm_vcpu *vcpu, > Reviewed-by: Thomas Huth