Re: [PATCH v2 4/4] kvm/ppc: IRQ disabling cleanup

["tiejun.chen" <tiejun.chen@windriver.com>]

>> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
>> index 4e05f8c..f8659aa 100644
>> --- a/arch/powerpc/kvm/powerpc.c
>> +++ b/arch/powerpc/kvm/powerpc.c
>> @@ -64,12 +64,14 @@ int kvmppc_prepare_to_enter(struct kvm_vcpu *vcpu)
>>   {
>>   	int r = 1;
>>
>> -	WARN_ON_ONCE(!irqs_disabled());
>> +	WARN_ON(irqs_disabled());
>> +	hard_irq_disable();
>
> Here we hard disable in kvmppc_prepare_to_enter(), so my comment in other patch about interrupt loss is no more valid.
>
> So here
>    MSR.EE = 0
>    local_paca->soft_enabled = 0
>    local_paca->irq_happened |= PACA_IRQ_HARD_DIS;
>
>> +
>>   	while (true) {
>>   		if (need_resched()) {
>>   			local_irq_enable();
>
> This will make the state:
>    MSR.EE = 1
>    local_paca->soft_enabled = 1
>    local_paca->irq_happened = PACA_IRQ_HARD_DIS;  //same as before

Why is this same the above state? local_irq_enable() can call 
__check_irq_replay() to clear PACA_IRQ_HARD_DIS.

>
> Is that a valid state where interrupts are fully enabled and irq_happend in not 0?
>
>>   			cond_resched();
>> -			local_irq_disable();
>> +			hard_irq_disable();
>>   			continue;
>>   		}
>>
>> @@ -95,7 +97,7 @@ int kvmppc_prepare_to_enter(struct kvm_vcpu *vcpu)
>>   			local_irq_enable();
>>   			trace_kvm_check_requests(vcpu);
>>   			r = kvmppc_core_check_requests(vcpu);
>> -			local_irq_disable();
>> +			hard_irq_disable();
>>   			if (r > 0)
>>   				continue;
>>   			break;
>> @@ -108,21 +110,14 @@ int kvmppc_prepare_to_enter(struct kvm_vcpu *vcpu)
>>   		}
>>
>>   #ifdef CONFIG_PPC64
>> -		/* lazy EE magic */
>> -		hard_irq_disable();
>> -		if (lazy_irq_pending()) {
>> -			/* Got an interrupt in between, try again */
>> -			local_irq_enable();
>> -			local_irq_disable();
>> -			kvm_guest_exit();
>> -			continue;
>> -		}
>> +		WARN_ON(lazy_irq_pending());
>>   #endif
>>
>>   		kvm_guest_enter();
>> -		break;
>> +		return r;
>>   	}
>>
>> +	local_irq_enable();
>>   	return r;
>>   }
>
>
> int kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
> {
>          int r = 0;
>          WARN_ON_ONCE(!irqs_disabled());
>
>          kvmppc_core_check_exceptions(vcpu);
>
>          if (vcpu->requests) {
>                  /* Exception delivery raised request; start over */
>                  return 1;
>          }
>
>          if (vcpu->arch.shared->msr & MSR_WE) {
>                  local_irq_enable();
>                  kvm_vcpu_block(vcpu);
>                  clear_bit(KVM_REQ_UNHALT, &vcpu->requests);
>                  local_irq_disable();
> ^^^
> We do not require hard_irq_disable() here?

Between kvmppc_core_prepare_to_enter() and kvmppc_prepare_to_enter(), as I 
recall Scott had some discussions with Ben earlier.

Tiejun

>