From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexander Graf Date: Wed, 16 May 2012 09:23:42 +0000 Subject: Re: [PATCH] KVM: PPC: booke: Added DECAR support Message-Id: <4FB3721E.6060105@suse.de> List-Id: References: <1337067209-26435-1-git-send-email-bharat.bhushan@freescale.com> In-Reply-To: <1337067209-26435-1-git-send-email-bharat.bhushan@freescale.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: kvm-ppc@vger.kernel.org On 05/16/2012 11:21 AM, Bhushan Bharat-R65777 wrote: > >> -----Original Message----- >> From: Alexander Graf [mailto:agraf@suse.de] >> Sent: Wednesday, May 16, 2012 2:36 PM >> To: Bhushan Bharat-R65777 >> Cc: kvm-ppc@vger.kernel.org >> Subject: Re: [PATCH] KVM: PPC: booke: Added DECAR support >> >> On 05/16/2012 08:57 AM, Bhushan Bharat-R65777 wrote: >>>> -----Original Message----- >>>> From: Alexander Graf [mailto:agraf@suse.de] >>>> Sent: Tuesday, May 15, 2012 7:59 PM >>>> To: Bhushan Bharat-R65777 >>>> Cc: kvm-ppc@vger.kernel.org; Bhushan Bharat-R65777 >>>> Subject: Re: [PATCH] KVM: PPC: booke: Added DECAR support >>>> >>>> On 05/15/2012 09:33 AM, Bharat Bhushan wrote: >>>>> Added the decrementer auto-reload support. >>>>> >>>>> Signed-off-by: Bharat Bhushan >>>>> --- >>>>> arch/powerpc/include/asm/kvm_host.h | 2 ++ >>>>> arch/powerpc/kvm/booke.c | 5 +++++ >>>>> arch/powerpc/kvm/booke_emulate.c | 7 ++++++- >>>>> 3 files changed, 13 insertions(+), 1 deletions(-) >>>>> >>>>> diff --git a/arch/powerpc/include/asm/kvm_host.h >>>>> b/arch/powerpc/include/asm/kvm_host.h >>>>> index d848cdc..1d6f89e 100644 >>>>> --- a/arch/powerpc/include/asm/kvm_host.h >>>>> +++ b/arch/powerpc/include/asm/kvm_host.h >>>>> @@ -414,7 +414,9 @@ struct kvm_vcpu_arch { >>>>> ulong mcsrr1; >>>>> ulong mcsr; >>>>> u32 dec; >>>>> +#ifdef CONFIG_BOOKE >>>>> u32 decar; >>>>> +#endif >>>>> u32 tbl; >>>>> u32 tbu; >>>>> u32 tcr; >>>>> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c >>>>> index 72f13f4..86681ee 100644 >>>>> --- a/arch/powerpc/kvm/booke.c >>>>> +++ b/arch/powerpc/kvm/booke.c >>>>> @@ -1267,6 +1267,11 @@ void kvmppc_decrementer_func(unsigned long data) >>>>> { >>>>> struct kvm_vcpu *vcpu = (struct kvm_vcpu *)data; >>>>> >>>>> + if (vcpu->arch.tcr& TCR_ARE) { >>>>> + vcpu->arch.dec = vcpu->arch.decar; >>>>> + kvmppc_emulate_dec(vcpu); >>>>> + } >>>>> + >>>>> kvmppc_set_tsr_bits(vcpu, TSR_DIS); >>>>> } >>>>> >>>>> diff --git a/arch/powerpc/kvm/booke_emulate.c >>>>> b/arch/powerpc/kvm/booke_emulate.c >>>>> index 6c76397..83c3796 100644 >>>>> --- a/arch/powerpc/kvm/booke_emulate.c >>>>> +++ b/arch/powerpc/kvm/booke_emulate.c >>>>> @@ -129,6 +129,9 @@ int kvmppc_booke_emulate_mtspr(struct kvm_vcpu >>>>> *vcpu, int >>>> sprn, ulong spr_val) >>>>> kvmppc_set_tcr(vcpu, spr_val); >>>>> break; >>>>> >>>>> + case SPRN_DECAR: >>>>> + vcpu->arch.decar = spr_val; >>>>> + break; >>>>> /* >>>>> * Note: SPRG4-7 are user-readable. >>>>> * These values are loaded into the real SPRGs when resuming the >>>>> @@ >>>>> -244,7 +247,9 @@ int kvmppc_booke_emulate_mfspr(struct kvm_vcpu >>>>> *vcpu, int >>>> sprn, ulong *spr_val) >>>>> case SPRN_TCR: >>>>> *spr_val = vcpu->arch.tcr; >>>>> break; >>>>> - >>>>> + case SPRN_DECAR: >>>>> + *spr_val = vcpu->arch.decar; >>>>> + break; >>>> DECAR can't be read. Otherwise looks good to me. >>> DECAR can be read on e500mc cores. So I will make this under >> CONFIG_KVM_E500MC. >>> What happens if DECAR is read on non e500mc? Is it treated as NOP or illegal >> instruction exception? >> >> I would assume the latter. NOPs usually don't happen. If anything, it would >> return 0. >> >> See section 9.4 in the PowerISA: >> >> The contents of the Decrementer Auto-Reload Register cannot be read. The >> contents of bits 32:63 of register RS can be written to the Decrementer Auto- >> Reload Register using the mtspr instruction. >> >> Could you please paste the respective passage of the e500mc manual that declares >> DECAR as readable? > I have booke -4 version 1.05. > There are remarks at 3 place > > 1) > Table 2-1: > There is remark " DECAR is defined by the architecture to be write-only, however the e500mc allows it to be read." > ------------ > > 2) > 2.8.5 Decrementer Auto-Reload Register (DECAR) > > "For e500V4, the DECAR can be read in hypervisor state, although the architecture defines it as a > write-only register." > -------------- > > 3) > Table 4.37 > > Hypervisor Privilege on Read - Yes > Hypervisor Privilege on Write - Yes > > Remark: e500mc allows reading of DECAR although Power ISA does not define it. > ---------------- > > I verified with my unit test that reading DECAR traps in KVM on e500mc. Alrighty, so how about putting the read into e500_emulate.c then? What does e500v2 do here? Alex