From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexander Graf Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support Date: Wed, 3 Apr 2013 18:26:47 +0200 Message-ID: References: <1363847101-26503-1-git-send-email-Bharat.Bhushan@freescale.com> <1363847101-26503-5-git-send-email-Bharat.Bhushan@freescale.com> <7355982C-05D1-4C58-A189-2F8F926AD11B@suse.de> <6A3DF150A5B70D4F9B66A25E3F7C888D06FB82BE@039-SN2MPN1-013.039d.mgd> <7C9A47BE-0385-4B45-B4F9-D5069C9ADBA2@suse.de> <6A3DF150A5B70D4F9B66A25E3F7C888D06FBBF38@039-SN2MPN1-013.039d.mgd.msft.net> <1364925601.24520.9@snotra> <6A3DF150A5B70D4F9B66A25E3F7C888D06FBDD76@039-SN2MPN1-013.039d.mgd.msft.net> <32389E01-F199-49CF-8789-F1A4D5070F76@suse.de> <6A3DF150A5B70D4F9B66A25E3F7C888D06FBE2D4@039-SN2MPN1-013.039d.mgd.msft.net> <6A3DF150A5B70D4F9B66A25E3F7C888D06FBE624@039-SN2MPN1-013.039d.mgd.msft.net> Mime-Version: 1.0 (Apple Message framework v1278) Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 8BIT Cc: Wood Scott-B07421 , "kvm-ppc@vger.kernel.org" , "kvm@vger.kernel.org" To: Bhushan Bharat-R65777 Return-path: In-Reply-To: <6A3DF150A5B70D4F9B66A25E3F7C888D06FBE624@039-SN2MPN1-013.039d.mgd.msft.net> Sender: kvm-ppc-owner@vger.kernel.org List-Id: kvm.vger.kernel.org On 03.04.2013, at 17:18, Bhushan Bharat-R65777 wrote: > > >> -----Original Message----- >> From: Alexander Graf [mailto:agraf@suse.de] >> Sent: Wednesday, April 03, 2013 7:39 PM >> To: Bhushan Bharat-R65777 >> Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org >> Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support >> >> >> On 03.04.2013, at 15:50, Bhushan Bharat-R65777 wrote: >> >>> >>> >>>> -----Original Message----- >>>> From: kvm-ppc-owner@vger.kernel.org >>>> [mailto:kvm-ppc-owner@vger.kernel.org] On Behalf Of Alexander Graf >>>> Sent: Wednesday, April 03, 2013 3:58 PM >>>> To: Bhushan Bharat-R65777 >>>> Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org >>>> Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub >>>> support >>>> >>>> >>>> >>>> Am 03.04.2013 um 12:03 schrieb Bhushan Bharat-R65777 : >>>> >>>>> >>>>> >>>>>> -----Original Message----- >>>>>> From: Wood Scott-B07421 >>>>>> Sent: Tuesday, April 02, 2013 11:30 PM >>>>>> To: Bhushan Bharat-R65777 >>>>>> Cc: Alexander Graf; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; >>>>>> Wood Scott- >>>>>> B07421 >>>>>> Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub >>>>>> support >>>>>> >>>>>> On 04/02/2013 09:09:34 AM, Bhushan Bharat-R65777 wrote: >>>>>>> >>>>>>> >>>>>>>> -----Original Message----- >>>>>>>> From: Alexander Graf [mailto:agraf@suse.de] >>>>>>>> Sent: Tuesday, April 02, 2013 1:57 PM >>>>>>>> To: Bhushan Bharat-R65777 >>>>>>>> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Wood >>>>>>>> Scott-B07421 >>>>>>>> Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub >>>>>>> support >>>>>>>> >>>>>>>> >>>>>>>> On 29.03.2013, at 07:04, Bhushan Bharat-R65777 wrote: >>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>>> -----Original Message----- >>>>>>>>>> From: Alexander Graf [mailto:agraf@suse.de] >>>>>>>>>> Sent: Thursday, March 28, 2013 10:06 PM >>>>>>>>>> To: Bhushan Bharat-R65777 >>>>>>>>>> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Wood >>>>>>> Scott-B07421; >>>>>>>>>> Bhushan >>>>>>>>>> Bharat-R65777 >>>>>>>>>> Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub >>>>>>>>>> support >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> How does the normal debug register switching code work in Linux? >>>>>>>>>> Can't we just reuse that? Or rely on it to restore working >>>>>>>>>> state >>>>>>> when >>>>>>>>>> another process gets scheduled in? >>>>>>>>> >>>>>>>>> Good point, I can see debug registers loading in function >>>>>>> __switch_to()- >>>>>>>>> switch_booke_debug_regs() in file arch/powerpc/kernel/process.c. >>>>>>>>> So as long as assume that host will not use debug resources we >>>>>>> can rely on >>>>>>>> this restore. But I am not sure that this is a fare assumption. >>>>>>>> As >>>>>>> Scott earlier >>>>>>>> mentioned someone can use debug resource for kernel debugging also. >>>>>>>> >>>>>>>> Someone in the kernel can also use floating point registers. But >>>>>>> then it's his >>>>>>>> responsibility to clean up the mess he leaves behind. >>>>>>> >>>>>>> I am neither convinced by what you said and nor even have much >>>>>>> reason to oppose :) >>>>>>> >>>>>>> Scott, >>>>>>> I remember you mentioned that host can use debug resources, you >>>>>>> comment on this ? >>>>>> >>>>>> I thought the conclusion we reached was that it was OK as long as >>>>>> KVM waits until it actually needs the debug resources to mess with the >> registers. >>>>> >>>>> Right, Are we also agreeing on that KVM will not save/restore host >>>>> debug >>>> context on vcpu_load/vcpu_put()? KVM will load its context in >>>> vcpu_load() if needed and on vcpu_put() it will clear DBCR0 and DBSR. >>>> >>>> That depends on whether the kernel restores the debug registers. >>>> Please double- check that. >>> >>> Currently the kernel code restore the debug state of new schedule process in >> context_switch(). >>> >>> switch_booke_debug_regs() from __switch_to() and defined as : >>> /* >>> * Unless neither the old or new thread are making use of the >>> * debug registers, set the debug registers from the values >>> * stored in the new thread. >>> */ >>> static void switch_booke_debug_regs(struct thread_struct *new_thread) >>> { >>> if ((current->thread.dbcr0 & DBCR0_IDM) >>> || (new_thread->dbcr0 & DBCR0_IDM)) >>> prime_debug_regs(new_thread); } >>> >>> static void prime_debug_regs(struct thread_struct *thread) { >>> mtspr(SPRN_IAC1, thread->iac1); >>> mtspr(SPRN_IAC2, thread->iac2); #if CONFIG_PPC_ADV_DEBUG_IACS > >>> 2 >>> mtspr(SPRN_IAC3, thread->iac3); >>> mtspr(SPRN_IAC4, thread->iac4); #endif >>> mtspr(SPRN_DAC1, thread->dac1); >>> mtspr(SPRN_DAC2, thread->dac2); #if CONFIG_PPC_ADV_DEBUG_DVCS > >>> 0 >>> mtspr(SPRN_DVC1, thread->dvc1); >>> mtspr(SPRN_DVC2, thread->dvc2); #endif >>> mtspr(SPRN_DBCR0, thread->dbcr0); >>> mtspr(SPRN_DBCR1, thread->dbcr1); #ifdef CONFIG_BOOKE >>> mtspr(SPRN_DBCR2, thread->dbcr2); #endif } This is analogous to >>> moving from guest to/from QEMU. so we can make prime_debug_regs() available to >> kvm code for heavyweight_exit. And vcpu_load() will load guest state and save >> host state (update thread->debug_registers). >> >> I don't think we need to do anything on vcpu_load if we just swap the thread- >>> debug_registers. Just make sure to restore them before you return from a heavy >> weight exit. > > My understanding is : > > 1) > When VCPU is running -> h/w debug registers have vcpu->arch.debug_registers > Goes for heavyweight_exit -> h/w debug registers are loaded with thread->debug_registers > Return from heavyweight_exit and run vcpu -> h/w debug registers have vcpu->arch.debug_registers. Do we need to save thread->debug_registers , probably no as the thread->debug_registers are upto date. I think I had a thinko here :). I figured that when we put the guest debug register state into thread->debug_registers, we don't need to worry about save/restore on vcpu_load/put. But that won't update the register values in thread->debug_registers when the guest modifies them, so we would need to write them back on vcpu_put again, rendering the whole exercise pointless. So yes, we should only make sure that we set thread->dbcr0 = 0 on vcpu_run and return it to its old value on a heavy weight exit. That way we don't swap in debug registers we don't care about if someone debugs the QEMU process. > > 2) > KVM to kernel -> clear DBSR and DBCR0 if debugging is active > Kernel to KVM -> load vcpu->arch.debug_registers. if debugging is active :) Alex From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexander Graf Date: Wed, 03 Apr 2013 16:26:47 +0000 Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support Message-Id: List-Id: References: <1363847101-26503-1-git-send-email-Bharat.Bhushan@freescale.com> <1363847101-26503-5-git-send-email-Bharat.Bhushan@freescale.com> <7355982C-05D1-4C58-A189-2F8F926AD11B@suse.de> <6A3DF150A5B70D4F9B66A25E3F7C888D06FB82BE@039-SN2MPN1-013.039d.mgd> <7C9A47BE-0385-4B45-B4F9-D5069C9ADBA2@suse.de> <6A3DF150A5B70D4F9B66A25E3F7C888D06FBBF38@039-SN2MPN1-013.039d.mgd.msft.net> <1364925601.24520.9@snotra> <6A3DF150A5B70D4F9B66A25E3F7C888D06FBDD76@039-SN2MPN1-013.039d.mgd.msft.net> <32389E01-F199-49CF-8789-F1A4D5070F76@suse.de> <6A3DF150A5B70D4F9B66A25E3F7C888D06FBE2D4@039-SN2MPN1-013.039d.mgd.msft.net> <6A3DF150A5B70D4F9B66A25E3F7C888D06FBE624@039-SN2MPN1-013.039d.mgd.msft.net> In-Reply-To: <6A3DF150A5B70D4F9B66A25E3F7C888D06FBE624@039-SN2MPN1-013.039d.mgd.msft.net> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Bhushan Bharat-R65777 Cc: Wood Scott-B07421 , "kvm-ppc@vger.kernel.org" , "kvm@vger.kernel.org" On 03.04.2013, at 17:18, Bhushan Bharat-R65777 wrote: > > >> -----Original Message----- >> From: Alexander Graf [mailto:agraf@suse.de] >> Sent: Wednesday, April 03, 2013 7:39 PM >> To: Bhushan Bharat-R65777 >> Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org >> Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub support >> >> >> On 03.04.2013, at 15:50, Bhushan Bharat-R65777 wrote: >> >>> >>> >>>> -----Original Message----- >>>> From: kvm-ppc-owner@vger.kernel.org >>>> [mailto:kvm-ppc-owner@vger.kernel.org] On Behalf Of Alexander Graf >>>> Sent: Wednesday, April 03, 2013 3:58 PM >>>> To: Bhushan Bharat-R65777 >>>> Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org >>>> Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub >>>> support >>>> >>>> >>>> >>>> Am 03.04.2013 um 12:03 schrieb Bhushan Bharat-R65777 : >>>> >>>>> >>>>> >>>>>> -----Original Message----- >>>>>> From: Wood Scott-B07421 >>>>>> Sent: Tuesday, April 02, 2013 11:30 PM >>>>>> To: Bhushan Bharat-R65777 >>>>>> Cc: Alexander Graf; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; >>>>>> Wood Scott- >>>>>> B07421 >>>>>> Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub >>>>>> support >>>>>> >>>>>> On 04/02/2013 09:09:34 AM, Bhushan Bharat-R65777 wrote: >>>>>>> >>>>>>> >>>>>>>> -----Original Message----- >>>>>>>> From: Alexander Graf [mailto:agraf@suse.de] >>>>>>>> Sent: Tuesday, April 02, 2013 1:57 PM >>>>>>>> To: Bhushan Bharat-R65777 >>>>>>>> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Wood >>>>>>>> Scott-B07421 >>>>>>>> Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub >>>>>>> support >>>>>>>> >>>>>>>> >>>>>>>> On 29.03.2013, at 07:04, Bhushan Bharat-R65777 wrote: >>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>>> -----Original Message----- >>>>>>>>>> From: Alexander Graf [mailto:agraf@suse.de] >>>>>>>>>> Sent: Thursday, March 28, 2013 10:06 PM >>>>>>>>>> To: Bhushan Bharat-R65777 >>>>>>>>>> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Wood >>>>>>> Scott-B07421; >>>>>>>>>> Bhushan >>>>>>>>>> Bharat-R65777 >>>>>>>>>> Subject: Re: [PATCH 4/4 v2] KVM: PPC: Add userspace debug stub >>>>>>>>>> support >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> How does the normal debug register switching code work in Linux? >>>>>>>>>> Can't we just reuse that? Or rely on it to restore working >>>>>>>>>> state >>>>>>> when >>>>>>>>>> another process gets scheduled in? >>>>>>>>> >>>>>>>>> Good point, I can see debug registers loading in function >>>>>>> __switch_to()- >>>>>>>>> switch_booke_debug_regs() in file arch/powerpc/kernel/process.c. >>>>>>>>> So as long as assume that host will not use debug resources we >>>>>>> can rely on >>>>>>>> this restore. But I am not sure that this is a fare assumption. >>>>>>>> As >>>>>>> Scott earlier >>>>>>>> mentioned someone can use debug resource for kernel debugging also. >>>>>>>> >>>>>>>> Someone in the kernel can also use floating point registers. But >>>>>>> then it's his >>>>>>>> responsibility to clean up the mess he leaves behind. >>>>>>> >>>>>>> I am neither convinced by what you said and nor even have much >>>>>>> reason to oppose :) >>>>>>> >>>>>>> Scott, >>>>>>> I remember you mentioned that host can use debug resources, you >>>>>>> comment on this ? >>>>>> >>>>>> I thought the conclusion we reached was that it was OK as long as >>>>>> KVM waits until it actually needs the debug resources to mess with the >> registers. >>>>> >>>>> Right, Are we also agreeing on that KVM will not save/restore host >>>>> debug >>>> context on vcpu_load/vcpu_put()? KVM will load its context in >>>> vcpu_load() if needed and on vcpu_put() it will clear DBCR0 and DBSR. >>>> >>>> That depends on whether the kernel restores the debug registers. >>>> Please double- check that. >>> >>> Currently the kernel code restore the debug state of new schedule process in >> context_switch(). >>> >>> switch_booke_debug_regs() from __switch_to() and defined as : >>> /* >>> * Unless neither the old or new thread are making use of the >>> * debug registers, set the debug registers from the values >>> * stored in the new thread. >>> */ >>> static void switch_booke_debug_regs(struct thread_struct *new_thread) >>> { >>> if ((current->thread.dbcr0 & DBCR0_IDM) >>> || (new_thread->dbcr0 & DBCR0_IDM)) >>> prime_debug_regs(new_thread); } >>> >>> static void prime_debug_regs(struct thread_struct *thread) { >>> mtspr(SPRN_IAC1, thread->iac1); >>> mtspr(SPRN_IAC2, thread->iac2); #if CONFIG_PPC_ADV_DEBUG_IACS > >>> 2 >>> mtspr(SPRN_IAC3, thread->iac3); >>> mtspr(SPRN_IAC4, thread->iac4); #endif >>> mtspr(SPRN_DAC1, thread->dac1); >>> mtspr(SPRN_DAC2, thread->dac2); #if CONFIG_PPC_ADV_DEBUG_DVCS > >>> 0 >>> mtspr(SPRN_DVC1, thread->dvc1); >>> mtspr(SPRN_DVC2, thread->dvc2); #endif >>> mtspr(SPRN_DBCR0, thread->dbcr0); >>> mtspr(SPRN_DBCR1, thread->dbcr1); #ifdef CONFIG_BOOKE >>> mtspr(SPRN_DBCR2, thread->dbcr2); #endif } This is analogous to >>> moving from guest to/from QEMU. so we can make prime_debug_regs() available to >> kvm code for heavyweight_exit. And vcpu_load() will load guest state and save >> host state (update thread->debug_registers). >> >> I don't think we need to do anything on vcpu_load if we just swap the thread- >>> debug_registers. Just make sure to restore them before you return from a heavy >> weight exit. > > My understanding is : > > 1) > When VCPU is running -> h/w debug registers have vcpu->arch.debug_registers > Goes for heavyweight_exit -> h/w debug registers are loaded with thread->debug_registers > Return from heavyweight_exit and run vcpu -> h/w debug registers have vcpu->arch.debug_registers. Do we need to save thread->debug_registers , probably no as the thread->debug_registers are upto date. I think I had a thinko here :). I figured that when we put the guest debug register state into thread->debug_registers, we don't need to worry about save/restore on vcpu_load/put. But that won't update the register values in thread->debug_registers when the guest modifies them, so we would need to write them back on vcpu_put again, rendering the whole exercise pointless. So yes, we should only make sure that we set thread->dbcr0 = 0 on vcpu_run and return it to its old value on a heavy weight exit. That way we don't swap in debug registers we don't care about if someone debugs the QEMU process. > > 2) > KVM to kernel -> clear DBSR and DBCR0 if debugging is active > Kernel to KVM -> load vcpu->arch.debug_registers. if debugging is active :) Alex