From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexander Graf Date: Thu, 03 Jul 2014 12:03:01 +0000 Subject: Re: [PATCH] KVM: PPC: e500: Emulate power management control SPR Message-Id: <53B54675.5040308@suse.de> List-Id: References: <1404132929-27308-1-git-send-email-mihai.caraman@freescale.com> <1404152404.2435.132.camel@snotra.buserror.net> In-Reply-To: <1404152404.2435.132.camel@snotra.buserror.net> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Scott Wood , Mihai Caraman Cc: kvm-ppc@vger.kernel.org, kvm@vger.kernel.org, linuxppc-dev@lists.ozlabs.org On 30.06.14 20:20, Scott Wood wrote: > On Mon, 2014-06-30 at 15:55 +0300, Mihai Caraman wrote: >> For FSL e6500 core the kernel uses power management SPR register (PWRMGTCR0) >> to enable idle power down for cores and devices by setting up the idle count >> period at boot time. With the host already controlling the power management >> configuration the guest could simply benefit from it, so emulate guest request >> as nop. >> >> Signed-off-by: Mihai Caraman >> --- >> arch/powerpc/kvm/e500_emulate.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/arch/powerpc/kvm/e500_emulate.c b/arch/powerpc/kvm/e500_emulate.c >> index 002d517..98a22e5 100644 >> --- a/arch/powerpc/kvm/e500_emulate.c >> +++ b/arch/powerpc/kvm/e500_emulate.c >> @@ -250,6 +250,10 @@ int kvmppc_core_emulate_mtspr_e500(struct kvm_vcpu *vcpu, int sprn, ulong spr_va >> spr_val); >> break; >> >> + case SPRN_PWRMGTCR0: >> + /* Guest relies on host power management configurations */ >> + break; >> + >> /* extra exceptions */ >> case SPRN_IVOR32: >> vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_UNAVAIL] = spr_val; >> @@ -355,6 +359,10 @@ int kvmppc_core_emulate_mfspr_e500(struct kvm_vcpu *vcpu, int sprn, ulong *spr_v >> *spr_val = 0; >> break; >> >> + case SPRN_PWRMGTCR0: >> + *spr_val = 0; >> + break; >> + >> case SPRN_MMUCFG: >> *spr_val = vcpu->arch.mmucfg; >> break; > When reading, is it better to return zero, or the current host value, or > the value last written by the guest (even though it wasn't written to > hardware)? I think it makes sense to treat it as general storage. I don't think leaking the host value into the guest is useful. And while zero works, the spec does say that the value gets retained, so I think we should do the same. Alex From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de (cantor2.suse.de [195.135.220.15]) (using TLSv1 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 564911A0004 for ; Thu, 3 Jul 2014 22:03:05 +1000 (EST) Message-ID: <53B54675.5040308@suse.de> Date: Thu, 03 Jul 2014 14:03:01 +0200 From: Alexander Graf MIME-Version: 1.0 To: Scott Wood , Mihai Caraman Subject: Re: [PATCH] KVM: PPC: e500: Emulate power management control SPR References: <1404132929-27308-1-git-send-email-mihai.caraman@freescale.com> <1404152404.2435.132.camel@snotra.buserror.net> In-Reply-To: <1404152404.2435.132.camel@snotra.buserror.net> Content-Type: text/plain; charset=UTF-8; format=flowed Cc: linuxppc-dev@lists.ozlabs.org, kvm@vger.kernel.org, kvm-ppc@vger.kernel.org List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On 30.06.14 20:20, Scott Wood wrote: > On Mon, 2014-06-30 at 15:55 +0300, Mihai Caraman wrote: >> For FSL e6500 core the kernel uses power management SPR register (PWRMGTCR0) >> to enable idle power down for cores and devices by setting up the idle count >> period at boot time. With the host already controlling the power management >> configuration the guest could simply benefit from it, so emulate guest request >> as nop. >> >> Signed-off-by: Mihai Caraman >> --- >> arch/powerpc/kvm/e500_emulate.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/arch/powerpc/kvm/e500_emulate.c b/arch/powerpc/kvm/e500_emulate.c >> index 002d517..98a22e5 100644 >> --- a/arch/powerpc/kvm/e500_emulate.c >> +++ b/arch/powerpc/kvm/e500_emulate.c >> @@ -250,6 +250,10 @@ int kvmppc_core_emulate_mtspr_e500(struct kvm_vcpu *vcpu, int sprn, ulong spr_va >> spr_val); >> break; >> >> + case SPRN_PWRMGTCR0: >> + /* Guest relies on host power management configurations */ >> + break; >> + >> /* extra exceptions */ >> case SPRN_IVOR32: >> vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_UNAVAIL] = spr_val; >> @@ -355,6 +359,10 @@ int kvmppc_core_emulate_mfspr_e500(struct kvm_vcpu *vcpu, int sprn, ulong *spr_v >> *spr_val = 0; >> break; >> >> + case SPRN_PWRMGTCR0: >> + *spr_val = 0; >> + break; >> + >> case SPRN_MMUCFG: >> *spr_val = vcpu->arch.mmucfg; >> break; > When reading, is it better to return zero, or the current host value, or > the value last written by the guest (even though it wasn't written to > hardware)? I think it makes sense to treat it as general storage. I don't think leaking the host value into the guest is useful. And while zero works, the spec does say that the value gets retained, so I think we should do the same. Alex From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexander Graf Subject: Re: [PATCH] KVM: PPC: e500: Emulate power management control SPR Date: Thu, 03 Jul 2014 14:03:01 +0200 Message-ID: <53B54675.5040308@suse.de> References: <1404132929-27308-1-git-send-email-mihai.caraman@freescale.com> <1404152404.2435.132.camel@snotra.buserror.net> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: kvm-ppc@vger.kernel.org, kvm@vger.kernel.org, linuxppc-dev@lists.ozlabs.org To: Scott Wood , Mihai Caraman Return-path: In-Reply-To: <1404152404.2435.132.camel@snotra.buserror.net> Sender: kvm-ppc-owner@vger.kernel.org List-Id: kvm.vger.kernel.org On 30.06.14 20:20, Scott Wood wrote: > On Mon, 2014-06-30 at 15:55 +0300, Mihai Caraman wrote: >> For FSL e6500 core the kernel uses power management SPR register (PWRMGTCR0) >> to enable idle power down for cores and devices by setting up the idle count >> period at boot time. With the host already controlling the power management >> configuration the guest could simply benefit from it, so emulate guest request >> as nop. >> >> Signed-off-by: Mihai Caraman >> --- >> arch/powerpc/kvm/e500_emulate.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/arch/powerpc/kvm/e500_emulate.c b/arch/powerpc/kvm/e500_emulate.c >> index 002d517..98a22e5 100644 >> --- a/arch/powerpc/kvm/e500_emulate.c >> +++ b/arch/powerpc/kvm/e500_emulate.c >> @@ -250,6 +250,10 @@ int kvmppc_core_emulate_mtspr_e500(struct kvm_vcpu *vcpu, int sprn, ulong spr_va >> spr_val); >> break; >> >> + case SPRN_PWRMGTCR0: >> + /* Guest relies on host power management configurations */ >> + break; >> + >> /* extra exceptions */ >> case SPRN_IVOR32: >> vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_UNAVAIL] = spr_val; >> @@ -355,6 +359,10 @@ int kvmppc_core_emulate_mfspr_e500(struct kvm_vcpu *vcpu, int sprn, ulong *spr_v >> *spr_val = 0; >> break; >> >> + case SPRN_PWRMGTCR0: >> + *spr_val = 0; >> + break; >> + >> case SPRN_MMUCFG: >> *spr_val = vcpu->arch.mmucfg; >> break; > When reading, is it better to return zero, or the current host value, or > the value last written by the guest (even though it wasn't written to > hardware)? I think it makes sense to treat it as general storage. I don't think leaking the host value into the guest is useful. And while zero works, the spec does say that the value gets retained, so I think we should do the same. Alex