* Re: [PATCH] KVM: PPC: booke: Added DECAR support
2012-05-15 7:45 [PATCH] KVM: PPC: booke: Added DECAR support Bharat Bhushan
@ 2012-05-15 14:28 ` Alexander Graf
2012-05-16 6:57 ` Bhushan Bharat-R65777
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Alexander Graf @ 2012-05-15 14:28 UTC (permalink / raw)
To: kvm-ppc
On 05/15/2012 09:33 AM, Bharat Bhushan wrote:
> Added the decrementer auto-reload support.
>
> Signed-off-by: Bharat Bhushan<bharat.bhushan@freescale.com>
> ---
> 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.
Alex
^ permalink raw reply [flat|nested] 8+ messages in thread* RE: [PATCH] KVM: PPC: booke: Added DECAR support
2012-05-15 7:45 [PATCH] KVM: PPC: booke: Added DECAR support Bharat Bhushan
2012-05-15 14:28 ` Alexander Graf
@ 2012-05-16 6:57 ` Bhushan Bharat-R65777
2012-05-16 9:06 ` Alexander Graf
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Bhushan Bharat-R65777 @ 2012-05-16 6:57 UTC (permalink / raw)
To: kvm-ppc
> -----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<bharat.bhushan@freescale.com>
> > ---
> > 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?
Thanks
-Bharat
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] KVM: PPC: booke: Added DECAR support
2012-05-15 7:45 [PATCH] KVM: PPC: booke: Added DECAR support Bharat Bhushan
2012-05-15 14:28 ` Alexander Graf
2012-05-16 6:57 ` Bhushan Bharat-R65777
@ 2012-05-16 9:06 ` Alexander Graf
2012-05-16 9:21 ` Bhushan Bharat-R65777
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Alexander Graf @ 2012-05-16 9:06 UTC (permalink / raw)
To: kvm-ppc
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<bharat.bhushan@freescale.com>
>>> ---
>>> 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?
Alex
^ permalink raw reply [flat|nested] 8+ messages in thread* RE: [PATCH] KVM: PPC: booke: Added DECAR support
2012-05-15 7:45 [PATCH] KVM: PPC: booke: Added DECAR support Bharat Bhushan
` (2 preceding siblings ...)
2012-05-16 9:06 ` Alexander Graf
@ 2012-05-16 9:21 ` Bhushan Bharat-R65777
2012-05-16 9:23 ` Alexander Graf
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Bhushan Bharat-R65777 @ 2012-05-16 9:21 UTC (permalink / raw)
To: kvm-ppc
> -----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<bharat.bhushan@freescale.com>
> >>> ---
> >>> 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.
Thanks
-Bharat
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] KVM: PPC: booke: Added DECAR support
2012-05-15 7:45 [PATCH] KVM: PPC: booke: Added DECAR support Bharat Bhushan
` (3 preceding siblings ...)
2012-05-16 9:21 ` Bhushan Bharat-R65777
@ 2012-05-16 9:23 ` Alexander Graf
2012-05-21 4:28 ` Bhushan Bharat-R65777
2012-05-21 6:18 ` Alexander Graf
6 siblings, 0 replies; 8+ messages in thread
From: Alexander Graf @ 2012-05-16 9:23 UTC (permalink / raw)
To: kvm-ppc
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<bharat.bhushan@freescale.com>
>>>>> ---
>>>>> 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
^ permalink raw reply [flat|nested] 8+ messages in thread* RE: [PATCH] KVM: PPC: booke: Added DECAR support
2012-05-15 7:45 [PATCH] KVM: PPC: booke: Added DECAR support Bharat Bhushan
` (4 preceding siblings ...)
2012-05-16 9:23 ` Alexander Graf
@ 2012-05-21 4:28 ` Bhushan Bharat-R65777
2012-05-21 6:18 ` Alexander Graf
6 siblings, 0 replies; 8+ messages in thread
From: Bhushan Bharat-R65777 @ 2012-05-21 4:28 UTC (permalink / raw)
To: kvm-ppc
> -----Original Message-----
> From: Alexander Graf [mailto:agraf@suse.de]
> Sent: Wednesday, May 16, 2012 2:54 PM
> To: Bhushan Bharat-R65777
> Cc: kvm-ppc@vger.kernel.org
> 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@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<bharat.bhushan@freescale.com>
> >>>>> ---
> >>>>> 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.
Thanks
-Bharat
>
>
> Alex
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] KVM: PPC: booke: Added DECAR support
2012-05-15 7:45 [PATCH] KVM: PPC: booke: Added DECAR support Bharat Bhushan
` (5 preceding siblings ...)
2012-05-21 4:28 ` Bhushan Bharat-R65777
@ 2012-05-21 6:18 ` Alexander Graf
6 siblings, 0 replies; 8+ messages in thread
From: Alexander Graf @ 2012-05-21 6:18 UTC (permalink / raw)
To: kvm-ppc
On 21.05.2012, at 06:28, Bhushan Bharat-R65777 <R65777@freescale.com> wrote:
>
>
>> -----Original Message-----
>> From: Alexander Graf [mailto:agraf@suse.de]
>> Sent: Wednesday, May 16, 2012 2:54 PM
>> To: Bhushan Bharat-R65777
>> Cc: kvm-ppc@vger.kernel.org
>> 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@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<bharat.bhushan@freescale.com>
>>>>>>> ---
>>>>>>> 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
^ permalink raw reply [flat|nested] 8+ messages in thread