Re: [PATCH v9 2/4] KVM: PPC: epapr: Add idle hcall support for host

[Scott Wood <scottwood@freescale.com>]

On 03/07/2012 05:27 PM, Alexander Graf wrote:
> On 08.03.2012, at 00:12, Stuart Yoder wrote:
>>
>> 	if (vcpu->requests) {
>> +		/* kvm_vcpu_block() sets KVM_REQ_UNHALT, but it is
>> + 		 * not cleared elsewhere as on x86.  Clear it here
>> + 		 * for now, otherwise we never go idle.
>> + 		 */
>> +		clear_bit(KVM_REQ_UNHALT, &vcpu->requests);
> 
> Shouldn't the same thing hit us on non-booke as well? Also, it sounds unrelated to me and probably shouldn't be in this patch.

Until recently we didn't check for requests in kvm_arch_vcpu_runnable().

And yes, book3s will need this too.

>> 		if (kvm_check_request(KVM_REQ_PENDING_TIMER, vcpu)) {
>> 			smp_mb();
>> 			update_timer_ints(vcpu);
>> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
>> index ee489f4..2595916 100644
>> --- a/arch/powerpc/kvm/powerpc.c
>> +++ b/arch/powerpc/kvm/powerpc.c
>> @@ -48,8 +48,7 @@ static unsigned int perfmon_refcount;
>>
>> int kvm_arch_vcpu_runnable(struct kvm_vcpu *v)
>> {
>> -	bool ret = !(v->arch.shared->msr & MSR_WE) ||
>> -		   !!(v->arch.pending_exceptions) ||
>> +	bool ret = !!(v->arch.pending_exceptions) ||
>> 		   v->requests;
> 
> Huh?

MSR_WE is not going to get set if the idle hcall is used, so this check
was preventing us from blocking.

The check isn't needed anyway, as nothing can actually change MSR_WE
while we're in kvm_vcpu_block(), which is the only user of
kvm_arch_vcpu_runnable(), and the MSR_WE path won't call
kvm_vcpu_block() if MSR_WE isn't set.

-Scott