Re: [PATCH/RFC 2/2] KVM: push down irq_save from kvm_guest_exit

[Christian Borntraeger <borntraeger@de.ibm.com>]

Am 28.04.2015 um 13:37 schrieb Paolo Bonzini:
>> --- a/arch/powerpc/kvm/book3s_pr.c
>> +++ b/arch/powerpc/kvm/book3s_pr.c
>> @@ -891,7 +891,9 @@ int kvmppc_handle_exit_pr(struct kvm_run *run, struct kvm_vcpu *vcpu,
>>  
>>  	/* We get here with MSR.EE=1 */
>>  
>> +	local_irq_disable();
>>  	trace_kvm_exit(exit_nr, vcpu);
>> +	local_irq_enable();
>>  	kvm_guest_exit();
> 
> This looks wrong.
> 
Yes it is.

>> --- a/include/linux/kvm_host.h
>> +++ b/include/linux/kvm_host.h
>> @@ -773,11 +773,7 @@ static inline void kvm_guest_enter(void)
>>  
>>  static inline void kvm_guest_exit(void)
>>  {
>> -	unsigned long flags;
>> -
>> -	local_irq_save(flags);
> 
> Why no WARN_ON here?

Yes,WARN_ON makes sense.
Hmm, on the other hand the  irqs_disabled call might be somewhat expensive again
(would boil down on s390 to call stnsm (store and and system mask) once for 
query and once for setting.

so...
> 
> I think the patches are sensible, especially since you can add warnings.
>  This kind of code definitely knows if it has interrupts disabled or enabled
> 
> Alternatively, the irq-disabled versions could be called
> __kvm_guest_{enter,exit}.  Then you can use those directly when it makes
> sense.

..having a special  __kvm_guest_{enter,exit} without the WARN_ON might be even
the cheapest way. In that way I could leave everything besides s390 alone and
arch maintainers can do a followup patch if appropriate.