On Tuesday 17 June 2014 02:24 PM, Alexander Graf wrote:
>
> On 14.06.14 23:08, 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 mandates use of
>> "abs" instruction
>> (primary opcode 31 and extended opcode 360) as sw breakpoint instruction.
>> Based on PowerISA v2.01, ABS instruction has been dropped from the
>> architecture
>> and treated an illegal instruction.
>>
>> Signed-off-by: Madhavan Srinivasan <maddy@xxxxxxxxxxxxxxxxxx>
>> ---
>> arch/powerpc/kvm/book3s.c | 3 ++-
>> arch/powerpc/kvm/book3s_hv.c | 23 +++++++++++++++++++----
>> 2 files changed, 21 insertions(+), 5 deletions(-)
>>
>> 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..688421d 100644
>> --- a/arch/powerpc/kvm/book3s_hv.c
>> +++ b/arch/powerpc/kvm/book3s_hv.c
>> @@ -67,6 +67,14 @@
>> /* Used as a "null" value for timebase values */
>> #define TB_NIL (~(u64)0)
>> +/*
>> + * SW_BRK_DBG_INT 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 SW_BRK_DBG_INT 0x7c0002d0
>
> The instruction we use to trap needs to get exposed to user space via a
> ONE_REG property.
>
Yes. I got to know about that from Bharat (patchset "ppc debug: Add
debug stub support"). I will change it.
> Also, why don't we use twi always or something else that actually is
> defined as illegal instruction? I would like to see this shared with
> book3s_32 PR.
>
>> +
>> static void kvmppc_end_cede(struct kvm_vcpu *vcpu);
>> static int kvmppc_hv_setup_htab_rma(struct kvm_vcpu *vcpu);
>> @@ -721,12 +729,19 @@ static int kvmppc_handle_exit_hv(struct
>> kvm_run *run, struct kvm_vcpu *vcpu,
>> break;
>> /*
>> * This occurs if the guest executes an illegal instruction.
>> - * We just generate a program interrupt to the guest, since
>> - * we don't emulate any guest instructions at this stage.
>> + * To support software breakpoint, we check for the sw breakpoint
>> + * instruction to return back to host, if not we just generate a
>> + * program interrupt to the guest.
>> */
>> case BOOK3S_INTERRUPT_H_EMUL_ASSIST:
>> - kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
>> - r = RESUME_GUEST;
>> + if (vcpu->arch.last_inst == SW_BRK_DBG_INT) {
>
> Don't access last_inst directly. Instead use the provided helpers.
>
Ok. Will look and replace it.
>> + run->exit_reason = KVM_EXIT_DEBUG;
>> + run->debug.arch.address = vcpu->arch.pc;
>> + r = RESUME_HOST;
>> + } else {
>> + kvmppc_core_queue_program(vcpu, 0x80000);
>
> magic numbers
^^^^^
I did not understand this?
>> + r = RESUME_GUEST;
>> + }
>> break;
>> /*
>> * This occurs if the guest (kernel or userspace), does
>> something that
>
> Please enable PR KVM as well while you're at it.
>
My bad, I did not try the PR KVM. I will try it out.
>
> Alex
>
Thanks for review
Regards
Maddy
--
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
