On Friday 04 July 2014 12:18 PM, Alexander Graf wrote: > > On 04.07.14 06:34, Madhavan Srinivasan wrote: >> On Thursday 03 July 2014 05:21 PM, Alexander Graf wrote: >>> On 01.07.14 10:41, Madhavan Srinivasan wrote: >>>> This patch adds kernel side support for software breakpoint. >>>> Design is that, by using an illegal instruction, we trap to hypervisor >>>> via Emulation Assistance interrupt, where we check for the illegal >>>> instruction >>>> and accordingly we return to Host or Guest. Patch also adds support for >>>> software breakpoint in PR KVM. >>>> >>>> Patch mandates use of "abs" instruction as sw breakpoint instruction >>>> (primary opcode 31 and extended opcode 360). Based on PowerISA v2.01, >>>> ABS instruction has been dropped from the architecture and treated an >>>> illegal instruction. >>>> >>>> Changes v1->v2: >>>> >>>> Moved the debug instruction #def to kvm_book3s.h. This way PR_KVM >>>> can also share it. >>>> Added code to use KVM get one reg infrastructure to get debug >>>> opcode. >>>> Updated emulate.c to include emulation of debug instruction incase >>>> of PR_KVM. >>>> Made changes to commit message. >>>> >>>> Signed-off-by: Madhavan Srinivasan <maddy@xxxxxxxxxxxxxxxxxx> >>>> --- >>>> arch/powerpc/include/asm/kvm_book3s.h | 8 ++++++++ >>>> arch/powerpc/include/asm/ppc-opcode.h | 5 +++++ >>>> arch/powerpc/kvm/book3s.c | 3 ++- >>>> arch/powerpc/kvm/book3s_hv.c | 9 +++++++++ >>>> arch/powerpc/kvm/book3s_pr.c | 3 +++ >>>> arch/powerpc/kvm/emulate.c | 10 ++++++++++ >>>> 6 files changed, 37 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/arch/powerpc/include/asm/kvm_book3s.h >>>> b/arch/powerpc/include/asm/kvm_book3s.h >>>> index f52f656..180d549 100644 >>>> --- a/arch/powerpc/include/asm/kvm_book3s.h >>>> +++ b/arch/powerpc/include/asm/kvm_book3s.h >>>> @@ -24,6 +24,14 @@ >>>> #include <linux/kvm_host.h> >>>> #include <asm/kvm_book3s_asm.h> >>>> +/* >>>> + * KVMPPC_INST_BOOK3S_DEBUG is debug Instruction for supporting >>>> Software Breakpoint. >>>> + * Instruction mnemonic is ABS, primary opcode is 31 and extended >>>> opcode is 360. >>>> + * Based on PowerISA v2.01, ABS instruction has been dropped from the >>>> architecture >>>> + * and treated an illegal instruction. >>>> + */ >>>> +#define KVMPPC_INST_BOOK3S_DEBUG 0x7c0002d0 >>> This will still break with LE guests. >>> >> I am told to try with all 0s opcode. So rewriting the patch. > > The problem with "all 0s" is that it's reasonably likely to occur on > real world code. Hence Segher was proposing something like 0x00dddd00 > which should be the same regardless of endianness, but has a certain > appeal of intentional placement ;). > Ok Sure. >> >>>> + >>>> struct kvmppc_bat { >>>> u64 raw; >>>> u32 bepi; >>>> diff --git a/arch/powerpc/include/asm/ppc-opcode.h >>>> b/arch/powerpc/include/asm/ppc-opcode.h >>>> index 3132bb9..3fbb4c1 100644 >>>> --- a/arch/powerpc/include/asm/ppc-opcode.h >>>> +++ b/arch/powerpc/include/asm/ppc-opcode.h >>>> @@ -111,6 +111,11 @@ >>>> #define OP_31_XOP_LHBRX 790 >>>> #define OP_31_XOP_STHBRX 918 >>>> +/* KVMPPC_INST_BOOK3S_DEBUG -- Software breakpoint Instruction >>>> + * Instruction mnemonic is ABS, primary opcode is 31 and extended >>>> opcode is 360. >>>> + */ >>>> +#define OP_31_XOP_ABS 360 >>>> + >>>> #define OP_LWZ 32 >>>> #define OP_LD 58 >>>> #define OP_LWZU 33 >>>> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c >>>> index c254c27..b40fe5d 100644 >>>> --- a/arch/powerpc/kvm/book3s.c >>>> +++ b/arch/powerpc/kvm/book3s.c >>>> @@ -789,7 +789,8 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu >>>> *vcpu, >>>> int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu, >>>> struct kvm_guest_debug *dbg) >>>> { >>>> - return -EINVAL; >>>> + vcpu->guest_debug = dbg->control; >>>> + return 0; >>>> } >>>> void kvmppc_decrementer_func(unsigned long data) >>>> diff --git a/arch/powerpc/kvm/book3s_hv.c >>>> b/arch/powerpc/kvm/book3s_hv.c >>>> index 7a12edb..402c1ec 100644 >>>> --- a/arch/powerpc/kvm/book3s_hv.c >>>> +++ b/arch/powerpc/kvm/book3s_hv.c >>>> @@ -725,8 +725,14 @@ static int kvmppc_handle_exit_hv(struct kvm_run >>>> *run, struct kvm_vcpu *vcpu, >>>> * we don't emulate any guest instructions at this stage. >>>> */ >>>> case BOOK3S_INTERRUPT_H_EMUL_ASSIST: >>>> + if (kvmppc_get_last_inst(vcpu) == KVMPPC_INST_BOOK3S_DEBUG ) { >>>> + run->exit_reason = KVM_EXIT_DEBUG; >>>> + run->debug.arch.address = kvmppc_get_pc(vcpu); >>>> + r = RESUME_HOST; >>> Phew - why can't we just go into the normal instruction emulator for >>> EMUL_ASSIST? >>> >> IIUC, using the emulation_assist_interrupt function (kernel/trap.c) ? > > I was more thinking of kvmppc_emulate_instruction() :). > This makes sense. Can use the same call for pr kvm also. awesome :) > > Alex > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html