From patchwork Thu Jan 31 18:21:07 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexander Graf X-Patchwork-Id: 217232 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3D4242C008D for ; Fri, 1 Feb 2013 05:21:16 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753737Ab3AaSVP (ORCPT ); Thu, 31 Jan 2013 13:21:15 -0500 Received: from cantor2.suse.de ([195.135.220.15]:50868 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750921Ab3AaSVO convert rfc822-to-8bit (ORCPT ); Thu, 31 Jan 2013 13:21:14 -0500 Received: from relay2.suse.de (unknown [195.135.220.254]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx2.suse.de (Postfix) with ESMTP id 5CC49A39D2; Thu, 31 Jan 2013 19:21:13 +0100 (CET) Subject: Re: [PATCH 8/8] KVM:PPC:booke: Allow debug interrupt injection to guest Mime-Version: 1.0 (Apple Message framework v1278) From: Alexander Graf In-Reply-To: <6A3DF150A5B70D4F9B66A25E3F7C888D06591B44@039-SN2MPN1-023.039d.mgd.msft.net> Date: Thu, 31 Jan 2013 19:21:07 +0100 Cc: "kvm-ppc@vger.kernel.org" , "kvm@vger.kernel.org" Message-Id: References: <1358324685-30225-1-git-send-email-bharat.bhushan@freescale.com> <1358324685-30225-7-git-send-email-bharat.bhushan@freescale.com> <4145C65C-BC91-494C-8C90-2DC2538F131F@suse.de> <6A3DF150A5B70D4F9B66A25E3F7C888D065829EE@039-SN2MPN1-022.039d.mgd <11819845-A3C0-41F1-90E3-8E22E97863CB@suse.de> <6A3DF150A5B70D4F9B66A25E3F7C888D06591B44@039-SN2MPN1-023.039d.mgd.msft.net> To: Bhushan Bharat-R65777 X-Mailer: Apple Mail (2.1278) Sender: kvm-ppc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm-ppc@vger.kernel.org On 31.01.2013, at 18:59, Bhushan Bharat-R65777 wrote: > > >> -----Original Message----- >> From: kvm-owner@vger.kernel.org [mailto:kvm-owner@vger.kernel.org] On Behalf Of >> Alexander Graf >> Sent: Thursday, January 31, 2013 5:34 PM >> To: Bhushan Bharat-R65777 >> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org >> Subject: Re: [PATCH 8/8] KVM:PPC:booke: Allow debug interrupt injection to guest >> >> >> On 30.01.2013, at 12:12, 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: Friday, January 25, 2013 5:44 PM >>>> To: Bhushan Bharat-R65777 >>>> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Bhushan >>>> Bharat-R65777 >>>> Subject: Re: [PATCH 8/8] KVM:PPC:booke: Allow debug interrupt >>>> injection to guest >>>> >>>> >>>> On 16.01.2013, at 09:24, Bharat Bhushan wrote: >>>> >>>>> Allow userspace to inject debug interrupt to guest. QEMU can >>>> >>>> s/QEMU/user space. >>>> >>>>> inject the debug interrupt to guest if it is not able to handle the >>>>> debug interrupt. >>>>> >>>>> Signed-off-by: Bharat Bhushan >>>>> --- >>>>> arch/powerpc/kvm/booke.c | 32 +++++++++++++++++++++++++++++++- >>>>> arch/powerpc/kvm/e500mc.c | 10 +++++++++- >>>>> 2 files changed, 40 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c >>>>> index faa0a0b..547797f 100644 >>>>> --- a/arch/powerpc/kvm/booke.c >>>>> +++ b/arch/powerpc/kvm/booke.c >>>>> @@ -133,6 +133,13 @@ static void kvmppc_vcpu_sync_fpu(struct >>>>> kvm_vcpu >>>>> *vcpu) #endif } >>>>> >>>>> +#ifdef CONFIG_KVM_BOOKE_HV >>>>> +static int kvmppc_core_pending_debug(struct kvm_vcpu *vcpu) { >>>>> + return test_bit(BOOKE_IRQPRIO_DEBUG, >>>>> +&vcpu->arch.pending_exceptions); } #endif >>>>> + >>>>> /* >>>>> * Helper function for "full" MSR writes. No need to call this if >>>>> only >>>>> * EE/CE/ME/DE/RI are changing. >>>>> @@ -144,7 +151,11 @@ void kvmppc_set_msr(struct kvm_vcpu *vcpu, u32 >>>>> new_msr) #ifdef CONFIG_KVM_BOOKE_HV >>>>> new_msr |= MSR_GS; >>>>> >>>>> - if (vcpu->guest_debug) >>>>> + /* >>>>> + * Set MSR_DE if the hardware debug resources are owned by user-space >>>>> + * and there is no debug interrupt pending for guest to handle. >>>> >>>> Why? >>> >>> QEMU is using the IAC/DAC registers to set hardware breakpoint/watchpoints via >> debug ioctls. As debug events are enabled/gated by MSR_DE so somehow we need to >> set MSR_DE on hardware MSR when guest is running in this case. >> >> Reading this 5 times I still have no idea what you're really checking for here. >> Maybe the naming for kvmppc_core_pending_debug is just unnatural? What does that >> function do really? >> >>> >>> On bookehv this is how I am controlling the MSR_DE in hardware MSR. >>> >>>> And why is this whole thing only executed on HV? >>> >>> On e500v2 we always enable MSR_DE using vcpu->arch.shadow_msr in >>> e500.c #ifndef CONFIG_KVM_BOOKE_HV >>> - vcpu->arch.shadow_msr = MSR_USER | MSR_IS | MSR_DS; >>> + vcpu->arch.shadow_msr = MSR_USER | MSR_DE | MSR_IS | MSR_DS; > > > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c > index b340a62..1e2d663 100644 > --- a/arch/powerpc/kvm/booke.c > +++ b/arch/powerpc/kvm/booke.c > @@ -151,10 +151,14 @@ void kvmppc_set_msr(struct kvm_vcpu *vcpu, u32 new_msr) > > /* > * Set MSR_DE if the hardware debug resources are owned by user-space > - * and there is no debug interrupt pending for guest to handle. > */ > - if (vcpu->guest_debug && !kvmppc_core_pending_debug(vcpu)) > + if (vcpu->guest_debug) > new_msr |= MSR_DE; > +#else > + if (vcpu->guest_debug) > + vcpu->arch.shadow_msr |= MSR_DE; > #endif > > But do not when I should clear? How about something like this? Then both targets at least suck as much :). Thanks to e500mc's awful hardware design, we don't know who sets the MSR_DE bit. Once we forced it onto the guest, we have no change to know whether the guest also set it or not. We could only guess. So I would assume it's for the best to just treat both the same: always expose MSR_DE into guest visibility. This will break when the guest disables MSR_DE. But I have no good idea on how to solve this properly - except for hypercalls to tell us that MSR_DE is set or not. Scott, do you have an idea? Alex --- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 38a62ef..3f8cbbd 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -133,6 +133,19 @@ static void kvmppc_vcpu_sync_fpu(struct kvm_vcpu *vcpu) #endif } +static void kvmppc_vcpu_sync_debug(struct kvm_vcpu *vcpu) +{ + /* Force debug to on in guest space when user space wants to debug */ + if (vcpu->guest_debug) + vcpu->arch.shared->msr |= MSR_DE; + +#if !defined(CONFIG_KVM_BOOKE_HV) + /* Synchronize MSR_DE into shadow MSR */ + vcpu->arch.shadow_msr &= ~MSR_DE; + vcpu->arch.shadow_msr |= vcpu->arch.shared->msr & MSR_DE; +#endif +} + /* * Helper function for "full" MSR writes. No need to call this if only * EE/CE/ME/DE/RI are changing. @@ -150,6 +163,7 @@ void kvmppc_set_msr(struct kvm_vcpu *vcpu, u32 new_msr) kvmppc_mmu_msr_notify(vcpu, old_msr); kvmppc_vcpu_sync_spe(vcpu); kvmppc_vcpu_sync_fpu(vcpu); + kvmppc_vcpu_sync_debug(vcpu); } static void kvmppc_booke_queue_irqprio(struct kvm_vcpu *vcpu,