Re: [PATCH] powerpc/kvm: support to handle sw breakpoint

[Alexander Graf <agraf@suse.de>]

On 17.06.14 13:13, Madhavan Srinivasan wrote:
> On Tuesday 17 June 2014 04:38 PM, Alexander Graf wrote:
>> On 17.06.14 13:07, Madhavan Srinivasan wrote:
>>> 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@linux.vnet.ibm.com>
>>>>> ---
>>>>>     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 please make sure to pick an instruction that preferably looks 
identical regardless of guest endianness. Segher suggested 0x00dddd00. 
Does that trap properly for you?


Alex