Re: [PATCH 4/6 v5] KVM: PPC: exit to user space on "ehpriv" instruction

From: tiejun.chen
Date: Wed Jun 26 2013 - 05:17:27 EST


On 06/26/2013 04:44 PM, Bhushan Bharat-R65777 wrote:


-----Original Message-----
From: tiejun.chen [mailto:tiejun.chen@xxxxxxxxxxxxx]
Sent: Wednesday, June 26, 2013 12:25 PM
To: Bhushan Bharat-R65777
Cc: kvm-ppc@xxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx; agraf@xxxxxxx; Wood Scott-
B07421; benh@xxxxxxxxxxxxxxxxxxx; linuxppc-dev@xxxxxxxxxxxxxxxx; linux-
kernel@xxxxxxxxxxxxxxx; mikey@xxxxxxxxxxx; Bhushan Bharat-R65777
Subject: Re: [PATCH 4/6 v5] KVM: PPC: exit to user space on "ehpriv" instruction

On 06/26/2013 01:42 PM, Bharat Bhushan wrote:
"ehpriv" instruction is used for setting software breakpoints
by user space. This patch adds support to exit to user space
with "run->debug" have relevant information.

As this is the first point we are using run->debug, also defined
the run->debug structure.

Signed-off-by: Bharat Bhushan <bharat.bhushan@xxxxxxxxxxxxx>
---
arch/powerpc/include/asm/disassemble.h | 4 ++++
arch/powerpc/include/uapi/asm/kvm.h | 21 +++++++++++++++++----
arch/powerpc/kvm/e500_emulate.c | 27 +++++++++++++++++++++++++++
3 files changed, 48 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/include/asm/disassemble.h
b/arch/powerpc/include/asm/disassemble.h
index 9b198d1..856f8de 100644
--- a/arch/powerpc/include/asm/disassemble.h
+++ b/arch/powerpc/include/asm/disassemble.h
@@ -77,4 +77,8 @@ static inline unsigned int get_d(u32 inst)
return inst & 0xffff;
}

+static inline unsigned int get_oc(u32 inst)
+{
+ return (inst >> 11) & 0x7fff;
+}
#endif /* __ASM_PPC_DISASSEMBLE_H__ */
diff --git a/arch/powerpc/include/uapi/asm/kvm.h
b/arch/powerpc/include/uapi/asm/kvm.h
index 0fb1a6e..ded0607 100644
--- a/arch/powerpc/include/uapi/asm/kvm.h
+++ b/arch/powerpc/include/uapi/asm/kvm.h
@@ -269,7 +269,24 @@ struct kvm_fpu {
__u64 fpr[32];
};

+/*
+ * Defines for h/w breakpoint, watchpoint (read, write or both) and
+ * software breakpoint.
+ * These are used as "type" in KVM_SET_GUEST_DEBUG ioctl and "status"
+ * for KVM_DEBUG_EXIT.
+ */
+#define KVMPPC_DEBUG_NONE 0x0
+#define KVMPPC_DEBUG_BREAKPOINT (1UL << 1)
+#define KVMPPC_DEBUG_WATCH_WRITE (1UL << 2)
+#define KVMPPC_DEBUG_WATCH_READ (1UL << 3)
struct kvm_debug_exit_arch {
+ __u64 address;
+ /*
+ * exiting to userspace because of h/w breakpoint, watchpoint
+ * (read, write or both) and software breakpoint.
+ */
+ __u32 status;
+ __u32 reserved;
};

/* for KVM_SET_GUEST_DEBUG */
@@ -281,10 +298,6 @@ struct kvm_guest_debug_arch {
* Type denotes h/w breakpoint, read watchpoint, write
* watchpoint or watchpoint (both read and write).
*/
-#define KVMPPC_DEBUG_NONE 0x0
-#define KVMPPC_DEBUG_BREAKPOINT (1UL << 1)
-#define KVMPPC_DEBUG_WATCH_WRITE (1UL << 2)
-#define KVMPPC_DEBUG_WATCH_READ (1UL << 3)
__u32 type;
__u32 reserved;
} bp[16];
diff --git a/arch/powerpc/kvm/e500_emulate.c b/arch/powerpc/kvm/e500_emulate.c
index b10a012..dab9d07 100644
--- a/arch/powerpc/kvm/e500_emulate.c
+++ b/arch/powerpc/kvm/e500_emulate.c
@@ -26,6 +26,8 @@
#define XOP_TLBRE 946
#define XOP_TLBWE 978
#define XOP_TLBILX 18
+#define XOP_EHPRIV 270
+#define EHPRIV_OC_DEBUG 0

As I think the case, "OC = 0", is a bit specific since IIRC, if the OC
operand is omitted, its equal 0 by default. So I think we should start this OC
value from 1 or other magic number.

ehpriv instruction is defined to be used as:
ehpriv OC // where OC can be 0,1, ... n
and in extended for it can be used as
ehpriv // With no OC, and here it assumes OC = 0
So OC = 0 is not specific but "ehpriv" is same as "ehpriv 0".

Yes, this is just what I mean.


I do not think of any special reason to reserve "ehpriv" and "ehpriv 0".

So I still prefer we can reserve the 'ehpriv' without OC operand as one simple approach to test or develop something for KVM quickly because its really convenient to trap into the hypervisor only with one 'ehpriv' instruction easily.

But I have no further objection if you guys are fine to this ;-)

Tiejun


Thanks
-Bharat


And if possible, we'd better add some comments to describe this to make the OC
definition readable.

Tiejun


#ifdef CONFIG_KVM_E500MC
static int dbell2prio(ulong param)
@@ -82,6 +84,26 @@ static int kvmppc_e500_emul_msgsnd(struct kvm_vcpu *vcpu,
int rb)
}
#endif

+static int kvmppc_e500_emul_ehpriv(struct kvm_run *run, struct kvm_vcpu
*vcpu,
+ unsigned int inst, int *advance)
+{
+ int emulated = EMULATE_DONE;
+
+ switch (get_oc(inst)) {
+ case EHPRIV_OC_DEBUG:
+ run->exit_reason = KVM_EXIT_DEBUG;
+ run->debug.arch.address = vcpu->arch.pc;
+ run->debug.arch.status = 0;
+ kvmppc_account_exit(vcpu, DEBUG_EXITS);
+ emulated = EMULATE_EXIT_USER;
+ *advance = 0;
+ break;
+ default:
+ emulated = EMULATE_FAIL;
+ }
+ return emulated;
+}
+
int kvmppc_core_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
unsigned int inst, int *advance)
{
@@ -130,6 +152,11 @@ int kvmppc_core_emulate_op(struct kvm_run *run, struct
kvm_vcpu *vcpu,
emulated = kvmppc_e500_emul_tlbivax(vcpu, ea);
break;

+ case XOP_EHPRIV:
+ emulated = kvmppc_e500_emul_ehpriv(run, vcpu, inst,
+ advance);
+ break;
+
default:
emulated = EMULATE_FAIL;
}




--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/