On 31.01.2013, at 18:08, Alexander Graf wrote: > > On 31.01.2013, at 17:58, Bhushan Bharat-R65777 wrote: > >> >> >>> -----Original Message----- >>> From: Alexander Graf [mailto:agraf@xxxxxxx] >>> Sent: Thursday, January 31, 2013 5:47 PM >>> To: Bhushan Bharat-R65777 >>> Cc: kvm-ppc@xxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx >>> Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler >>> >>> >>> On 30.01.2013, at 12:30, Bhushan Bharat-R65777 wrote: >>> >>>> >>>> >>>>> -----Original Message----- >>>>> From: Alexander Graf [mailto:agraf@xxxxxxx] >>>>> Sent: Friday, January 25, 2013 5:13 PM >>>>> To: Bhushan Bharat-R65777 >>>>> Cc: kvm-ppc@xxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx; Bhushan >>>>> Bharat-R65777 >>>>> Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler >>>>> >>>>> >>>>> On 16.01.2013, at 09:24, Bharat Bhushan wrote: >>>>> >>>>>> From: Bharat Bhushan <Bharat.Bhushan@xxxxxxxxxxxxx> >>>>>> >>>>>> Installed debug handler will be used for guest debug support and >>>>>> debug facility emulation features (patches for these features will >>>>>> follow this patch). >>>>>> >>>>>> Signed-off-by: Liu Yu <yu.liu@xxxxxxxxxxxxx> >>>>>> [bharat.bhushan@xxxxxxxxxxxxx: Substantial changes] >>>>>> Signed-off-by: Bharat Bhushan <bharat.bhushan@xxxxxxxxxxxxx> >>>>>> --- >>>>>> arch/powerpc/include/asm/kvm_host.h | 1 + >>>>>> arch/powerpc/kernel/asm-offsets.c | 1 + >>>>>> arch/powerpc/kvm/booke_interrupts.S | 49 ++++++++++++++++++++++++++++++--- >>> -- >>>>>> 3 files changed, 44 insertions(+), 7 deletions(-) >>>>>> >>>>>> diff --git a/arch/powerpc/include/asm/kvm_host.h >>>>>> b/arch/powerpc/include/asm/kvm_host.h >>>>>> index 8a72d59..f4ba881 100644 >>>>>> --- a/arch/powerpc/include/asm/kvm_host.h >>>>>> +++ b/arch/powerpc/include/asm/kvm_host.h >>>>>> @@ -503,6 +503,7 @@ struct kvm_vcpu_arch { >>>>>> u32 tlbcfg[4]; >>>>>> u32 mmucfg; >>>>>> u32 epr; >>>>>> + u32 crit_save; >>>>>> struct kvmppc_booke_debug_reg dbg_reg; #endif >>>>>> gpa_t paddr_accessed; >>>>>> diff --git a/arch/powerpc/kernel/asm-offsets.c >>>>>> b/arch/powerpc/kernel/asm-offsets.c >>>>>> index 46f6afd..02048f3 100644 >>>>>> --- a/arch/powerpc/kernel/asm-offsets.c >>>>>> +++ b/arch/powerpc/kernel/asm-offsets.c >>>>>> @@ -562,6 +562,7 @@ int main(void) >>>>>> DEFINE(VCPU_LAST_INST, offsetof(struct kvm_vcpu, arch.last_inst)); >>>>>> DEFINE(VCPU_FAULT_DEAR, offsetof(struct kvm_vcpu, arch.fault_dear)); >>>>>> DEFINE(VCPU_FAULT_ESR, offsetof(struct kvm_vcpu, arch.fault_esr)); >>>>>> + DEFINE(VCPU_CRIT_SAVE, offsetof(struct kvm_vcpu, arch.crit_save)); >>>>>> #endif /* CONFIG_PPC_BOOK3S */ >>>>>> #endif /* CONFIG_KVM */ >>>>>> >>>>>> diff --git a/arch/powerpc/kvm/booke_interrupts.S >>>>>> b/arch/powerpc/kvm/booke_interrupts.S >>>>>> index eae8483..dd9c5d4 100644 >>>>>> --- a/arch/powerpc/kvm/booke_interrupts.S >>>>>> +++ b/arch/powerpc/kvm/booke_interrupts.S >>>>>> @@ -52,12 +52,7 @@ >>>>>> (1<<BOOKE_INTERRUPT_PROGRAM) | \ >>>>>> (1<<BOOKE_INTERRUPT_DTLB_MISS)) >>>>>> >>>>>> -.macro KVM_HANDLER ivor_nr scratch srr0 >>>>>> -_GLOBAL(kvmppc_handler_\ivor_nr) >>>>>> - /* Get pointer to vcpu and record exit number. */ >>>>>> - mtspr \scratch , r4 >>>>>> - mfspr r4, SPRN_SPRG_THREAD >>>>>> - lwz r4, THREAD_KVM_VCPU(r4) >>>>>> +.macro __KVM_HANDLER ivor_nr scratch srr0 >>>>>> stw r3, VCPU_GPR(R3)(r4) >>>>>> stw r5, VCPU_GPR(R5)(r4) >>>>>> stw r6, VCPU_GPR(R6)(r4) >>>>>> @@ -74,6 +69,46 @@ _GLOBAL(kvmppc_handler_\ivor_nr) >>>>>> bctr >>>>>> .endm >>>>>> >>>>>> +.macro KVM_HANDLER ivor_nr scratch srr0 >>>>>> +_GLOBAL(kvmppc_handler_\ivor_nr) >>>>>> + /* Get pointer to vcpu and record exit number. */ >>>>>> + mtspr \scratch , r4 >>>>>> + mfspr r4, SPRN_SPRG_THREAD >>>>>> + lwz r4, THREAD_KVM_VCPU(r4) >>>>>> + __KVM_HANDLER \ivor_nr \scratch \srr0 .endm >>>>>> + >>>>>> +.macro KVM_DBG_HANDLER ivor_nr scratch srr0 >>>>>> +_GLOBAL(kvmppc_handler_\ivor_nr) >>>>>> + mtspr \scratch, r4 >>>>>> + mfspr r4, SPRN_SPRG_THREAD >>>>>> + lwz r4, THREAD_KVM_VCPU(r4) >>>>>> + stw r3, VCPU_CRIT_SAVE(r4) >>>>>> + mfcr r3 >>>>>> + mfspr r4, SPRN_CSRR1 >>>>>> + andi. r4, r4, MSR_PR >>>>>> + bne 1f >>>>> >>>>> >>>>>> + /* debug interrupt happened in enter/exit path */ >>>>>> + mfspr r4, SPRN_CSRR1 >>>>>> + rlwinm r4, r4, 0, ~MSR_DE >>>>>> + mtspr SPRN_CSRR1, r4 >>>>>> + lis r4, 0xffff >>>>>> + ori r4, r4, 0xffff >>>>>> + mtspr SPRN_DBSR, r4 >>>>>> + mfspr r4, SPRN_SPRG_THREAD >>>>>> + lwz r4, THREAD_KVM_VCPU(r4) >>>>>> + mtcr r3 >>>>>> + lwz r3, VCPU_CRIT_SAVE(r4) >>>>>> + mfspr r4, \scratch >>>>>> + rfci >>>>> >>>>> What is this part doing? Try to ignore the debug exit? >>>> >>>> As BOOKE doesn't have hardware support for virtualization, hardware never know >>> current pc is in guest or in host. >>>> So when enable hardware single step for guest, it cannot be disabled at the >>> time guest exit. Thus, we'll see that an single step interrupt happens at the >>> beginning of guest exit path. >>>> >>>> With the above code we recognize this kind of single step interrupt disable >>> single step and rfci. >>>> >>>>> Why would we have MSR_DE >>>>> enabled in the first place when we can't handle it? >>>> >>>> When QEMU is using hardware debug resource then we always set MSR_DE during >>> guest is running. >>> >>> Right, but why is MSR_DE enabled during the exit path? If MSR_DE wasn't set, you >>> wouldn't get a single step exit. >> >> We always set MSR_DE in hw MSR when qemu using the debug resource. > > In the _guest_ MSR, yes. But once we exit the guest, it shouldn't be set anymore, because we're in an interrupt handler, no? Or is MSR_DE kept alive on interrupts? Ah, it's kept for non-debug interrupts. That explains things. 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
