From mboxrd@z Thu Jan  1 00:00:00 1970
From: Christoffer Dall <christoffer.dall@linaro.org>
Subject: Re: [PATCH v2 06/10] ARM: KVM: fix handling of trapped 64bit
 coprocessor accesses
Date: Wed, 29 Jan 2014 12:07:48 -0800
Message-ID: <20140129200748.GI3570@cbox>
References: <1390402602-22777-1-git-send-email-marc.zyngier@arm.com>
 <1390402602-22777-7-git-send-email-marc.zyngier@arm.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org,
	kvm@vger.kernel.org
To: Marc Zyngier <marc.zyngier@arm.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mail-pd0-f169.google.com ([209.85.192.169]:37045 "EHLO
	mail-pd0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1750942AbaA2UHt (ORCPT <rfc822;kvm@vger.kernel.org>);
	Wed, 29 Jan 2014 15:07:49 -0500
Received: by mail-pd0-f169.google.com with SMTP id v10so2129707pde.28
        for <kvm@vger.kernel.org>; Wed, 29 Jan 2014 12:07:49 -0800 (PST)
Content-Disposition: inline
In-Reply-To: <1390402602-22777-7-git-send-email-marc.zyngier@arm.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Wed, Jan 22, 2014 at 02:56:38PM +0000, Marc Zyngier wrote:
> Commit 240e99cbd00a (ARM: KVM: Fix 64-bit coprocessor handling)
> changed the way we match the 64bit coprocessor access from
> user space, but didn't update the trap handler for the same
> set of registers.
> 
> The effect is that a trapped 64bit access is never matched, leading
> to a fault being injected into the guest. This went unnoticed as we
> didn;t really trap any 64bit register so far.

didn't

> 
> Placing the CRm field of the access into the CRn field of the matching
> structure fixes the problem. Also update the debug feature to emit the
> expected string in case of failing match.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm/kvm/coproc.c | 4 ++--
>  arch/arm/kvm/coproc.h | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/kvm/coproc.c b/arch/arm/kvm/coproc.c
> index 78c0885..126c90d 100644
> --- a/arch/arm/kvm/coproc.c
> +++ b/arch/arm/kvm/coproc.c
> @@ -443,7 +443,7 @@ int kvm_handle_cp15_64(struct kvm_vcpu *vcpu, struct kvm_run *run)
>  {
>  	struct coproc_params params;
>  
> -	params.CRm = (kvm_vcpu_get_hsr(vcpu) >> 1) & 0xf;
> +	params.CRn = (kvm_vcpu_get_hsr(vcpu) >> 1) & 0xf;
>  	params.Rt1 = (kvm_vcpu_get_hsr(vcpu) >> 5) & 0xf;
>  	params.is_write = ((kvm_vcpu_get_hsr(vcpu) & 1) == 0);
>  	params.is_64bit = true;
> @@ -451,7 +451,7 @@ int kvm_handle_cp15_64(struct kvm_vcpu *vcpu, struct kvm_run *run)
>  	params.Op1 = (kvm_vcpu_get_hsr(vcpu) >> 16) & 0xf;
>  	params.Op2 = 0;
>  	params.Rt2 = (kvm_vcpu_get_hsr(vcpu) >> 10) & 0xf;
> -	params.CRn = 0;
> +	params.CRm = 0;
>  
>  	return emulate_cp15(vcpu, &params);
>  }
> diff --git a/arch/arm/kvm/coproc.h b/arch/arm/kvm/coproc.h
> index 0461d5c..c5ad7ff 100644
> --- a/arch/arm/kvm/coproc.h
> +++ b/arch/arm/kvm/coproc.h
> @@ -58,8 +58,8 @@ static inline void print_cp_instr(const struct coproc_params *p)
>  {
>  	/* Look, we even formatted it for you to paste into the table! */
>  	if (p->is_64bit) {
> -		kvm_pr_unimpl(" { CRm(%2lu), Op1(%2lu), is64, func_%s },\n",
> -			      p->CRm, p->Op1, p->is_write ? "write" : "read");
> +		kvm_pr_unimpl(" { CRm64(%2lu), Op1(%2lu), is64, func_%s },\n",
> +			      p->CRn, p->Op1, p->is_write ? "write" : "read");
>  	} else {
>  		kvm_pr_unimpl(" { CRn(%2lu), CRm(%2lu), Op1(%2lu), Op2(%2lu), is32,"
>  			      " func_%s },\n",
> -- 
> 1.8.3.4
> 

Thanks for fixing my broken fix!

Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>