Re: [PATCH v4] powerpc/kvm: support to handle sw breakpoint
> Am 23.08.2014 um 12:17 schrieb Madhavan Srinivasan :
>
>> On Thursday 21 August 2014 02:40 PM, Alexander Graf wrote:
>>
>>
>>> On 20.08.14 07:52, 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.
>>>
>>> Changes v3->v4:
>>> Made changes to code comments and removed #define of zero opcode
>>> Added a new function to handle the debug instruction emulation in book3s_hv
>>> Rebased the code to latest upstream source.
>>>
>>> Changes v2->v3:
>>> Changed the debug instructions. Using the all zero opcode in the
>>> instruction word
>>> as illegal instruction as mentioned in Power ISA instead of ABS
>>> Removed reg updated in emulation assist and added a call to
>>> kvmppc_emulate_instruction for reg update.
>>>
>>> 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
>>> ---
>>> arch/powerpc/include/asm/kvm_book3s.h | 7 +++
>>> arch/powerpc/kvm/book3s.c | 3 ++-
>>> arch/powerpc/kvm/book3s_hv.c | 32 ++--
>>> arch/powerpc/kvm/book3s_pr.c | 3 +++
>>> arch/powerpc/kvm/emulate.c| 11 +++
>>> 5 files changed, 53 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/powerpc/include/asm/kvm_book3s.h
>>> b/arch/powerpc/include/asm/kvm_book3s.h
>>> index 6acf0c2..a1944f8 100644
>>> --- a/arch/powerpc/include/asm/kvm_book3s.h
>>> +++ b/arch/powerpc/include/asm/kvm_book3s.h
>>> @@ -24,6 +24,13 @@
>>> #include
>>> #include
>>>
>>> +/*
>>> + * KVMPPC_INST_BOOK3S_DEBUG is debug Instruction for supporting Software
>>> Breakpoint.
>>> + * Based on PowerISA v2.07, Instruction with primary opcode 0 will be
>>> treated as illegal
>>> + * instruction.
>>> + */
>>> +#define KVMPPC_INST_BOOK3S_DEBUG0x0000
>>
>> Please change the BookE version of this as well, put the define in a
>> common header and use a non book specific name.
>
> I first wanted to get the server side in and then take up this, but i
> can do it with this, just concerned incase of booke testing :(
I can test, no problem. You can at least do compile tests on your side already
;)
>
>>> +
>>> struct kvmppc_bat {
>>>u64 raw;
>>>u32 bepi;
>>> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
>>> index dd03f6b..00e9c9f 100644
>>> --- a/arch/powerpc/kvm/book3s.c
>>> +++ b/arch/powerpc/kvm/book3s.c
>>> @@ -778,7 +778,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 27cced9..0a92e45 100644
>>> --- a/arch/powerpc/kvm/book3s_hv.c
>>> +++ b/arch/powerpc/kvm/book3s_hv.c
>>> @@ -725,6 +725,14 @@ static int kvmppc_hcall_impl_hv(unsigned long cmd)
>>>return kvmppc_hcall_impl_hv_realmode(cmd);
>>> }
>>>
>>> +static int kvmppc_emulate_debug_instruction_hv(struct kvm_run *run,
>>> +struct kvm_vcpu *vcpu)
>>> +{
>>> +run->exit_reason = KVM_EXIT_DEBUG;
>>> +run->debug.arch.address = kvmppc_get_pc(vcpu);
>>> +return 0;
>>> +}
>>> +
>>> static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu,
>>> struct task_struct *tsk)
>>> {
>>> @@ -811,9 +819,26 @@ static int kvmppc_handle_exit_hv(struct kvm_run *run,
>>> struct kvm_vcpu *vcpu,
>>> * we don't emulate any guest instructions at this stage.
>>
>> This comment is no longer true, it should get changed.
>
> Will change it.
>
>>> */
>>>case BOOK3S_INTERRUPT_H_EMUL_ASSIST:
>>> -kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
>>> -r = RESUME_GUEST;
>>> +{
>>> +u32 last_inst;
>>> +if(kvmppc_get_last_inst(vcpu, INST_GENERIC, &last_inst) !=
>>> +EMULATE_DONE) {
>>
>> Please only do this if debugging is active.
>
> Ok sure. But i will add in the else part of the check. Since we do the
> instruction check there.
We should only fetch the instruction if sw breakpoint debugging is active.
Alex
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[question] e1000 interrupt storm happened because of its corresponding ioapic->irr bit always set
Hi, all I use a qemu-1.4.1/qemu-2.0.0 to run win7 guest, and encounter e1000 NIC interrupt storm, because "if (!ent->fields.mask && (ioapic->irr & (1 << i)))" is always true in __kvm_ioapic_update_eoi(). Any ideas? Thanks, Zhang Haoyu -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] powerpc/kvm: support to handle sw breakpoint
On Thursday 21 August 2014 02:40 PM, Alexander Graf wrote:
>
>
> On 20.08.14 07:52, 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.
>>
>> Changes v3->v4:
>> Made changes to code comments and removed #define of zero opcode
>> Added a new function to handle the debug instruction emulation in book3s_hv
>> Rebased the code to latest upstream source.
>>
>> Changes v2->v3:
>> Changed the debug instructions. Using the all zero opcode in the
>> instruction word
>> as illegal instruction as mentioned in Power ISA instead of ABS
>> Removed reg updated in emulation assist and added a call to
>> kvmppc_emulate_instruction for reg update.
>>
>> 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
>> ---
>> arch/powerpc/include/asm/kvm_book3s.h | 7 +++
>> arch/powerpc/kvm/book3s.c | 3 ++-
>> arch/powerpc/kvm/book3s_hv.c | 32 ++--
>> arch/powerpc/kvm/book3s_pr.c | 3 +++
>> arch/powerpc/kvm/emulate.c| 11 +++
>> 5 files changed, 53 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/kvm_book3s.h
>> b/arch/powerpc/include/asm/kvm_book3s.h
>> index 6acf0c2..a1944f8 100644
>> --- a/arch/powerpc/include/asm/kvm_book3s.h
>> +++ b/arch/powerpc/include/asm/kvm_book3s.h
>> @@ -24,6 +24,13 @@
>> #include
>> #include
>>
>> +/*
>> + * KVMPPC_INST_BOOK3S_DEBUG is debug Instruction for supporting Software
>> Breakpoint.
>> + * Based on PowerISA v2.07, Instruction with primary opcode 0 will be
>> treated as illegal
>> + * instruction.
>> + */
>> +#define KVMPPC_INST_BOOK3S_DEBUG0x0000
>
> Please change the BookE version of this as well, put the define in a
> common header and use a non book specific name.
>
I first wanted to get the server side in and then take up this, but i
can do it with this, just concerned incase of booke testing :(
>> +
>> struct kvmppc_bat {
>> u64 raw;
>> u32 bepi;
>> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
>> index dd03f6b..00e9c9f 100644
>> --- a/arch/powerpc/kvm/book3s.c
>> +++ b/arch/powerpc/kvm/book3s.c
>> @@ -778,7 +778,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 27cced9..0a92e45 100644
>> --- a/arch/powerpc/kvm/book3s_hv.c
>> +++ b/arch/powerpc/kvm/book3s_hv.c
>> @@ -725,6 +725,14 @@ static int kvmppc_hcall_impl_hv(unsigned long cmd)
>> return kvmppc_hcall_impl_hv_realmode(cmd);
>> }
>>
>> +static int kvmppc_emulate_debug_instruction_hv(struct kvm_run *run,
>> +struct kvm_vcpu *vcpu)
>> +{
>> +run->exit_reason = KVM_EXIT_DEBUG;
>> +run->debug.arch.address = kvmppc_get_pc(vcpu);
>> +return 0;
>> +}
>> +
>> static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu,
>> struct task_struct *tsk)
>> {
>> @@ -811,9 +819,26 @@ static int kvmppc_handle_exit_hv(struct kvm_run *run,
>> struct kvm_vcpu *vcpu,
>> * we don't emulate any guest instructions at this stage.
>
> This comment is no longer true, it should get changed.
>
Will change it.
>> */
>> case BOOK3S_INTERRUPT_H_EMUL_ASSIST:
>> -kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
>> -r = RESUME_GUEST;
>> +{
>> +u32 last_inst;
>> +if(kvmppc_get_last_inst(vcpu, INST_GENERIC, &last_inst) !=
>> +EMULATE_DONE) {
>
> Please only do this if debugging is active.
>
Ok sure. But i will add in the else part of the check. Since we do the
instruction check there.
>> +/*
>> + * Fetch failed, so return to guest and
>> + * try executing it again.
>> + */
>> +r = RESUME_GUEST;
>> +} else {
>> +if (last_inst == KVMPPC_INST_BOOK3S_DEBUG) {
>> +kvmppc_emulate_debug_instruction_hv(run, vcpu);
>> +r = RESUME_HOST;
>>
