From mboxrd@z Thu Jan  1 00:00:00 1970
From: Segher Boessenkool <segher@kernel.crashing.org>
Date: Sun, 03 Aug 2014 15:51:08 +0000
Subject: Re: [PATCH v3] powerpc/kvm: support to handle sw breakpoint
Message-Id: <20140803155108.GA19710@gate.crashing.org>
List-Id: <kvm-ppc.vger.kernel.org>
References: <1406868643-26291-1-git-send-email-maddy@linux.vnet.ibm.com>
In-Reply-To: <1406868643-26291-1-git-send-email-maddy@linux.vnet.ibm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
Cc: agraf@suse.de, benh@kernel.crashing.org, paulus@samba.org, linuxppc-dev@lists.ozlabs.org, kvm-ppc@vger.kernel.org, kvm@vger.kernel.org

> +/*
> + * KVMPPC_INST_BOOK3S_DEBUG is debug Instruction for supporting Software Breakpoint.
> + * Based on PowerISA v2.07, Instruction with opcode 0s will be treated as illegal
> + * instruction.
> + */

"primary opcode 0" instead?

> +#define OP_ZERO                 0x0

Using 0x0 where you mean 0, making a #define for 0 in the first place...
This all looks rather silly doesn't it.

> +	case OP_ZERO:
> +		if((inst & 0x00FFFF00) = KVMPPC_INST_BOOK3S_DEBUG) {

You either shouldn't mask at all here, or the mask is wrong (the primary
op is the top six bits, not the top eight).


Segher