From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754219AbbDMOsk (ORCPT ); Mon, 13 Apr 2015 10:48:40 -0400 Received: from mail-lb0-f173.google.com ([209.85.217.173]:33550 "EHLO mail-lb0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753847AbbDMOsi (ORCPT ); Mon, 13 Apr 2015 10:48:38 -0400 Date: Mon, 13 Apr 2015 16:48:43 +0200 From: Christoffer Dall To: Alex =?iso-8859-1?Q?Benn=E9e?= Cc: kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, marc.zyngier@arm.com, peter.maydell@linaro.org, agraf@suse.de, drjones@redhat.com, pbonzini@redhat.com, zhichao.huang@linaro.org, jan.kiszka@siemens.com, dahi@linux.vnet.ibm.com, r65777@freescale.com, bp@suse.de, Gleb Natapov , Russell King , Catalin Marinas , Will Deacon , Andre Przywara , Lorenzo Pieralisi , open list Subject: Re: [PATCH v2 05/10] KVM: arm: introduce kvm_arch_setup/clear_debug() Message-ID: <20150413144843.GQ6186@cbox> References: <1427814488-28467-1-git-send-email-alex.bennee@linaro.org> <1427814488-28467-6-git-send-email-alex.bennee@linaro.org> <20150413143623.GP6186@cbox> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150413143623.GP6186@cbox> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Apr 13, 2015 at 04:36:23PM +0200, Christoffer Dall wrote: [...] > > + > > +/** > > + * kvm_arch_setup_debug - set-up debug related stuff > > nit: I think you want "set up" when it's a verb. > > > + * > > + * @vcpu: the vcpu pointer > > + * > > + * This is called before each entry in to the hypervisor to setup any > > s/in to/into/ > s/setup/set up/ > > > + * debug related registers. Currently this just ensures we will trap > > + * access to: > > guest accesses to: > > > + * - Performance monitors (MDCR_EL2_TPM/MDCR_EL2_TPMCR) > > + * - Debug ROM Address (MDCR_EL2_TDRA) > > + * - Power down debug registers (MDCR_EL2_TDOSA) > > + * > > + * Additionally the hypervisor lazily saves/restores the debug > > + * register state. If it is not currently doing so (arch.debug_flags) > > + * then we also need to ensure we trap if the guest messes with them > > + * so we know we need to save them. > > This paragraph is a little hard to make sense of. If I understand it > correctly, the point is that when debugging the guest we need to make > sure guest accesses to the debug registers traps? If so, I would > suggest something like: > > Additionally, KVM only traps guest accesses to the debug registers if > the guest is not actively using them (see the KVM_ARM64_DEBUG_DIRTY > flag on vcpu->arch.debug_flags). Since the guest must not interfere > with the hardware state when debugging the guest, we must ensure that > trapping is enabled whenever we are debugging the guest. > thinking about this, I don't think we're enforcing this yet, but maybe that will come in the later patches or I misread the original paragraph. -Christoffer From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christoffer Dall Subject: Re: [PATCH v2 05/10] KVM: arm: introduce kvm_arch_setup/clear_debug() Date: Mon, 13 Apr 2015 16:48:43 +0200 Message-ID: <20150413144843.GQ6186@cbox> References: <1427814488-28467-1-git-send-email-alex.bennee@linaro.org> <1427814488-28467-6-git-send-email-alex.bennee@linaro.org> <20150413143623.GP6186@cbox> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: Lorenzo Pieralisi , Russell King , kvm@vger.kernel.org, marc.zyngier@arm.com, jan.kiszka@siemens.com, Will Deacon , open list , dahi@linux.vnet.ibm.com, Andre Przywara , Catalin Marinas , zhichao.huang@linaro.org, r65777@freescale.com, pbonzini@redhat.com, bp@suse.de, Gleb Natapov , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org To: Alex =?iso-8859-1?Q?Benn=E9e?= Return-path: Content-Disposition: inline In-Reply-To: <20150413143623.GP6186@cbox> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kvmarm-bounces@lists.cs.columbia.edu Sender: kvmarm-bounces@lists.cs.columbia.edu List-Id: kvm.vger.kernel.org On Mon, Apr 13, 2015 at 04:36:23PM +0200, Christoffer Dall wrote: [...] > > + > > +/** > > + * kvm_arch_setup_debug - set-up debug related stuff > > nit: I think you want "set up" when it's a verb. > > > + * > > + * @vcpu: the vcpu pointer > > + * > > + * This is called before each entry in to the hypervisor to setup any > > s/in to/into/ > s/setup/set up/ > > > + * debug related registers. Currently this just ensures we will trap > > + * access to: > > guest accesses to: > > > + * - Performance monitors (MDCR_EL2_TPM/MDCR_EL2_TPMCR) > > + * - Debug ROM Address (MDCR_EL2_TDRA) > > + * - Power down debug registers (MDCR_EL2_TDOSA) > > + * > > + * Additionally the hypervisor lazily saves/restores the debug > > + * register state. If it is not currently doing so (arch.debug_flags) > > + * then we also need to ensure we trap if the guest messes with them > > + * so we know we need to save them. > > This paragraph is a little hard to make sense of. If I understand it > correctly, the point is that when debugging the guest we need to make > sure guest accesses to the debug registers traps? If so, I would > suggest something like: > > Additionally, KVM only traps guest accesses to the debug registers if > the guest is not actively using them (see the KVM_ARM64_DEBUG_DIRTY > flag on vcpu->arch.debug_flags). Since the guest must not interfere > with the hardware state when debugging the guest, we must ensure that > trapping is enabled whenever we are debugging the guest. > thinking about this, I don't think we're enforcing this yet, but maybe that will come in the later patches or I misread the original paragraph. -Christoffer From mboxrd@z Thu Jan 1 00:00:00 1970 From: christoffer.dall@linaro.org (Christoffer Dall) Date: Mon, 13 Apr 2015 16:48:43 +0200 Subject: [PATCH v2 05/10] KVM: arm: introduce kvm_arch_setup/clear_debug() In-Reply-To: <20150413143623.GP6186@cbox> References: <1427814488-28467-1-git-send-email-alex.bennee@linaro.org> <1427814488-28467-6-git-send-email-alex.bennee@linaro.org> <20150413143623.GP6186@cbox> Message-ID: <20150413144843.GQ6186@cbox> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Mon, Apr 13, 2015 at 04:36:23PM +0200, Christoffer Dall wrote: [...] > > + > > +/** > > + * kvm_arch_setup_debug - set-up debug related stuff > > nit: I think you want "set up" when it's a verb. > > > + * > > + * @vcpu: the vcpu pointer > > + * > > + * This is called before each entry in to the hypervisor to setup any > > s/in to/into/ > s/setup/set up/ > > > + * debug related registers. Currently this just ensures we will trap > > + * access to: > > guest accesses to: > > > + * - Performance monitors (MDCR_EL2_TPM/MDCR_EL2_TPMCR) > > + * - Debug ROM Address (MDCR_EL2_TDRA) > > + * - Power down debug registers (MDCR_EL2_TDOSA) > > + * > > + * Additionally the hypervisor lazily saves/restores the debug > > + * register state. If it is not currently doing so (arch.debug_flags) > > + * then we also need to ensure we trap if the guest messes with them > > + * so we know we need to save them. > > This paragraph is a little hard to make sense of. If I understand it > correctly, the point is that when debugging the guest we need to make > sure guest accesses to the debug registers traps? If so, I would > suggest something like: > > Additionally, KVM only traps guest accesses to the debug registers if > the guest is not actively using them (see the KVM_ARM64_DEBUG_DIRTY > flag on vcpu->arch.debug_flags). Since the guest must not interfere > with the hardware state when debugging the guest, we must ensure that > trapping is enabled whenever we are debugging the guest. > thinking about this, I don't think we're enforcing this yet, but maybe that will come in the later patches or I misread the original paragraph. -Christoffer