Re: [PATCH] KVM: PPC: BOOK3S: PR: Fix PURR and SPURR emulation

["Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>]

Paul Mackerras <paulus@samba.org> writes:

> On Tue, Jun 03, 2014 at 05:46:11PM +0530, Aneesh Kumar K.V wrote:
>> We use time base for PURR and SPURR emulation with PR KVM since we
>> are emulating a single threaded core. When using time base
>> we need to make sure that we don't accumulate time spent in the host
>> in PURR and SPURR value.
>
> Mostly looks good except for this...
>
>> @@ -170,6 +175,11 @@ void kvmppc_copy_from_svcpu(struct kvm_vcpu *vcpu,
>>  
>>  out:
>>  	preempt_enable();
>> +	/*
>> +	 * Update purr and spurr using time base
>> +	 */
>> +	vcpu->arch.purr += get_tb() - vcpu->arch.entry_tb;
>> +	vcpu->arch.spurr += get_tb() - vcpu->arch.entry_tb;
>
> You need to do those updates before the "out:" label.  Otherwise if
> this function gets called with !svcpu->in_use (which can happen if
> CONFIG_PREEMPT is enabled) we would do these updates a second time for
> one guest exit.  The thing is that kvmppc_copy_from_svcpu() can get
> called from kvmppc_core_vcpu_put_pr() if the vcpu task gets preempted
> on the way out from the guest before we get to the regular call of
> kvmppc_copy_from_svcpu().  It would then get called again when the
> task gets to run, but this time it does nothing because svcpu->in_use
> is false.

Looking at the code, since we enable MSR.EE early now, we might possibly
end up calling this function late in the guest exit path. That
implies, we may account that time (time spent after a preempt immediately
following a guest exit) in purr/spurr. I guess that amount of inaccuracy is
ok, because that is the best we could do here ?

-aneesh