On 21.05.2012, at 06:28, Bhushan Bharat-R65777 <R65777@xxxxxxxxxxxxx> wrote: > > >> -----Original Message----- >> From: Alexander Graf [mailto:agraf@xxxxxxx] >> Sent: Wednesday, May 16, 2012 2:54 PM >> To: Bhushan Bharat-R65777 >> Cc: kvm-ppc@xxxxxxxxxxxxxxx >> Subject: Re: [PATCH] KVM: PPC: booke: Added DECAR support >> >> On 05/16/2012 11:21 AM, Bhushan Bharat-R65777 wrote: >>> >>>> -----Original Message----- >>>> From: Alexander Graf [mailto:agraf@xxxxxxx] >>>> Sent: Wednesday, May 16, 2012 2:36 PM >>>> To: Bhushan Bharat-R65777 >>>> Cc: kvm-ppc@xxxxxxxxxxxxxxx >>>> 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@xxxxxxx] >>>>>> Sent: Tuesday, May 15, 2012 7:59 PM >>>>>> To: Bhushan Bharat-R65777 >>>>>> Cc: kvm-ppc@xxxxxxxxxxxxxxx; 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<bharat.bhushan@xxxxxxxxxxxxx> >>>>>>> --- >>>>>>> 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? > > Ok > >> What does e500v2 do here? > > mfspr(DECAR) works ok e500v2 :), while spec says it is write-only. > > We should emulate DECAR as per specifications documented . Right? So we will not support trap and emulate for e500v2. Usually yes, but IMHO hardware always wins over specs, so I would enable it for v2 as well if it really does work properly there ;). Alex -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
